Skip to content

Commit 857b4b8

Browse files
Sergio0694Sergio0694
committed
Resolve symbols before registering actions in analyzers
1 parent c4d9041 commit 857b4b8

5 files changed

+99
-70
lines changed

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

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.Collections.Immutable;
77
using System.Linq;
8+
using CommunityToolkit.Mvvm.SourceGenerators.Extensions;
89
using Microsoft.CodeAnalysis;
910
using Microsoft.CodeAnalysis.Diagnostics;
1011
using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors;
@@ -56,37 +57,44 @@ public override void Initialize(AnalysisContext context)
5657
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
5758
context.EnableConcurrentExecution();
5859

59-
context.RegisterSymbolAction(static context =>
60+
context.RegisterCompilationStartAction(static context =>
6061
{
61-
// We're looking for class declarations
62-
if (context.Symbol is not INamedTypeSymbol { TypeKind: TypeKind.Class, IsRecord: false, IsStatic: false, IsImplicitlyDeclared: false } classSymbol)
62+
// Try to get all necessary type symbols
63+
if (!context.Compilation.TryBuildNamedTypeSymbolMap(GeneratorAttributeNamesToFullyQualifiedNamesMap, out ImmutableDictionary<string, INamedTypeSymbol>? typeSymbols))
6364
{
6465
return;
6566
}
6667

67-
foreach (AttributeData attribute in context.Symbol.GetAttributes())
68+
context.RegisterSymbolAction(context =>
6869
{
69-
// Same logic as in FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer to find target attributes
70-
if (attribute.AttributeClass is { Name: string attributeName } attributeClass &&
71-
GeneratorAttributeNamesToFullyQualifiedNamesMap.TryGetValue(attributeName, out string? fullyQualifiedAttributeName) &&
72-
context.Compilation.GetTypeByMetadataName(fullyQualifiedAttributeName) is INamedTypeSymbol attributeSymbol &&
73-
SymbolEqualityComparer.Default.Equals(attributeClass, attributeSymbol))
70+
// We're looking for class declarations that don't have any base type
71+
if (context.Symbol is not INamedTypeSymbol { TypeKind: TypeKind.Class, IsRecord: false, IsStatic: false, IsImplicitlyDeclared: false } classSymbol)
7472
{
75-
// The type is annotated with either [ObservableObject] or [INotifyPropertyChanged].
76-
// Next, we need to check whether it isn't already inheriting from another type.
77-
if (classSymbol.BaseType is { SpecialType: SpecialType.System_Object })
73+
return;
74+
}
75+
76+
foreach (AttributeData attribute in context.Symbol.GetAttributes())
77+
{
78+
// Same logic as in FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer to find target attributes
79+
if (attribute.AttributeClass is { Name: string attributeName } attributeClass &&
80+
typeSymbols.TryGetValue(attributeName, out INamedTypeSymbol? attributeSymbol) &&
81+
SymbolEqualityComparer.Default.Equals(attributeClass, attributeSymbol))
7882
{
79-
// This type is using the attribute when it could just inherit from ObservableObject, which is preferred
80-
context.ReportDiagnostic(Diagnostic.Create(
81-
GeneratorAttributeNamesToDiagnosticsMap[attributeClass.Name],
82-
context.Symbol.Locations.FirstOrDefault(),
83-
ImmutableDictionary.Create<string, string?>()
84-
.Add(TypeNameKey, classSymbol.Name)
85-
.Add(AttributeTypeNameKey, attributeName),
86-
context.Symbol));
83+
// The type is annotated with either [ObservableObject] or [INotifyPropertyChanged].
84+
if (classSymbol.BaseType is { SpecialType: SpecialType.System_Object })
85+
{
86+
// This type is using the attribute when it could just inherit from ObservableObject, which is preferred
87+
context.ReportDiagnostic(Diagnostic.Create(
88+
GeneratorAttributeNamesToDiagnosticsMap[attributeClass.Name],
89+
context.Symbol.Locations.FirstOrDefault(),
90+
ImmutableDictionary.Create<string, string?>()
91+
.Add(TypeNameKey, classSymbol.Name)
92+
.Add(AttributeTypeNameKey, attributeName),
93+
context.Symbol));
94+
}
8795
}
8896
}
89-
}
90-
}, SymbolKind.NamedType);
97+
}, SymbolKind.NamedType);
98+
});
9199
}
92100
}

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

Lines changed: 44 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -35,51 +35,59 @@ public override void Initialize(AnalysisContext context)
3535
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
3636
context.EnableConcurrentExecution();
3737

