Skip to content

Commit

Permalink
Merge pull request CommunityToolkit#434 from CommunityToolkit/dev/orp…
Browse files Browse the repository at this point in the history
…haned-fields-analyzer

Move diagnostics for orphaned observable property attributes to analyzer
  • Loading branch information
Sergio0694 authored Sep 10, 2022
2 parents 82ccd6c + b8749fe commit 5876242
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -204,19 +204,6 @@ internal static class Execute
forwardedAttributes.ToImmutable());
}

/// <summary>
/// Gets the diagnostics for a field with invalid attribute uses.
/// </summary>
/// <param name="fieldSymbol">The input <see cref="IFieldSymbol"/> instance to process.</param>
/// <returns>The resulting <see cref="Diagnostic"/> instance for <paramref name="fieldSymbol"/>.</returns>
public static Diagnostic GetDiagnosticForFieldWithOrphanedDependentAttributes(IFieldSymbol fieldSymbol)
{
return FieldWithOrphanedDependentObservablePropertyAttributesError.CreateDiagnostic(
fieldSymbol,
fieldSymbol.ContainingType,
fieldSymbol.Name);
}

/// <summary>
/// Validates the containing type for a given field being annotated.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,6 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
fieldSymbols
.Where(static item => item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute"));

// Get diagnostics for fields using [NotifyPropertyChangedFor], [NotifyCanExecuteChangedFor], [NotifyPropertyChangedRecipients] and [NotifyDataErrorInfo], but not [ObservableProperty]
IncrementalValuesProvider<Diagnostic> fieldSymbolsWithOrphanedDependentAttributeWithErrors =
fieldSymbols
.Where(static item =>
(item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.NotifyPropertyChangedForAttribute") ||
item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.NotifyCanExecuteChangedForAttribute") ||
item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.NotifyPropertyChangedRecipientsAttribute") ||
item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.NotifyDataErrorInfoAttribute")) &&
!item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute"))
.Select(static (item, _) => Execute.GetDiagnosticForFieldWithOrphanedDependentAttributes(item));

// Output the diagnostics
context.ReportDiagnostics(fieldSymbolsWithOrphanedDependentAttributeWithErrors);

// Gather info for all annotated fields
IncrementalValuesProvider<(HierarchyInfo Hierarchy, Result<PropertyInfo?> Info)> propertyInfoWithErrors =
fieldSymbolsWithAttribute
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using CommunityToolkit.Mvvm.SourceGenerators.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Diagnostics;
using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors;

namespace CommunityToolkit.Mvvm.SourceGenerators;

/// <summary>
/// A diagnostic analyzer that generates an error whenever a field has an orphaned attribute that depends on <c>[ObservableProperty]</c>.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer : DiagnosticAnalyzer
{
/// <summary>
/// The mapping of target attributes that will trigger the analyzer.
/// </summary>
private static readonly ImmutableDictionary<string, string> GeneratorAttributeNamesToFullyQualifiedNamesMap = ImmutableDictionary.CreateRange(new[]
{
new KeyValuePair<string, string>("NotifyCanExecuteChangedForAttribute", "CommunityToolkit.Mvvm.ComponentModel.NotifyCanExecuteChangedForAttribute"),
new KeyValuePair<string, string>("NotifyDataErrorInfoAttribute", "CommunityToolkit.Mvvm.ComponentModel.NotifyDataErrorInfoAttribute"),
new KeyValuePair<string, string>("NotifyPropertyChangedForAttribute", "CommunityToolkit.Mvvm.ComponentModel.NotifyPropertyChangedForAttribute"),
new KeyValuePair<string, string>("NotifyPropertyChangedRecipientsAttribute", "CommunityToolkit.Mvvm.ComponentModel.NotifyPropertyChangedRecipientsAttribute")
});

/// <inheritdoc/>
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(FieldWithOrphanedDependentObservablePropertyAttributesError);

/// <inheritdoc/>
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
context.EnableConcurrentExecution();

// Defer the registration so it can be skipped if C# 8.0 or more is not available.
// That is because in that case source generators are not supported at all anyaway.
context.RegisterCompilationStartAction(static context =>
{
if (!context.Compilation.HasLanguageVersionAtLeastEqualTo(LanguageVersion.CSharp8))
{
return;
}

context.RegisterSymbolAction(static context =>
{
ImmutableArray<AttributeData> attributes = context.Symbol.GetAttributes();

// If the symbol has no attributes, there's nothing left to do
if (attributes.IsEmpty)
{
return;
}

foreach (AttributeData dependentAttribute in attributes)
{
// Go over each attribute on the target symbol, anche check if any of them matches one of the trigger attributes.
// The logic here is the same as the one in UnsupportedCSharpLanguageVersionAnalyzer, to minimize retrieving symbols.
if (dependentAttribute.AttributeClass is { Name: string attributeName } dependentAttributeClass &&
GeneratorAttributeNamesToFullyQualifiedNamesMap.TryGetValue(attributeName, out string? fullyQualifiedDependentAttributeName) &&
context.Compilation.GetTypeByMetadataName(fullyQualifiedDependentAttributeName) is INamedTypeSymbol dependentAttributeSymbol &&
SymbolEqualityComparer.Default.Equals(dependentAttributeClass, dependentAttributeSymbol))
{
// If the attribute matches, iterate over the attributes to try to find [ObservableProperty]
foreach (AttributeData attribute in attributes)
{
if (attribute.AttributeClass is { Name: "ObservablePropertyAttribute" } attributeSymbol &&
context.Compilation.GetTypeByMetadataName("CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") is INamedTypeSymbol observablePropertySymbol &&
SymbolEqualityComparer.Default.Equals(attributeSymbol, observablePropertySymbol))
{
// If [ObservableProperty] is found, then this field is valid in that it doesn't have orphaned dependent attributes
return;
}
}

context.ReportDiagnostic(Diagnostic.Create(FieldWithOrphanedDependentObservablePropertyAttributesError, context.Symbol.Locations.FirstOrDefault(), context.Symbol.ContainingType, context.Symbol.Name));

// Just like in UnsupportedCSharpLanguageVersionAnalyzer, stop if a diagnostic has been emitted for the current symbol
return;
}
}
}, SymbolKind.Field);
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using System.Threading.Tasks;
using CommunityToolkit.Mvvm.SourceGenerators.UnitTests.Helpers;
using System.Text.RegularExpressions;
using Microsoft.CodeAnalysis.Diagnostics;

namespace CommunityToolkit.Mvvm.SourceGenerators.UnitTests;

Expand Down Expand Up @@ -247,7 +248,7 @@ public partial class {|MVVMTK0008:SampleViewModel|}
}
}";

await VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(source);
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<UnsupportedCSharpLanguageVersionAnalyzer>(source, LanguageVersion.CSharp7_3);
}

[TestMethod]
Expand All @@ -264,7 +265,7 @@ public partial class {|MVVMTK0008:SampleViewModel|}
}
}";

await VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(source);
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<UnsupportedCSharpLanguageVersionAnalyzer>(source, LanguageVersion.CSharp7_3);
}

