Skip to content

Commit fc42924

Browse files
committed
Add 'UseObservablePropertyOnSemiAutoPropertyAnalyzer'
1 parent 78348ca commit fc42924

File tree

4 files changed

+286
-0
lines changed

4 files changed

+286
-0
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,3 +97,11 @@ MVVMTK0052 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator
9797
MVVMTK0053 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0053
9898
MVVMTK0054 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0054
9999
MVVMTK0055 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0055
100+
101+
## Release 8.4.1
102+
103+
### New Rules
104+
105+
Rule ID | Category | Severity | Notes
106+
--------|----------|----------|-------
107+
MVVMTK0056 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Info | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0056

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
<Compile Include="$(MSBuildThisFileDirectory)ComponentModel\ObservableValidatorValidateAllPropertiesGenerator.Execute.cs" />
4040
<Compile Include="$(MSBuildThisFileDirectory)ComponentModel\TransitiveMembersGenerator.cs" />
4141
<Compile Include="$(MSBuildThisFileDirectory)ComponentModel\TransitiveMembersGenerator.Execute.cs" />
42+
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\UseObservablePropertyOnSemiAutoPropertyAnalyzer.cs" />
4243
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\AsyncVoidReturningRelayCommandMethodAnalyzer.cs" />
4344
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\InvalidGeneratedPropertyObservablePropertyAttributeAnalyzer.cs" />
4445
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\InvalidPointerTypeObservablePropertyAttributeAnalyzer.cs" />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
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+
#if ROSLYN_4_12_0_OR_GREATER
6+
7+
using System.Collections.Generic;
8+
using System.Collections.Immutable;
9+
using System.Diagnostics.CodeAnalysis;
10+
using System.Linq;
11+
using CommunityToolkit.Mvvm.SourceGenerators.Extensions;
12+
using Microsoft.CodeAnalysis;
13+
using Microsoft.CodeAnalysis.CSharp.Syntax;
14+
using Microsoft.CodeAnalysis.Diagnostics;
15+
using Microsoft.CodeAnalysis.Operations;
16+
using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors;
17+
18+
namespace CommunityToolkit.Mvvm.SourceGenerators;
19+
20+
/// <summary>
21+
/// A diagnostic analyzer that generates a suggestion whenever <c>[ObservableProperty]</c> is used on a semi-auto property when a partial property could be used instead.
22+
/// </summary>
23+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
24+
public sealed class UseObservablePropertyOnSemiAutoPropertyAnalyzer : DiagnosticAnalyzer
25+
{
26+
/// <inheritdoc/>
27+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(UseObservablePropertyOnSemiAutoProperty);
28+
29+
/// <inheritdoc/>
30+
public override void Initialize(AnalysisContext context)
31+
{
32+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
33+
context.EnableConcurrentExecution();
34+
35+
context.RegisterCompilationStartAction(static context =>
36+
{
37+
// Using [ObservableProperty] on partial properties is only supported when using C# preview.
38+
// As such, if that is not the case, return immediately, as no diagnostic should be produced.
39+
if (!context.Compilation.IsLanguageVersionPreview())
40+
{
41+
return;
42+
}
43+
44+
// Get the symbol for [ObservableProperty] and ObservableObject
45+
if (context.Compilation.GetTypeByMetadataName("CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") is not INamedTypeSymbol observablePropertySymbol ||
46+
context.Compilation.GetTypeByMetadataName("CommunityToolkit.Mvvm.ComponentModel.ObservableObject") is not INamedTypeSymbol observableObjectSymbol)
47+
{
48+
return;
49+
}
50+
51+
// Get the symbol for the SetProperty<T> method as well
52+
if (!TryGetSetPropertyMethodSymbol(observableObjectSymbol, out IMethodSymbol? setPropertySymbol))
53+
{
54+
return;
55+
}
56+
57+
context.RegisterSymbolStartAction(context =>
58+
{
59+
// We only care about types that could derive from ObservableObject
60+
if (context.Symbol is not INamedTypeSymbol { IsStatic: false, IsReferenceType: true, BaseType.SpecialType: not SpecialType.System_Object } typeSymbol)
61+
{
62+
return;
63+
}
64+
65+
// If the type does not derive from ObservableObject, ignore it
66+
if (!typeSymbol.InheritsFromType(observableObjectSymbol))
67+
{
68+
return;
69+
}
70+
71+
Dictionary<IPropertySymbol, bool[]> propertyMap = new(SymbolEqualityComparer.Default);
72+
73+
// Crawl all members to discover properties that might be of interest
74+
foreach (ISymbol memberSymbol in typeSymbol.GetMembers())
75+
{
76+
// We're only looking for properties that might be valid candidates for conversion
77+
if (memberSymbol is not IPropertySymbol
78+
{
79+
IsStatic: false,
80+
IsPartialDefinition: false,
81+
PartialDefinitionPart: null,
82+
PartialImplementationPart: null,
83+
ReturnsByRef: false,
84+
ReturnsByRefReadonly: false,
85+
Type.IsRefLikeType: false,
86+
GetMethod: not null,
87+
SetMethod.IsInitOnly: false
88+
} propertySymbol)
89+
{
90+
continue;
91+
}
92+
93+
// We can safely ignore properties that already have [ObservableProperty]
94+
if (typeSymbol.HasAttributeWithType(observablePropertySymbol))
95+
{
96+
continue;
97+
}
98+
99+
// Track the property for later
100+
propertyMap.Add(propertySymbol, new bool[2]);
101+
}
102+
103+
// We want to process both accessors, where we specifically need both the syntax
104+
// and their semantic model to verify what they're doing. We can use a code callback.
105+
context.RegisterOperationBlockAction(context =>
106+
{
107+
// Make sure the current symbol is a property accessor
108+
if (context.OwningSymbol is not IMethodSymbol { MethodKind: MethodKind.PropertyGet or MethodKind.PropertySet, AssociatedSymbol: IPropertySymbol propertySymbol })
109+
{
110+
return;
111+
}
112+
113+
// If so, check that we are actually processing one of the properties we care about
114+
if (!propertyMap.TryGetValue(propertySymbol, out bool[]? validFlags))
115+
{
116+
return;
117+
}
118+
119+
// Handle the 'get' logic
120+
if (SymbolEqualityComparer.Default.Equals(propertySymbol.GetMethod, context.OwningSymbol))
121+
{
122+
// We expect a top-level block operation, that immediately returns an expression
123+
if (context.OperationBlocks is not [IBlockOperation { Operations: [IReturnOperation returnOperation] }])
124+
{
125+
return;
126+
}
127+
128+
// Next, we expect the return to produce a field reference
129+
if (returnOperation is not { ReturnedValue: IFieldReferenceOperation fieldReferenceOperation })
130+
{
131+
return;
132+
}
133+
134+
// The field has to be implicitly declared and not constant (and not static)
135+
if (fieldReferenceOperation.Field is not { IsImplicitlyDeclared: true, IsStatic: false } fieldSymbol)
136+
{
137+
return;
138+
}
139+
140+
// Validate tha the field is indeed 'field' (it will be associated with the property)
141+
if (!SymbolEqualityComparer.Default.Equals(fieldSymbol.AssociatedSymbol, propertySymbol))
142+
{
143+
return;
144+
}
145+
146+
// The 'get' accessor is valid
147+
validFlags[0] = true;
148+
}
149+
else if (SymbolEqualityComparer.Default.Equals(propertySymbol.SetMethod, context.OwningSymbol))
150+
{
151+
// We expect a top-level block operation, that immediately performs an invocation
152+
if (context.OperationBlocks is not [IBlockOperation { Operations: [IExpressionStatementOperation { Operation: IInvocationOperation invocationOperation }] }])
153+
{
154+
return;
155+
}
156+
157+
// Brief filtering of the target method, also get the original definition
158+
if (invocationOperation.TargetMethod is not { Name: "SetProperty", IsGenericMethod: true, IsStatic: false } methodSymbol)
159+
{
160+
return;
161+
}
162+
163+
// First, check that we're calling 'ObservableObject.SetProperty'
164+
if (!SymbolEqualityComparer.Default.Equals(methodSymbol.ConstructedFrom, setPropertySymbol))
165+
{
166+
return;
167+
}
168+
169+
// We matched the method, now let's validate the arguments
170+
if (invocationOperation.Arguments is not [{ } locationArgument, { } valueArgument, { } propertyNameArgument])
171+
{
172+
return;
173+
}
174+
175+
// The field has to be implicitly declared and not constant (and not static)
176+
if (locationArgument.Value is not IFieldReferenceOperation { Field: { IsImplicitlyDeclared: true, IsStatic: false } fieldSymbol })
177+
{
178+
return;
179+
}
180+
181+
// Validate tha the field is indeed 'field' (it will be associated with the property)
182+
if (!SymbolEqualityComparer.Default.Equals(fieldSymbol.AssociatedSymbol, propertySymbol))
183+
{
184+
return;
185+
}
186+
187+
// The value is just the 'value' keyword
188+
if (valueArgument.Value is not IParameterReferenceOperation { Syntax: IdentifierNameSyntax { Identifier.Text: "value" } })
189+
{
190+
return;
191+
}
192+
193+
// The property name should be the default value
194+
if (propertyNameArgument is not { IsImplicit: true, ArgumentKind: ArgumentKind.DefaultValue })
195+
{
196+
return;
197+
}
198+
199+
// The 'set' accessor is valid
200+
validFlags[1] = true;
201+
}
202+
});
203+
204+
// Finally, we can consume this information when we finish processing the symbol
205+
context.RegisterSymbolEndAction(context =>
206+
{
207+
// Emit a diagnostic for each property that was a valid match
208+
foreach (KeyValuePair<IPropertySymbol, bool[]> pair in propertyMap)
209+
{
210+
if (pair.Value is [true, true])
211+
{
212+
context.ReportDiagnostic(Diagnostic.Create(
213+
UseObservablePropertyOnSemiAutoProperty,
214+
pair.Key.Locations.FirstOrDefault(),
215+
pair.Key));
216+
}
217+
}
218+
});
219+
}, SymbolKind.NamedType);
220+
});
221+
}
222+
223+
/// <summary>
224+
/// Tries to get the symbol for the target <c>SetProperty</c> method this analyzer looks for.
225+
/// </summary>
226+
/// <param name="observableObjectSymbol">The symbol for <c>ObservableObject</c>.</param>
227+
/// <param name="setPropertySymbol">The resulting method symbol, if found (this should always be the case).</param>
228+
/// <returns>Whether <paramref name="setPropertySymbol"/> could be resolved correctly.</returns>
229+
private static bool TryGetSetPropertyMethodSymbol(INamedTypeSymbol observableObjectSymbol, [NotNullWhen(true)] out IMethodSymbol? setPropertySymbol)
230+
{
231+
foreach (ISymbol symbol in observableObjectSymbol.GetMembers("SetProperty"))
232+
{
233+
// We're guaranteed to only match methods here
234+
IMethodSymbol methodSymbol = (IMethodSymbol)symbol;
235+
236+
// Match the exact signature we need (there's several overloads)
237+
if (methodSymbol.Parameters is not
238+
[
239+
{ Kind: SymbolKind.TypeParameter, RefKind: RefKind.Ref },
240+
{ Kind: SymbolKind.TypeParameter, RefKind: RefKind.None },
241+
{ Type.SpecialType: SpecialType.System_String }
242+
])
243+
{
244+
setPropertySymbol = methodSymbol;
245+
246+
return true;
247+
}
248+
}
249+
250+
setPropertySymbol = null;
251+
252+
return false;
253+
}
254+
}
255+
256+
#endif

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ internal static class DiagnosticDescriptors
4444
/// </summary>
4545
public const string WinRTObservablePropertyOnFieldsIsNotAotCompatibleId = "MVVMTK0045";
4646

