Merge pull request #587 from CommunityToolkit/dev/resolve-symbols-early

Resolve all needed symbols early in analyzers

Author: Sergio0694

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

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
+using CommunityToolkit.Mvvm.SourceGenerators.Extensions;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Diagnostics;
 using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors;
@@ -56,27 +57,31 @@ public override void Initialize(AnalysisContext context)
         context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
         context.EnableConcurrentExecution();
 
-        context.RegisterSymbolAction(static context =>
+        context.RegisterCompilationStartAction(static context =>
         {
-            // We're looking for class declarations
-            if (context.Symbol is not INamedTypeSymbol { TypeKind: TypeKind.Class, IsRecord: false, IsStatic: false, IsImplicitlyDeclared: false } classSymbol)
+            // Try to get all necessary type symbols
+            if (!context.Compilation.TryBuildNamedTypeSymbolMap(GeneratorAttributeNamesToFullyQualifiedNamesMap, out ImmutableDictionary<string, INamedTypeSymbol>? typeSymbols))
             {
                 return;
             }
 
-            foreach (AttributeData attribute in context.Symbol.GetAttributes())
+            context.RegisterSymbolAction(context =>
             {
-                // Same logic as in FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer to find target attributes
-                if (attribute.AttributeClass is { Name: string attributeName } attributeClass &&
-                    GeneratorAttributeNamesToFullyQualifiedNamesMap.TryGetValue(attributeName, out string? fullyQualifiedAttributeName) &&
-                    context.Compilation.GetTypeByMetadataName(fullyQualifiedAttributeName) is INamedTypeSymbol attributeSymbol &&
-                    SymbolEqualityComparer.Default.Equals(attributeClass, attributeSymbol))
+                // We're looking for class declarations that don't have any base type
+                if (context.Symbol is not INamedTypeSymbol { TypeKind: TypeKind.Class, IsRecord: false, IsStatic: false, IsImplicitlyDeclared: false, BaseType.SpecialType: SpecialType.System_Object } classSymbol)
                 {
-                    // The type is annotated with either [ObservableObject] or [INotifyPropertyChanged].
-                    // Next, we need to check whether it isn't already inheriting from another type.
-                    if (classSymbol.BaseType is { SpecialType: SpecialType.System_Object })
+                    return;
+                }
+
+                foreach (AttributeData attribute in context.Symbol.GetAttributes())
+                {
+                    // Same logic as in FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer to find target attributes
+                    if (attribute.AttributeClass is { Name: string attributeName } attributeClass &&
+                        typeSymbols.TryGetValue(attributeName, out INamedTypeSymbol? attributeSymbol) &&
+                        SymbolEqualityComparer.Default.Equals(attributeClass, attributeSymbol))
                     {
-                        // This type is using the attribute when it could just inherit from ObservableObject, which is preferred
+                        // The type is annotated with either [ObservableObject] or [INotifyPropertyChanged], and we already validated
+                        // that it has no other base type, so emit a diagnostic to suggest inheriting from ObservableObject instead.
                         context.ReportDiagnostic(Diagnostic.Create(
                             GeneratorAttributeNamesToDiagnosticsMap[attributeClass.Name],
                             context.Symbol.Locations.FirstOrDefault(),
@@ -86,7 +91,7 @@ public override void Initialize(AnalysisContext context)
                             context.Symbol));
                     }
                 }
-            }
-        }, SymbolKind.NamedType);
+            }, SymbolKind.NamedType);
+        });
     }
 }

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

@@ -35,51 +35,59 @@ public override void Initialize(AnalysisContext context)
         context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
         context.EnableConcurrentExecution();
 
