Skip to content

Commit aa4ff30

Browse files
Sergio0694Sergio0694
authored
Merge pull request #531 from CommunityToolkit/dev/base-attribute-analyzer
Add analyzer for incorrect [INotifyPropertyChanged] and [ObservableObject] use
2 parents a7cfaa2 + e3980a7 commit aa4ff30

13 files changed

+172
-25
lines changed

‎src/CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,5 @@ MVVMTK0028 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator
3838
MVVMTK0029 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Warning | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0029
3939
MVVMTK0030 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Warning | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0030
4040
MVVMTK0031 | CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator | Error | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0031
41+
MVVMTK0032 | CommunityToolkit.Mvvm.SourceGenerators.INotifyPropertyChangedGenerator | Warning | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0032
42+
MVVMTK0033 | CommunityToolkit.Mvvm.SourceGenerators.ObservableObjectGenerator | Warning | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0033

‎src/CommunityToolkit.Mvvm.SourceGenerators/CommunityToolkit.Mvvm.SourceGenerators.projitems‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
<Compile Include="$(MSBuildThisFileDirectory)ComponentModel\TransitiveMembersGenerator.cs" />
5050
<Compile Include="$(MSBuildThisFileDirectory)ComponentModel\TransitiveMembersGenerator.Execute.cs" />
5151
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer.cs" />
52+
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\ClassUsingAttributeInsteadOfInheritanceAnalyzer.cs" />
5253
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\UnsupportedCSharpLanguageVersionAnalyzer.cs" />
5354
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Suppressors\ObservablePropertyAttributeWithPropertyTargetDiagnosticSuppressor.cs" />
5455
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\DiagnosticDescriptors.cs" />

‎src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/Analyzers/ClassUsingAttributeInsteadOfInheritanceAnalyzer.cs‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
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 Microsoft.CodeAnalysis;
9+
using Microsoft.CodeAnalysis.Diagnostics;
10+
using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors;
11+
12+
namespace CommunityToolkit.Mvvm.SourceGenerators;
13+
14+
/// <summary>
15+
/// A diagnostic analyzer that generates a warning when a class is using a code generation attribute when it could inherit instead.
16+
/// </summary>
17+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
18+
public sealed class ClassUsingAttributeInsteadOfInheritanceAnalyzer : DiagnosticAnalyzer
19+
{
20+
/// <summary>
21+
/// The mapping of target attributes that will trigger the analyzer.
22+
/// </summary>
23+
private static readonly ImmutableDictionary<string, string> GeneratorAttributeNamesToFullyQualifiedNamesMap = ImmutableDictionary.CreateRange(new[]
24+
{
25+
new KeyValuePair<string, string>("ObservableObjectAttribute", "CommunityToolkit.Mvvm.ComponentModel.ObservableObjectAttribute"),
26+
new KeyValuePair<string, string>("INotifyPropertyChangedAttribute", "CommunityToolkit.Mvvm.ComponentModel.INotifyPropertyChangedAttribute"),
27+
});
28+
29+
/// <summary>
30+
/// The mapping of diagnostics for each target attribute.
31+
/// </summary>
32+
private static readonly ImmutableDictionary<string, DiagnosticDescriptor> GeneratorAttributeNamesToDiagnosticsMap = ImmutableDictionary.CreateRange(new[]
33+
{
34+
new KeyValuePair<string, DiagnosticDescriptor>("ObservableObjectAttribute", InheritFromObservableObjectInsteadOfUsingObservableObjectAttributeWarning),
35+
new KeyValuePair<string, DiagnosticDescriptor>("INotifyPropertyChangedAttribute", InheritFromObservableObjectInsteadOfUsingINotifyPropertyChangedAttributeWarning),
36+
});
37+
38+
/// <inheritdoc/>
39+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(
40+
InheritFromObservableObjectInsteadOfUsingObservableObjectAttributeWarning,
41+
InheritFromObservableObjectInsteadOfUsingINotifyPropertyChangedAttributeWarning);
42+
43+
/// <inheritdoc/>
44+
public override void Initialize(AnalysisContext context)
45+
{
46+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
47+
context.EnableConcurrentExecution();
48+
49+
// Defer the callback registration to when the compilation starts, so we can execute more
50+
// preliminary checks and skip registering any kind of symbol analysis at all if not needed.
51+
context.RegisterSymbolAction(static context =>
52+
{
53+
// We're looking for class declarations
54+
if (context.Symbol is not INamedTypeSymbol { TypeKind: TypeKind.Class, IsRecord: false, IsStatic: false, IsImplicitlyDeclared: false } classSymbol)
55+
{
56+
return;
57+
}
58+
59+
foreach (AttributeData attribute in context.Symbol.GetAttributes())
60+
{
61+
// Same logic as in FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer to find target attributes
62+
if (attribute.AttributeClass is { Name: string attributeName } attributeClass &&
63+
GeneratorAttributeNamesToFullyQualifiedNamesMap.TryGetValue(attributeName, out string? fullyQualifiedAttributeName) &&
64+
context.Compilation.GetTypeByMetadataName(fullyQualifiedAttributeName) is INamedTypeSymbol attributeSymbol &&
65+
SymbolEqualityComparer.Default.Equals(attributeClass, attributeSymbol))
66+
{
67+
// The type is annotated with either [ObservableObject] or [INotifyPropertyChanged].
68+
// Next, we need to check whether it isn't already inheriting from another type.
69+
if (classSymbol.BaseType is { SpecialType: SpecialType.System_Object })
70+
{
71+
// This type is using the attribute when it could just inherit from ObservableObject, which is preferred
72+
context.ReportDiagnostic(Diagnostic.Create(GeneratorAttributeNamesToDiagnosticsMap[attributeClass.Name], context.Symbol.Locations.FirstOrDefault(), context.Symbol));
73+
}
74+
}
75+
}
76+
}, SymbolKind.NamedType);
77+
}
78+
}

