Migrate S2589: Fix rethrow FP (#7706)

SonarSource · Aug 8, 2023 · f94f344 · f94f344
1 parent f9f794d
commit f94f344
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 0 deletions.
diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs
@@ -133,6 +133,12 @@ private ExplodedNode ProcessBranch(ExplodedNode node, ControlFlowBranch branch)
         }
         else
         {
+            if (branch.Source.BranchValue is { } branchValue)
+            {
+                // If a branch has no Destination but is part of conditional branching we need to call ConditionEvaluated. This happens when a Rethrow is following a condition.
+                var state = SetBranchingConstraints(branch, node.State, branchValue);
+                checks.ConditionEvaluated(new(branchValue.ToSonar(), state, false, node.VisitCount, lva.CapturedVariables));
+            }
             return null;    // We don't know where to continue
         }
 

diff --git a/.../SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs b/.../SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs
@@ -508,6 +508,30 @@ public void Branching_ConditionEvaluated()
         SETestContext.CreateCS(code, check).Validator.ValidateTagOrder("Begin", "End");
     }
 
+    [TestMethod]
+    public void Branching_Rethrow_CallsConditionEvaluated()
+    {
+        const string code = """
+            try
+            {
+                Console.WriteLine("may throw");
+            }
+            catch
+            {
+                if (boolParameter)
+                    throw;
+            }
+            """;
+        var count = 0;
+        var check = new ConditionEvaluatedTestCheck(x =>
+            {
+                count++;
+                return x.State;
+            });
+        SETestContext.CreateCS(code, check).Validator.ValidateTagOrder();
+        count.Should().Be(2);
+    }
+
     [TestMethod]
     public void Branching_NullConstraints_VisitsIfBranch()
     {

diff --git a/...SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs b/...SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs
@@ -1576,6 +1576,25 @@ public void ObjectsShouldNotBeDisposedMoreThanOnce()
             }
         }
 
+        public void Throw(bool condition)
+        {
+            if (condition)      // Compliant
+                throw new Exception();
+        }
+
+        public void Rethrow(bool condition)
+        {
+            try
+            {
+                Console.WriteLine("may throw");
+            }
+            catch
+            {
+                if (condition)  // Compliant
+                    throw;
+            }
+        }
+
         public void FalseNegatives()
         {
             // We cannot detect the case in ObjectsShouldNotBeDisposedMoreThanOnce method above