Skip to content

Commit 5876242

Browse files
authored
Merge pull request #434 from CommunityToolkit/dev/orphaned-fields-analyzer
Move diagnostics for orphaned observable property attributes to analyzer
2 parents 82ccd6c + b8749fe commit 5876242

File tree

4 files changed

+125
-57
lines changed

4 files changed

+125
-57
lines changed

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

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -204,19 +204,6 @@ internal static class Execute
204204
forwardedAttributes.ToImmutable());
205205
}
206206

207-
/// <summary>
208-
/// Gets the diagnostics for a field with invalid attribute uses.
209-
/// </summary>
210-
/// <param name="fieldSymbol">The input <see cref="IFieldSymbol"/> instance to process.</param>
211-
/// <returns>The resulting <see cref="Diagnostic"/> instance for <paramref name="fieldSymbol"/>.</returns>
212-
public static Diagnostic GetDiagnosticForFieldWithOrphanedDependentAttributes(IFieldSymbol fieldSymbol)
213-
{
214-
return FieldWithOrphanedDependentObservablePropertyAttributesError.CreateDiagnostic(
215-
fieldSymbol,
216-
fieldSymbol.ContainingType,
217-
fieldSymbol.Name);
218-
}
219-
220207
/// <summary>
221208
/// Validates the containing type for a given field being annotated.
222209
/// </summary>

CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.cs

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -46,20 +46,6 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
4646
fieldSymbols
4747
.Where(static item => item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute"));
4848

49-
// Get diagnostics for fields using [NotifyPropertyChangedFor], [NotifyCanExecuteChangedFor], [NotifyPropertyChangedRecipients] and [NotifyDataErrorInfo], but not [ObservableProperty]
50-
IncrementalValuesProvider<Diagnostic> fieldSymbolsWithOrphanedDependentAttributeWithErrors =
51-
fieldSymbols
52-
.Where(static item =>
53-
(item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.NotifyPropertyChangedForAttribute") ||
54-
item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.NotifyCanExecuteChangedForAttribute") ||
55-
item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.NotifyPropertyChangedRecipientsAttribute") ||
56-
item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.NotifyDataErrorInfoAttribute")) &&
57-
!item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute"))
58-
.Select(static (item, _) => Execute.GetDiagnosticForFieldWithOrphanedDependentAttributes(item));
59-
60-
// Output the diagnostics
61-
context.ReportDiagnostics(fieldSymbolsWithOrphanedDependentAttributeWithErrors);
62-
6349
// Gather info for all annotated fields
6450
IncrementalValuesProvider<(HierarchyInfo Hierarchy, Result<PropertyInfo?> Info)> propertyInfoWithErrors =
6551
fieldSymbolsWithAttribute
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Collections.Generic;
6+
using System.Collections.Immutable;
7+
using System.Linq;
8+
using CommunityToolkit.Mvvm.SourceGenerators.Extensions;
9+
using Microsoft.CodeAnalysis;
10+
using Microsoft.CodeAnalysis.CSharp;
11+
using Microsoft.CodeAnalysis.Diagnostics;
12+
using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors;
13+
14+
namespace CommunityToolkit.Mvvm.SourceGenerators;
15+
16+
/// <summary>
17+
/// A diagnostic analyzer that generates an error whenever a field has an orphaned attribute that depends on <c>[ObservableProperty]</c>.
18+
/// </summary>
19+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
20+
public sealed class FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer : DiagnosticAnalyzer
21+
{
22+
/// <summary>
23+
/// The mapping of target attributes that will trigger the analyzer.
24+
/// </summary>
25+
private static readonly ImmutableDictionary<string, string> GeneratorAttributeNamesToFullyQualifiedNamesMap = ImmutableDictionary.CreateRange(new[]
26+
{
27+
new KeyValuePair<string, string>("NotifyCanExecuteChangedForAttribute", "CommunityToolkit.Mvvm.ComponentModel.NotifyCanExecuteChangedForAttribute"),
28+
new KeyValuePair<string, string>("NotifyDataErrorInfoAttribute", "CommunityToolkit.Mvvm.ComponentModel.NotifyDataErrorInfoAttribute"),
29+
new KeyValuePair<string, string>("NotifyPropertyChangedForAttribute", "CommunityToolkit.Mvvm.ComponentModel.NotifyPropertyChangedForAttribute"),
30+
new KeyValuePair<string, string>("NotifyPropertyChangedRecipientsAttribute", "CommunityToolkit.Mvvm.ComponentModel.NotifyPropertyChangedRecipientsAttribute")
31+
});
32+
33+
/// <inheritdoc/>
34+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(FieldWithOrphanedDependentObservablePropertyAttributesError);
35+
36+
/// <inheritdoc/>
37+
public override void Initialize(AnalysisContext context)
38+
{
39+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
40+
context.EnableConcurrentExecution();
41+
42+
// Defer the registration so it can be skipped if C# 8.0 or more is not available.
43+
// That is because in that case source generators are not supported at all anyaway.
44+
context.RegisterCompilationStartAction(static context =>
45+
{
46+
if (!context.Compilation.HasLanguageVersionAtLeastEqualTo(LanguageVersion.CSharp8))
47+
{
48+
return;
49+
}
50+
51+
context.RegisterSymbolAction(static context =>
52+
{
53+
ImmutableArray<AttributeData> attributes = context.Symbol.GetAttributes();
54+
55+
// If the symbol has no attributes, there's nothing left to do
56+
if (attributes.IsEmpty)
57+
{
58+
return;
59+
}
60+
61+
foreach (AttributeData dependentAttribute in attributes)
62+
{
63+
// Go over each attribute on the target symbol, anche check if any of them matches one of the trigger attributes.
64+
// The logic here is the same as the one in UnsupportedCSharpLanguageVersionAnalyzer, to minimize retrieving symbols.
65+
if (dependentAttribute.AttributeClass is { Name: string attributeName } dependentAttributeClass &&
66+
GeneratorAttributeNamesToFullyQualifiedNamesMap.TryGetValue(attributeName, out string? fullyQualifiedDependentAttributeName) &&
67+
context.Compilation.GetTypeByMetadataName(fullyQualifiedDependentAttributeName) is INamedTypeSymbol dependentAttributeSymbol &&
68+
SymbolEqualityComparer.Default.Equals(dependentAttributeClass, dependentAttributeSymbol))
69+
{
70+
// If the attribute matches, iterate over the attributes to try to find [ObservableProperty]
71+
foreach (AttributeData attribute in attributes)
72+
{
73+
if (attribute.AttributeClass is { Name: "ObservablePropertyAttribute" } attributeSymbol &&
74+
context.Compilation.GetTypeByMetadataName("CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") is INamedTypeSymbol observablePropertySymbol &&
75+
SymbolEqualityComparer.Default.Equals(attributeSymbol, observablePropertySymbol))
76+
{
77+
// If [ObservableProperty] is found, then this field is valid in that it doesn't have orphaned dependent attributes
78+
return;
79+
}
80+
}
81+
82+
context.ReportDiagnostic(Diagnostic.Create(FieldWithOrphanedDependentObservablePropertyAttributesError, context.Symbol.Locations.FirstOrDefault(), context.Symbol.ContainingType, context.Symbol.Name));
83+
84+
// Just like in UnsupportedCSharpLanguageVersionAnalyzer, stop if a diagnostic has been emitted for the current symbol
85+
return;
86+
}
87+
}
88+
}, SymbolKind.Field);
89+
});
90+
}
91+
}

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

Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
using System.Threading.Tasks;
1515
using CommunityToolkit.Mvvm.SourceGenerators.UnitTests.Helpers;
1616
using System.Text.RegularExpressions;
17+
using Microsoft.CodeAnalysis.Diagnostics;
1718

1819
namespace CommunityToolkit.Mvvm.SourceGenerators.UnitTests;
1920

@@ -247,7 +248,7 @@ public partial class {|MVVMTK0008:SampleViewModel|}
247248
}
248249
}";
249250

250-
await VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(source);
251+
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<UnsupportedCSharpLanguageVersionAnalyzer>(source, LanguageVersion.CSharp7_3);
251252
}
252253