[TestMethod]
Expand All @@ -283,7 +284,7 @@ public partial class {|MVVMTK0008:SampleViewModel|}
}
}";

await VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(source);
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<UnsupportedCSharpLanguageVersionAnalyzer>(source, LanguageVersion.CSharp7_3);
}

[TestMethod]
Expand All @@ -308,7 +309,7 @@ public partial class {|MVVMTK0008:SampleViewModel|}
}
}";

await VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(source);
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<UnsupportedCSharpLanguageVersionAnalyzer>(source, LanguageVersion.CSharp7_3);
}

[TestMethod]
Expand All @@ -327,7 +328,7 @@ public partial class SampleViewModel : ObservableValidator
}
}";

await VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(source);
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<UnsupportedCSharpLanguageVersionAnalyzer>(source, LanguageVersion.CSharp7_3);
}

[TestMethod]
Expand All @@ -347,7 +348,7 @@ public partial class SampleViewModel
}
}";

await VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(source);
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<UnsupportedCSharpLanguageVersionAnalyzer>(source, LanguageVersion.CSharp7_3);
}

[TestMethod]
Expand All @@ -370,7 +371,7 @@ public void Receive(MyMessage message)
}
}";

await VerifyUnsupportedCSharpVersionAndSuccessfulGeneration(source);
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<UnsupportedCSharpLanguageVersionAnalyzer>(source, LanguageVersion.CSharp7_3);
}