‎src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/Analyzers/UnsupportedCSharpLanguageVersionAnalyzer.cs‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public sealed class UnsupportedCSharpLanguageVersionAnalyzer : DiagnosticAnalyze
3232
new KeyValuePair<string, string>("ObservableObjectAttribute", "CommunityToolkit.Mvvm.ComponentModel.ObservableObjectAttribute"),
3333
new KeyValuePair<string, string>("ObservablePropertyAttribute", "CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute"),
3434
new KeyValuePair<string, string>("ObservableRecipientAttribute", "CommunityToolkit.Mvvm.ComponentModel.ObservableRecipientAttribute"),
35-
new KeyValuePair<string, string>("RelayCommandAttribute", "CommunityToolkit.Mvvm.Input.RelayCommandAttribute"),
35+
new KeyValuePair<string, string>("RelayCommandAttribute", "CommunityToolkit.Mvvm.Input.RelayCommandAttribute")
3636
});
3737

3838
/// <inheritdoc/>
@@ -62,15 +62,7 @@ public override void Initialize(AnalysisContext context)
6262
return;
6363
}
6464

65-
ImmutableArray<AttributeData> attributes = context.Symbol.GetAttributes();
66-
67-
// If the symbol has no attributes, there's nothing left to do
68-
if (attributes.IsEmpty)
69-
{
70-
return;
71-
}
72-
73-
foreach (AttributeData attribute in attributes)
65+
foreach (AttributeData attribute in context.Symbol.GetAttributes())
7466
{
7567
// Go over each attribute on the target symbol, and check if the attribute type name is a candidate.
7668
// If it is, double check by actually resolving the symbol from the compilation and comparing against it.

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

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ internal static class DiagnosticDescriptors
134134
id: "MVVMTK0008",
135135
title: "Unsupported C# language version",
136136
messageFormat: "The source generator features from the MVVM Toolkit require consuming projects to set the C# language version to at least C# 8.0",
137-
category: typeof(CSharpParseOptions).FullName,
137+
category: typeof(UnsupportedCSharpLanguageVersionAnalyzer).FullName,
138138
defaultSeverity: DiagnosticSeverity.Error,
139139
isEnabledByDefault: true,
140140
description: "The source generator features from the MVVM Toolkit require consuming projects to set the C# language version to at least C# 8.0. Make sure to add <LangVersion>8.0</LangVersion> (or above) to your .csproj file.",
@@ -507,4 +507,40 @@ internal static class DiagnosticDescriptors
507507
isEnabledByDefault: true,
508508
description: "Cannot apply the [RelayCommand] attribute specifying a task scheduler exception flow option to methods mapping to non-asynchronous command types.",
509509
helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0031");
510+
511+
/// <summary>
512+
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when <c>[INotifyPropertyChanged]</c> is used on a type that could inherit from <c>ObservableObject</c> instead.
513+
/// <para>
514+
/// Format: <c>"The type {0} is using the [INotifyPropertyChanged] attribute while having no base type, and it should instead inherit from ObservableObject"</c>.
515+
/// </para>
516+
/// </summary>
517+
public static readonly DiagnosticDescriptor InheritFromObservableObjectInsteadOfUsingINotifyPropertyChangedAttributeWarning = new DiagnosticDescriptor(
518+
id: "MVVMTK0032",
519+
title: "Inherit from ObservableObject instead of using [INotifyPropertyChanged]",
520+
messageFormat: "The type {0} is using the [INotifyPropertyChanged] attribute while having no base type, and it should instead inherit from ObservableObject",
521+
category: typeof(INotifyPropertyChangedGenerator).FullName,
522+
defaultSeverity: DiagnosticSeverity.Warning,
523+
isEnabledByDefault: true,
524+
description:
525+
"Classes with no base types should prefer inheriting from ObservableObject instead of using attributes to generate INotifyPropertyChanged code, as that will " +
526+
"reduce the binary size of the application (the attributes are only meant to support cases where the annotated types are already inheriting from a different type).",
527+
helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0032");
528+
529+
/// <summary>
530+
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when <c>[ObservableObject]</c> is used on a type that could inherit from <c>ObservableObject</c> instead.
531+
/// <para>
532+
/// Format: <c>"The type {0} is using the [ObservableObject] attribute while having no base type, and it should instead inherit from ObservableObject"</c>.
533+
/// </para>
534+
/// </summary>
535+
public static readonly DiagnosticDescriptor InheritFromObservableObjectInsteadOfUsingObservableObjectAttributeWarning = new DiagnosticDescriptor(
536+
id: "MVVMTK0033",
537+
title: "Inherit from ObservableObject instead of using [ObservableObject]",
538+
messageFormat: "The type {0} is using the [ObservableObject] attribute while having no base type, and it should instead inherit from ObservableObject",
539+
category: typeof(ObservableObjectGenerator).FullName,
540+
defaultSeverity: DiagnosticSeverity.Warning,
541+
isEnabledByDefault: true,
542+
description:
543+
"Classes with no base types should prefer inheriting from ObservableObject instead of using attributes to generate INotifyPropertyChanged code, as that will " +
544+
"reduce the binary size of the application (the attributes are only meant to support cases where the annotated types are already inheriting from a different type).",
545+
helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0032");
510546
}

‎tests/CommunityToolkit.Mvvm.ExternalAssembly/ModelWithObservableObjectAttribute.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
using CommunityToolkit.Mvvm.ComponentModel;
66

7+
#pragma warning disable MVVMTK0033
8+
79
namespace CommunityToolkit.Mvvm.ExternalAssembly;
810

911
/// <summary>

‎tests/CommunityToolkit.Mvvm.ExternalAssembly/SampleModelWithINPCAndObservableProperties.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
using CommunityToolkit.Mvvm.ComponentModel;
66

7+
#pragma warning disable MVVMTK0032
8+
79
namespace CommunityToolkit.Mvvm.ExternalAssembly;
810

911
/// <summary>

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

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1495,6 +1495,42 @@ private void GreetUser(User user)
14951495
VerifyGeneratedDiagnostics<RelayCommandGenerator>(source, "MVVMTK0031");
14961496
}
14971497

1498+
[TestMethod]
1499+
public async Task UsingINotifyPropertyChangedAttributeOnClassWithNoBaseType()
1500+
{
1501+
string source = """
1502+
using CommunityToolkit.Mvvm.ComponentModel;
1503+
1504+
namespace MyApp
1505+
{
1506+
[INotifyPropertyChanged]
1507+
public partial class {|MVVMTK0032:SampleViewModel|}
1508+
{
1509+
}
1510+
}
1511+
""";
1512+
1513+
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<ClassUsingAttributeInsteadOfInheritanceAnalyzer>(source, LanguageVersion.CSharp8);
1514+
}
1515+
1516+
[TestMethod]
1517+
public async Task UsingObservableObjectAttributeOnClassWithNoBaseType()
1518+
{
1519+
string source = """
1520+
using CommunityToolkit.Mvvm.ComponentModel;
1521+
1522+
namespace MyApp
1523+
{
1524+
[ObservableObject]
1525+
public partial class {|MVVMTK0033:SampleViewModel|}
1526+
{
1527+
}
1528+
}
1529+
""";
1530+
1531+
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<ClassUsingAttributeInsteadOfInheritanceAnalyzer>(source, LanguageVersion.CSharp8);
1532+
}
1533+
14981534
/// <summary>
14991535
/// Verifies the diagnostic errors for a given analyzer, and that all available source generators can run successfully with the input source (including subsequent compilation).
15001536
/// </summary>
@@ -1538,20 +1574,6 @@ private static void VerifyGeneratedDiagnostics<TGenerator>(string source, params
15381574
VerifyGeneratedDiagnostics(CSharpSyntaxTree.ParseText(source, CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp8)), new[] { generator }, diagnosticsIds);
15391575
}
15401576