253254
[TestMethod]
@@ -264,7 +265,7 @@ public partial class {|MVVMTK0008:SampleViewModel|}
264265
}
265266
}";
266267

267-
await VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(source);
268+
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<UnsupportedCSharpLanguageVersionAnalyzer>(source, LanguageVersion.CSharp7_3);
268269
}
269270

270271
[TestMethod]
@@ -283,7 +284,7 @@ public partial class {|MVVMTK0008:SampleViewModel|}
283284
}
284285
}";
285286

286-
await VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(source);
287+
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<UnsupportedCSharpLanguageVersionAnalyzer>(source, LanguageVersion.CSharp7_3);
287288
}
288289

289290
[TestMethod]
@@ -308,7 +309,7 @@ public partial class {|MVVMTK0008:SampleViewModel|}
308309
}
309310
}";
310311

311-
await VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(source);
312+
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<UnsupportedCSharpLanguageVersionAnalyzer>(source, LanguageVersion.CSharp7_3);
312313
}
313314

314315
[TestMethod]
@@ -327,7 +328,7 @@ public partial class SampleViewModel : ObservableValidator
327328
}
328329
}";
329330

330-
await VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(source);
331+
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<UnsupportedCSharpLanguageVersionAnalyzer>(source, LanguageVersion.CSharp7_3);
331332
}
332333

333334
[TestMethod]
@@ -347,7 +348,7 @@ public partial class SampleViewModel
347348
}
348349
}";
349350

350-
await VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(source);
351+
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<UnsupportedCSharpLanguageVersionAnalyzer>(source, LanguageVersion.CSharp7_3);
351352
}
352353

353354
[TestMethod]
@@ -370,7 +371,7 @@ public void Receive(MyMessage message)
370371
}
371372
}";
372373

373-
await VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(source);
374+
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<UnsupportedCSharpLanguageVersionAnalyzer>(source, LanguageVersion.CSharp7_3);
374375
}
375376

376377
[TestMethod]
@@ -980,7 +981,7 @@ public partial class MyViewModel : INotifyPropertyChanged
980981
}
981982

