Merge pull request #267 from CommunityToolkit/dev/validation-attribute-improvements

Add [AlsoValidateProperty] attribute

Author: Sergio0694

CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md

@@ -31,3 +31,5 @@ MVVMTK0021 | CommunityToolkit.Mvvm.SourceGenerators.ObservableRecipientGenerator
 MVVMTK0022 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/error
 MVVMTK0023 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error | See https://aka.ms/mvvmtoolkit/error
 MVVMTK0024 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error | See https://aka.ms/mvvmtoolkit/error
+MVVMTK0025 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/error
+MVVMTK0026 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/error

CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs

@@ -21,7 +21,8 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.ComponentModel.Models;
 /// <param name="PropertyChangedNames">The sequence of property changed properties to notify.</param>
 /// <param name="NotifiedCommandNames">The sequence of commands to notify.</param>
 /// <param name="AlsoBroadcastChange">Whether or not the generated property also broadcasts changes.</param>
-/// <param name="ValidationAttributes">The sequence of validation attributes for the generated property.</param>
+/// <param name="AlsoValidateProperty">Whether or not the generated property also validates its value.</param>
+/// <param name="ForwardedAttributes">The sequence of forwarded attributes for the generated property.</param>
 internal sealed record PropertyInfo(
     string TypeNameWithNullabilityAnnotations,
     string FieldName,
@@ -30,7 +31,8 @@ internal sealed record PropertyInfo(
     ImmutableArray<string> PropertyChangedNames,
     ImmutableArray<string> NotifiedCommandNames,
     bool AlsoBroadcastChange,
-    ImmutableArray<AttributeInfo> ValidationAttributes)
+    bool AlsoValidateProperty,
+    ImmutableArray<AttributeInfo> ForwardedAttributes)
 {
     /// <summary>
     /// An <see cref="IEqualityComparer{T}"/> implementation for <see cref="PropertyInfo"/>.
@@ -47,7 +49,8 @@ protected override void AddToHashCode(ref HashCode hashCode, PropertyInfo obj)
             hashCode.AddRange(obj.PropertyChangedNames);
             hashCode.AddRange(obj.NotifiedCommandNames);
             hashCode.Add(obj.AlsoBroadcastChange);
-            hashCode.AddRange(obj.ValidationAttributes, AttributeInfo.Comparer.Default);
+            hashCode.Add(obj.AlsoValidateProperty);
+            hashCode.AddRange(obj.ForwardedAttributes, AttributeInfo.Comparer.Default);
         }
 
         /// <inheritdoc/>
@@ -61,7 +64,8 @@ protected override bool AreEqual(PropertyInfo x, PropertyInfo y)
                 x.PropertyChangedNames.SequenceEqual(y.PropertyChangedNames) &&
                 x.NotifiedCommandNames.SequenceEqual(y.NotifiedCommandNames) &&
                 x.AlsoBroadcastChange == y.AlsoBroadcastChange &&
-                x.ValidationAttributes.SequenceEqual(y.ValidationAttributes, AttributeInfo.Comparer.Default);
+                x.AlsoValidateProperty == y.AlsoValidateProperty &&
+                x.ForwardedAttributes.SequenceEqual(y.ForwardedAttributes, AttributeInfo.Comparer.Default);
         }
     }
 }

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

@@ -88,8 +88,10 @@ internal static class Execute
             ImmutableArray<string>.Builder propertyChangedNames = ImmutableArray.CreateBuilder<string>();
             ImmutableArray<string>.Builder propertyChangingNames = ImmutableArray.CreateBuilder<string>();
             ImmutableArray<string>.Builder notifiedCommandNames = ImmutableArray.CreateBuilder<string>();
-            ImmutableArray<AttributeInfo>.Builder validationAttributes = ImmutableArray.CreateBuilder<AttributeInfo>();
+            ImmutableArray<AttributeInfo>.Builder forwardedAttributes = ImmutableArray.CreateBuilder<AttributeInfo>();
             bool alsoBroadcastChange = false;
+            bool alsoValidateProperty = false;
+            bool hasAnyValidationAttributes = false;
 
             // Track the property changing event for the property, if the type supports it
             if (shouldInvokeOnPropertyChanging)
@@ -118,28 +120,58 @@ internal static class Execute
                     continue;
                 }
 
