Merge pull request #4221 from Sergio0694/bugfix/validate-all-generated-props

Fixed ValidateAllProperties generation for generated properties

Microsoft.Toolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.cs

@@ -459,7 +459,7 @@ namespace Microsoft.Toolkit.Mvvm.SourceGenerators
         /// <param name="fieldSymbol">The input <see cref="IFieldSymbol"/> instance to process.</param>
         /// <returns>The generated property name for <paramref name="fieldSymbol"/>.</returns>
         [Pure]
-        private static string GetGeneratedPropertyName(IFieldSymbol fieldSymbol)
+        public static string GetGeneratedPropertyName(IFieldSymbol fieldSymbol)
         {
             string propertyName = fieldSymbol.Name;
 

Microsoft.Toolkit.Mvvm.SourceGenerators/ComponentModel/ObservableValidatorValidateAllPropertiesGenerator.cs

@@ -16,6 +16,8 @@ using Microsoft.Toolkit.Mvvm.SourceGenerators.Extensions;
 using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
 using static Microsoft.Toolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors;
 
+#pragma warning disable SA1008
+
 namespace Microsoft.Toolkit.Mvvm.SourceGenerators
 {
     /// <summary>
@@ -46,8 +48,10 @@ namespace Microsoft.Toolkit.Mvvm.SourceGenerators
                 context.ReportDiagnostic(Diagnostic.Create(UnsupportedCSharpLanguageVersionError, null));
             }
 
-            // Get the symbol for the ValidationAttribute type
-            INamedTypeSymbol validationSymbol = context.Compilation.GetTypeByMetadataName("System.ComponentModel.DataAnnotations.ValidationAttribute")!;
+            // Get the symbol for the required attributes
+            INamedTypeSymbol
+                validationSymbol = context.Compilation.GetTypeByMetadataName("System.ComponentModel.DataAnnotations.ValidationAttribute")!,
+                observablePropertySymbol = context.Compilation.GetTypeByMetadataName("Microsoft.Toolkit.Mvvm.ComponentModel.ObservablePropertyAttribute")!;
 
             // Prepare the attributes to add to the first class declaration
             AttributeListSyntax[] classAttributes = new[]
@@ -145,14 +149,14 @@ namespace Microsoft.Toolkit.Mvvm.SourceGenerators
                                 Parameter(Identifier("obj")).WithType(PredefinedType(Token(SyntaxKind.ObjectKeyword))))
                             .WithBody(Block(
                                 LocalDeclarationStatement(
-                                    VariableDeclaration(IdentifierName("var")) // Cannot Token(SyntaxKind.VarKeyword) here (throws an ArgumentException)
+                                    VariableDeclaration(IdentifierName("var")) // Cannot use Token(SyntaxKind.VarKeyword) here (throws an ArgumentException)
                                     .AddVariables(
                                         VariableDeclarator(Identifier("instance"))
                                         .WithInitializer(EqualsValueClause(
                                             CastExpression(
                                                 IdentifierName(classSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)),
                                                 IdentifierName("obj")))))))
-                                .AddStatements(EnumerateValidationStatements(classSymbol, validationSymbol).ToArray())),
+                                .AddStatements(EnumerateValidationStatements(classSymbol, validationSymbol, observablePropertySymbol).ToArray())),
                             ReturnStatement(IdentifierName("ValidateAllProperties")))))))
                     .NormalizeWhitespace()
                     .ToFullString();
@@ -166,28 +170,47 @@ namespace Microsoft.Toolkit.Mvvm.SourceGenerators
         }
 
         /// <summary>
-        /// Gets a sequence of statements to validate declared properties.
+        /// Gets a sequence of statements to validate declared properties (including generated ones).
         /// </summary>
         /// <param name="classSymbol">The input <see cref="INamedTypeSymbol"/> instance to process.</param>
         /// <param name="validationSymbol">The type symbol for the <c>ValidationAttribute</c> type.</param>
+        /// <param name="observablePropertySymbol">The type symbol for the <c>ObservablePropertyAttribute</c> type.</param>
         /// <returns>The sequence of <see cref="StatementSyntax"/> instances to validate declared properties.</returns>
         [Pure]