38-
context.RegisterOperationAction(static context =>
38+
context.RegisterCompilationStartAction(static context =>
3939
{
40-
// We're only looking for references to fields that could potentially be observable properties
41-
if (context.Operation is not IFieldReferenceOperation
42-
{
43-
Field: IFieldSymbol { IsStatic: false, IsConst: false, IsImplicitlyDeclared: false, ContainingType: INamedTypeSymbol } fieldSymbol,
44-
Instance.Type: ITypeSymbol typeSymbol
45-
})
40+
// Get the symbol for [ObservableProperty]
41+
if (context.Compilation.GetTypeByMetadataName("CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") is not INamedTypeSymbol observablePropertySymbol)
4642
{
4743
return;
4844
}
4945

50-
// Special case field references from within a constructor and don't ever emit warnings for them. The point of this
51-
// analyzer is to prevent mistakes when users assign a field instead of a property and then get confused when the
52-
// property changed event is not raised. But this would never be the case from a constructur anyway, given that
53-
// no handler for that event would possibly be present. Suppressing warnings in this cases though will help to
54-
// avoid scenarios where people get nullability warnings they cannot suppress, in case they were pushed by the
55-
// analyzer in the MVVM Toolkit to not assign a field marked with a non-nullable reference type. Ideally this
56-
// would be solved by habing the generated setter be marked with [MemberNotNullIfNotNull("field", "value")],
57-
// but such an annotation does not currently exist.
58-
if (context.ContainingSymbol is IMethodSymbol { MethodKind: MethodKind.Constructor, ContainingType: INamedTypeSymbol instanceType } &&
59-
SymbolEqualityComparer.Default.Equals(instanceType, typeSymbol))
46+
context.RegisterOperationAction(context =>
6047
{
61-
return;
62-
}
63-
64-
foreach (AttributeData attribute in fieldSymbol.GetAttributes())
65-
{
66-
// Look for the [ObservableProperty] attribute (there can only ever be one per field)
67-
if (attribute.AttributeClass is { Name: "ObservablePropertyAttribute" } attributeClass &&
68-
context.Compilation.GetTypeByMetadataName("CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") is INamedTypeSymbol attributeSymbol &&
69-
SymbolEqualityComparer.Default.Equals(attributeClass, attributeSymbol))
48+
// We're only looking for references to fields that could potentially be observable properties
49+
if (context.Operation is not IFieldReferenceOperation
50+
{
51+
Field: IFieldSymbol { IsStatic: false, IsConst: false, IsImplicitlyDeclared: false, ContainingType: INamedTypeSymbol } fieldSymbol,
52+
Instance.Type: ITypeSymbol typeSymbol
53+
})
7054
{
71-
// Emit a warning to redirect users to access the generated property instead
72-
context.ReportDiagnostic(Diagnostic.Create(
73-
FieldReferenceForObservablePropertyFieldWarning,
74-
context.Operation.Syntax.GetLocation(),
75-
ImmutableDictionary.Create<string, string?>()
76-
.Add(FieldNameKey, fieldSymbol.Name)
77-
.Add(PropertyNameKey, ObservablePropertyGenerator.Execute.GetGeneratedPropertyName(fieldSymbol)),
78-
fieldSymbol));
55+
return;
56+
}
7957

58+
// Special case field references from within a constructor and don't ever emit warnings for them. The point of this
59+
// analyzer is to prevent mistakes when users assign a field instead of a property and then get confused when the
60+
// property changed event is not raised. But this would never be the case from a constructur anyway, given that
61+
// no handler for that event would possibly be present. Suppressing warnings in this cases though will help to
62+
// avoid scenarios where people get nullability warnings they cannot suppress, in case they were pushed by the
63+
// analyzer in the MVVM Toolkit to not assign a field marked with a non-nullable reference type. Ideally this
64+
// would be solved by habing the generated setter be marked with [MemberNotNullIfNotNull("field", "value")],
65+
// but such an annotation does not currently exist.
66+
if (context.ContainingSymbol is IMethodSymbol { MethodKind: MethodKind.Constructor, ContainingType: INamedTypeSymbol instanceType } &&
67+
SymbolEqualityComparer.Default.Equals(instanceType, typeSymbol))
68+
{
8069
return;
8170
}
82-
}
83-
}, OperationKind.FieldReference);
71+
72+
foreach (AttributeData attribute in fieldSymbol.GetAttributes())
73+
{
74+
// Look for the [ObservableProperty] attribute (there can only ever be one per field)
75+
if (attribute.AttributeClass is { Name: "ObservablePropertyAttribute" } attributeClass &&
76+
SymbolEqualityComparer.Default.Equals(attributeClass, observablePropertySymbol))
77+
{
78+
// Emit a warning to redirect users to access the generated property instead
79+
context.ReportDiagnostic(Diagnostic.Create(
80+
FieldReferenceForObservablePropertyFieldWarning,
81+
context.Operation.Syntax.GetLocation(),
82+
ImmutableDictionary.Create<string, string?>()
83+
.Add(FieldNameKey, fieldSymbol.Name)
84+
.Add(PropertyNameKey, ObservablePropertyGenerator.Execute.GetGeneratedPropertyName(fieldSymbol)),
85+
fieldSymbol));
86+
87+
return;
88+
}
89+
}
90+
}, OperationKind.FieldReference);
91+
});
8492
}
8593
}

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

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,19 @@ public override void Initialize(AnalysisContext context)
4848
return;
4949
}
5050

