Skip to content

Commit 48587bc

Browse files
committed
Add diagnostics for invalid target of [AlsoNotifyChangeFor]
1 parent 53f9abf commit 48587bc

File tree

4 files changed

+63
-10
lines changed

4 files changed

+63
-10
lines changed

CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Unshipped.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,4 @@ MVVMTK0011 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error |
1919
MVVMTK0012 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error | See https://aka.ms/mvvmtoolkit/error
2020
MVVMTK0013 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error | See https://aka.ms/mvvmtoolkit/error
2121
MVVMTK0014 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error | See https://aka.ms/mvvmtoolkit/error
22+
MVVMTK0015 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error | See https://aka.ms/mvvmtoolkit/error

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

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,8 @@ public static PropertyInfo GetInfo(IFieldSymbol fieldSymbol, out ImmutableArray<
7373
// Gather attributes info
7474
foreach (AttributeData attributeData in fieldSymbol.GetAttributes())
7575
{
76-
// Add dependent property notifications, if needed
77-
if (attributeData.AttributeClass?.HasFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.AlsoNotifyChangeForAttribute") == true)
76+
if (TryGatherDependentPropertyChangedNames(fieldSymbol, attributeData, propertyChangedNames, builder))
7877
{
79-
foreach (string dependentPropertyName in attributeData.GetConstructorArguments<string>())
80-
{
81-
propertyChangedNames.Add(dependentPropertyName);
82-
}
8378
}
8479
else if (attributeData.AttributeClass?.HasFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.AlsoNotifyCanExecuteForAttribute") == true)
8580
{
@@ -121,6 +116,46 @@ public static PropertyInfo GetInfo(IFieldSymbol fieldSymbol, out ImmutableArray<
121116
validationAttributes.ToImmutable());
122117
}
123118

119+
/// <summary>
120+
/// Tries to gather dependent properties from the given attribute.
121+
/// </summary>
122+
/// <param name="fieldSymbol">The input <see cref="IFieldSymbol"/> instance to process.</param>
123+
/// <param name="attributeData">The <see cref="AttributeData"/> instance for <paramref name="fieldSymbol"/>.</param>
124+
/// <param name="propertyChangedNames">The target collection of dependent property names to populate.</param>
125+
/// <param name="diagnostics">The current collection of gathered diagnostics.</param>
126+
/// <returns>Whether or not <paramref name="attributeData"/> was an attribute containing any dependent properties.</returns>
127+
private static bool TryGatherDependentPropertyChangedNames(
128+
IFieldSymbol fieldSymbol,
129+
AttributeData attributeData,
130+
ImmutableArray<string>.Builder propertyChangedNames,
131+
ImmutableArray<Diagnostic>.Builder diagnostics)
132+
{
133+
if (attributeData.AttributeClass?.HasFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.AlsoNotifyChangeForAttribute") == true)
134+
{
135+
foreach (string? dependentPropertyName in attributeData.GetConstructorArguments<string>())
136+
{
137+
// Each target must be a string matching the name of a property from the containing type of the annotated field
138+
if (dependentPropertyName is { Length: > 0 } &&
139+
fieldSymbol.ContainingType.GetMembers(dependentPropertyName).OfType<IPropertySymbol>().Any())
140+
{
141+
propertyChangedNames.Add(dependentPropertyName);
142+
}
143+
else
144+
{
145+
diagnostics.Add(
146+
AlsoNotifyChangeForInvalidTargetError,
147+
fieldSymbol,
148+
dependentPropertyName ?? "",
149+
fieldSymbol.ContainingType);
150+
}
151+
}
152+
153+
return true;
154+
}
155+
156+
return false;
157+
}
158+
124159
/// <summary>
125160
/// Gets a <see cref="CompilationUnitSyntax"/> instance with the cached args for property changing notifications.
126161
/// </summary>

CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,4 +235,20 @@ internal static class DiagnosticDescriptors
235235
isEnabledByDefault: true,
236236
description: $"The name of fields annotated with [ObservableProperty] should use \"lowerCamel\", \"_lowerCamel\" or \"m_lowerCamel\" pattern to avoid collisions with the generated properties.",
237237
helpLinkUri: "https://aka.ms/mvvmtoolkit");
238+
239+
/// <summary>
240+
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when the specified target for <c>[AlsoNotifyChangeFor]</c> is not valid.
241+
/// <para>
242+
/// Format: <c>"The target(s) of [AlsoNotifyChangeFor] must be an accessible property, but "{0}" has no matches in type {1}</c>.
243+
/// </para>
244+
/// </summary>
245+
public static readonly DiagnosticDescriptor AlsoNotifyChangeForInvalidTargetError = new DiagnosticDescriptor(
246+
id: "MVVMTK0015",
247+
title: "Name collision for generated property",
248+
messageFormat: "The target(s) of [AlsoNotifyChangeFor] must be an accessible property, but \"{0}\" has no matches in type {1}",
249+
category: typeof(ObservablePropertyGenerator).FullName,
250+
defaultSeverity: DiagnosticSeverity.Error,
251+
isEnabledByDefault: true,
252+
description: "The target(s) of [AlsoNotifyChangeFor] must be an accessible property in its parent type.",
253+
helpLinkUri: "https://aka.ms/mvvmtoolkit");
238254
}

CommunityToolkit.Mvvm.SourceGenerators/Extensions/AttributeDataExtensions.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,16 @@ public static bool TryGetNamedArgument<T>(this AttributeData attributeData, stri
8787
/// <typeparam name="T">The type of constructor arguments to retrieve.</typeparam>
8888
/// <param name="attributeData">The target <see cref="AttributeData"/> instance to get the arguments from.</param>
8989
/// <returns>A sequence of all constructor arguments of the specified type from <paramref name="attributeData"/>.</returns>
90-
public static IEnumerable<T> GetConstructorArguments<T>(this AttributeData attributeData)
90+
public static IEnumerable<T?> GetConstructorArguments<T>(this AttributeData attributeData)
91+
where T : class
9192
{
92-
static IEnumerable<T> Enumerate(IEnumerable<TypedConstant> constants)
93+
static IEnumerable<T?> Enumerate(IEnumerable<TypedConstant> constants)
9394
{
9495
foreach (TypedConstant constant in constants)
9596
{
9697
if (constant.IsNull)
9798
{
98-
continue;
99+
yield return null;
99100
}
100101

101102
if (constant.Kind == TypedConstantKind.Primitive &&
@@ -105,7 +106,7 @@ static IEnumerable<T> Enumerate(IEnumerable<TypedConstant> constants)
105106
}
106107
else if (constant.Kind == TypedConstantKind.Array)
107108
{
108-
foreach (T item in Enumerate(constant.Values))
109+
foreach (T? item in Enumerate(constant.Values))
109110
{
110111
yield return item;
111112
}

0 commit comments

Comments
 (0)