-                // Track the current validation attribute, if applicable
+                // Check whether the property should also be validated
+                if (TryGetIsValidatingProperty(fieldSymbol, attributeData, builder, out bool isValidationTargetValid))
+                {
+                    alsoValidateProperty = isValidationTargetValid;
+
+                    continue;
+                }
+
+                // Track the current attribute for forwarding if it is a validation attribute
                 if (attributeData.AttributeClass?.InheritsFromFullyQualifiedName("global::System.ComponentModel.DataAnnotations.ValidationAttribute") == true)
                 {
-                    validationAttributes.Add(AttributeInfo.From(attributeData));
+                    hasAnyValidationAttributes = true;
+
+                    forwardedAttributes.Add(AttributeInfo.From(attributeData));
+                }
+
+                // Also track the current attribute for forwarding if it is of any of the following types:
+                //   - Display attributes (System.ComponentModel.DataAnnotations.DisplayAttribute)
+                //   - UI hint attributes(System.ComponentModel.DataAnnotations.UIHintAttribute)
+                //   - Scaffold column attributes (System.ComponentModel.DataAnnotations.ScaffoldColumnAttribute)
+                //   - Editable attributes (System.ComponentModel.DataAnnotations.EditableAttribute)
+                //   - Key attributes (System.ComponentModel.DataAnnotations.KeyAttribute)
+                if (attributeData.AttributeClass?.HasOrInheritsFromFullyQualifiedName("global::System.ComponentModel.DataAnnotations.UIHintAttribute") == true ||
+                    attributeData.AttributeClass?.HasOrInheritsFromFullyQualifiedName("global::System.ComponentModel.DataAnnotations.ScaffoldColumnAttribute") == true ||
+                    attributeData.AttributeClass?.HasFullyQualifiedName("global::System.ComponentModel.DataAnnotations.DisplayAttribute") == true ||
+                    attributeData.AttributeClass?.HasFullyQualifiedName("global::System.ComponentModel.DataAnnotations.EditableAttribute") == true ||
+                    attributeData.AttributeClass?.HasFullyQualifiedName("global::System.ComponentModel.DataAnnotations.KeyAttribute") == true)
+                {
+                    forwardedAttributes.Add(AttributeInfo.From(attributeData));
                 }
             }
 
-            // Log the diagnostics if needed
-            if (validationAttributes.Count > 0 &&
+            // Log the diagnostic for missing ObservableValidator, if needed
+            if (hasAnyValidationAttributes &&
                 !fieldSymbol.ContainingType.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableValidator"))
             {
                 builder.Add(
-                    MissingObservableValidatorInheritanceError,
+                    MissingObservableValidatorInheritanceForValidationAttributeError,
                     fieldSymbol,
                     fieldSymbol.ContainingType,
                     fieldSymbol.Name,
-                    validationAttributes.Count);
+                    forwardedAttributes.Count);
+            }
 
-                // Remove all validation attributes so that the generated code doesn't cause a build error about the
-                // "ValidateProperty" method not existing (as the type doesn't inherit from ObservableValidator). The
-                // compilation will still fail due to this diagnostics, but with just this easier to understand error.
-                validationAttributes.Clear();
+            // Log the diagnostic for missing validation attributes, if any
+            if (alsoValidateProperty && !hasAnyValidationAttributes)
+            {
+                builder.Add(
+                    MissingValidationAttributesForAlsoValidatePropertyError,
+                    fieldSymbol,
+                    fieldSymbol.ContainingType,
+                    fieldSymbol.Name);
             }
 
             diagnostics = builder.ToImmutable();
@@ -152,7 +184,8 @@ internal static class Execute
                 propertyChangedNames.ToImmutable(),
                 notifiedCommandNames.ToImmutable(),
                 alsoBroadcastChange,
-                validationAttributes.ToImmutable());
+                alsoValidateProperty,
+                forwardedAttributes.ToImmutable());
         }
 
         /// <summary>
@@ -412,6 +445,47 @@ private static bool TryGetIsBroadcastingChanges(
             return false;
         }
 
