Skip to content

Commit ccb8a9d

Browse files
Sergio0694Sergio0694
committed
Fixed bugs in new diagnostics logic
1 parent 929bef6 commit ccb8a9d

File tree

7 files changed

+112
-67
lines changed

7 files changed

+112
-67
lines changed

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

Lines changed: 61 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ public static PropertyInfo GetInfo(IFieldSymbol fieldSymbol, out ImmutableArray<
3535
ImmutableArray<Diagnostic>.Builder builder = ImmutableArray.CreateBuilder<Diagnostic>();
3636

3737
// Check whether the containing type implements INotifyPropertyChanging and whether it inherits from ObservableValidator
38-
bool isObservableObject = fieldSymbol.ContainingType.InheritsFrom("global::CommunityToolkit.Mvvm.ComponentModel.ObservableObject");
39-
bool isObservableValidator = fieldSymbol.ContainingType.InheritsFrom("global::CommunityToolkit.Mvvm.ComponentModel.ObservableValidator");
38+
bool isObservableObject = fieldSymbol.ContainingType.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableObject");
39+
bool isObservableValidator = fieldSymbol.ContainingType.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableValidator");
4040
bool isNotifyPropertyChanging = fieldSymbol.ContainingType.AllInterfaces.Any(static i => i.HasFullyQualifiedName("global::System.ComponentModel.INotifyPropertyChanging"));
4141
bool hasObservableObjectAttribute = fieldSymbol.ContainingType.GetAttributes().Any(static a => a.AttributeClass?.HasFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableObjectAttribute") == true);
4242

@@ -81,7 +81,7 @@ public static PropertyInfo GetInfo(IFieldSymbol fieldSymbol, out ImmutableArray<
8181
}
8282

8383
// Track the current validation attribute, if applicable
84-
if (attributeData.AttributeClass?.InheritsFrom("global::System.ComponentModel.DataAnnotations.ValidationAttribute") == true)
84+
if (attributeData.AttributeClass?.InheritsFromFullyQualifiedName("global::System.ComponentModel.DataAnnotations.ValidationAttribute") == true)
8585
{
8686
validationAttributes.Add(AttributeInfo.From(attributeData));
8787
}
@@ -130,42 +130,44 @@ private static bool TryGatherDependentPropertyChangedNames(
130130
{
131131
foreach (string? dependentPropertyName in attributeData.GetConstructorArguments<string>())
132132
{
133+
if (dependentPropertyName is null or "")
134+
{
135+
goto InvalidProperty;
136+
}
137+
133138
// Each target must be a string matching the name of a property from the containing type of the annotated field
134-
if (dependentPropertyName is { Length: > 0 } &&
135-
fieldSymbol.ContainingType.GetMembers(dependentPropertyName).OfType<IPropertySymbol>().Any())
139+
if (fieldSymbol.ContainingType.GetMembers(dependentPropertyName).OfType<IPropertySymbol>().Any())
136140
{
137141
propertyChangedNames.Add(dependentPropertyName);
142+
143+
goto ValidProperty;
138144
}
139-
else
140-
{
141-
bool isTargetValid = false;
142145

143-
// Check for generated properties too
144-
foreach (ISymbol member in fieldSymbol.ContainingType.GetMembers())
146+
// Check for generated properties too
147+
foreach (ISymbol member in fieldSymbol.ContainingType.GetMembers())
148+
{
149+
if (member is IFieldSymbol otherFieldSymbol &&
150+
!SymbolEqualityComparer.Default.Equals(fieldSymbol, otherFieldSymbol) &&
151+
otherFieldSymbol.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") &&
152+
dependentPropertyName == GetGeneratedPropertyName(otherFieldSymbol))
145153
{
146-
if (member is IFieldSymbol otherFieldSymbol &&
147-
!SymbolEqualityComparer.Default.Equals(fieldSymbol, otherFieldSymbol) &&
148-
otherFieldSymbol.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") &&
149-
dependentPropertyName == GetGeneratedPropertyName(otherFieldSymbol))
150-
{
151-
propertyChangedNames.Add(dependentPropertyName);
154+
propertyChangedNames.Add(dependentPropertyName);
152155

153-
isTargetValid = true;
154-
155-
break;
156-
}
157-
}
158-
159-
// Add the diagnostic if the target is definitely invalid
160-
if (!isTargetValid)
161-
{
162-
diagnostics.Add(
163-
AlsoNotifyChangeForInvalidTargetError,
164-
fieldSymbol,
165-
dependentPropertyName ?? "",
166-
fieldSymbol.ContainingType);
156+
goto ValidProperty;
167157
}
168158
}
159+
160+
ValidProperty:
161+
162+
continue;
163+
164+
InvalidProperty:
165+
166+
diagnostics.Add(
167+
AlsoNotifyChangeForInvalidTargetError,
168+
fieldSymbol,
169+
dependentPropertyName ?? "",
170+
fieldSymbol.ContainingType);
169171
}
170172

171173
return true;
@@ -192,44 +194,46 @@ private static bool TryGatherDependentCommandNames(
192194
{
193195
foreach (string? commandName in attributeData.GetConstructorArguments<string>())
194196
{
197+
if (commandName is null or "")
198+
{
199+
goto InvalidProperty;
200+
}
201+
195202
// Each target must be a string matching the name of a property from the containing type of the annotated field, and the
196203
// property must be of type IRelayCommand, or any type that implements that interface (to avoid generating invalid code).
197-
if (commandName is { Length: > 0 } &&
198-
fieldSymbol.ContainingType.GetMembers(commandName).OfType<IPropertySymbol>().FirstOrDefault() is IPropertySymbol propertySymbol &&
204+
if (fieldSymbol.ContainingType.GetMembers(commandName).OfType<IPropertySymbol>().FirstOrDefault() is IPropertySymbol propertySymbol &&
199205
propertySymbol is INamedTypeSymbol typeSymbol &&
200-
typeSymbol.InheritsFrom("global::CommunityToolkit.Mvvm.Input.IRelayCommand"))
206+
typeSymbol.HasInterfaceWithFullyQualifiedName("global::CommunityToolkit.Mvvm.Input.IRelayCommand"))
201207
{
202208
notifiedCommandNames.Add(commandName);
209+
210+
goto ValidProperty;
203211
}
204-
else
205-
{
206-
bool isTargetValid = false;
207212

208-
// Check for generated commands too
209-
foreach (ISymbol member in fieldSymbol.ContainingType.GetMembers())
213+
// Check for generated commands too
214+
foreach (ISymbol member in fieldSymbol.ContainingType.GetMembers())
215+
{
216+
if (member is IMethodSymbol methodSymbol &&
217+
methodSymbol.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.Input.ICommandAttribute") &&
218+
commandName == ICommandGenerator.Execute.GetGeneratedFieldAndPropertyNames(methodSymbol).PropertyName)
210219
{
211-
if (member is IMethodSymbol methodSymbol &&
212-
methodSymbol.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.Input.ICommandAttribute") &&
213-
commandName == ICommandGenerator.Execute.GetGeneratedFieldAndPropertyNames(methodSymbol).PropertyName)
214-
{
215-
notifiedCommandNames.Add(commandName);
220+
notifiedCommandNames.Add(commandName);
216221

217-
isTargetValid = true;
218-
219-
break;
220-
}
221-
}
222-
223-
// Add the diagnostic if the target is definitely invalid
224-
if (!isTargetValid)
225-
{
226-
diagnostics.Add(
227-
AlsoNotifyCanExecuteForInvalidTargetError,
228-
fieldSymbol,
229-
commandName ?? "",
230-
fieldSymbol.ContainingType);
222+
goto ValidProperty;
231223
}
232224
}
225+
226+
ValidProperty:
227+
228+
continue;
229+
230+
InvalidProperty:
231+
232+
diagnostics.Add(
233+
AlsoNotifyCanExecuteForInvalidTargetError,
234+
fieldSymbol,
235+
commandName ?? "",
236+
fieldSymbol.ContainingType);
233237
}
234238

235239
return true;

‎CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservableRecipientGenerator.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ static ObservableRecipientInfo GetInfo(INamedTypeSymbol typeSymbol, AttributeDat
3939
string typeName = typeSymbol.Name;
4040
bool hasExplicitConstructors = !(typeSymbol.InstanceConstructors.Length == 1 && typeSymbol.InstanceConstructors[0] is { Parameters.IsEmpty: true, IsImplicitlyDeclared: true });
4141
bool isAbstract = typeSymbol.IsAbstract;
42-
bool isObservableValidator = typeSymbol.InheritsFrom("global::CommunityToolkit.Mvvm.ComponentModel.ObservableValidator");
42+
bool isObservableValidator = typeSymbol.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableValidator");
4343
bool hasOnActivatedMethod = typeSymbol.GetMembers().Any(m => m is IMethodSymbol { Parameters.IsEmpty: true, Name: "OnActivated" });
4444
bool hasOnDeactivatedMethod = typeSymbol.GetMembers().Any(m => m is IMethodSymbol { Parameters.IsEmpty: true, Name: "OnDeactivated" });
4545

@@ -70,7 +70,7 @@ protected override bool ValidateTargetType(INamedTypeSymbol typeSymbol, Observab
7070
ImmutableArray<Diagnostic>.Builder builder = ImmutableArray.CreateBuilder<Diagnostic>();
7171

7272
// Check if the type already inherits from ObservableRecipient
73-
if (typeSymbol.InheritsFrom("global::CommunityToolkit.Mvvm.ComponentModel.ObservableRecipient"))
73+
if (typeSymbol.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableRecipient"))
7474
{
7575
builder.Add(DuplicateObservableRecipientError, typeSymbol, typeSymbol);
7676

@@ -81,7 +81,7 @@ protected override bool ValidateTargetType(INamedTypeSymbol typeSymbol, Observab
8181

8282
// In order to use [ObservableRecipient], the target type needs to inherit from ObservableObject,
8383
// or be annotated with [ObservableObject] or [INotifyPropertyChanged] (with additional helpers).
84-
if (!typeSymbol.InheritsFrom("global::CommunityToolkit.Mvvm.ComponentModel.ObservableObject") &&
84+
if (!typeSymbol.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableObject") &&
8585
!typeSymbol.HasOrInheritsAttribute(static a => a.AttributeClass?.HasFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableObjectAttribute") == true) &&
8686
!typeSymbol.HasOrInheritsAttribute(static a =>
8787
a.AttributeClass?.HasFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.INotifyPropertyChangedAttribute") == true &&

‎CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservableValidatorValidateAllPropertiesGenerator.Execute.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ private static class Execute
2929
/// <returns>Whether <paramref name="typeSymbol"/> inherits from <c>ObservableValidator</c>.</returns>
3030
public static bool IsObservableValidator(INamedTypeSymbol typeSymbol)
3131
{
32-
return typeSymbol.InheritsFrom("global::CommunityToolkit.Mvvm.ComponentModel.ObservableValidator");
32+
return typeSymbol.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableValidator");
3333
}
3434

3535
/// <summary>
@@ -61,7 +61,7 @@ public static ValidationInfo GetInfo(INamedTypeSymbol typeSymbol)
6161
}
6262

6363
// Skip the current member if there are no validation attributes applied to it
64-
if (!attributes.Any(a => a.AttributeClass?.InheritsFrom(
64+
if (!attributes.Any(a => a.AttributeClass?.InheritsFromFullyQualifiedName(
6565
"global::System.ComponentModel.DataAnnotations.ValidationAttribute") == true))
6666
{
6767
continue;

‎CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,17 +239,17 @@ internal static class DiagnosticDescriptors
239239
/// <summary>
240240
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when the specified target for <c>[AlsoNotifyChangeFor]</c> is not valid.
241241
/// <para>
242-
/// Format: <c>"The target(s) of [AlsoNotifyChangeFor] must be an accessible property, but "{0}" has no matches in type {1}</c>.
242+
/// Format: <c>"The target(s) of [AlsoNotifyChangeFor] must be a (different) accessible property, but "{0}" has no (other) matches in type {1}</c>.
243243
/// </para>
244244
/// </summary>
245245
public static readonly DiagnosticDescriptor AlsoNotifyChangeForInvalidTargetError = new DiagnosticDescriptor(
246246
id: "MVVMTK0015",
247247
title: "Invalid target name for [AlsoNotifyChangeFor]",
248-
messageFormat: "The target(s) of [AlsoNotifyChangeFor] must be an accessible property, but \"{0}\" has no matches in type {1}",
248+
messageFormat: "The target(s) of [AlsoNotifyChangeFor] must be a (different) accessible property, but \"{0}\" has no (other) matches in type {1}",
249249
category: typeof(ObservablePropertyGenerator).FullName,
250250
defaultSeverity: DiagnosticSeverity.Error,
251251
isEnabledByDefault: true,
252-
description: "The target(s) of [AlsoNotifyChangeFor] must be an accessible property in its parent type.",
252+
description: "The target(s) of [AlsoNotifyChangeFor] must be a (different) accessible property in its parent type.",
253253
helpLinkUri: "https://aka.ms/mvvmtoolkit");
254254

255255
/// <summary>

‎CommunityToolkit.Mvvm.SourceGenerators/Extensions/INamedTypeSymbolExtensions.cs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ static StringBuilder BuildFrom(ISymbol? symbol, StringBuilder builder)
4848
/// <param name="typeSymbol">The target <see cref="INamedTypeSymbol"/> instance to check.</param>
4949
/// <param name="name">The full name of the type to check for inheritance.</param>
5050
/// <returns>Whether or not <paramref name="typeSymbol"/> inherits from <paramref name="name"/>.</returns>
51-
public static bool InheritsFrom(this INamedTypeSymbol typeSymbol, string name)
51+
public static bool InheritsFromFullyQualifiedName(this INamedTypeSymbol typeSymbol, string name)
5252
{
5353
INamedTypeSymbol? baseType = typeSymbol.BaseType;
5454

@@ -65,6 +65,25 @@ public static bool InheritsFrom(this INamedTypeSymbol typeSymbol, string name)
6565
return false;
6666
}
6767

68+
/// <summary>
69+
/// Checks whether or not a given <see cref="INamedTypeSymbol"/> implements an interface with a specied name.
70+
/// </summary>
71+
/// <param name="typeSymbol">The target <see cref="INamedTypeSymbol"/> instance to check.</param>
72+
/// <param name="name">The full name of the type to check for interface implementation.</param>
73+
/// <returns>Whether or not <paramref name="typeSymbol"/> has an interface with the specified name.</returns>
74+
public static bool HasInterfaceWithFullyQualifiedName(this INamedTypeSymbol typeSymbol, string name)
75+
{
76+
foreach (INamedTypeSymbol interfaceType in typeSymbol.AllInterfaces)
77+
{
78+
if (interfaceType.HasFullyQualifiedName(name))
79+
{
80+
return true;
81+
}
82+
}
83+
84+
return false;
85+
}
86+
6887
/// <summary>
6988
/// Checks whether or not a given <see cref="INamedTypeSymbol"/> has or inherits a specified attribute.
7089
/// </summary>

‎tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,26 @@ public partial class SampleViewModel : ObservableObject
655655
VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0015");
656656
}
657657

658+
[TestMethod]
659+
public void AlsoNotifyChangeForInvalidTargetError_SamePropertyAsGeneratedOneFromSelf()
660+
{
661+
string source = @"
662+
using System.ComponentModel.DataAnnotations;
663+
using CommunityToolkit.Mvvm.ComponentModel;
664+
665+
namespace MyApp
666+
{
667+
public partial class SampleViewModel : ObservableObject
668+
{
669+
[ObservableProperty]
670+
[AlsoNotifyChangeFor(nameof(Name))]
671+
private string Name;
672+
}
673+
}";
674+
675+
VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0015");
676+
}
677+
658678
[TestMethod]
659679
public void AlsoNotifyChangeForInvalidTargetError_Missing()
660680
{

‎tests/CommunityToolkit.Mvvm.UnitTests/Test_SourceGeneratorAttributes.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,12 @@ public partial class INotifyPropertyChangedModel
3636
public partial class TestModel
3737
{
3838
[ObservableProperty]
39-
[AlsoNotifyChangeFor(nameof(Name))]
39+
[AlsoNotifyChangeFor(nameof(Foo))]
4040
[AlsoNotifyCanExecuteFor(nameof(TestCommand))]
4141
public string? name;
4242

43+
public string? Foo { get; set; }
44+
4345
[ICommand]
4446
public void Test()
4547
{

0 commit comments

Comments
 (0)