Skip to content

Commit 202138f

Browse files
committed
Fixed ValidateAllProperties generation for generated properties
1 parent 0ce4553 commit 202138f

File tree

3 files changed

+82
-12
lines changed

3 files changed

+82
-12
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ private static PropertyDeclarationSyntax CreatePropertyDeclaration(
450450
/// <param name="fieldSymbol">The input <see cref="IFieldSymbol"/> instance to process.</param>
451451
/// <returns>The generated property name for <paramref name="fieldSymbol"/>.</returns>
452452
[Pure]
453-
private static string GetGeneratedPropertyName(IFieldSymbol fieldSymbol)
453+
public static string GetGeneratedPropertyName(IFieldSymbol fieldSymbol)
454454
{
455455
string propertyName = fieldSymbol.Name;
456456

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

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
using Microsoft.Toolkit.Mvvm.SourceGenerators.Extensions;
1616
using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
1717

18+
#pragma warning disable SA1008
19+
1820
namespace Microsoft.Toolkit.Mvvm.SourceGenerators
1921
{
2022
/// <summary>
@@ -39,8 +41,10 @@ public void Execute(GeneratorExecutionContext context)
3941
return;
4042
}
4143

42-
// Get the symbol for the ValidationAttribute type
43-
INamedTypeSymbol validationSymbol = context.Compilation.GetTypeByMetadataName("System.ComponentModel.DataAnnotations.ValidationAttribute")!;
44+
// Get the symbol for the required attributes
45+
INamedTypeSymbol
46+
validationSymbol = context.Compilation.GetTypeByMetadataName("System.ComponentModel.DataAnnotations.ValidationAttribute")!,
47+
observablePropertySymbol = context.Compilation.GetTypeByMetadataName("Microsoft.Toolkit.Mvvm.ComponentModel.ObservablePropertyAttribute")!;
4448

4549
// Prepare the attributes to add to the first class declaration
4650
AttributeListSyntax[] classAttributes = new[]
@@ -138,14 +142,14 @@ public void Execute(GeneratorExecutionContext context)
138142
Parameter(Identifier("obj")).WithType(PredefinedType(Token(SyntaxKind.ObjectKeyword))))
139143
.WithBody(Block(
140144
LocalDeclarationStatement(
141-
VariableDeclaration(IdentifierName("var")) // Cannot Token(SyntaxKind.VarKeyword) here (throws an ArgumentException)
145+
VariableDeclaration(IdentifierName("var")) // Cannot use Token(SyntaxKind.VarKeyword) here (throws an ArgumentException)
142146
.AddVariables(
143147
VariableDeclarator(Identifier("instance"))
144148
.WithInitializer(EqualsValueClause(
145149
CastExpression(
146150
IdentifierName(classSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)),
147151
IdentifierName("obj")))))))
148-
.AddStatements(EnumerateValidationStatements(classSymbol, validationSymbol).ToArray())),
152+
.AddStatements(EnumerateValidationStatements(classSymbol, validationSymbol, observablePropertySymbol).ToArray())),
149153
ReturnStatement(IdentifierName("ValidateAllProperties")))))))
150154
.NormalizeWhitespace()
151155
.ToFullString();
@@ -159,28 +163,47 @@ public void Execute(GeneratorExecutionContext context)
159163
}
160164

161165
/// <summary>
162-
/// Gets a sequence of statements to validate declared properties.
166+
/// Gets a sequence of statements to validate declared properties (including generated ones).
163167
/// </summary>
164168
/// <param name="classSymbol">The input <see cref="INamedTypeSymbol"/> instance to process.</param>
165169
/// <param name="validationSymbol">The type symbol for the <c>ValidationAttribute</c> type.</param>
170+
/// <param name="observablePropertySymbol">The type symbol for the <c>ObservablePropertyAttribute</c> type.</param>
166171
/// <returns>The sequence of <see cref="StatementSyntax"/> instances to validate declared properties.</returns>
167172
[Pure]
168-
private static IEnumerable<StatementSyntax> EnumerateValidationStatements(INamedTypeSymbol classSymbol, INamedTypeSymbol validationSymbol)
173+
private static IEnumerable<StatementSyntax> EnumerateValidationStatements(INamedTypeSymbol classSymbol, INamedTypeSymbol validationSymbol, INamedTypeSymbol observablePropertySymbol)
169174
{
170-
foreach (var propertySymbol in classSymbol.GetMembers().OfType<IPropertySymbol>())
175+
foreach (var memberSymbol in classSymbol.GetMembers())
171176
{
172-
if (propertySymbol.IsIndexer)
177+
if (memberSymbol is not (IPropertySymbol { IsIndexer: false } or IFieldSymbol))
173178
{
174179
continue;
175180
}
176181

177-
ImmutableArray<AttributeData> attributes = propertySymbol.GetAttributes();
182+
ImmutableArray<AttributeData> attributes = memberSymbol.GetAttributes();
178183

184+
// Also include fields that are annotated with [ObservableProperty]. This is necessary because
185+
// all generators run in an undefined order and looking at the same original compilation, so the
186+
// current one wouldn't be able to see generated properties from other generators directly.
187+
if (memberSymbol is IFieldSymbol &&
188+
!attributes.Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, observablePropertySymbol)))
189+
{
190+
continue;
191+
}
192+
193+
// Skip the current member if there are no validation attributes applied to it
179194
if (!attributes.Any(a => a.AttributeClass?.InheritsFrom(validationSymbol) == true))
180195
{
181196
continue;
182197
}
183198

