Skip to content

Commit d8af528

Browse files
committed
Fixed ValidateAllProperties generation for generated properties
1 parent b6ec0bc commit d8af528

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
@@ -459,7 +459,7 @@ private static PropertyDeclarationSyntax CreatePropertyDeclaration(
459459
/// <param name="fieldSymbol">The input <see cref="IFieldSymbol"/> instance to process.</param>
460460
/// <returns>The generated property name for <paramref name="fieldSymbol"/>.</returns>
461461
[Pure]
462-
private static string GetGeneratedPropertyName(IFieldSymbol fieldSymbol)
462+
public static string GetGeneratedPropertyName(IFieldSymbol fieldSymbol)
463463
{
464464
string propertyName = fieldSymbol.Name;
465465

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

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

19+
#pragma warning disable SA1008
20+
1921
namespace Microsoft.Toolkit.Mvvm.SourceGenerators
2022
{
2123
/// <summary>
@@ -46,8 +48,10 @@ public void Execute(GeneratorExecutionContext context)
4648
context.ReportDiagnostic(Diagnostic.Create(UnsupportedCSharpLanguageVersionError, null));
4749
}
4850

49-
// Get the symbol for the ValidationAttribute type
50-
INamedTypeSymbol validationSymbol = context.Compilation.GetTypeByMetadataName("System.ComponentModel.DataAnnotations.ValidationAttribute")!;
51+
// Get the symbol for the required attributes
52+
INamedTypeSymbol
53+
validationSymbol = context.Compilation.GetTypeByMetadataName("System.ComponentModel.DataAnnotations.ValidationAttribute")!,
54+
observablePropertySymbol = context.Compilation.GetTypeByMetadataName("Microsoft.Toolkit.Mvvm.ComponentModel.ObservablePropertyAttribute")!;
5155

5256
// Prepare the attributes to add to the first class declaration
5357
AttributeListSyntax[] classAttributes = new[]
@@ -145,14 +149,14 @@ public void Execute(GeneratorExecutionContext context)
145149
Parameter(Identifier("obj")).WithType(PredefinedType(Token(SyntaxKind.ObjectKeyword))))
146150
.WithBody(Block(
147151
LocalDeclarationStatement(
148-
VariableDeclaration(IdentifierName("var")) // Cannot Token(SyntaxKind.VarKeyword) here (throws an ArgumentException)
152+
VariableDeclaration(IdentifierName("var")) // Cannot use Token(SyntaxKind.VarKeyword) here (throws an ArgumentException)
149153
.AddVariables(
150154
VariableDeclarator(Identifier("instance"))
151155
.WithInitializer(EqualsValueClause(
152156
CastExpression(
153157
IdentifierName(classSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)),
154158
IdentifierName("obj")))))))
155-
.AddStatements(EnumerateValidationStatements(classSymbol, validationSymbol).ToArray())),
159+
.AddStatements(EnumerateValidationStatements(classSymbol, validationSymbol, observablePropertySymbol).ToArray())),
156160
ReturnStatement(IdentifierName("ValidateAllProperties")))))))
157161
.NormalizeWhitespace()
158162
.ToFullString();
@@ -166,28 +170,47 @@ public void Execute(GeneratorExecutionContext context)
166170
}
167171

168172
/// <summary>
169-
/// Gets a sequence of statements to validate declared properties.
173+
/// Gets a sequence of statements to validate declared properties (including generated ones).
170174
/// </summary>
171175
/// <param name="classSymbol">The input <see cref="INamedTypeSymbol"/> instance to process.</param>
172176
/// <param name="validationSymbol">The type symbol for the <c>ValidationAttribute</c> type.</param>
177+
/// <param name="observablePropertySymbol">The type symbol for the <c>ObservablePropertyAttribute</c> type.</param>
173178
/// <returns>The sequence of <see cref="StatementSyntax"/> instances to validate declared properties.</returns>
174179
[Pure]
175-
private static IEnumerable<StatementSyntax> EnumerateValidationStatements(INamedTypeSymbol classSymbol, INamedTypeSymbol validationSymbol)
180+
private static IEnumerable<StatementSyntax> EnumerateValidationStatements(INamedTypeSymbol classSymbol, INamedTypeSymbol validationSymbol, INamedTypeSymbol observablePropertySymbol)
176181
{
177-
foreach (var propertySymbol in classSymbol.GetMembers().OfType<IPropertySymbol>())
182+
foreach (var memberSymbol in classSymbol.GetMembers())
178183
{
179-
if (propertySymbol.IsIndexer)
184+
if (memberSymbol is not (IPropertySymbol { IsIndexer: false } or IFieldSymbol))
180185
{
181186
continue;
182187
}
183188

184-
ImmutableArray<AttributeData> attributes = propertySymbol.GetAttributes();
189+
ImmutableArray<AttributeData> attributes = memberSymbol.GetAttributes();
185190

191+
// Also include fields that are annotated with [ObservableProperty]. This is necessary because
192+
// all generators run in an undefined order and looking at the same original compilation, so the
193+
// current one wouldn't be able to see generated properties from other generators directly.
194+
if (memberSymbol is IFieldSymbol &&
195+
!attributes.Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, observablePropertySymbol)))
196+
{
197+
continue;
198+
}
199+
200+
// Skip the current member if there are no validation attributes applied to it
186201
if (!attributes.Any(a => a.AttributeClass?.InheritsFrom(validationSymbol) == true))
187202
{
188203
continue;
189204
}
190205

206+
// Get the target property name either directly or matching the generated one
207+
string propertyName = memberSymbol switch
208+
{
209+
IPropertySymbol propertySymbol => propertySymbol.Name,
210+
IFieldSymbol fieldSymbol => ObservablePropertyGenerator.GetGeneratedPropertyName(fieldSymbol),
211+
_ => throw new InvalidOperationException("Invalid symbol type")
212+
};
213+
191214
// This enumerator produces a sequence of statements as follows:
192215
//
193216
// __ObservableValidatorHelper.ValidateProperty(instance, instance.<PROPERTY_0>, nameof(instance.<PROPERTY_0>));
@@ -207,14 +230,14 @@ private static IEnumerable<StatementSyntax> EnumerateValidationStatements(INamed
207230
MemberAccessExpression(
208231
SyntaxKind.SimpleMemberAccessExpression,
209232
IdentifierName("instance"),
210-
IdentifierName(propertySymbol.Name))),
233+
IdentifierName(propertyName))),
211234
Argument(
212235
InvocationExpression(IdentifierName("nameof"))
213236
.AddArgumentListArguments(Argument(
214237
MemberAccessExpression(
215238
SyntaxKind.SimpleMemberAccessExpression,
216239
IdentifierName("instance"),
217-
IdentifierName(propertySymbol.Name)))))));
240+
IdentifierName(propertyName)))))));
218241
}
219242
}
220243
}

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)