Review 2

SonarSource · Dec 20, 2023 · 3cb2551 · 3cb2551
1 parent 30da70e
commit 3cb2551
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 16 deletions.
diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs
@@ -243,7 +243,13 @@ private IEnumerable<ExplodedNode> ExceptionSuccessors(ExplodedNode node, Excepti
             var reachableHandlers = tryRegion.EnclosingRegion.NestedRegions.Where(x => x.Kind != ControlFlowRegionKind.Try && IsReachable(x, state.Exception)).ToArray();
             foreach (var handler in reachableHandlers)  // CatchRegion, FinallyRegion or FilterAndHandlerRegion
             {
-                handlersAreExhaustive |= CatchesAll(handler) || state.Exception.Type.DerivesFrom(handler.ExceptionType);
+                handlersAreExhaustive |= handler.Kind switch
+                {
+                    ControlFlowRegionKind.Finally => true,
+                    ControlFlowRegionKind.FilterAndHandler => false,
+                    _ => state.Exception.Type.DerivesFrom(handler.ExceptionType)
+                        || handler.ExceptionType.IsAny(KnownType.System_Exception, KnownType.System_Object) // relevant for UnkonwnException: 'catch (Exception) { ... }' and 'catch { ... }'
+                };
                 yield return new(cfg.Blocks[handler.FirstBlockOrdinal], state, null);
             }
             tryRegion = tryRegion.EnclosingRegion.EnclosingRegionOrSelf(ControlFlowRegionKind.Try); // Inner catch for specific exception doesn't match. Go to outer one.
@@ -297,15 +303,7 @@ private static bool IsReachable(ControlFlowBranch branch, bool condition, bool c
         branch.IsConditionalSuccessor ? condition == constraint : condition != constraint;
 
     private static bool IsReachable(ControlFlowRegion region, ExceptionState thrown) =>
-        CatchesAll(region)
+        region.Kind == ControlFlowRegionKind.Finally
         || thrown == ExceptionState.UnknownException
         || thrown.Type.DerivesFrom(region.ExceptionType);
-
-    private static bool CatchesAll(ControlFlowRegion region) =>
-        region.Kind switch
-        {
-            ControlFlowRegionKind.FilterAndHandler => false,
-            ControlFlowRegionKind.Finally => true,
-            _ => region.ExceptionType.Is(KnownType.System_Exception) || region.ExceptionType.Is(KnownType.System_Object)
-        };
 }
diff --git a/...ests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Throw.cs b/...ests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Throw.cs
@@ -610,11 +610,13 @@ public void Throw_TryCatch_Throw_CatchBaseTypeAndSpecificType_WithWhenCondition_
         validator.ValidateTagOrder(
             "BeforeTry",
             "InTry",
-            "InCatchSpecificWithCondition", // should be followed by InCatchSpecificNoCondition
+            "InCatchSpecificNoCondition",
+            "InCatchBase",    // It would be better not visit this state, but it gets tricky with conditions
+            "InCatchSpecificWithCondition",
             "End");
-        validator.TagStates("InCatchBase").Should().BeEmpty();
+        validator.TagStates("InCatchBase").Should().HaveCount(1).And.ContainSingle(x => HasExceptionOfType(x, "FileNotFoundException"));    // Should().BeEmpt()
         validator.TagStates("InCatchSpecificWithCondition").Should().HaveCount(1).And.ContainSingle(x => HasExceptionOfType(x, "FileNotFoundException"));
-        validator.TagStates("InCatchSpecificNoCondition").Should().BeEmpty();   // Should().HaveCount(1).And.ContainSingle(x => HasExceptionOfType(x, "FileNotFoundException"))
+        validator.TagStates("InCatchSpecificNoCondition").Should().HaveCount(1).And.ContainSingle(x => HasExceptionOfType(x, "FileNotFoundException"));
     }
 
     [TestMethod]

diff --git a/...Analyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.TryCatchFinally.cs b/...Analyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.TryCatchFinally.cs
@@ -810,7 +810,9 @@ public void CatchWhen_Simple()
             "End");
 
         validator.TagStates("InCatch").Should().HaveCount(1).And.ContainSingle(x => HasUnknownException(x));
-        validator.TagStates("InFinally").Should().HaveCount(2).And.ContainSingle(x => HasNoException(x)).And.ContainSingle(x => HasUnknownException(x));
+        validator.TagStates("InFinally").Should().HaveCount(2)
+            .And.ContainSingle(x => HasNoException(x), "if no exception is thrown, execution goes from 'InTry' to 'InFinally'.")
+            .And.ContainSingle(x => HasUnknownException(x), "if an exception is thrown, execution goes fron 'InTry' to 'InCatch' to 'InFinally'.");
         validator.TagStates("End").Should().HaveCount(1).And.ContainSingle(x => HasNoException(x));
     }
 

diff --git a/...SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs b/...SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs
@@ -3533,7 +3533,7 @@ public void TestMethod()
     private void Callback() { }
 }
 
-// Reproducer for https://github.com/SonarSource/sonar-dotnet/issues/8484
+// https://github.com/SonarSource/sonar-dotnet/issues/8484
 public class Repro_8484
 {
     void TestMethod()

diff --git a/...zer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Monitor.TryCatch.cs b/...zer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Monitor.TryCatch.cs
@@ -96,7 +96,7 @@ public void Method6(string arg)
 
         public void CatchWhen_SpecificException(string arg)
         {
-            Monitor.Enter(obj); // FN
+            Monitor.Enter(obj); // Noncompliant
             try
             {
                 throw new NotImplementedException();