47+
/// <summary>
48+
/// The diagnostic id for <see cref="UseObservablePropertyOnSemiAutoProperty"/>.
49+
/// </summary>
50+
public const string UseObservablePropertyOnSemiAutoPropertyId = "MVVMTK0056";
51+
4752
/// <summary>
4853
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when a duplicate declaration of <see cref="INotifyPropertyChanged"/> would happen.
4954
/// <para>
@@ -923,4 +928,20 @@ internal static class DiagnosticDescriptors
923928
isEnabledByDefault: true,
924929
description: "A property using [ObservableProperty] returns a pointer-like value ([ObservableProperty] must be used on properties of a non pointer-like type).",
925930
helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0055");
931+
932+
/// <summary>
933+
/// Gets a <see cref="DiagnosticDescriptor"/> for when a semi-auto property can be converted to use <c>[ObservableProperty]</c> instead.
934+
/// <para>
935+
/// Format: <c>"The semi-auto property {0}.{1} can be converted to a partial property using [ObservableProperty], which is recommended (doing so makes the code less verbose and results in more optimized code)"</c>.
936+
/// </para>
937+
/// </summary>
938+
public static readonly DiagnosticDescriptor UseObservablePropertyOnSemiAutoProperty = new DiagnosticDescriptor(
939+
id: UseObservablePropertyOnSemiAutoPropertyId,
940+
title: "Prefer using [ObservableProperty] over semi-auto properties",
941+
messageFormat: """The semi-auto property {0}.{1} can be converted to a partial property using [ObservableProperty], which is recommended (doing so makes the code less verbose and results in more optimized code)""",
942+
category: typeof(ObservablePropertyGenerator).FullName,
943+
defaultSeverity: DiagnosticSeverity.Info,
944+
isEnabledByDefault: true,
945+
description: "Semi-auto properties should be converted to partial properties using [ObservableProperty] when possible, which is recommended (doing so makes the code less verbose and results in more optimized code).",
946+
helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0056");
926947
}

0 commit comments

Comments
 (0)