51-
context.RegisterSymbolAction(static context =>
51+
// Try to get all necessary type symbols to map
52+
if (!context.Compilation.TryBuildNamedTypeSymbolMap(GeneratorAttributeNamesToFullyQualifiedNamesMap, out ImmutableDictionary<string, INamedTypeSymbol>? typeSymbols))
53+
{
54+
return;
55+
}
56+
57+
// We also need the symbol for [ObservableProperty], separately
58+
if (context.Compilation.GetTypeByMetadataName("CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") is not INamedTypeSymbol observablePropertySymbol)
59+
{
60+
return;
61+
}
62+
63+
context.RegisterSymbolAction(context =>
5264
{
5365
ImmutableArray<AttributeData> attributes = context.Symbol.GetAttributes();
5466

@@ -61,17 +73,15 @@ public override void Initialize(AnalysisContext context)
6173
foreach (AttributeData dependentAttribute in attributes)
6274
{
6375
// 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.
76+
// The logic here is the same as the one in UnsupportedCSharpLanguageVersionAnalyzer.
6577
if (dependentAttribute.AttributeClass is { Name: string attributeName } dependentAttributeClass &&
66-
GeneratorAttributeNamesToFullyQualifiedNamesMap.TryGetValue(attributeName, out string? fullyQualifiedDependentAttributeName) &&
67-
context.Compilation.GetTypeByMetadataName(fullyQualifiedDependentAttributeName) is INamedTypeSymbol dependentAttributeSymbol &&
78+
typeSymbols.TryGetValue(attributeName, out INamedTypeSymbol? dependentAttributeSymbol) &&
6879
SymbolEqualityComparer.Default.Equals(dependentAttributeClass, dependentAttributeSymbol))
6980
{
7081
// If the attribute matches, iterate over the attributes to try to find [ObservableProperty]
7182
foreach (AttributeData attribute in attributes)
7283
{
7384
if (attribute.AttributeClass is { Name: "ObservablePropertyAttribute" } attributeSymbol &&
74-
context.Compilation.GetTypeByMetadataName("CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") is INamedTypeSymbol observablePropertySymbol &&
7585
SymbolEqualityComparer.Default.Equals(attributeSymbol, observablePropertySymbol))
7686
{
7787
// If [ObservableProperty] is found, then this field is valid in that it doesn't have orphaned dependent attributes

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public override void Initialize(AnalysisContext context)
4444
return;
4545
}
4646

47-
// Emit a diagnstic for types that use [NotifyPropertyChangedRecipients] but are neither inheriting from ObservableRecipient nor using [ObservableRecipient]
47+
// Emit a diagnostic for types that use [NotifyPropertyChangedRecipients] but are neither inheriting from ObservableRecipient nor using [ObservableRecipient]
4848
if (classSymbol.HasAttributeWithType(notifyPropertyChangedRecipientsAttributeSymbol) &&
4949
!classSymbol.InheritsFromType(observableRecipientSymbol) &&
5050
!classSymbol.HasOrInheritsAttributeWithType(observableRecipientAttributeSymbol))

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,13 @@ public override void Initialize(AnalysisContext context)
5454
return;
5555
}
5656

57-
context.RegisterSymbolAction(static context =>
57+
// Try to get all necessary type symbols
58+
if (!context.Compilation.TryBuildNamedTypeSymbolMap(GeneratorAttributeNamesToFullyQualifiedNamesMap, out ImmutableDictionary<string, INamedTypeSymbol>? typeSymbols))
59+
{
60+
return;
61+
}
62+
63+
context.RegisterSymbolAction(context =>
5864
{
5965
// The possible attribute targets are only fields, classes and methods
6066
if (context.Symbol is not (IFieldSymbol or INamedTypeSymbol { TypeKind: TypeKind.Class, IsImplicitlyDeclared: false } or IMethodSymbol))
@@ -65,12 +71,9 @@ public override void Initialize(AnalysisContext context)
6571
foreach (AttributeData attribute in context.Symbol.GetAttributes())
6672
{
6773
// Go over each attribute on the target symbol, and check if the attribute type name is a candidate.
68-
// If it is, double check by actually resolving the symbol from the compilation and comparing against it.
69-
// This minimizes the calls to CompilationGetTypeByMetadataName(string) to only cases where it's almost
70-
// guaranteed we'll actually get a match. If we do have one, then we can emit the diagnostic for the symbol.
74+
// If it is, double check by actually resolving the symbol from the mapping and comparing against it.
7175
if (attribute.AttributeClass is { Name: string attributeName } attributeClass &&
72-
GeneratorAttributeNamesToFullyQualifiedNamesMap.TryGetValue(attributeName, out string? fullyQualifiedAttributeName) &&
73-
context.Compilation.GetTypeByMetadataName(fullyQualifiedAttributeName) is INamedTypeSymbol attributeSymbol &&
76+
typeSymbols.TryGetValue(attributeName, out INamedTypeSymbol? attributeSymbol) &&
7477
SymbolEqualityComparer.Default.Equals(attributeClass, attributeSymbol))
7578
{
7679
context.ReportDiagnostic(Diagnostic.Create(UnsupportedCSharpLanguageVersionError, context.Symbol.Locations.FirstOrDefault()));

0 commit comments

Comments
 (0)