199+
// Get the target property name either directly or matching the generated one
200+
string propertyName = memberSymbol switch
201+
{
202+
IPropertySymbol propertySymbol => propertySymbol.Name,
203+
IFieldSymbol fieldSymbol => ObservablePropertyGenerator.GetGeneratedPropertyName(fieldSymbol),
204+
_ => throw new InvalidOperationException("Invalid symbol type")
205+
};
206+
184207
// This enumerator produces a sequence of statements as follows:
185208
//
186209
// __ObservableValidatorHelper.ValidateProperty(instance, instance.<PROPERTY_0>, nameof(instance.<PROPERTY_0>));
@@ -200,14 +223,14 @@ private static IEnumerable<StatementSyntax> EnumerateValidationStatements(INamed
200223
MemberAccessExpression(
201224
SyntaxKind.SimpleMemberAccessExpression,
202225
IdentifierName("instance"),
203-
IdentifierName(propertySymbol.Name))),
226+
IdentifierName(propertyName))),
204227
Argument(
205228
InvocationExpression(IdentifierName("nameof"))
206229
.AddArgumentListArguments(Argument(
207230
MemberAccessExpression(
208231
SyntaxKind.SimpleMemberAccessExpression,
209232
IdentifierName("instance"),
210-
IdentifierName(propertySymbol.Name)))))));
233+
IdentifierName(propertyName)))))));
211234
}
212235
}
213236
}

UnitTests/UnitTests.NetCore/Mvvm/Test_ObservablePropertyAttribute.cs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.ComponentModel;
88
using System.ComponentModel.DataAnnotations;
99
using System.Diagnostics.CodeAnalysis;
10+
using System.Linq;
1011
using System.Reflection;
1112
using Microsoft.Toolkit.Mvvm.ComponentModel;
1213
using Microsoft.VisualStudio.TestTools.UnitTesting;
@@ -155,6 +156,33 @@ public void Test_ObservablePropertyWithValueNamedField_WithValidationAttributes(
155156
CollectionAssert.AreEqual(new[] { nameof(model.Value) }, propertyNames);
156157
}
157158

159+
// See https://github.com/CommunityToolkit/WindowsCommunityToolkit/issues/4184
160+
[TestCategory("Mvvm")]
161+
[TestMethod]
162+
public void Test_GeneratedPropertiesWithValidationAttributesOverFields()
163+
{
164+
var model = new ViewModelWithValidatableGeneratedProperties();
165+
166+
List<string?> propertyNames = new();
167+
168+
model.PropertyChanged += (s, e) => propertyNames.Add(e.PropertyName);
169+
170+
// Assign these fields directly to bypass the validation that is executed in the generated setters.
171+
// We only need those generated properties to be there to check whether they are correctly detected.
172+
model.first = "A";
173+
model.last = "This is a very long name that exceeds the maximum length of 60 for this property";
174+
175+
Assert.IsFalse(model.HasErrors);
176+
177+
model.RunValidation();
178+
179+
Assert.IsTrue(model.HasErrors);
180+
181+
ValidationResult[] validationErrors = model.GetErrors().ToArray();
182+
183+
Assert.AreEqual(validationErrors.Length, 2);
184+
}
185+
158186
public partial class SampleModel : ObservableObject
159187
{
160188
/// <summary>
@@ -245,5 +273,24 @@ public partial class ModelWithValuePropertyWithValidation : ObservableValidator
245273
[MinLength(5)]
246274
private string value;
247275
}
276+
277+
public partial class ViewModelWithValidatableGeneratedProperties : ObservableValidator
278+
{
279+
[Required]
280+
[MinLength(2)]
281+
[MaxLength(60)]
282+
[Display(Name = "FirstName")]
283+
[ObservableProperty]
284+
public string first = "Bob";
285+
286+
[Display(Name = "LastName")]
287+
[Required]
288+
[MinLength(2)]
289+
[MaxLength(60)]
290+
[ObservableProperty]
291+
public string last = "Jones";
292+
293+
public void RunValidation() => ValidateAllProperties();
294+
}
248295
}
249296
}

0 commit comments

Comments
 (0)