1541-
/// <summary>
1542-
/// Verifies the output of a source generator.
1543-
/// </summary>
1544-
/// <typeparam name="TGenerator">The generator type to use.</typeparam>
1545-
/// <param name="syntaxTree">The input source to process.</param>
1546-
/// <param name="diagnosticsIds">The diagnostic ids to expect for the input source code.</param>
1547-
private static void VerifyGeneratedDiagnostics<TGenerator>(SyntaxTree syntaxTree, params string[] diagnosticsIds)
1548-
where TGenerator : class, IIncrementalGenerator, new()
1549-
{
1550-
IIncrementalGenerator generator = new TGenerator();
1551-
1552-
VerifyGeneratedDiagnostics(syntaxTree, new[] { generator }, diagnosticsIds);
1553-
}
1554-
15551577
/// <summary>
15561578
/// Verifies the output of one or more source generators.
15571579
/// </summary>

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
using CommunityToolkit.Mvvm.ComponentModel;
99
using Microsoft.VisualStudio.TestTools.UnitTesting;
1010

11+
#pragma warning disable MVVMTK0032
12+
1113
namespace CommunityToolkit.Mvvm.UnitTests;
1214

1315
[TestClass]

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
using CommunityToolkit.Mvvm.ComponentModel;
77
using Microsoft.VisualStudio.TestTools.UnitTesting;
88

9+
#pragma warning disable MVVMTK0033
10+
911
namespace CommunityToolkit.Mvvm.UnitTests;
1012

1113
[TestClass]

0 commit comments

Comments
 (0)