diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ExplodedNode.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ExplodedNode.cs index 2e5925237dc..5328b1b3031 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ExplodedNode.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ExplodedNode.cs @@ -58,7 +58,7 @@ public int AddVisit() } public override int GetHashCode() => - HashCode.Combine(programPointHash, State); + HashCode.Combine(programPointHash, State, FinallyPoint?.BlockIndex); public override bool Equals(object obj) => Equals(obj as ExplodedNode); @@ -66,10 +66,29 @@ public int AddVisit() public bool Equals(ExplodedNode other) => other is not null && other.programPointHash == programPointHash - && other.State.Equals(State); + && other.State.Equals(State) + && HasSameFinallyPointChain(other.FinallyPoint); public override string ToString() => Operation.Instance is { } operation ? $"Block #{Block.Ordinal}, Operation #{index}, {operation.Serialize()}{Environment.NewLine}{State}" : $"Block #{Block.Ordinal}, Branching{Environment.NewLine}{State}"; + + private bool HasSameFinallyPointChain(FinallyPoint other) + { + var current = FinallyPoint; + while (current is not null && other is not null) + { + if (current.BlockIndex == other.BlockIndex && current.BranchDestination == other.BranchDestination) + { + current = current.Previous; + other = other.Previous; + } + else + { + return false; + } + } + return current is null && other is null; + } } diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/FinallyPoint.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/FinallyPoint.cs index d02c043e71a..270795e3984 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/FinallyPoint.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/FinallyPoint.cs @@ -29,6 +29,7 @@ public sealed class FinallyPoint public bool IsFinallyBlock => finallyIndex < branch.FinallyRegions.Length; public int BlockIndex => IsFinallyBlock ? branch.FinallyRegions[finallyIndex].FirstBlockOrdinal : branch.Destination.Ordinal; + public int BranchDestination => branch.Destination.Ordinal; public FinallyPoint Previous { get; } public FinallyPoint(FinallyPoint previous, ControlFlowBranch branch, int finallyIndex = 0) diff --git a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/ExplodedNodeTest.cs b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/ExplodedNodeTest.cs index 45b07dd640e..18496ef925a 100644 --- a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/ExplodedNodeTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/ExplodedNodeTest.cs @@ -18,9 +18,9 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using System.Xml.Linq; using SonarAnalyzer.Extensions; using SonarAnalyzer.SymbolicExecution.Roslyn; -using SonarAnalyzer.Test.Helpers; namespace SonarAnalyzer.Test.SymbolicExecution.Roslyn; @@ -113,19 +113,101 @@ public void Equals_ReturnsTrueForEquivalent() basic.Equals((ExplodedNode)null).Should().BeFalse(); // Explicit cast to ensure correct overload } + [TestMethod] + public void Equals_FinallyPoints_Single() + { + var cfg = TestHelper.CompileCfgBodyCS(""" + try + { + if (condition) + { + return; // Branch goes to exit 1->5, skipping AfterFinally + } + if (condition) + { + return; // Branch goes to exit 2->5, skipping AfterFinally + } + // Branch goes AfterFinally 2->4 + } + finally + { + System.Console.WriteLine("Finally"); + } + System.Console.WriteLine("AfterFinally"); + """, "bool condition"); + var block = cfg.Blocks[0]; + var from1to5 = cfg.Blocks[1].Successors.Single(x => x.Destination.Ordinal == 5); + var from2to5 = cfg.Blocks[2].Successors.Single(x => x.Destination.Ordinal == 5); + var from2to4 = cfg.Blocks[2].Successors.Single(x => x.Destination.Ordinal == 4); + var node = new ExplodedNode(block, ProgramState.Empty, new FinallyPoint(null, from1to5)); + var noFinally = new ExplodedNode(block, ProgramState.Empty, null); + + node.Equals(new ExplodedNode(block, ProgramState.Empty, new FinallyPoint(null, from2to5))).Should().BeTrue("We consider only same destination block"); + node.Equals(new ExplodedNode(block, ProgramState.Empty, new FinallyPoint(null, from2to4))).Should().BeFalse("It has different destination block"); + node.Equals(new ExplodedNode(block, ProgramState.Empty, new FinallyPoint(null, from1to5, 1))).Should().BeFalse("It has different BlockIndex"); + node.Equals(noFinally).Should().BeFalse(); + noFinally.Equals(node).Should().BeFalse(); + } + + [TestMethod] + public void Equals_FinallyPoints_WithPrevious() + { + var cfg = TestHelper.CompileCfgBodyCS(""" + try + { + if (condition) + { + return; // Branch goes to exit, skipping AfterFinally + } + if (condition) + { + return; // Branch goes to exit, skipping AfterFinally + } + // Branch goes AfterFinally + } + finally + { + System.Console.WriteLine("Finally"); + } + System.Console.WriteLine("AfterFinally"); + """, "bool condition"); + var block = cfg.Blocks[0]; + var outerSame = cfg.Blocks[0].Successors.Single(); + var from1to5 = cfg.Blocks[1].Successors.Single(x => x.Destination.Ordinal == 5); + var from2to5 = cfg.Blocks[2].Successors.Single(x => x.Destination.Ordinal == 5); + var from2to4 = cfg.Blocks[2].Successors.Single(x => x.Destination.Ordinal == 4); + var node = new ExplodedNode(block, ProgramState.Empty, new FinallyPoint(new FinallyPoint(null, from1to5), outerSame)); + var noFinally = new ExplodedNode(block, ProgramState.Empty, Wrap(null)); + + node.Equals(new ExplodedNode(block, ProgramState.Empty, Wrap(new FinallyPoint(null, from2to5)))).Should().BeTrue("We consider only same destination block"); + node.Equals(new ExplodedNode(block, ProgramState.Empty, Wrap(new FinallyPoint(null, from2to4)))).Should().BeFalse("It has different destination block"); + node.Equals(new ExplodedNode(block, ProgramState.Empty, Wrap(new FinallyPoint(null, from1to5, 1)))).Should().BeFalse("It has different BlockIndex"); + node.Equals(noFinally).Should().BeFalse(); + noFinally.Equals(node).Should().BeFalse(); + + FinallyPoint Wrap(FinallyPoint inner) => + new(inner, outerSame); + } + [TestMethod] public void GetHashCode_ReturnsSameForEquivalent() { - var block = TestHelper.CompileCfgBodyCS("var a = true;").Blocks[1]; - var basic = new ExplodedNode(block, ProgramState.Empty, null); - var same = new ExplodedNode(block, ProgramState.Empty, null); + var cfg = TestHelper.CompileCfgBodyCS("var a = true;"); + var block = cfg.Blocks[1]; + var finallyPoint = new FinallyPoint(null, block.Successors.Single()); + var basic = new ExplodedNode(block, ProgramState.Empty, finallyPoint); + var same = new ExplodedNode(block, ProgramState.Empty, finallyPoint); var differentLocation = basic.CreateNext(ProgramState.Empty); - var differentState = new ExplodedNode(block, ProgramState.Empty.SetOperationValue(block.Operations[0], SymbolicValue.Empty), null); + var differentState = new ExplodedNode(block, ProgramState.Empty.SetOperationValue(block.Operations[0], SymbolicValue.Empty), finallyPoint); + var differentNoFinallyPoint = new ExplodedNode(block, ProgramState.Empty, null); + var differentFinallyPointBlockIndex = new ExplodedNode(block, ProgramState.Empty, new(null, cfg.Blocks[0].Successors.Single())); basic.GetHashCode().Should().Be(basic.GetHashCode(), "value should be deterministic"); basic.GetHashCode().Should().Be(same.GetHashCode()); basic.GetHashCode().Should().NotBe(differentLocation.GetHashCode()); basic.GetHashCode().Should().NotBe(differentState.GetHashCode()); + basic.GetHashCode().Should().NotBe(differentNoFinallyPoint.GetHashCode()); + basic.GetHashCode().Should().NotBe(differentFinallyPointBlockIndex.GetHashCode()); } [TestMethod] @@ -134,21 +216,24 @@ public void ToString_SerializeOperationAndState() var cfg = TestHelper.CompileCfgBodyCS("var a = true;"); var state = ProgramState.Empty.SetSymbolValue(cfg.Blocks[1].Operations[0].ChildOperations.First().TrackedSymbol(ProgramState.Empty), SymbolicValue.Empty); - new ExplodedNode(cfg.Blocks[0], ProgramState.Empty, null).ToString().Should().BeIgnoringLineEndings( -@"Block #0, Branching -Empty -"); - - new ExplodedNode(cfg.Blocks[1], state, null).ToString().Should().BeIgnoringLineEndings( -@"Block #1, Operation #0, LocalReferenceOperation: a = true -Symbols: -a: No constraints -"); - new ExplodedNode(cfg.ExitBlock, state, null).ToString().Should().BeIgnoringLineEndings( -@"Block #2, Branching -Symbols: -a: No constraints -"); + new ExplodedNode(cfg.Blocks[0], ProgramState.Empty, null).ToString().Should().BeIgnoringLineEndings(""" + Block #0, Branching + Empty + + """); + + new ExplodedNode(cfg.Blocks[1], state, null).ToString().Should().BeIgnoringLineEndings(""" + Block #1, Operation #0, LocalReferenceOperation: a = true + Symbols: + a: No constraints + + """); + new ExplodedNode(cfg.ExitBlock, state, null).ToString().Should().BeIgnoringLineEndings(""" + Block #2, Branching + Symbols: + a: No constraints + + """); } [TestMethod] diff --git a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/FinallyPointTest.cs b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/FinallyPointTest.cs index 1d468d421b3..d4d928acf83 100644 --- a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/FinallyPointTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/FinallyPointTest.cs @@ -36,22 +36,23 @@ public void Constructor_Null_Throws() [TestMethod] public void CreateNext_ReturnsAllFinally_AndThenDestination() { - const string code = @" -try -{ - try - { - true.ToString(); - } - finally - { - true.ToString(); - } -} -finally -{ - true.ToString(); -}"; + const string code = """ + try + { + try + { + true.ToString(); + } + finally + { + true.ToString(); + } + } + finally + { + true.ToString(); + } + """; var cfg = TestHelper.CompileCfgBodyCS(code); var sut = new FinallyPoint(null, cfg.Blocks[1].FallThroughSuccessor); sut.BlockIndex.Should().Be(2); // Inner finally diff --git a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.TryCatchFinally.cs b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.TryCatchFinally.cs index 471fbcc9444..a6b4d4435ca 100644 --- a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.TryCatchFinally.cs +++ b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.TryCatchFinally.cs @@ -29,17 +29,18 @@ public partial class RoslynSymbolicExecutionTest [TestMethod] public void Finally_Simple() { - const string code = @" -Tag(""BeforeTry""); -try -{ - Tag(""InTry""); -} -finally -{ - Tag(""InFinally""); -} -Tag(""AfterFinally"");"; + const string code = """ + Tag("BeforeTry"); + try + { + Tag("InTry"); + } + finally + { + Tag("InFinally"); + } + Tag("AfterFinally"); + """; SETestContext.CreateCS(code).Validator.ValidateTagOrder( "BeforeTry", "InTry", @@ -51,28 +52,29 @@ public void Finally_Simple() [TestMethod] public void Finally_Nested_ExitingTwoFinallyOnSameBranch() { - const string code = @" -Tag(""BeforeOuterTry""); -try -{ - Tag(""InOuterTry""); - try - { - Tag(""InInnerTry""); - } - finally - { - true.ToString(); // Put some operations in the way - Tag(""InInnerFinally""); - } -} -finally -{ - true.ToString(); // Put some operations in the way - true.ToString(); - Tag(""InOuterFinally""); -} -Tag(""AfterOuterFinally"");"; + const string code = """ + Tag("BeforeOuterTry"); + try + { + Tag("InOuterTry"); + try + { + Tag("InInnerTry"); + } + finally + { + true.ToString(); // Put some operations in the way + Tag("InInnerFinally"); + } + } + finally + { + true.ToString(); // Put some operations in the way + true.ToString(); + Tag("InOuterFinally"); + } + Tag("AfterOuterFinally"); + """; var validator = SETestContext.CreateCS(code).Validator; validator.ValidateTagOrder( "BeforeOuterTry", @@ -98,28 +100,29 @@ public void Finally_Nested_ExitingTwoFinallyOnSameBranch() [TestMethod] public void Finally_Nested_InstructionAfterFinally() { - const string code = @" -Tag(""BeforeOuterTry""); -try -{ - Tag(""InOuterTry""); - try - { - Tag(""InInnerTry""); - } - finally - { - true.ToString(); // Put some operations in the way - Tag(""InInnerFinally""); - } - Tag(""AfterInnerFinally""); -} -finally -{ - true.ToString(); // Put some operations in the way - Tag(""InOuterFinally""); -} -Tag(""AfterOuterFinally"");"; + const string code = """ + Tag("BeforeOuterTry"); + try + { + Tag("InOuterTry"); + try + { + Tag("InInnerTry"); + } + finally + { + true.ToString(); // Put some operations in the way + Tag("InInnerFinally"); + } + Tag("AfterInnerFinally"); + } + finally + { + true.ToString(); // Put some operations in the way + Tag("InOuterFinally"); + } + Tag("AfterOuterFinally"); + """; var validator = SETestContext.CreateCS(code).Validator; validator.ValidateTagOrder( "BeforeOuterTry", @@ -148,27 +151,28 @@ public void Finally_Nested_InstructionAfterFinally() [TestMethod] public void Finally_Nested_InstructionAfterFinally_NoThrowsInFinally() { - const string code = @" -var tag = ""BeforeOuterTry""; -var value = false; -try -{ - try - { - Tag(""InInnerTry""); // This can throw - } - finally - { - tag = ""InInnerFinally""; - } - value = true; - tag = ""AfterInnerFinally""; -} -finally -{ - Tag(""InOuterFinally"", value); -} -Tag(""AfterOuterFinally"", value);"; + const string code = """ + var tag = "BeforeOuterTry"; + var value = false; + try + { + try + { + Tag("InInnerTry"); // This can throw + } + finally + { + tag = "InInnerFinally"; + } + value = true; + tag = "AfterInnerFinally"; + } + finally + { + Tag("InOuterFinally", value); + } + Tag("AfterOuterFinally", value); + """; var validator = SETestContext.CreateCS(code, new PreserveTestCheck("value")).Validator; validator.ValidateTagOrder( "BeforeOuterTry", @@ -191,30 +195,31 @@ public void Finally_Nested_InstructionAfterFinally_NoThrowsInFinally() [TestMethod] public void Finally_NestedInFinally_InstructionAfterFinally_NoThrowsInFinally() { - const string code = @" -var tag = ""BeforeOuterTry""; -var value = false; -try -{ - Tag(""InOuterTry""); // This can throw -} -finally -{ - tag = ""BeforeInnerTry""; - try - { - value = false; // Operation that cannot throw - this doesn't do anything - tag = ""InInnerTry""; - } - finally - { - tag = ""InInnerFinally""; // Operation that cannot throw - } - value = true; - - Tag(""InOuterFinally"", value); -} -Tag(""AfterOuterFinally"", value);"; + const string code = """ + var tag = "BeforeOuterTry"; + var value = false; + try + { + Tag("InOuterTry"); // This can throw + } + finally + { + tag = "BeforeInnerTry"; + try + { + value = false; // Operation that cannot throw - this doesn't do anything + tag = "InInnerTry"; + } + finally + { + tag = "InInnerFinally"; // Operation that cannot throw + } + value = true; + + Tag("InOuterFinally", value); + } + Tag("AfterOuterFinally", value); + """; var validator = SETestContext.CreateCS(code, new PreserveTestCheck("value")).Validator; validator.ValidateTagOrder( "BeforeOuterTry", @@ -240,33 +245,34 @@ public void Finally_NestedInFinally_InstructionAfterFinally_NoThrowsInFinally() [TestMethod] public void Finally_BranchInNested() { - const string code = @" -Tag(""BeforeOuterTry""); -try -{ - Tag(""InOuterTry""); - try - { - Tag(""InInnerTry""); - if (Condition) - { - Tag(""1""); - } - else - { - Tag(""2""); - } - } - finally - { - Tag(""InInnerFinally""); - } -} -finally -{ - Tag(""InOuterFinally""); -} -Tag(""AfterOuterFinally"");"; + const string code = """ + Tag("BeforeOuterTry"); + try + { + Tag("InOuterTry"); + try + { + Tag("InInnerTry"); + if (Condition) + { + Tag("1"); + } + else + { + Tag("2"); + } + } + finally + { + Tag("InInnerFinally"); + } + } + finally + { + Tag("InOuterFinally"); + } + Tag("AfterOuterFinally"); + """; SETestContext.CreateCS(code).Validator.ValidateTagOrder( "BeforeOuterTry", "InOuterTry", @@ -283,25 +289,26 @@ public void Finally_BranchInNested() [TestMethod] public void Finally_BranchAfterFinally() { - const string code = @" -Tag(""BeforeTry""); -try -{ - Tag(""InTry""); -} -finally -{ - true.ToString(); // Put some operations in the way - Tag(""InFinally""); -} -if (boolParameter) // No operation between the finally and this. This will create a single follow up block with BranchValue -{ - Tag(""1""); -} -else -{ - Tag(""2""); -}"; + const string code = """ + Tag("BeforeTry"); + try + { + Tag("InTry"); + } + finally + { + true.ToString(); // Put some operations in the way + Tag("InFinally"); + } + if (boolParameter) // No operation between the finally and this. This will create a single follow up block with BranchValue + { + Tag("1"); + } + else + { + Tag("2"); + } + """; SETestContext.CreateCS(code).Validator.ValidateTagOrder( "BeforeTry", "InTry", @@ -314,26 +321,27 @@ public void Finally_BranchAfterFinally() [TestMethod] public void Finally_BranchInFinally() { - const string code = @" -Tag(""BeforeTry""); -try -{ - Tag(""InTry""); -} -finally -{ - Tag(""InFinallyBeforeCondition""); - if (Condition) - { - Tag(""1""); - } - else - { - Tag(""2""); - } - Tag(""InFinallyAfterCondition""); -} -Tag(""AfterFinally"");"; + const string code = """ + Tag("BeforeTry"); + try + { + Tag("InTry"); + } + finally + { + Tag("InFinallyBeforeCondition"); + if (Condition) + { + Tag("1"); + } + else + { + Tag("2"); + } + Tag("InFinallyAfterCondition"); + } + Tag("AfterFinally"); + """; SETestContext.CreateCS(code).Validator.ValidateTagOrder( "BeforeTry", "InTry", @@ -351,19 +359,20 @@ public void Finally_BranchInFinally() [TestMethod] public void Finally_WrappedInLocalLifetimeRegion() { - const string code = @" -Tag(""BeforeTry""); -try -{ - Tag(""InTry""); -} -finally -{ - var local = true; // This creates LocalLifeTime region - Tag(""InFinally""); - // Here is Block#4 outside the LocalLifeTime region -} -Tag(""AfterFinally"");"; + const string code = """ + Tag("BeforeTry"); + try + { + Tag("InTry"); + } + finally + { + var local = true; // This creates LocalLifeTime region + Tag("InFinally"); + // Here is Block#4 outside the LocalLifeTime region + } + Tag("AfterFinally"); + """; SETestContext.CreateCS(code).Validator.ValidateTagOrder( "BeforeTry", "InTry", @@ -375,26 +384,27 @@ public void Finally_WrappedInLocalLifetimeRegion() [TestMethod] public void Finally_NestedFinally() { - const string code = @" -var tag = ""BeforeOuterTry""; -try -{ - Tag(""InOuterTry""); -} -finally -{ - Tag(""InOuterFinally""); - try - { - Tag(""InInnerTry""); - } - finally - { - Tag(""InInnerFinally""); - } - Tag(""AfterInnerFinally""); -} -Tag(""AfterOuterFinally"");"; + const string code = """ + var tag = "BeforeOuterTry"; + try + { + Tag("InOuterTry"); + } + finally + { + Tag("InOuterFinally"); + try + { + Tag("InInnerTry"); + } + finally + { + Tag("InInnerFinally"); + } + Tag("AfterInnerFinally"); + } + Tag("AfterOuterFinally"); + """; var validator = SETestContext.CreateCS(code).Validator; validator.ValidateTagOrder( "BeforeOuterTry", @@ -403,44 +413,45 @@ public void Finally_NestedFinally() "InOuterFinally", "InInnerTry", // With Exception thrown by Tag("InOuterTry") "InInnerTry", - "InInnerFinally", // With Exception thrown by Tag("InOuterTry") - "InInnerFinally", - "AfterInnerFinally", + "InInnerFinally", // With Exception thrown by Tag("InOuterTry"), that visits AfterInnerFinally + "InInnerFinally", // With Exception thrown by Tag("InInnerTry"), that skips AfterInnerFinally and goes to exit block + "InInnerFinally", // No exception + "AfterInnerFinally", // With Exception thrown by Tag("InOuterTry"), that visits AfterInnerFinally + "AfterInnerFinally", // No exception "AfterOuterFinally"); - ValidateHasOnlyNoExceptionAndUnknownException(validator, "InOuterFinally"); - ValidateHasOnlyNoExceptionAndUnknownException(validator, "InInnerFinally"); ValidateHasOnlyNoExceptionAndUnknownException(validator, "InInnerTry"); - - validator.TagStates("AfterInnerFinally").Should().HaveCount(1) - .And.ContainSingle(x => HasNoException(x)); - - validator.TagStates("AfterOuterFinally").Should().HaveCount(1) - .And.ContainSingle(x => HasNoException(x)); + ValidateHasOnlyNoExceptionAndUnknownException(validator, "AfterInnerFinally"); + validator.TagStates("InInnerFinally").Should().SatisfyRespectively( + x => HasUnknownException(x).Should().BeTrue(), + x => HasUnknownException(x).Should().BeTrue(), + x => HasNoException(x).Should().BeTrue()); + validator.TagStates("AfterOuterFinally").Should().HaveCount(1).And.ContainSingle(x => HasNoException(x)); } [TestMethod] public void Finally_NestedInCatch() { - const string code = @" -var tag = ""BeforeOuterTry""; -try -{ - try - { - Tag(""InInnerTry""); - } - finally - { - tag = ""InInnerFinally""; - } - tag = ""AfterInnerTry""; -} -catch -{ - tag = ""InOuterCatch""; -} -Tag(""End"");"; + const string code = """ + var tag = "BeforeOuterTry"; + try + { + try + { + Tag("InInnerTry"); + } + finally + { + tag = "InInnerFinally"; + } + tag = "AfterInnerTry"; + } + catch + { + tag = "InOuterCatch"; + } + Tag("End"); + """; var validator = SETestContext.CreateCS(code).Validator; validator.ValidateTagOrder( "BeforeOuterTry", @@ -460,35 +471,36 @@ public void Finally_NestedInCatch() [TestMethod] public void Finally_NestedInCatch_NestedInFinally() { - const string code = @" -var tag = ""BeforeOuterTry""; -try -{ - tag = ""BeforeMiddleTry""; - try - { - tag = ""BeforeInnerTry""; - try - { - Tag(""InInnerTry""); - } - finally - { - tag = ""InInnerFinally""; - } - tag = ""AfterInnerTry""; - } - finally - { - tag = ""InMiddleFinally""; - } - tag = ""AfterMiddleTry""; -} -catch -{ - tag = ""InOuterCatch""; -} -Tag(""End"");"; + const string code = """ + var tag = "BeforeOuterTry"; + try + { + tag = "BeforeMiddleTry"; + try + { + tag = "BeforeInnerTry"; + try + { + Tag("InInnerTry"); + } + finally + { + tag = "InInnerFinally"; + } + tag = "AfterInnerTry"; + } + finally + { + tag = "InMiddleFinally"; + } + tag = "AfterMiddleTry"; + } + catch + { + tag = "InOuterCatch"; + } + Tag("End"); + """; var validator = SETestContext.CreateCS(code).Validator; validator.ValidateTagOrder( "BeforeOuterTry", @@ -514,17 +526,18 @@ public void Finally_NestedInCatch_NestedInFinally() [TestMethod] public void Catch_Simple_NoFilter() { - const string code = @" -var tag = ""BeforeTry""; -try -{ - Tag(""InTry""); -} -catch -{ - tag = ""InCatch""; -} -tag = ""End"";"; + const string code = """ + var tag = "BeforeTry"; + try + { + Tag("InTry"); + } + catch + { + tag = "InCatch"; + } + tag = "End"; + """; var validator = SETestContext.CreateCS(code).Validator; validator.ValidateTagOrder( "BeforeTry", @@ -539,21 +552,22 @@ public void Catch_Simple_NoFilter() [TestMethod] public void Catch_WrappedInLocalLifetimeRegion() { - const string code = @" -var tag = ""BeforeTry""; -try -{ - Tag(""InTry""); -} -catch -{ - tag = ""InCatch""; - if (true) - { - var local = true; // Block #4 is wrapped in LocalLifeTime region - } -} -tag = ""End"";"; + const string code = """ + var tag = "BeforeTry"; + try + { + Tag("InTry"); + } + catch + { + tag = "InCatch"; + if (true) + { + var local = true; // Block #4 is wrapped in LocalLifeTime region + } + } + tag = "End"; + """; var validator = SETestContext.CreateCS(code).Validator; validator.ValidateTagOrder( "BeforeTry", @@ -568,17 +582,18 @@ public void Catch_WrappedInLocalLifetimeRegion() [TestMethod] public void Catch_Simple_TypeFilter() { - const string code = @" -var tag = ""BeforeTry""; -try -{ - Tag(""InTry""); -} -catch (Exception) -{ - tag = ""InCatch""; -} -tag = ""End"";"; + const string code = """ + var tag = "BeforeTry"; + try + { + Tag("InTry"); + } + catch (Exception) + { + tag = "InCatch"; + } + tag = "End"; + """; var validator = SETestContext.CreateCS(code).Validator; validator.ValidateTagOrder( "BeforeTry", @@ -593,25 +608,26 @@ public void Catch_Simple_TypeFilter() [TestMethod] public void Catch_Multiple() { - const string code = @" -var tag = ""BeforeTry""; -try -{ - Tag(""InTry""); -} -catch (ArgumentNullException ex) -{ - tag = ""InCatchArgumentNull""; -} -catch (NotSupportedException ex) -{ - tag = ""InCatchNotSupported""; -} -catch (Exception ex) -{ - tag = ""InCatchEverything""; -} -tag = ""End"";"; + const string code = """ + var tag = "BeforeTry"; + try + { + Tag("InTry"); + } + catch (ArgumentNullException ex) + { + tag = "InCatchArgumentNull"; + } + catch (NotSupportedException ex) + { + tag = "InCatchNotSupported"; + } + catch (Exception ex) + { + tag = "InCatchEverything"; + } + tag = "End"; + """; var validator = SETestContext.CreateCS(code).Validator; validator.ValidateTagOrder( "BeforeTry", @@ -630,21 +646,22 @@ public void Catch_Multiple() [TestMethod] public void Catch_Finally() { - const string code = @" -var tag = ""BeforeTry""; -try -{ - Tag(""InTry""); -} -catch (Exception ex) -{ - tag = ""InCatch""; -} -finally -{ - tag = ""InFinally""; -} -tag = ""End"";"; + const string code = """ + var tag = "BeforeTry"; + try + { + Tag("InTry"); + } + catch (Exception ex) + { + tag = "InCatch"; + } + finally + { + tag = "InFinally"; + } + tag = "End"; + """; var validator = SETestContext.CreateCS(code).Validator; validator.ValidateTagOrder( "BeforeTry", @@ -661,26 +678,27 @@ public void Catch_Finally() [TestMethod] public void Catch_NestedInTry() { - const string code = @" -var tag = ""BeforeOuterTry""; -try -{ - Tag(""BeforeInnerTry""); // Can throw - try - { - Tag(""InInnerTry""); // Can throw - } - catch (Exception exInner) - { - tag = ""InInnerCatch""; - } - tag = ""AfterInnerTry""; -} -catch (Exception ex) -{ - tag = ""InOuterCatch""; -} -tag = ""End"";"; + const string code = """ + var tag = "BeforeOuterTry"; + try + { + Tag("BeforeInnerTry"); // Can throw + try + { + Tag("InInnerTry"); // Can throw + } + catch (Exception exInner) + { + tag = "InInnerCatch"; + } + tag = "AfterInnerTry"; + } + catch (Exception ex) + { + tag = "InOuterCatch"; + } + tag = "End"; + """; var validator = SETestContext.CreateCS(code).Validator; validator.ValidateTagOrder( "BeforeOuterTry", @@ -699,26 +717,27 @@ public void Catch_NestedInTry() [TestMethod] public void Catch_NestedInCatch() { - const string code = @" -var tag = ""BeforeOuterTry""; -try -{ - Tag(""InOuterTry""); -} -catch (Exception exOuter) -{ - tag = ""BeforeInnerTry""; - try - { - Tag(""InInnerTry""); - } - catch (Exception exInner) - { - tag = ""InInnerCatch""; - } - tag = ""AfterInnerTry""; -} -tag = ""End"";"; + const string code = """ + var tag = "BeforeOuterTry"; + try + { + Tag("InOuterTry"); + } + catch (Exception exOuter) + { + tag = "BeforeInnerTry"; + try + { + Tag("InInnerTry"); + } + catch (Exception exInner) + { + tag = "InInnerCatch"; + } + tag = "AfterInnerTry"; + } + tag = "End"; + """; var validator = SETestContext.CreateCS(code).Validator; validator.ValidateTagOrder( "BeforeOuterTry", @@ -739,30 +758,31 @@ public void Catch_NestedInCatch() [TestMethod] public void Catch_NestedInFinally() { - const string code = @" -var tag = ""BeforeOuterTry""; -try -{ - Tag(""InOuterTry""); -} -catch (Exception ex) -{ - tag = ""InOuterCatch""; -} -finally -{ - tag = ""BeforeInnerTry""; - try - { - Tag(""InInnerTry""); - } - catch (Exception exInner) - { - tag = ""InInnerCatch""; - } - tag = ""AfterInnerTry""; -} -tag = ""End"";"; + const string code = """ + var tag = "BeforeOuterTry"; + try + { + Tag("InOuterTry"); + } + catch (Exception ex) + { + tag = "InOuterCatch"; + } + finally + { + tag = "BeforeInnerTry"; + try + { + Tag("InInnerTry"); + } + catch (Exception exInner) + { + tag = "InInnerCatch"; + } + tag = "AfterInnerTry"; + } + tag = "End"; + """; var validator = SETestContext.CreateCS(code).Validator; validator.ValidateTagOrder( "BeforeOuterTry", @@ -772,12 +792,13 @@ public void Catch_NestedInFinally() "InInnerTry", "AfterInnerTry", "InInnerCatch", - "End"); + "End", + "AfterInnerTry"); validator.TagStates("InOuterCatch").Should().HaveCount(1).And.ContainSingle(x => HasUnknownException(x)); validator.TagStates("BeforeInnerTry").Should().HaveCount(1).And.ContainSingle(x => HasNoException(x)); validator.TagStates("InInnerCatch").Should().HaveCount(1).And.ContainSingle(x => HasUnknownException(x)); - validator.TagStates("AfterInnerTry").Should().HaveCount(1).And.ContainSingle(x => HasNoException(x)); + validator.TagStates("AfterInnerTry").Should().HaveCount(2).And.OnlyContain(x => HasNoException(x)); validator.TagStates("End").Should().HaveCount(1).And.ContainSingle(x => HasNoException(x)); } @@ -819,33 +840,34 @@ public void CatchWhen_Simple() [TestMethod] public void CatchWhen_Multiple() { - const string code = @" -var tag = ""BeforeTry""; -try -{ - Tag(""InTry""); -} -catch (ArgumentNullException ex) when (ex.ParamName == ""value"") -{ - tag = ""InCatchArgumentWhen""; -} -catch (ArgumentNullException ex) -{ - tag = ""InCatchArgument""; -} -catch (Exception ex) when (ex is ArgumentNullException) -{ - tag = ""InCatchAllWhen""; -} -catch (Exception ex) -{ - tag = ""InCatchAll""; -} -finally -{ - tag = ""InFinally""; -} -tag = ""End"";"; + const string code = """ + var tag = "BeforeTry"; + try + { + Tag("InTry"); + } + catch (ArgumentNullException ex) when (ex.ParamName == "value") + { + tag = "InCatchArgumentWhen"; + } + catch (ArgumentNullException ex) + { + tag = "InCatchArgument"; + } + catch (Exception ex) when (ex is ArgumentNullException) + { + tag = "InCatchAllWhen"; + } + catch (Exception ex) + { + tag = "InCatchAll"; + } + finally + { + tag = "InFinally"; + } + tag = "End"; + """; var validator = SETestContext.CreateCS(code).Validator; validator.ValidateTagOrder( "BeforeTry", @@ -868,39 +890,41 @@ public void CatchWhen_Multiple() [TestMethod] public void Finally_NestedLambda_ShouldNotFail() { - const string code = @" -Tag(""Unreachable - outer CFG is not analyzed""); -try -{ - Action a = () => - { - // Lambda marker - var tag = ""Before""; - Tag(""CanThrow""); - tag = ""After""; - }; -} -finally -{ - Tag(""Unreachable - outer CFG is not analyzed""); -}"; + const string code = """ + Tag("Unreachable - outer CFG is not analyzed"); + try + { + Action a = () => + { + // Lambda marker + var tag = "Before"; + Tag("CanThrow"); + tag = "After"; + }; + } + finally + { + Tag("Unreachable - outer CFG is not analyzed"); + } + """; SETestContext.CreateCSLambda(code, "// Lambda marker").Validator.ValidateTagOrder("Before", "CanThrow", "After"); } [TestMethod] public void Catch_ExceptionVariableIsNotNull() { - const string code = @" -try -{ - InstanceMethod(); -} -catch(InvalidOperationException ex) when (Tag(""InFilter"", ex)) -{ - Tag(""InCatch"", ex); -} + const string code = """ + try + { + InstanceMethod(); + } + catch(InvalidOperationException ex) when (Tag("InFilter", ex)) + { + Tag("InCatch", ex); + } -static bool Tag(string name, T value) => true;"; + static bool Tag(string name, T value) => true; + """; var validator = SETestContext.CreateCS(code).Validator; validator.TagValue("InFilter").Should().HaveOnlyConstraint(ObjectConstraint.NotNull); validator.TagValue("InCatch").Should().HaveOnlyConstraint(ObjectConstraint.NotNull); diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs index 604f1813c21..50f4f4f3e73 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs @@ -3961,9 +3961,9 @@ public bool WithLock(bool a, bool b) flag = false; } } - if (flag) // Noncompliant FP + if (flag) // Compliant { - return true; // Secondary FP + return true; } else { @@ -3985,9 +3985,9 @@ public bool WithUsing(bool a, bool b) flag = false; } } - if (flag) // Noncompliant FP + if (flag) // Compliant { - return true; // Secondary FP + return true; } else { @@ -4058,13 +4058,13 @@ public void Method(bool condition1, bool condition2) { if (condition1) { - return; + return; // Goes via Finally with success=true directly to exit } - Console.WriteLine("invocation"); + Console.WriteLine("Invocation can throw"); // Goes via Finally with success=true, follows to the final condition } catch { - success = false; + success = false; // Goes via Finally with success=false if (condition2) { throw; @@ -4072,12 +4072,12 @@ public void Method(bool condition1, bool condition2) } finally { - Console.WriteLine("do something"); + Console.WriteLine("Finally"); } - if (success) // Noncompliant FP + if (success) // Compliant { - Console.WriteLine("Notify of success"); // Secondary FP + Console.WriteLine("Final condition"); } } }