-        private static IEnumerable<StatementSyntax> EnumerateValidationStatements(INamedTypeSymbol classSymbol, INamedTypeSymbol validationSymbol)
+        private static IEnumerable<StatementSyntax> EnumerateValidationStatements(INamedTypeSymbol classSymbol, INamedTypeSymbol validationSymbol, INamedTypeSymbol observablePropertySymbol)
         {
-            foreach (var propertySymbol in classSymbol.GetMembers().OfType<IPropertySymbol>())
+            foreach (var memberSymbol in classSymbol.GetMembers())
             {
-                if (propertySymbol.IsIndexer)
+                if (memberSymbol is not (IPropertySymbol { IsIndexer: false } or IFieldSymbol))
                 {
                     continue;
                 }
 
-                ImmutableArray<AttributeData> attributes = propertySymbol.GetAttributes();
+                ImmutableArray<AttributeData> attributes = memberSymbol.GetAttributes();
 
+                // Also include fields that are annotated with [ObservableProperty]. This is necessary because
+                // all generators run in an undefined order and looking at the same original compilation, so the
+                // current one wouldn't be able to see generated properties from other generators directly.
+                if (memberSymbol is IFieldSymbol &&
+                    !attributes.Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, observablePropertySymbol)))
+                {
+                    continue;
+                }
+
+                // Skip the current member if there are no validation attributes applied to it
                 if (!attributes.Any(a => a.AttributeClass?.InheritsFrom(validationSymbol) == true))
                 {
                     continue;
                 }
 
+                // Get the target property name either directly or matching the generated one
+                string propertyName = memberSymbol switch
+                {
+                    IPropertySymbol propertySymbol => propertySymbol.Name,
+                    IFieldSymbol fieldSymbol => ObservablePropertyGenerator.GetGeneratedPropertyName(fieldSymbol),
+                    _ => throw new InvalidOperationException("Invalid symbol type")
+                };
+
                 // This enumerator produces a sequence of statements as follows:
                 //
                 // __ObservableValidatorHelper.ValidateProperty(instance, instance.<PROPERTY_0>, nameof(instance.<PROPERTY_0>));
@@ -207,14 +230,14 @@ namespace Microsoft.Toolkit.Mvvm.SourceGenerators
                                 MemberAccessExpression(
                                     SyntaxKind.SimpleMemberAccessExpression,
                                     IdentifierName("instance"),
-                                    IdentifierName(propertySymbol.Name))),
+                                    IdentifierName(propertyName))),
                             Argument(
                                 InvocationExpression(IdentifierName("nameof"))
                                 .AddArgumentListArguments(Argument(
                                     MemberAccessExpression(
                                         SyntaxKind.SimpleMemberAccessExpression,
                                         IdentifierName("instance"),
-                                        IdentifierName(propertySymbol.Name)))))));
+                                        IdentifierName(propertyName)))))));
             }
         }
     }

UnitTests/UnitTests.NetCore/Mvvm/Test_ObservablePropertyAttribute.cs

@@ -7,6 +7,7 @@ using System.Collections.Generic;
 using System.ComponentModel;
 using System.ComponentModel.DataAnnotations;
 using System.Diagnostics.CodeAnalysis;
+using System.Linq;
 using System.Reflection;
 using Microsoft.Toolkit.Mvvm.ComponentModel;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
@@ -237,6 +238,36 @@ namespace UnitTests.Mvvm
             CollectionAssert.AreEqual(new[] { nameof(model.Value) }, propertyNames);
         }
 
+        // See https://github.com/CommunityToolkit/WindowsCommunityToolkit/issues/4184
+        [TestCategory("Mvvm")]
+        [TestMethod]
+        public void Test_GeneratedPropertiesWithValidationAttributesOverFields()
+        {
+            var model = new ViewModelWithValidatableGeneratedProperties();
+
+            List<string?> propertyNames = new();
+
+            model.PropertyChanged += (s, e) => propertyNames.Add(e.PropertyName);
+
+            // Assign these fields directly to bypass the validation that is executed in the generated setters.
+            // We only need those generated properties to be there to check whether they are correctly detected.
+            model.first = "A";
+            model.last = "This is a very long name that exceeds the maximum length of 60 for this property";
+
+            Assert.IsFalse(model.HasErrors);
+
+            model.RunValidation();
+
+            Assert.IsTrue(model.HasErrors);
+
+            ValidationResult[] validationErrors = model.GetErrors().ToArray();
+
+            Assert.AreEqual(validationErrors.Length, 2);
+
+            CollectionAssert.AreEqual(new[] { nameof(ViewModelWithValidatableGeneratedProperties.First) }, validationErrors[0].MemberNames.ToArray());
+            CollectionAssert.AreEqual(new[] { nameof(ViewModelWithValidatableGeneratedProperties.Last) }, validationErrors[1].MemberNames.ToArray());
+        }
+
         public partial class SampleModel : ObservableObject
         {
             /// <summary>
@@ -340,5 +371,24 @@ namespace UnitTests.Mvvm
             [MinLength(5)]
             private string value;
         }
+  
+        public partial class ViewModelWithValidatableGeneratedProperties : ObservableValidator
+        {
+            [Required]
+            [MinLength(2)]
+            [MaxLength(60)]
+            [Display(Name = "FirstName")]
+            [ObservableProperty]
+            public string first = "Bob";
+
+            [Display(Name = "LastName")]
+            [Required]
+            [MinLength(2)]
+            [MaxLength(60)]
+            [ObservableProperty]
+            public string last = "Jones";
+
+            public void RunValidation() => ValidateAllProperties();
+        }
     }
 }