Add warning for unnecessary [NotifyDataErrorInfo] attribute

Author: Sergio0694

CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md

@@ -35,4 +35,5 @@ MVVMTK0025 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator
 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
+MVVMTK0029 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Warning | See https://aka.ms/mvvmtoolkit/error
+MVVMTK0030 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Warning | See https://aka.ms/mvvmtoolkit/error

CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs

@@ -92,6 +92,7 @@ internal static class Execute
             bool notifyRecipients = false;
             bool notifyDataErrorInfo = false;
             bool hasOrInheritsClassLevelNotifyRecipients = false;
+            bool hasOrInheritsClassLevelNotifyDataErrorInfo = false;
             bool hasAnyValidationAttributes = false;
 
             // Track the property changing event for the property, if the type supports it
@@ -114,6 +115,7 @@ internal static class Execute
             if (TryGetNotifyDataErrorInfo(fieldSymbol, out bool isValidationTargetValid))
             {
                 notifyDataErrorInfo = isValidationTargetValid;
+                hasOrInheritsClassLevelNotifyDataErrorInfo = true;
             }
 
             // Gather attributes info
@@ -135,7 +137,7 @@ internal static class Execute
                 }
 
                 // Check whether the property should also be validated
-                if (TryGetNotifyDataErrorInfo(fieldSymbol, attributeData, builder, out isValidationTargetValid))
+                if (TryGetNotifyDataErrorInfo(fieldSymbol, attributeData, builder, hasOrInheritsClassLevelNotifyDataErrorInfo, out isValidationTargetValid))
                 {
                     notifyDataErrorInfo = isValidationTargetValid;
 
@@ -557,16 +559,28 @@ private static bool TryGetNotifyDataErrorInfo(IFieldSymbol fieldSymbol, out bool
         /// <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="hasOrInheritsClassLevelNotifyDataErrorInfo">Indicates wether the containing type of <paramref name="fieldSymbol"/> has or inherits <c>[NotifyDataErrorInfo]</c>.</param>
         /// <param name="isValidationTargetValid">Whether or not the the property is in a valid target that can validate values.</param>
         /// <returns>Whether or not the generated property for <paramref name="fieldSymbol"/> used <c>[NotifyDataErrorInfo]</c>.</returns>
         private static bool TryGetNotifyDataErrorInfo(
             IFieldSymbol fieldSymbol,
             AttributeData attributeData,
             ImmutableArray<Diagnostic>.Builder diagnostics,
+            bool hasOrInheritsClassLevelNotifyDataErrorInfo,
             out bool isValidationTargetValid)
         {
             if (attributeData.AttributeClass?.HasFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.NotifyDataErrorInfoAttribute") == true)
             {
+                // Emit a diagnostic if the attribute is unnecessary
+                if (hasOrInheritsClassLevelNotifyDataErrorInfo)
+                {
+                    diagnostics.Add(
+                        UnnecessaryNotifyDataErrorInfoAttributeOnFieldWarning,
+                        fieldSymbol,
+                        fieldSymbol.ContainingType,
+                        fieldSymbol.Name);
+                }
+
                 // If the containing type is valid, track it
                 if (fieldSymbol.ContainingType.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableValidator"))
                 {

CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs

@@ -475,4 +475,20 @@ internal static class DiagnosticDescriptors
         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");
+
+    /// <summary>
+    /// Gets a <see cref="DiagnosticDescriptor"/> indicating when <c>[NotifyDataErrorInfo]</c> is applied to a field in a class with <c>[NotifyDataErrorInfo]</c> used at the class-level.
+    /// <para>
+    /// Format: <c>"The field {0}.{1} is annotated with [NotifyDataErrorInfo], but that is not needed since its containing type already uses or inherits [NotifyDataErrorInfo] at the class-level"</c>.
+    /// </para>
+    /// </summary>
+    public static readonly DiagnosticDescriptor UnnecessaryNotifyDataErrorInfoAttributeOnFieldWarning = new DiagnosticDescriptor(
+        id: "MVVMTK0030",
+        title: "Unnecessary [NotifyDataErrorInfo] field annotation",
+        messageFormat: "The field {0}.{1} is annotated with [NotifyDataErrorInfo], but that is not needed since its containing type already uses or inherits [NotifyDataErrorInfo] at the class-level",
+        category: typeof(ObservablePropertyGenerator).FullName,
+        defaultSeverity: DiagnosticSeverity.Warning,
+        isEnabledByDefault: true,
+        description: "Annotating a field with [NotifyDataErrorInfo] is not necessary if the containing type has or inherits [NotifyDataErrorInfo] at the class-level.",
+        helpLinkUri: "https://aka.ms/mvvmtoolkit");
 }

tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs

@@ -1342,6 +1342,54 @@ public partial class MyViewModel : MyBaseViewModel
         VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0029");
     }
 
+    [TestMethod]
+    public void UnnecessaryNotifyDataErrorInfoWarning_SameType()
+    {
+        string source = @"
+            using System.ComponentModel.DataAnnotations;
+            using CommunityToolkit.Mvvm.ComponentModel;
+
+            namespace MyApp
+            {
+                [NotifyDataErrorInfo]
+                public partial class MyViewModel : ObservableValidator
+                {
+                    [ObservableProperty]
+                    [Required]
+                    [NotifyDataErrorInfo]
+                    public int number;
+                }
+            }";
+
+        VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0030");
+    }
+
+    [TestMethod]
+    public void UnnecessaryNotifyDataErrorInfoWarning_BaseType()
+    {
+        string source = @"
+            using System.ComponentModel.DataAnnotations;
+            using CommunityToolkit.Mvvm.ComponentModel;
+
+            namespace MyApp
+            {
+                [NotifyDataErrorInfo]
+                public class MyBaseViewModel : ObservableValidator
+                {
+                }
+
+                public partial class MyViewModel : MyBaseViewModel
+                {
+                    [ObservableProperty]
+                    [Required]
+                    [NotifyDataErrorInfo]
+                    public int number;
+                }
+            }";
+
+        VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0030");
+    }
+
     /// <summary>
     /// Verifies the output of a source generator.
     /// </summary>