[TestMethod]
Expand Down Expand Up @@ -980,7 +981,7 @@ public partial class MyViewModel : INotifyPropertyChanged
}

[TestMethod]
public void FieldWithOrphanedDependentObservablePropertyAttributesError_NotifyPropertyChangedFor()
public async Task FieldWithOrphanedDependentObservablePropertyAttributesError_NotifyPropertyChangedFor()
{
string source = @"
using CommunityToolkit.Mvvm.ComponentModel;
Expand All @@ -989,16 +990,16 @@ namespace MyApp
{
public partial class MyViewModel
{
[NotifyPropertyChangedFor("")]
public int number;
[NotifyPropertyChangedFor("""")]
public int {|MVVMTK0020:number|};
}
}";

VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0020");
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer>(source, LanguageVersion.CSharp8);
}

[TestMethod]
public void FieldWithOrphanedDependentObservablePropertyAttributesError_NotifyCanExecuteChangedFor()
public async Task FieldWithOrphanedDependentObservablePropertyAttributesError_NotifyCanExecuteChangedFor()
{
string source = @"
using CommunityToolkit.Mvvm.ComponentModel;
Expand All @@ -1007,16 +1008,16 @@ namespace MyApp
{
public partial class MyViewModel
{
[NotifyCanExecuteChangedFor("")]
public int number;
[NotifyCanExecuteChangedFor("""")]
public int {|MVVMTK0020:number|};
}
}";

VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0020");
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer>(source, LanguageVersion.CSharp8);
}

[TestMethod]
public void FieldWithOrphanedDependentObservablePropertyAttributesError_NotifyPropertyChangedRecipients()
public async Task FieldWithOrphanedDependentObservablePropertyAttributesError_NotifyPropertyChangedRecipients()
{
string source = @"
using CommunityToolkit.Mvvm.ComponentModel;
Expand All @@ -1026,15 +1027,15 @@ namespace MyApp
public partial class MyViewModel
{
[NotifyPropertyChangedRecipients]
public int number;
public int {|MVVMTK0020:number|};
}
}";

VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0020");
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer>(source, LanguageVersion.CSharp8);
}

[TestMethod]
public void FieldWithOrphanedDependentObservablePropertyAttributesError_MultipleUsesStillGenerateOnlyASingleDiagnostic()
public async Task FieldWithOrphanedDependentObservablePropertyAttributesError_MultipleUsesStillGenerateOnlyASingleDiagnostic()
{
string source = @"
using CommunityToolkit.Mvvm.ComponentModel;
Expand All @@ -1043,17 +1044,17 @@ namespace MyApp
{
public partial class MyViewModel
{
[NotifyPropertyChangedFor("")]
[NotifyPropertyChangedFor("")]
[NotifyPropertyChangedFor("")]
[NotifyCanExecuteChangedFor("")]
[NotifyCanExecuteChangedFor("")]
[NotifyPropertyChangedFor("""")]
[NotifyPropertyChangedFor("""")]
[NotifyPropertyChangedFor("""")]
[NotifyCanExecuteChangedFor("""")]
[NotifyCanExecuteChangedFor("""")]
[NotifyPropertyChangedRecipients]
public int number;
public int {|MVVMTK0020:number|};
}
}";

VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0020");
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer>(source, LanguageVersion.CSharp8);
}

[TestMethod]
Expand Down Expand Up @@ -1428,12 +1429,15 @@ private void GreetUser(User user)
}

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

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

VerifyGeneratedDiagnostics(CSharpSyntaxTree.ParseText(source, CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp7_3)), generators, Array.Empty<string>());
VerifyGeneratedDiagnostics(CSharpSyntaxTree.ParseText(source, CSharpParseOptions.Default.WithLanguageVersion(languageVersion)), generators, Array.Empty<string>());
}

/// <summary>
Expand Down

0 comments on commit 5876242

Please sign in to comment.