NonNullabkeFieldsSuppressor: Recognize (indirect) recursive methods.

nunit · Sep 27, 2023 · 3fc4980 · 3fc4980
1 parent b6d9d2b
commit 3fc4980
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 10 deletions.
diff --git a/...s.tests/DiagnosticSuppressors/NonNullableFieldOrPropertyIsUninitializedSuppressorTests.cs b/...s.tests/DiagnosticSuppressors/NonNullableFieldOrPropertyIsUninitializedSuppressorTests.cs
@@ -401,5 +401,39 @@ public void Test()
 
             RoslynAssert.Suppressed(suppressor, testCode);
         }
+
+        [Test]
+        public void ShouldDealWithRecursiveMethods()
+        {
+            var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
+                private Dictionary<string, bool> _values = new();
+                private string ↓_lastOne;
+
+                [SetUp]
+                public void SetUp()
+                {
+                    Recurse(""SetUp"");
+                }
+
+                [Test]
+                public void MinimalRepro()
+                {
+                    Recurse(""help"");
+                }
+
+                private void Recurse(string one, bool keepGoing = true)
+                {
+                    _values[one] = keepGoing;
+                    if (!keepGoing)
+                    {
+                        return;
+                    }
+                    Recurse(one, false);
+                    _lastOne = one;
+                }
+            ", "using System.Collections.Generic;");
+
+            RoslynAssert.Suppressed(suppressor, testCode);
+        }
     }
 }
diff --git a/...it.analyzers/DiagnosticSuppressors/NonNullableFieldOrPropertyIsUninitializedSuppressor.cs b/...it.analyzers/DiagnosticSuppressors/NonNullableFieldOrPropertyIsUninitializedSuppressor.cs
@@ -107,7 +107,8 @@ public override void ReportSuppressions(SuppressionAnalysisContext context)
                     if (isSetup)
                     {
                         // Find (OneTime)SetUps method and check for assignment to this field.
-                        if (IsAssignedIn(model, classDeclaration, method, fieldOrPropertyName))
+                        HashSet<MethodDeclarationSyntax> visitedMethods = new();
+                        if (IsAssignedIn(model, classDeclaration, visitedMethods, method, fieldOrPropertyName))
                         {
                             context.ReportSuppression(Suppression.Create(NullableFieldOrPropertyInitializedInSetUp, diagnostic));
                         }
@@ -119,17 +120,18 @@ public override void ReportSuppressions(SuppressionAnalysisContext context)
         private static bool IsAssignedIn(
             SemanticModel model,
             ClassDeclarationSyntax classDeclaration,
+            HashSet<MethodDeclarationSyntax> visitedMethods,
             MethodDeclarationSyntax method,
             string fieldOrPropertyName)
         {
             if (method.ExpressionBody is not null)
             {
-                return IsAssignedIn(model, classDeclaration, method.ExpressionBody.Expression, fieldOrPropertyName);
+                return IsAssignedIn(model, classDeclaration, visitedMethods, method.ExpressionBody.Expression, fieldOrPropertyName);
             }
 
             if (method.Body is not null)
             {
-                return IsAssignedIn(model, classDeclaration, method.Body, fieldOrPropertyName);
+                return IsAssignedIn(model, classDeclaration, visitedMethods, method.Body, fieldOrPropertyName);
             }
 
             return false;
@@ -138,21 +140,22 @@ private static bool IsAssignedIn(
         private static bool IsAssignedIn(
             SemanticModel model,
             ClassDeclarationSyntax classDeclaration,
+            HashSet<MethodDeclarationSyntax> visitedMethods,
             StatementSyntax statement,
             string fieldOrPropertyName)
         {
             switch (statement)
             {
                 case ExpressionStatementSyntax expressionStatement:
-                    return IsAssignedIn(model, classDeclaration, expressionStatement.Expression, fieldOrPropertyName);
+                    return IsAssignedIn(model, classDeclaration, visitedMethods, expressionStatement.Expression, fieldOrPropertyName);
 
                 case BlockSyntax block:
-                    return IsAssignedIn(model, classDeclaration, block.Statements, fieldOrPropertyName);
+                    return IsAssignedIn(model, classDeclaration, visitedMethods, block.Statements, fieldOrPropertyName);
 
                 case TryStatementSyntax tryStatement:
-                    return IsAssignedIn(model, classDeclaration, tryStatement.Block, fieldOrPropertyName) ||
+                    return IsAssignedIn(model, classDeclaration, visitedMethods, tryStatement.Block, fieldOrPropertyName) ||
                         (tryStatement.Finally is not null &&
-                        IsAssignedIn(model, classDeclaration, tryStatement.Finally.Block, fieldOrPropertyName));
+                        IsAssignedIn(model, classDeclaration, visitedMethods, tryStatement.Finally.Block, fieldOrPropertyName));
 
                 default:
                     // Any conditional statement does not guarantee assignment.
@@ -163,12 +166,13 @@ private static bool IsAssignedIn(
         private static bool IsAssignedIn(
             SemanticModel model,
             ClassDeclarationSyntax classDeclaration,
+            HashSet<MethodDeclarationSyntax> visitedMethods,
             SyntaxList<StatementSyntax> statements,
             string fieldOrPropertyName)
         {
             foreach (var statement in statements)
             {
-                if (IsAssignedIn(model, classDeclaration, statement, fieldOrPropertyName))
+                if (IsAssignedIn(model, classDeclaration, visitedMethods, statement, fieldOrPropertyName))
                     return true;
             }
 
@@ -178,6 +182,7 @@ private static bool IsAssignedIn(
         private static bool IsAssignedIn(
             SemanticModel model,
             ClassDeclarationSyntax classDeclaration,
+            HashSet<MethodDeclarationSyntax> visitedMethods,
             InvocationExpressionSyntax invocationExpression,
             string fieldOrPropertyName)
         {
@@ -190,7 +195,11 @@ private static bool IsAssignedIn(
             if (method?.Parent == classDeclaration)
             {
                 // We only get here if the method is in our source code and our class.
-                return IsAssignedIn(model, classDeclaration, method, fieldOrPropertyName);
+                if (!visitedMethods.Contains(method))
+                {
+                    visitedMethods.Add(method);
+                    return IsAssignedIn(model, classDeclaration, visitedMethods, method, fieldOrPropertyName);
+                }
             }
 
             return false;
@@ -199,6 +208,7 @@ private static bool IsAssignedIn(
         private static bool IsAssignedIn(
             SemanticModel model,
             ClassDeclarationSyntax classDeclaration,
+            HashSet<MethodDeclarationSyntax> visitedMethods,
             ExpressionSyntax? expressionStatement,
             string fieldOrPropertyName)
         {
@@ -245,7 +255,7 @@ memberAccessExpression.Expression is InvocationExpressionSyntax awaitedInvocatio
                 string? identifier = GetIdentifier(invocationExpression.Expression);
 
                 if (!string.IsNullOrEmpty(identifier) &&
-                    IsAssignedIn(model, classDeclaration, invocationExpression, fieldOrPropertyName))
+                    IsAssignedIn(model, classDeclaration, visitedMethods, invocationExpression, fieldOrPropertyName))
                 {
                     return true;
                 }