Fix incorrect analyzer warning around tests assigning instance data (#…

…1634)
thomhurst · Jan 20, 2025 · 7054957 · 7054957
1 parent 0fe529f
commit 7054957
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 45 deletions.
diff --git a/TUnit.Analyzers.Tests/InstanceValuesInTestClassAnalyzerTests.cs b/TUnit.Analyzers.Tests/InstanceValuesInTestClassAnalyzerTests.cs
@@ -54,4 +54,51 @@ public void MyTest(string value)
                 """
             );
     }
+
+    [Test]
+    public async Task Do_Not_Flag_When_Not_Assigning_To_New_Class()
+    {
+        await Verifier
+            .VerifyAnalyzerAsync(
+                """
+                using System;
+                using TUnit.Core;
+
+                public record SomeDataClass
+                {
+                    public required Guid SomeGuid { get; set; }
+                    public required Guid SomeGuid2 { get; set; }
+                    public required Guid MyCoolGuid { get; set; }
+                }
+                
+                public class TestTest
+                {
+                
+                    private readonly Guid SomeGuid;
+                    private readonly Guid SomeGuid2;
+                    private Guid SomeGuid3; // IDE0052 => for this context, ignored
+                    private readonly Guid SomeVeryCoolGuid;
+                
+                    public TestTest()
+                    {
+                        SomeGuid = Guid.NewGuid();
+                        SomeGuid2 = Guid.NewGuid();
+                        SomeGuid3 = Guid.NewGuid();
+                        SomeVeryCoolGuid = Guid.NewGuid();
+                    }
+                
+                    [Test]
+                    public void SomeTest()
+                    {
+                        var _ = new SomeDataClass()
+                        {
+                            SomeGuid = SomeGuid, // 2 => Warning: TUnit0018
+                            MyCoolGuid = SomeGuid2, // 3 => nothing
+                            SomeGuid2 = SomeVeryCoolGuid // 4 => Warning: TUnit0018
+                        };
+                    }
+                }
+                """
+            );
+    }
 }
diff --git a/TUnit.Analyzers/InstanceValuesInTestClassAnalyzer.cs b/TUnit.Analyzers/InstanceValuesInTestClassAnalyzer.cs
@@ -1,9 +1,9 @@
 using System.Collections.Immutable;
+using System.Diagnostics.CodeAnalysis;
 using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
 using TUnit.Analyzers.Extensions;
-using TUnit.Analyzers.Helpers;
 
 namespace TUnit.Analyzers;
 
@@ -15,81 +15,90 @@ public class InstanceValuesInTestClassAnalyzer : ConcurrentDiagnosticAnalyzer
 
     protected override void InitializeInternal(AnalysisContext context)
     { 
-        context.RegisterSymbolAction(AnalyzeSymbol, SymbolKind.NamedType);
+        context.RegisterOperationAction(AnalyzeOperation, OperationKind.SimpleAssignment);
     }
-    
-    private void AnalyzeSymbol(SymbolAnalysisContext context)
-    { 
-        if (context.Symbol is not INamedTypeSymbol namedTypeSymbol)
+
+    private void AnalyzeOperation(OperationAnalysisContext context)
+    {
+        if (context.Operation is not IAssignmentOperation assignmentOperation)
         {
             return;
         }
-
-        var classMembers = namedTypeSymbol.GetMembers();
 
-        var tests = classMembers
-            .OfType<IMethodSymbol>()
-            .Where(x => x.GetAttributes()
-                .Any(a => WellKnown.AttributeFullyQualifiedClasses.Test.WithGlobalPrefix == a.AttributeClass?.GloballyQualifiedNonGeneric())
-            )
-            .ToList();
+        if (!TryGetParentMethodBody(assignmentOperation, out var methodBodyOperation))
+        {
+            return;
+        }
 
-        if (!tests.Any())
+        if (context.Operation.SemanticModel?.GetDeclaredSymbol(methodBodyOperation.Syntax) is not IMethodSymbol
+            methodSymbol)
         {
             return;
         }
 
-        var fieldsAndProperties = classMembers
+        if (!methodSymbol.IsTestMethod(context.Compilation))
+        {
+            return;
+        }
+
+        var testClass = methodSymbol.ContainingType;
+
+        var typeMembers = testClass.GetMembers();
+
+        var fieldsAndProperties = typeMembers
             .OfType<IFieldSymbol>()
-            .Concat<ISymbol>(classMembers.OfType<IPropertySymbol>())
+            .Concat<ISymbol>(typeMembers.OfType<IPropertySymbol>())
             .Where(x => !x.IsStatic);
 
         foreach (var fieldOrProperty in fieldsAndProperties)
         {
-            var fieldOrPropertySyntax = fieldOrProperty.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax();
+            var targetSymbol = GetTarget(assignmentOperation);
 
-            var methodDeclarationSyntaxes = tests
-                .Select(x => x.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax())
-                .OfType<MethodDeclarationSyntax>();
-
-            foreach (var methodDeclarationSyntax in methodDeclarationSyntaxes)
+            if (!SymbolEqualityComparer.Default.Equals(targetSymbol?.ContainingType, testClass))
             {
-                CheckMethod(context, methodDeclarationSyntax, fieldOrPropertySyntax);
+                return;
             }
-        }
-    }
-
-    private void CheckMethod(SymbolAnalysisContext context, MethodDeclarationSyntax methodDeclarationSyntax,
-        SyntaxNode? fieldOrPropertySyntax)
-    {
-        var descendantNodes = methodDeclarationSyntax.DescendantNodes();
-
-        foreach (var descendantNode in descendantNodes)
-        {
-            if (IsAssignment(descendantNode, fieldOrPropertySyntax))
+
+            if (SymbolEqualityComparer.Default.Equals(targetSymbol, fieldOrProperty))
             {
                 context.ReportDiagnostic(
                     Diagnostic.Create(Rules.InstanceAssignmentInTestClass,
-                        descendantNode.GetLocation())
-                );
+                        assignmentOperation.Syntax.GetLocation()));
             }
         }
     }
 
-    private bool IsAssignment(SyntaxNode syntaxNode, SyntaxNode? fieldOrPropertySyntax)
+    private static ISymbol? GetTarget(IAssignmentOperation assignmentOperation)
     {
-        if (syntaxNode is not AssignmentExpressionSyntax assignmentExpressionSyntax)
+        if (assignmentOperation.Target is IPropertyReferenceOperation propertyReferenceOperation)
         {
-            return false;
+            return propertyReferenceOperation.Property;
         }
+
+        if (assignmentOperation.Target is IFieldReferenceOperation fieldReferenceOperation)
+        {
+            return fieldReferenceOperation.Field;
+        }
+
+        return null;
+    }
 
-        var assignmentSyntaxChild = assignmentExpressionSyntax.ChildNodes().FirstOrDefault();
+    private static bool TryGetParentMethodBody(IAssignmentOperation assignmentOperation, [NotNullWhen(true)] out IMethodBodyOperation? methodBodyOperation)
+    {
+        var parent = assignmentOperation.Parent;
 
-        if (assignmentSyntaxChild is not IdentifierNameSyntax identifierNameSyntax)
+        while (parent is not null)
         {
-            return false;
+            if (parent is IMethodBodyOperation methodBody)
+            {
+                methodBodyOperation = methodBody;
+                return true;
+            }
+
+            parent = parent.Parent;
         }
 
-        return identifierNameSyntax.Identifier.ValueText == fieldOrPropertySyntax?.ToString();
+        methodBodyOperation  = null;
+        return false;
     }
 }