diff --git a/CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md b/CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md index 3266ed4..618f48c 100644 --- a/CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md +++ b/CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md @@ -35,3 +35,4 @@ ### New Rules MVVMTK0026 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/error MVVMTK0027 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/error MVVMTK0028 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/error +MVVMTK0029 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/error diff --git a/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs b/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs index ff825b3..a426df7 100644 --- a/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs +++ b/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs @@ -91,6 +91,7 @@ internal static class Execute ImmutableArray<AttributeInfo>.Builder forwardedAttributes = ImmutableArray.CreateBuilder<AttributeInfo>(); bool notifyRecipients = false; bool notifyDataErrorInfo = false; + bool hasOrInheritsClassLevelNotifyRecipients = false; bool hasAnyValidationAttributes = false; // Track the property changing event for the property, if the type supports it @@ -106,6 +107,7 @@ internal static class Execute if (TryGetIsNotifyingRecipients(fieldSymbol, out bool isBroadcastTargetValid)) { notifyRecipients = isBroadcastTargetValid; + hasOrInheritsClassLevelNotifyRecipients = true; } // Get the class-level [NotifyDataErrorInfo] setting, if any @@ -125,7 +127,7 @@ internal static class Execute } // Check whether the property should also notify recipients - if (TryGetIsNotifyingRecipients(fieldSymbol, attributeData, builder, out isBroadcastTargetValid)) + if (TryGetIsNotifyingRecipients(fieldSymbol, attributeData, builder, hasOrInheritsClassLevelNotifyRecipients, out isBroadcastTargetValid)) { notifyRecipients = isBroadcastTargetValid; @@ -452,16 +454,28 @@ private static bool TryGetIsNotifyingRecipients(IFieldSymbol fieldSymbol, out bo /// <param name="fieldSymbol">The input <see cref="IFieldSymbol"/> instance to process.</param> /// <param name="attributeData">The <see cref="AttributeData"/> instance for <paramref name="fieldSymbol"/>.</param> /// <param name="diagnostics">The current collection of gathered diagnostics.</param> + /// <param name="hasOrInheritsClassLevelNotifyRecipients">Indicates wether the containing type of <paramref name="fieldSymbol"/> has or inherits <c>[NotifyRecipients]</c>.</param> /// <param name="isBroadcastTargetValid">Whether or not the the property is in a valid target that can notify recipients.</param> /// <returns>Whether or not the generated property for <paramref name="fieldSymbol"/> used <c>[NotifyRecipients]</c>.</returns> private static bool TryGetIsNotifyingRecipients( IFieldSymbol fieldSymbol, AttributeData attributeData, ImmutableArray<Diagnostic>.Builder diagnostics, + bool hasOrInheritsClassLevelNotifyRecipients, out bool isBroadcastTargetValid) { if (attributeData.AttributeClass?.HasFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.NotifyRecipientsAttribute") == true) { + // Emit a diagnostic if the attribute is unnecessary + if (hasOrInheritsClassLevelNotifyRecipients) + { + diagnostics.Add( + UnnecessaryNotifyRecipientsAttributeOnFieldWarning, + fieldSymbol, + fieldSymbol.ContainingType, + fieldSymbol.Name); + } + // If the containing type is valid, track it if (fieldSymbol.ContainingType.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableRecipient") || fieldSymbol.ContainingType.HasOrInheritsAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableRecipientAttribute")) diff --git a/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs b/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs index b09a70a..70ac1bf 100644 --- a/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs +++ b/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs @@ -459,4 +459,20 @@ internal static class DiagnosticDescriptors isEnabledByDefault: true, description: "Types annotated with [NotifyDataErrorInfo] must inherit from ObservableRecipient.", helpLinkUri: "https://aka.ms/mvvmtoolkit"); + + /// <summary> + /// Gets a <see cref="DiagnosticDescriptor"/> indicating when <c>[NotifyRecipients]</c> is applied to a field in a class with <c>[NotifyRecipients]</c> used at the class-level. + /// <para> + /// Format: <c>"The field {0}.{1} is annotated with [NotifyRecipients], but that is not needed since its containing type already uses or inherits [NotifyRecipients] at the class-level"</c>. + /// </para> + /// </summary> + public static readonly DiagnosticDescriptor UnnecessaryNotifyRecipientsAttributeOnFieldWarning = new DiagnosticDescriptor( + id: "MVVMTK0029", + title: "Unnecessary [NotifyRecipients] field annotation", + messageFormat: "The field {0}.{1} is annotated with [NotifyRecipients], but that is not needed since its containing type already uses or inherits [NotifyRecipients] at the class-level", + category: typeof(ObservablePropertyGenerator).FullName, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "Annotating a field with [NotifyRecipients] is not necessary if the containing type has or inherits [NotifyRecipients] at the class-level.", + helpLinkUri: "https://aka.ms/mvvmtoolkit"); } diff --git a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs index 583cc4f..99f98fc 100644 --- a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs +++ b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs @@ -1298,6 +1298,50 @@ public partial class SampleViewModel : ObservableObject VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0006", "MVVMTK0028"); } + [TestMethod] + public void UnnecessaryNotifyRecipientsWarning_SameType() + { + string source = @" + using CommunityToolkit.Mvvm.ComponentModel; + + namespace MyApp + { + [NotifyRecipients] + public partial class MyViewModel : ObservableRecipient + { + [ObservableProperty] + [NotifyRecipients] + public int number; + } + }"; + + VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0029"); + } + + [TestMethod] + public void UnnecessaryNotifyRecipientsWarning_BaseType() + { + string source = @" + using CommunityToolkit.Mvvm.ComponentModel; + + namespace MyApp + { + [NotifyRecipients] + public class MyBaseViewModel : ObservableRecipient + { + } + + public partial class MyViewModel : MyBaseViewModel + { + [ObservableProperty] + [NotifyRecipients] + public int number; + } + }"; + + VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0029"); + } + /// <summary> /// Verifies the output of a source generator. /// </summary>