Merge pull request #532 from CommunityToolkit/dev/observable-property-field-reference-analyzer

Add analyzer for references to fields with [ObservableProperty]

Author: Sergio0694

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

@@ -40,3 +40,4 @@ MVVMTK0030 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator
 MVVMTK0031 | CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator | Error | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0031
 MVVMTK0032 | CommunityToolkit.Mvvm.SourceGenerators.INotifyPropertyChangedGenerator | Warning | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0032
 MVVMTK0033 | CommunityToolkit.Mvvm.SourceGenerators.ObservableObjectGenerator | Warning | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0033
+MVVMTK0034 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Warning | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0034

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

@@ -48,8 +48,9 @@
     <Compile Include="$(MSBuildThisFileDirectory)ComponentModel\ObservableValidatorValidateAllPropertiesGenerator.Execute.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)ComponentModel\TransitiveMembersGenerator.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)ComponentModel\TransitiveMembersGenerator.Execute.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\ClassUsingAttributeInsteadOfInheritanceAnalyzer.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\FieldWithOrphanedDependentObservablePropertyAttributesAnalyzer.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\FieldReferenceForObservablePropertyFieldAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\UnsupportedCSharpLanguageVersionAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Suppressors\ObservablePropertyAttributeWithPropertyTargetDiagnosticSuppressor.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Diagnostics\DiagnosticDescriptors.cs" />

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

@@ -46,8 +46,6 @@ public override void Initialize(AnalysisContext context)
         context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
         context.EnableConcurrentExecution();
 
-        // Defer the callback registration to when the compilation starts, so we can execute more
-        // preliminary checks and skip registering any kind of symbol analysis at all if not needed.
         context.RegisterSymbolAction(static context =>
         {
             // We're looking for class declarations

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

@@ -0,0 +1,51 @@
+// 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.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors;
+
+namespace CommunityToolkit.Mvvm.SourceGenerators;
+
+/// <summary>
+/// A diagnostic analyzer that generates a warning when accessing a field instead of a generated observable property.
+/// </summary>
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class FieldReferenceForObservablePropertyFieldAnalyzer : DiagnosticAnalyzer
+{
+    /// <inheritdoc/>
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(FieldReferenceForObservablePropertyFieldWarning);
+
+    /// <inheritdoc/>
+    public override void Initialize(AnalysisContext context)
+    {
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
+        context.EnableConcurrentExecution();
+
+        context.RegisterOperationAction(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 })
+            {
+                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))
+                {
+                    // Emit a warning to redirect users to access the generated property instead
+                    context.ReportDiagnostic(Diagnostic.Create(FieldReferenceForObservablePropertyFieldWarning, context.Operation.Syntax.GetLocation(), fieldSymbol));
+
+                    return;
+                }
+            }
+        }, OperationKind.FieldReference);
+    }
+}

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

@@ -4,7 +4,6 @@
 
 using System.ComponentModel;
 using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.CSharp;
 
 #pragma warning disable IDE0090 // Use 'new DiagnosticDescriptor(...)'
 
@@ -543,4 +542,20 @@ internal static class DiagnosticDescriptors
             "Classes with no base types should prefer inheriting from ObservableObject instead of using attributes to generate INotifyPropertyChanged code, as that will " +
             "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).",
         helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0032");
+
+    /// <summary>
+    /// Gets a <see cref="DiagnosticDescriptor"/> indicating when a field with <c>[ObservableProperty]</c> is being directly referenced.
+    /// <para>
+    /// Format: <c>"The field {0} is annotated with [ObservableProperty] and should not be directly referenced (use the generated property instead)"</c>.
+    /// </para>
+    /// </summary>
+    public static readonly DiagnosticDescriptor FieldReferenceForObservablePropertyFieldWarning = new DiagnosticDescriptor(
+        id: "MVVMTK0034",
+        title: "Invalid task scheduler exception flow option usage",
+        messageFormat: "The field {0} is annotated with [ObservableProperty] and should not be directly referenced (use the generated property instead)",
+        category: typeof(ObservablePropertyGenerator).FullName,
+        defaultSeverity: DiagnosticSeverity.Warning,
+        isEnabledByDefault: true,
+        description: "Fields with [ObservableProperty] should not be directly referenced, and the generated properties should be used instead.",
+        helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0033");
 }

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

@@ -1531,6 +1531,77 @@ public partial class {|MVVMTK0033:SampleViewModel|}
         await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<ClassUsingAttributeInsteadOfInheritanceAnalyzer>(source, LanguageVersion.CSharp8);
     }
 
+    // See https://github.com/CommunityToolkit/dotnet/issues/518
+    [TestMethod]
+    public async Task FieldReferenceToFieldWithObservablePropertyAttribute_Assignment()
+    {
+        string source = """
+            using CommunityToolkit.Mvvm.ComponentModel;
+
+            partial class MyViewModel : ObservableObject
+            {
+                [ObservableProperty]
+                int number;
+
+                void M1()
+                {
+                    {|MVVMTK0034:number|} = 1;
+                }
+
+                void M2()
+                {
+                    {|MVVMTK0034:this.number|} = 1;
+                }
+            }
+            """;
+
+        await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<FieldReferenceForObservablePropertyFieldAnalyzer>(source, LanguageVersion.CSharp8);
+    }
+
+    // See https://github.com/CommunityToolkit/dotnet/issues/518
+    [TestMethod]
+    public async Task FieldReferenceToFieldWithObservablePropertyAttribute_Read()
+    {
+        string source = """
+            using CommunityToolkit.Mvvm.ComponentModel;
+
+            partial class MyViewModel : ObservableObject
+            {
+                [ObservableProperty]
+                int number;
+
+                void M()
+                {
+                    var temp = {|MVVMTK0034:number|};
+                }
+            }
+            """;
+
+        await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<FieldReferenceForObservablePropertyFieldAnalyzer>(source, LanguageVersion.CSharp8);
+    }
+
+    // See https://github.com/CommunityToolkit/dotnet/issues/518
+    [TestMethod]
+    public async Task FieldReferenceToFieldWithObservablePropertyAttribute_Ref()
+    {
+        string source = """
+            using CommunityToolkit.Mvvm.ComponentModel;
+
+            partial class MyViewModel : ObservableObject
+            {
+                [ObservableProperty]
+                int number;
+
+                void M()
+                {
+                    ref var x = ref {|MVVMTK0034:number|};
+                }
+            }
+            """;
+
+        await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<FieldReferenceForObservablePropertyFieldAnalyzer>(source, LanguageVersion.CSharp8);
+    }
+
     /// <summary>
     /// 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>

tests/CommunityToolkit.Mvvm.UnitTests/Test_ObservablePropertyAttribute.cs

@@ -21,7 +21,7 @@
 using CommunityToolkit.Mvvm.Messaging.Messages;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
-#pragma warning disable MVVMTK0032, MVVMTK0033
+#pragma warning disable MVVMTK0032, MVVMTK0033, MVVMTK0034
 
 namespace CommunityToolkit.Mvvm.UnitTests;