982983
[TestMethod]
983-
public void FieldWithOrphanedDependentObservablePropertyAttributesError_NotifyPropertyChangedFor()
984+
public async Task FieldWithOrphanedDependentObservablePropertyAttributesError_NotifyPropertyChangedFor()
984985
{
985986
string source = @"
986987
using CommunityToolkit.Mvvm.ComponentModel;
@@ -989,16 +990,16 @@ namespace MyApp
989990
{
990991
public partial class MyViewModel
991992
{
992-
[NotifyPropertyChangedFor("")]
993-
public int number;
993+
[NotifyPropertyChangedFor("""")]
994+
public int {|MVVMTK0020:number|};
994995
}
995996
}";
996997

997-
VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0020");
998+
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer>(source, LanguageVersion.CSharp8);
998999
}
9991000

10001001
[TestMethod]
1001-
public void FieldWithOrphanedDependentObservablePropertyAttributesError_NotifyCanExecuteChangedFor()
1002+
public async Task FieldWithOrphanedDependentObservablePropertyAttributesError_NotifyCanExecuteChangedFor()
10021003
{
10031004
string source = @"
10041005
using CommunityToolkit.Mvvm.ComponentModel;
@@ -1007,16 +1008,16 @@ namespace MyApp
10071008
{
10081009
public partial class MyViewModel
10091010
{
1010-
[NotifyCanExecuteChangedFor("")]
1011-
public int number;
1011+
[NotifyCanExecuteChangedFor("""")]
1012+
public int {|MVVMTK0020:number|};
10121013
}
10131014
}";
10141015

1015-
VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0020");
1016+
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer>(source, LanguageVersion.CSharp8);
10161017
}
10171018

10181019
[TestMethod]
1019-
public void FieldWithOrphanedDependentObservablePropertyAttributesError_NotifyPropertyChangedRecipients()
1020+
public async Task FieldWithOrphanedDependentObservablePropertyAttributesError_NotifyPropertyChangedRecipients()
10201021
{
10211022
string source = @"
10221023
using CommunityToolkit.Mvvm.ComponentModel;
@@ -1026,15 +1027,15 @@ namespace MyApp
10261027
public partial class MyViewModel
10271028
{
10281029
[NotifyPropertyChangedRecipients]
1029-
public int number;
1030+
public int {|MVVMTK0020:number|};
10301031
}
10311032
}";
10321033

1033-
VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0020");
1034+
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer>(source, LanguageVersion.CSharp8);
10341035
}
10351036

10361037
[TestMethod]
1037-
public void FieldWithOrphanedDependentObservablePropertyAttributesError_MultipleUsesStillGenerateOnlyASingleDiagnostic()
1038+
public async Task FieldWithOrphanedDependentObservablePropertyAttributesError_MultipleUsesStillGenerateOnlyASingleDiagnostic()
10381039
{
10391040
string source = @"
10401041
using CommunityToolkit.Mvvm.ComponentModel;
@@ -1043,17 +1044,17 @@ namespace MyApp
10431044
{
10441045
public partial class MyViewModel
10451046
{
1046-
[NotifyPropertyChangedFor("")]
1047-
[NotifyPropertyChangedFor("")]
1048-
[NotifyPropertyChangedFor("")]
1049-
[NotifyCanExecuteChangedFor("")]
1050-
[NotifyCanExecuteChangedFor("")]
1047+
[NotifyPropertyChangedFor("""")]
1048+
[NotifyPropertyChangedFor("""")]
1049+
[NotifyPropertyChangedFor("""")]
1050+
[NotifyCanExecuteChangedFor("""")]
1051+
[NotifyCanExecuteChangedFor("""")]
10511052
[NotifyPropertyChangedRecipients]
1052-
public int number;
1053+
public int {|MVVMTK0020:number|};
10531054
}
10541055
}";
10551056

1056-
VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0020");
1057+
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer>(source, LanguageVersion.CSharp8);
10571058
}
10581059

10591060
[TestMethod]
@@ -1428,12 +1429,15 @@ private void GreetUser(User user)
14281429
}
14291430

14301431
/// <summary>
1431-
/// Verifies the diagnostic error for unsupported C# version, and that all available source generators can run successfully with the input source (including subsequent compilation).
1432+
/// Verifies the diagnostic errors for a given analyzer, and that all available source generators can run successfully with the input source (including subsequent compilation).
14321433
/// </summary>
1434+
/// <typeparam name="TAnalyzer">The type of the analyzer to test.</typeparam>
14331435
/// <param name="markdownSource">The input source to process with diagnostic annotations.</param>
1434-
private static async Task VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(string markdownSource)
1436+
/// <param name="languageVersion">The language version to use to parse code and run tests.</param>
1437+
private static async Task VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<TAnalyzer>(string markdownSource, LanguageVersion languageVersion)
1438+
where TAnalyzer : DiagnosticAnalyzer, new()
14351439
{
1436-
await CSharpAnalyzerWithLanguageVersionTest<UnsupportedCSharpLanguageVersionAnalyzer>.VerifyAnalyzerAsync(markdownSource, LanguageVersion.CSharp7_3);
1440+
await CSharpAnalyzerWithLanguageVersionTest<TAnalyzer>.VerifyAnalyzerAsync(markdownSource, languageVersion);
14371441

14381442
IIncrementalGenerator[] generators =
14391443
{
@@ -1450,7 +1454,7 @@ private static async Task VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(
14501454
// Transform diagnostic annotations back to normal C# (eg. "{|MVVMTK0008:Foo()|}" ---> "Foo()")
14511455
string source = Regex.Replace(markdownSource, @"{\|((?:,?\w+)+):(.+)\|}", m => m.Groups[2].Value);
14521456

1453-
VerifyGeneratedDiagnostics(CSharpSyntaxTree.ParseText(source, CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp7_3)), generators, Array.Empty<string>());
1457+
VerifyGeneratedDiagnostics(CSharpSyntaxTree.ParseText(source, CSharpParseOptions.Default.WithLanguageVersion(languageVersion)), generators, Array.Empty<string>());
14541458
}
14551459

14561460
/// <summary>

0 commit comments

Comments
 (0)