-        context.RegisterOperationAction(static context =>
+        context.RegisterCompilationStartAction(static context =>
         {
-            // We're only looking for references to fields that could potentially be observable properties
-            if (context.Operation is not IFieldReferenceOperation
-                {
-                    Field: IFieldSymbol { IsStatic: false, IsConst: false, IsImplicitlyDeclared: false, ContainingType: INamedTypeSymbol } fieldSymbol,
-                    Instance.Type: ITypeSymbol typeSymbol
-                })
+            // Get the symbol for [ObservableProperty]
+            if (context.Compilation.GetTypeByMetadataName("CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") is not INamedTypeSymbol observablePropertySymbol)
             {
                 return;
             }
 
-            // Special case field references from within a constructor and don't ever emit warnings for them. The point of this
-            // analyzer is to prevent mistakes when users assign a field instead of a property and then get confused when the
-            // property changed event is not raised. But this would never be the case from a constructur anyway, given that
-            // no handler for that event would possibly be present. Suppressing warnings in this cases though will help to
-            // avoid scenarios where people get nullability warnings they cannot suppress, in case they were pushed by the
-            // analyzer in the MVVM Toolkit to not assign a field marked with a non-nullable reference type. Ideally this
-            // would be solved by habing the generated setter be marked with [MemberNotNullIfNotNull("field", "value")],
-            // but such an annotation does not currently exist.
-            if (context.ContainingSymbol is IMethodSymbol { MethodKind: MethodKind.Constructor, ContainingType: INamedTypeSymbol instanceType } &&
-                SymbolEqualityComparer.Default.Equals(instanceType, typeSymbol))
+            context.RegisterOperationAction(context =>
             {
-                return;
-            }
-
-            foreach (AttributeData attribute in fieldSymbol.GetAttributes())
-            {
-                // Look for the [ObservableProperty] attribute (there can only ever be one per field)
-                if (attribute.AttributeClass is { Name: "ObservablePropertyAttribute" } attributeClass &&
-                    context.Compilation.GetTypeByMetadataName("CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") is INamedTypeSymbol attributeSymbol &&
-                    SymbolEqualityComparer.Default.Equals(attributeClass, attributeSymbol))
+                // We're only looking for references to fields that could potentially be observable properties
+                if (context.Operation is not IFieldReferenceOperation
+                    {
+                        Field: IFieldSymbol { IsStatic: false, IsConst: false, IsImplicitlyDeclared: false, ContainingType: INamedTypeSymbol } fieldSymbol,
+                        Instance.Type: ITypeSymbol typeSymbol
+                    })
                 {
-                    // Emit a warning to redirect users to access the generated property instead
-                    context.ReportDiagnostic(Diagnostic.Create(
-                        FieldReferenceForObservablePropertyFieldWarning,
-                        context.Operation.Syntax.GetLocation(),
-                        ImmutableDictionary.Create<string, string?>()
-                            .Add(FieldNameKey, fieldSymbol.Name)
-                            .Add(PropertyNameKey, ObservablePropertyGenerator.Execute.GetGeneratedPropertyName(fieldSymbol)),
-                        fieldSymbol));
+                    return;
+                }
 
+                // Special case field references from within a constructor and don't ever emit warnings for them. The point of this
+                // analyzer is to prevent mistakes when users assign a field instead of a property and then get confused when the
+                // property changed event is not raised. But this would never be the case from a constructur anyway, given that
+                // no handler for that event would possibly be present. Suppressing warnings in this cases though will help to
+                // avoid scenarios where people get nullability warnings they cannot suppress, in case they were pushed by the
+                // analyzer in the MVVM Toolkit to not assign a field marked with a non-nullable reference type. Ideally this
+                // would be solved by habing the generated setter be marked with [MemberNotNullIfNotNull("field", "value")],
+                // but such an annotation does not currently exist.
+                if (context.ContainingSymbol is IMethodSymbol { MethodKind: MethodKind.Constructor, ContainingType: INamedTypeSymbol instanceType } &&
+                    SymbolEqualityComparer.Default.Equals(instanceType, typeSymbol))
+                {
                     return;
                 }
-            }
-        }, OperationKind.FieldReference);
+
+                foreach (AttributeData attribute in fieldSymbol.GetAttributes())
+                {
+                    // Look for the [ObservableProperty] attribute (there can only ever be one per field)
+                    if (attribute.AttributeClass is { Name: "ObservablePropertyAttribute" } attributeClass &&
+                        SymbolEqualityComparer.Default.Equals(attributeClass, observablePropertySymbol))
+                    {
+                        // Emit a warning to redirect users to access the generated property instead
+                        context.ReportDiagnostic(Diagnostic.Create(
+                            FieldReferenceForObservablePropertyFieldWarning,
+                            context.Operation.Syntax.GetLocation(),
+                            ImmutableDictionary.Create<string, string?>()
+                                .Add(FieldNameKey, fieldSymbol.Name)
+                                .Add(PropertyNameKey, ObservablePropertyGenerator.Execute.GetGeneratedPropertyName(fieldSymbol)),
+                            fieldSymbol));
+
+                        return;
+                    }
+                }
+            }, OperationKind.FieldReference);
+        });
     }
 }

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

@@ -48,7 +48,19 @@ public override void Initialize(AnalysisContext context)
                 return;
             }
 
