From a140882df3e541df7a0e2414a94d70a28a0c2358 Mon Sep 17 00:00:00 2001 From: Andrei Epure Date: Wed, 4 Nov 2020 18:35:16 +0100 Subject: [PATCH] Address comments and found issue --- .../ConditionalsWithSameConditionTest.cs | 8 +- .../ConditionalSimplification.FromCSharp9.cs | 2 +- ...nditionalStructureSameCondition.CSharp9.cs | 27 +++-- ...ameCondition.CSharp9.TopLevelStatements.cs | 19 +++ .../ConditionalsWithSameCondition.CSharp9.cs | 109 ++++++++++-------- 5 files changed, 107 insertions(+), 58 deletions(-) create mode 100644 sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalsWithSameCondition.CSharp9.TopLevelStatements.cs diff --git a/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/Rules/ConditionalsWithSameConditionTest.cs b/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/Rules/ConditionalsWithSameConditionTest.cs index 3352a31db9a..ed4c347788c 100644 --- a/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/Rules/ConditionalsWithSameConditionTest.cs +++ b/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/Rules/ConditionalsWithSameConditionTest.cs @@ -38,7 +38,13 @@ public void ConditionalsWithSameCondition() => [TestCategory("Rule")] public void ConditionalsWithSameCondition_CSharp9() => Verifier.VerifyAnalyzer(@"TestCases\ConditionalsWithSameCondition.CSharp9.cs", new ConditionalsWithSameCondition(), - ParseOptionsHelper.FromCSharp9, OutputKind.ConsoleApplication); + ParseOptionsHelper.FromCSharp9); + + [TestMethod] + [TestCategory("Rule")] + public void ConditionalsWithSameCondition_CSharp9_TopLevelStatements() => + Verifier.VerifyAnalyzer(@"TestCases\ConditionalsWithSameCondition.CSharp9.TopLevelStatements.cs", + new ConditionalsWithSameCondition(), ParseOptionsHelper.FromCSharp9, OutputKind.ConsoleApplication); } } diff --git a/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalSimplification.FromCSharp9.cs b/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalSimplification.FromCSharp9.cs index 88d3420d3a8..868f1bfcd7c 100644 --- a/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalSimplification.FromCSharp9.cs +++ b/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalSimplification.FromCSharp9.cs @@ -25,7 +25,7 @@ a = a ?? new(); // Noncompliant {{Use the '??=' operator here.}} Fruit elem; -if (condition) // FN, in C# 9 has target typed conditionals +if (condition) // FN, C# 9 has target typed conditionals { elem = new Apple(); } diff --git a/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalStructureSameCondition.CSharp9.cs b/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalStructureSameCondition.CSharp9.cs index b8a996e50e1..728b431d686 100644 --- a/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalStructureSameCondition.CSharp9.cs +++ b/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalStructureSameCondition.CSharp9.cs @@ -3,12 +3,12 @@ void SimpleTest() { - if (o is not null) // Secondary [line11] + if (o is not null) // Secondary [flow1] // ^^^^^^^^^^^^^ { // foo } - else if (o is not null) // Noncompliant [line11] {{This branch duplicates the one on line 6.}} + else if (o is not null) // Noncompliant [flow1] {{This branch duplicates the one on line 6.}} { var x = 1; } @@ -16,36 +16,45 @@ void SimpleTest() void Test(Apple a, Orange b, bool cond) { - if (i is > 0 and < 100) // Secondary [line23,line31] + if (i is > 0 and < 100) { } // Secondary [flow2,flow3] { } - else if (i is > 0 and < 100) // Noncompliant [line23] + else if (i is > 0 and < 100) // Noncompliant [flow2] { } - else if (i is < 0 or > 100) // Secondary [line35] + else if (i is < 0 or > 100) // Secondary [flow4] { } - else if (i is > 0 and < 100) // Noncompliant [line31] + else if (i is > 0 and < 100) // Noncompliant [flow3] { } - else if (i is < 0 or > 100) // Noncompliant [line35] + else if (i is < 0 or > 100) // Noncompliant [flow4] { } Fruit f; - if ((f = cond ? a : b) is Orange) // Secondary [line44] + if ((f = cond ? a : b) is Orange) // Secondary [flow5] { } - else if ((f = cond ? a : b) is Orange) // Noncompliant [line44] + else if ((f = cond ? a : b) is Orange) // Noncompliant [flow5] { } } +void AnotherTest(object o) +{ + if (o is not null) { } + else if (o != null) { } // FN, same as above + + if (o is null) { } + else if (o == null) { } // FN, same as above +} + abstract class Fruit { } class Apple : Fruit { } class Orange : Fruit { } diff --git a/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalsWithSameCondition.CSharp9.TopLevelStatements.cs b/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalsWithSameCondition.CSharp9.TopLevelStatements.cs new file mode 100644 index 00000000000..44eded0640b --- /dev/null +++ b/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalsWithSameCondition.CSharp9.TopLevelStatements.cs @@ -0,0 +1,19 @@ +using System; + +int a = 0, b = 1; + +if (a == b) { Foo(); } +if (a == b) { Bar(); } + +if (args[0] == args[1]) +{ + Foo(); +} + +if (args[0] == args[1]) // FN +{ + Bar(); +} + +void Foo() { } +void Bar() { } diff --git a/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalsWithSameCondition.CSharp9.cs b/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalsWithSameCondition.CSharp9.cs index 5b134480e7d..57ce5b8cf4c 100644 --- a/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalsWithSameCondition.CSharp9.cs +++ b/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ConditionalsWithSameCondition.CSharp9.cs @@ -1,53 +1,68 @@ -string c = null; -if (c is null) +namespace Tests.TestCases { - doTheThing(c); -} -if (c is null) // FN -{ - doTheThing(c); -} -if (c is not null) // Compliant -{ - doTheThing(c); -} + class ConditionalsWithSameCondition + { + public void Foo() + { + string c = null; + if (c is null) + { + doTheThing(c); + } + if (c is null) // Noncompliant {{This condition was just checked on line 8.}} + { + doTheThing(c); + } + if (c is not null) // Compliant + { + doTheThing(c); + } -if (c is "banana") // Compliant, c might be updated in the previous if -{ - c += "a"; -} -if (c is "banana") // Compliant, c might be updated in the previous if -{ - c = ""; -} -if (c is "banana") // Compliant, c might be updated in the previous if -{ - c = ""; -} + if (c is "banana") // Compliant, c might be updated in the previous if + { + c += "a"; + } + if (c is "banana") // Compliant, c might be updated in the previous if + { + c = ""; + } + if (c is "banana") // Compliant, c might be updated in the previous if + { + c = ""; + } -int i = 0; -if (i is > 0 and < 100) -{ - doTheThing(i); -} -if (i is > 0 and < 100) // FN -{ - doTheThing(i); -} + int i = 0; + if (i is > 0 and < 100) + { + doTheThing(i); + } + if (i is > 0 and < 100) // Noncompliant {{This condition was just checked on line 35.}} + { + doTheThing(i); + } -Fruit f = null; -bool cond = false; -if (f is Apple) -{ - f = cond ? new Apple() : new Orange(); -} -if (f is Apple) // Compliant as f may change -{ - f = cond ? new Apple() : new Orange(); -} + Fruit f = null; + bool cond = false; + if (f is Apple) + { + f = cond ? new Apple() : new Orange(); + } + if (f is Apple) // Compliant as f may change + { + f = cond ? new Apple() : new Orange(); + } + + if (f is not null) { } + if (f != null) { } // FN, same as above -void doTheThing(object o) { } + if (f is null) { } + if (f == null) { } // FN, same as above + } -abstract class Fruit { } -class Apple : Fruit { } -class Orange : Fruit { } + void doTheThing(object o) { } + } + + abstract class Fruit { } + class Apple : Fruit { } + class Orange : Fruit { } +}