Skip to content

Commit

Permalink
Merge pull request #267 from CommunityToolkit/dev/validation-attribut…
Browse files Browse the repository at this point in the history 
…e-improvements

Add [AlsoValidateProperty] attribute
  • Loading branch information
@Sergio0694
Sergio0694 authored May 31, 2022
2 parents 7efbb55 + 0b15992 commit cbafd72
Show file tree
Hide file tree
Showing 8 changed files with 324 additions and 32 deletions.
2 changes: 2 additions & 0 deletions CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 8 additions & 4 deletions CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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"/>.
Expand All @@ -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/>
Expand All @@ -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);
}
}
}
112 changes: 93 additions & 19 deletions CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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();
Expand All @@ -152,7 +184,8 @@ internal static class Execute
propertyChangedNames.ToImmutable(),
notifiedCommandNames.ToImmutable(),
alsoBroadcastChange,
validationAttributes.ToImmutable());
alsoValidateProperty,
forwardedAttributes.ToImmutable());
}

/// <summary>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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();

Expand All @@ -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>;
Expand All @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
Loading

0 comments on commit cbafd72

Please sign in to comment.