+        /// <summary>
+        /// Checks whether a given generated property should also validate its value.
+        /// </summary>
+        /// <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="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>[AlsoValidateProperty]</c>.</returns>
+        private static bool TryGetIsValidatingProperty(
+            IFieldSymbol fieldSymbol,
+            AttributeData attributeData,
+            ImmutableArray<Diagnostic>.Builder diagnostics,
+            out bool isValidationTargetValid)
+        {
+            if (attributeData.AttributeClass?.HasFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.AlsoValidatePropertyAttribute") == true)
+            {
+                // If the containing type is valid, track it
+                if (fieldSymbol.ContainingType.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableValidator"))
+                {
+                    isValidationTargetValid = true;
+
+                    return true;
+                }
+
+                // Otherwise just emit the diagnostic and then ignore the attribute
+                diagnostics.Add(
+                    MissingObservableValidatorInheritanceForAlsoValidatePropertyError,
+                    fieldSymbol,
+                    fieldSymbol.ContainingType,
+                    fieldSymbol.Name);
+
+                isValidationTargetValid = false;
+
+                return true;
+            }
+
+            isValidationTargetValid = false;
+
+            return false;
+        }
+
         /// <summary>
         /// Gets a <see cref="CompilationUnitSyntax"/> instance with the cached args for property changing notifications.
         /// </summary>
@@ -505,10 +579,10 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf
                         fieldExpression,
                         IdentifierName("value"))));
 
-            // If there are validation attributes, add a call to ValidateProperty:
+            // If validation is requested, add a call to ValidateProperty:
             //
             // ValidateProperty(value, <PROPERTY_NAME>);
-            if (propertyInfo.ValidationAttributes.Length > 0)
+            if (propertyInfo.AlsoValidateProperty)
             {
                 setterStatements.Add(
                     ExpressionStatement(
@@ -594,9 +668,9 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf
                             Argument(IdentifierName("value")))),
                     Block(setterStatements));
 
-            // Prepare the validation attributes, if any
-            ImmutableArray<AttributeListSyntax> validationAttributes =
-                propertyInfo.ValidationAttributes
+            // Prepare the forwarded attributes, if any
+            ImmutableArray<AttributeListSyntax> forwardedAttributes =
+                propertyInfo.ForwardedAttributes
                 .Select(static a => AttributeList(SingletonSeparatedList(a.GetSyntax())))
                 .ToImmutableArray();
 
@@ -605,7 +679,7 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf
             // /// <inheritdoc cref="<FIELD_NAME>"/>
             // [global::System.CodeDom.Compiler.GeneratedCode("...", "...")]
             // [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
-            // <VALIDATION_ATTRIBUTES>
+            // <FORWARDED_ATTRIBUTES>
             // public <FIELD_TYPE><NULLABLE_ANNOTATION?> <PROPERTY_NAME>
             // {
             //     get => <FIELD_NAME>;
@@ -624,7 +698,7 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf
                             AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).Assembly.GetName().Version.ToString()))))))
                     .WithOpenBracketToken(Token(TriviaList(Comment($"/// <inheritdoc cref=\"{propertyInfo.FieldName}\"/>")), SyntaxKind.OpenBracketToken, TriviaList())),
                     AttributeList(SingletonSeparatedList(Attribute(IdentifierName("global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage")))))
-                .AddAttributeLists(validationAttributes.ToArray())
+                .AddAttributeLists(forwardedAttributes.ToArray())
                 .AddModifiers(Token(SyntaxKind.PublicKeyword))
                 .AddAccessorListAccessors(
                     AccessorDeclaration(SyntaxKind.GetAccessorDeclaration)

CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.cs

@@ -38,13 +38,14 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
             fieldSymbols
             .Where(static item => item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute"));
 
-        // Get diagnostics for fields using [AlsoNotifyChangeFor], [AlsoNotifyCanExecuteFor] and [AlsoBroadcastChange], but not [ObservableProperty]
+        // Get diagnostics for fields using [AlsoNotifyChangeFor], [AlsoNotifyCanExecuteFor], [AlsoBroadcastChange] and [AlsoValidateProperty], but not [ObservableProperty]
         IncrementalValuesProvider<Diagnostic> fieldSymbolsWithOrphanedDependentAttributeWithErrors =
             fieldSymbols
             .Where(static item =>
                 (item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.AlsoNotifyChangeForAttribute") ||
                  item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.AlsoNotifyCanExecuteForAttribute") ||
-                 item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.AlsoBroadcastChangeAttribute")) &&
+                 item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.AlsoBroadcastChangeAttribute") ||
+                 item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.AlsoValidatePropertyAttribute")) &&
                  !item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute"))
             .Select(static (item, _) => Execute.GetDiagnosticForFieldWithOrphanedDependentAttributes(item));