-            context.RegisterSymbolAction(static context =>
+            // Try to get all necessary type symbols to map
+            if (!context.Compilation.TryBuildNamedTypeSymbolMap(GeneratorAttributeNamesToFullyQualifiedNamesMap, out ImmutableDictionary<string, INamedTypeSymbol>? typeSymbols))
+            {
+                return;
+            }
+
+            // We also need the symbol for [ObservableProperty], separately
+            if (context.Compilation.GetTypeByMetadataName("CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") is not INamedTypeSymbol observablePropertySymbol)
+            {
+                return;
+            }
+
+            context.RegisterSymbolAction(context =>
             {
                 ImmutableArray<AttributeData> attributes = context.Symbol.GetAttributes();
 
@@ -61,17 +73,15 @@ public override void Initialize(AnalysisContext context)
                 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.
+                    // The logic here is the same as the one in UnsupportedCSharpLanguageVersionAnalyzer.
                     if (dependentAttribute.AttributeClass is { Name: string attributeName } dependentAttributeClass &&
-                        GeneratorAttributeNamesToFullyQualifiedNamesMap.TryGetValue(attributeName, out string? fullyQualifiedDependentAttributeName) &&
-                        context.Compilation.GetTypeByMetadataName(fullyQualifiedDependentAttributeName) is INamedTypeSymbol dependentAttributeSymbol &&
+                        typeSymbols.TryGetValue(attributeName, out 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

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

@@ -44,7 +44,7 @@ public override void Initialize(AnalysisContext context)
                     return;
                 }
 
-                // Emit a diagnstic for types that use [NotifyPropertyChangedRecipients] but are neither inheriting from ObservableRecipient nor using [ObservableRecipient]
+                // Emit a diagnostic for types that use [NotifyPropertyChangedRecipients] but are neither inheriting from ObservableRecipient nor using [ObservableRecipient]
                 if (classSymbol.HasAttributeWithType(notifyPropertyChangedRecipientsAttributeSymbol) &&
                     !classSymbol.InheritsFromType(observableRecipientSymbol) &&
                     !classSymbol.HasOrInheritsAttributeWithType(observableRecipientAttributeSymbol))

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

@@ -54,7 +54,13 @@ public override void Initialize(AnalysisContext context)
                 return;
             }
 
-            context.RegisterSymbolAction(static context =>
+            // Try to get all necessary type symbols
+            if (!context.Compilation.TryBuildNamedTypeSymbolMap(GeneratorAttributeNamesToFullyQualifiedNamesMap, out ImmutableDictionary<string, INamedTypeSymbol>? typeSymbols))
+            {
+                return;
+            }
+
+            context.RegisterSymbolAction(context =>
             {
                 // The possible attribute targets are only fields, classes and methods
                 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)
                 foreach (AttributeData attribute in context.Symbol.GetAttributes())
                 {
                     // Go over each attribute on the target symbol, and check if the attribute type name is a candidate.
-                    // If it is, double check by actually resolving the symbol from the compilation and comparing against it.
-                    // This minimizes the calls to CompilationGetTypeByMetadataName(string) to only cases where it's almost
-                    // guaranteed we'll actually get a match. If we do have one, then we can emit the diagnostic for the symbol.
+                    // If it is, double check by actually resolving the symbol from the mapping and comparing against it.
                     if (attribute.AttributeClass is { Name: string attributeName } attributeClass &&
-                        GeneratorAttributeNamesToFullyQualifiedNamesMap.TryGetValue(attributeName, out string? fullyQualifiedAttributeName) &&
-                        context.Compilation.GetTypeByMetadataName(fullyQualifiedAttributeName) is INamedTypeSymbol attributeSymbol &&
+                        typeSymbols.TryGetValue(attributeName, out INamedTypeSymbol? attributeSymbol) &&
                         SymbolEqualityComparer.Default.Equals(attributeClass, attributeSymbol))
                     {
                         context.ReportDiagnostic(Diagnostic.Create(UnsupportedCSharpLanguageVersionError, context.Symbol.Locations.FirstOrDefault()));

src/CommunityToolkit.Mvvm.SourceGenerators/Extensions/CompilationExtensions.cs

@@ -2,6 +2,10 @@
 // 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;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Diagnostics.CodeAnalysis;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 
@@ -87,4 +91,37 @@ public static bool HasAccessibleTypeWithMetadataName(this Compilation compilatio
 
         return false;
     }
+
+    /// <summary>
+    /// Tries to build a map of <see cref="INamedTypeSymbol"/> instances form the input mapping of names.
+    /// </summary>
+    /// <typeparam name="T">The type of keys for each symbol.</typeparam>
+    /// <param name="compilation">The <see cref="Compilation"/> to consider for analysis.</param>
+    /// <param name="typeNames">The input mapping of <typeparamref name="T"/> keys to fully qualified type names.</param>
+    /// <param name="typeSymbols">The resulting mapping of <typeparamref name="T"/> keys to resolved <see cref="INamedTypeSymbol"/> instances.</param>
+    /// <returns>Whether all requested <see cref="INamedTypeSymbol"/> instances could be resolved.</returns>
+    public static bool TryBuildNamedTypeSymbolMap<T>(
+        this Compilation compilation,
+        IEnumerable<KeyValuePair<T, string>> typeNames,
+        [NotNullWhen(true)] out ImmutableDictionary<T, INamedTypeSymbol>? typeSymbols)
+        where T : IEquatable<T>
+    {
+        ImmutableDictionary<T, INamedTypeSymbol>.Builder builder = ImmutableDictionary.CreateBuilder<T, INamedTypeSymbol>();
+
+        foreach (KeyValuePair<T, string> pair in typeNames)
+        {
+            if (compilation.GetTypeByMetadataName(pair.Value) is not INamedTypeSymbol attributeSymbol)
+            {
+                typeSymbols = null;
+
+                return false;
+            }
+
+            builder.Add(pair.Key, attributeSymbol);
+        }
+
+        typeSymbols = builder.ToImmutable();
+
+        return true;
+    }
 }