diff --git a/analyzers/its/expected/Ember-MM/S1871-Ember Media Manager.json b/analyzers/its/expected/Ember-MM/S1871-Ember Media Manager.json new file mode 100644 index 00000000000..a32c9c492a3 --- /dev/null +++ b/analyzers/its/expected/Ember-MM/S1871-Ember Media Manager.json @@ -0,0 +1,10 @@ +{ + "Issues": [ + { + "Id": "S1871", + "Message": "Either merge this branch with the identical one on line 6404 or change one of the implementations.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/Ember-MM/Ember%20Media%20Manager/frmMain.vb#L6406", + "Location": "Line 6406 Position 25-62" + } + ] +} \ No newline at end of file diff --git a/analyzers/its/expected/Ember-MM/S1871-EmberAPI.json b/analyzers/its/expected/Ember-MM/S1871-EmberAPI.json index 9f41d3c7368..c0442769af7 100644 --- a/analyzers/its/expected/Ember-MM/S1871-EmberAPI.json +++ b/analyzers/its/expected/Ember-MM/S1871-EmberAPI.json @@ -35,6 +35,12 @@ "Message": "Either merge this case with the identical one on line 343 or change one of the implementations.", "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/Ember-MM/EmberAPI/clsAPIScanner.vb#L358-L359", "Location": "Lines 358-359 Position 33-41" + }, + { + "Id": "S1871", + "Message": "Either merge this branch with the identical one on line 808 or change one of the implementations.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/Ember-MM/EmberAPI/clsAPIScanner.vb#L811", + "Location": "Line 811 Position 29-51" } ] } \ No newline at end of file diff --git a/analyzers/its/expected/Ember-MM/S1871-generic.EmberCore.Notifications.json b/analyzers/its/expected/Ember-MM/S1871-generic.EmberCore.Notifications.json new file mode 100644 index 00000000000..397a2498df1 --- /dev/null +++ b/analyzers/its/expected/Ember-MM/S1871-generic.EmberCore.Notifications.json @@ -0,0 +1,28 @@ +{ + "Issues": [ + { + "Id": "S1871", + "Message": "Either merge this case with the identical one on line 113 or change one of the implementations.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/Ember-MM/Addons/generic.EmberCore.Notifications/Module.Notifications.vb#L115", + "Location": "Line 115 Position 25-38" + }, + { + "Id": "S1871", + "Message": "Either merge this case with the identical one on line 113 or change one of the implementations.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/Ember-MM/Addons/generic.EmberCore.Notifications/Module.Notifications.vb#L117", + "Location": "Line 117 Position 25-38" + }, + { + "Id": "S1871", + "Message": "Either merge this case with the identical one on line 113 or change one of the implementations.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/Ember-MM/Addons/generic.EmberCore.Notifications/Module.Notifications.vb#L119", + "Location": "Line 119 Position 25-38" + }, + { + "Id": "S1871", + "Message": "Either merge this case with the identical one on line 113 or change one of the implementations.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/Ember-MM/Addons/generic.EmberCore.Notifications/Module.Notifications.vb#L121", + "Location": "Line 121 Position 25-38" + } + ] +} \ No newline at end of file diff --git a/analyzers/its/expected/akka.net/S1871-Akka.Cluster.Sharding-netstandard2.0.json b/analyzers/its/expected/akka.net/S1871-Akka.Cluster.Sharding-netstandard2.0.json new file mode 100644 index 00000000000..32852589154 --- /dev/null +++ b/analyzers/its/expected/akka.net/S1871-Akka.Cluster.Sharding-netstandard2.0.json @@ -0,0 +1,10 @@ +{ + "Issues": [ + { + "Id": "S1871", + "Message": "Either merge this case with the identical one on line 1348 or change one of the implementations.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/akka.net/src/contrib/cluster/Akka.Cluster.Sharding/PersistentShardCoordinator.cs#L1351-L1353", + "Location": "Lines 1351-1353 Position 25-41" + } + ] +} \ No newline at end of file diff --git a/analyzers/its/expected/akka.net/S1871-Akka.Remote.TestKit-netstandard2.0.json b/analyzers/its/expected/akka.net/S1871-Akka.Remote.TestKit-netstandard2.0.json new file mode 100644 index 00000000000..68772c48361 --- /dev/null +++ b/analyzers/its/expected/akka.net/S1871-Akka.Remote.TestKit-netstandard2.0.json @@ -0,0 +1,10 @@ +{ + "Issues": [ + { + "Id": "S1871", + "Message": "Either merge this case with the identical one on line 334 or change one of the implementations.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/akka.net/src/core/Akka.Remote.TestKit/Controller.cs#L335", + "Location": "Line 335 Position 21-75" + } + ] +} \ No newline at end of file diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/ConditionalStructureSameImplementation.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/ConditionalStructureSameImplementation.cs index aa1595363eb..aa6e5ec71f7 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/ConditionalStructureSameImplementation.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/ConditionalStructureSameImplementation.cs @@ -18,110 +18,135 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -namespace SonarAnalyzer.Rules.CSharp +namespace SonarAnalyzer.Rules.CSharp; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class ConditionalStructureSameImplementation : ConditionalStructureSameImplementationBase { - [DiagnosticAnalyzer(LanguageNames.CSharp)] - public sealed class ConditionalStructureSameImplementation : ConditionalStructureSameImplementationBase + private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); + + private static readonly ISet IgnoredStatementsInSwitch = new HashSet { - private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); + SyntaxKind.BreakStatement, + SyntaxKind.ReturnStatement, + SyntaxKind.ThrowStatement, + }; - public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Rule); + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Rule); - private static readonly ISet IgnoredStatementsInSwitch = new HashSet - { - SyntaxKind.BreakStatement, - SyntaxKind.ReturnStatement, - SyntaxKind.ThrowStatement, - }; + protected override void Initialize(SonarAnalysisContext context) + { + context.RegisterNodeAction( + c => + { + var ifStatement = (IfStatementSyntax)c.Node; - protected override void Initialize(SonarAnalysisContext context) - { - context.RegisterNodeAction( - c => - { - var ifStatement = (IfStatementSyntax)c.Node; + var precedingStatements = ifStatement + .GetPrecedingStatementsInConditionChain() + .ToList(); - var precedingStatements = ifStatement - .GetPrecedingStatementsInConditionChain() - .ToList(); + CheckStatement(c, ifStatement.Statement, precedingStatements, c.SemanticModel, ifStatement.Else is not null, "branch"); - CheckStatement(c, ifStatement.Statement, precedingStatements); + if (ifStatement.Else is null) + { + return; + } - if (ifStatement.Else == null) - { - return; - } + precedingStatements.Add(ifStatement.Statement); + CheckStatement(c, ifStatement.Else.Statement, precedingStatements, c.SemanticModel, true, "branch"); + }, + SyntaxKind.IfStatement); - precedingStatements.Add(ifStatement.Statement); - CheckStatement(c, ifStatement.Else.Statement, precedingStatements); - }, - SyntaxKind.IfStatement); + context.RegisterNodeAction( + c => + { + var switchSection = (SwitchSectionSyntax)c.Node; - context.RegisterNodeAction( - c => - { - var switchSection = (SwitchSectionSyntax)c.Node; - - if (GetStatements(switchSection).Count(IsApprovedStatement) < 2) - { - return; - } - - var precedingSection = switchSection - .GetPrecedingSections() - .FirstOrDefault(preceding => CSharpEquivalenceChecker.AreEquivalent(switchSection.Statements, preceding.Statements) - && HaveTheSameInvocations(switchSection.Statements, preceding.Statements, c.SemanticModel)); - - if (precedingSection != null) - { - ReportSyntaxNode(c, switchSection, precedingSection, "case"); - } - }, - SyntaxKind.SwitchSection); - } + var precedingSections = switchSection + .GetPrecedingSections() + .ToList(); - private static IEnumerable GetStatements(SwitchSectionSyntax switchSection) => - Enumerable.Empty() - .Union(switchSection.Statements.OfType().SelectMany(block => block.Statements)) - .Union(switchSection.Statements.Where(s => !s.IsKind(SyntaxKind.Block))); + CheckStatement(c, switchSection, precedingSections, c.SemanticModel, HasDefaultClause((SwitchStatementSyntax)switchSection.Parent), "case"); + }, + SyntaxKind.SwitchSection); + } - private static void CheckStatement(SonarSyntaxNodeReportingContext context, SyntaxNode statement, IEnumerable precedingStatements) - { - if (statement.ChildNodes().Count() < 2) - { - return; - } + private static bool HasDefaultClause(SwitchStatementSyntax switchStatement) => + switchStatement.Sections.SelectMany(x => x.Labels).Any(x => x.IsKind(SyntaxKind.DefaultSwitchLabel)); - var precedingStatement = precedingStatements.FirstOrDefault(preceding => CSharpEquivalenceChecker.AreEquivalent(statement, preceding)); - if (precedingStatement != null) + private static void CheckStatement(SonarSyntaxNodeReportingContext context, SyntaxNode node, IReadOnlyList precedingBranches, SemanticModel model, bool hasElse, string discriminator) + { + var numberOfStatements = GetStatementsCount(node); + if (!hasElse && numberOfStatements == 1) + { + if (precedingBranches.Any() && precedingBranches.All(x => AreEquivalentStatements(node, x, model))) { - ReportSyntaxNode(context, statement, precedingStatement, "branch"); + ReportSyntaxNode(context, node, precedingBranches[0], discriminator); } } + else if (numberOfStatements > 1 && precedingBranches.FirstOrDefault(x => AreEquivalentStatements(node, x, model)) is { } equivalentStatement) + { + ReportSyntaxNode(context, node, equivalentStatement, discriminator); + } + } - private static void ReportSyntaxNode(SonarSyntaxNodeReportingContext context, SyntaxNode node, SyntaxNode precedingNode, string errorMessageDiscriminator) => - context.ReportIssue(Rule, node, [precedingNode.ToSecondaryLocation()], precedingNode.GetLineNumberToReport().ToString(), errorMessageDiscriminator); + private static bool AreEquivalentStatements(SyntaxNode node, SyntaxNode otherNode, SemanticModel model) => + new EquivalentNodeCompare(node, otherNode) switch + { + (SwitchSectionSyntax { Statements: var statements }, SwitchSectionSyntax { Statements: var otherStatements }) => + CSharpEquivalenceChecker.AreEquivalent(statements, otherStatements) && HaveTheSameInvocations(statements, otherStatements, model), + (BlockSyntax refBlock, BlockSyntax otherBlock) => + CSharpEquivalenceChecker.AreEquivalent(refBlock, otherBlock) && HaveTheSameInvocations(refBlock, otherBlock, model), + _ => false, // Should not happen + }; + + private static int GetStatementsCount(SyntaxNode node) + { + // Get all child statements from the node, in case of a switch section, we need to handle the case where the statements are wrapped in a block + var statements = node is SwitchSectionSyntax switchSection + ? switchSection.Statements.OfType().SelectMany(x => x.Statements) + .Union(switchSection.Statements.Where(x => !x.IsKind(SyntaxKind.Block))) + : node.ChildNodes(); + + return statements.Count(IsApprovedStatement); + } + + private static void ReportSyntaxNode(SonarSyntaxNodeReportingContext context, SyntaxNode node, SyntaxNode precedingNode, string errorMessageDiscriminator) => + context.ReportIssue(Rule, node, [precedingNode.ToSecondaryLocation()], precedingNode.GetLineNumberToReport().ToString(), errorMessageDiscriminator); + + private static bool IsApprovedStatement(SyntaxNode statement) => + !statement.IsAnyKind(IgnoredStatementsInSwitch); + + private static bool HaveTheSameInvocations(SyntaxList first, SyntaxList second, SemanticModel model) + { + var referenceInvocations = first.SelectMany(x => x.DescendantNodes().OfType()).ToArray(); + var candidateInvocations = second.SelectMany(x => x.DescendantNodes().OfType()).ToArray(); + return HaveTheSameInvocations(referenceInvocations, candidateInvocations, model); + } + + private static bool HaveTheSameInvocations(SyntaxNode first, SyntaxNode second, SemanticModel model) + { + var referenceInvocations = first.DescendantNodes().OfType().ToArray(); + var candidateInvocations = second.DescendantNodes().OfType().ToArray(); + return HaveTheSameInvocations(referenceInvocations, candidateInvocations, model); + } - private static bool IsApprovedStatement(StatementSyntax statement) => - !statement.IsAnyKind(IgnoredStatementsInSwitch); + private static bool HaveTheSameInvocations(InvocationExpressionSyntax[] referenceInvocations, InvocationExpressionSyntax[] candidateInvocations, SemanticModel model) + { + if (referenceInvocations.Length != candidateInvocations.Length) + { + return false; + } - public static bool HaveTheSameInvocations(SyntaxList first, SyntaxList second, SemanticModel model) + for (var i = 0; i < referenceInvocations.Length; i++) { - var referenceInvocations = first.SelectMany(x => x.DescendantNodes().OfType()).ToArray(); - var candidateInvocations = second.SelectMany(x => x.DescendantNodes().OfType()).ToArray(); - if (referenceInvocations.Length != candidateInvocations.Length) + if (!referenceInvocations[i].IsEqualTo(candidateInvocations[i], model)) { return false; } - - for (var i = 0; i < referenceInvocations.Length; i++) - { - if (!referenceInvocations[i].IsEqualTo(candidateInvocations[i], model)) - { - return false; - } - } - return true; } + return true; } + + private readonly record struct EquivalentNodeCompare(SyntaxNode RefNode, SyntaxNode OtherNode); } diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/ConditionalStructureSameImplementationBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/ConditionalStructureSameImplementationBase.cs index 5bc58d3bc76..58f414f9975 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/ConditionalStructureSameImplementationBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/ConditionalStructureSameImplementationBase.cs @@ -18,11 +18,10 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -namespace SonarAnalyzer.Rules +namespace SonarAnalyzer.Rules; + +public abstract class ConditionalStructureSameImplementationBase : SonarDiagnosticAnalyzer { - public abstract class ConditionalStructureSameImplementationBase : SonarDiagnosticAnalyzer - { - internal const string DiagnosticId = "S1871"; - internal const string MessageFormat = "Either merge this {1} with the identical one on line {0} or change one of the implementations."; - } + internal const string DiagnosticId = "S1871"; + internal const string MessageFormat = "Either merge this {1} with the identical one on line {0} or change one of the implementations."; } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ConditionalStructureSameImplementation.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ConditionalStructureSameImplementation.cs index 420057396cc..05f076e5c8a 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ConditionalStructureSameImplementation.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ConditionalStructureSameImplementation.cs @@ -18,76 +18,78 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -namespace SonarAnalyzer.Rules.VisualBasic -{ - [DiagnosticAnalyzer(LanguageNames.VisualBasic)] - public sealed class ConditionalStructureSameImplementation : ConditionalStructureSameImplementationBase - { - private static readonly DiagnosticDescriptor rule = - DescriptorFactory.Create(DiagnosticId, MessageFormat); +namespace SonarAnalyzer.Rules.VisualBasic; - public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(rule); +[DiagnosticAnalyzer(LanguageNames.VisualBasic)] +public sealed class ConditionalStructureSameImplementation : ConditionalStructureSameImplementationBase +{ + private static readonly DiagnosticDescriptor Rule = + DescriptorFactory.Create(DiagnosticId, MessageFormat); - private static readonly ISet ignoredStatementsInSwitch = new HashSet - { - SyntaxKind.ReturnStatement, - SyntaxKind.ThrowStatement, - }; + private static readonly ISet IgnoredStatementsInSwitch = new HashSet { SyntaxKind.ReturnStatement, SyntaxKind.ThrowStatement }; - protected override void Initialize(SonarAnalysisContext context) - { - context.RegisterNodeAction( - c => - { - var ifStatement = (SingleLineIfStatementSyntax)c.Node; + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Rule); - if (ifStatement.ElseClause != null && - VisualBasicEquivalenceChecker.AreEquivalent(ifStatement.ElseClause.Statements, ifStatement.Statements)) - { - ReportIssue(c, ifStatement.ElseClause.Statements, ifStatement.Statements, "branch"); - } - }, - SyntaxKind.SingleLineIfStatement); + protected override void Initialize(SonarAnalysisContext context) + { + context.RegisterNodeAction( + c => + { + var ifStatement = (SingleLineIfStatementSyntax)c.Node; - context.RegisterNodeAction( - c => + if (ifStatement.ElseClause is not null && + VisualBasicEquivalenceChecker.AreEquivalent(ifStatement.ElseClause.Statements, ifStatement.Statements)) { - var ifBlock = (MultiLineIfBlockSyntax)c.Node; + ReportIssue(c, ifStatement.ElseClause.Statements, ifStatement.Statements, "branch"); + } + }, + SyntaxKind.SingleLineIfStatement); - var statements = new[] { ifBlock.Statements } - .Concat(ifBlock.ElseIfBlocks.Select(elseIf => elseIf.Statements)) - .Concat(new[] { ifBlock.ElseBlock?.Statements ?? new SyntaxList() }) - .Where(l => l.Any()) - .ToList(); + context.RegisterNodeAction( + c => + { + var ifBlock = (MultiLineIfBlockSyntax)c.Node; - for (var i = 1; i < statements.Count; i++) - { - CheckStatementsAt(c, statements, i, "branch"); - } - }, - SyntaxKind.MultiLineIfBlock); + var statements = new[] { ifBlock.Statements } + .Concat(ifBlock.ElseIfBlocks.Select(elseIf => elseIf.Statements)) + .Concat(new[] { ifBlock.ElseBlock?.Statements ?? new SyntaxList() }) + .Where(l => l.Any()) + .ToList(); - context.RegisterNodeAction( - c => + for (var i = 1; i < statements.Count; i++) { - var select = (SelectBlockSyntax)c.Node; - var statements = select.CaseBlocks.Select(b => b.Statements).ToList(); - for (var i = 1; i < statements.Count; i++) - { - CheckStatementsAt(c, statements, i, "case"); - } - }, - SyntaxKind.SelectBlock); - } + CheckStatementsAt(c, statements, i, ifBlock.ElseBlock is not null, "branch"); + } + }, + SyntaxKind.MultiLineIfBlock); - private static void CheckStatementsAt(SonarSyntaxNodeReportingContext context, List> statements, int currentIndex, string constructType) + context.RegisterNodeAction( + c => + { + var select = (SelectBlockSyntax)c.Node; + var statements = select.CaseBlocks.Select(b => b.Statements).ToList(); + var hasCaseElse = select.CaseBlocks.Any(b => b.IsKind(SyntaxKind.CaseElseBlock)); + for (var i = 1; i < statements.Count; i++) + { + CheckStatementsAt(c, statements, i, hasCaseElse, "case"); + } + }, + SyntaxKind.SelectBlock); + } + + private static void CheckStatementsAt(SonarSyntaxNodeReportingContext context, List> statements, int currentIndex, bool hasElse, string constructType) + { + var currentBlockStatements = statements[currentIndex]; + var numberOfStatements = currentBlockStatements.Count(IsApprovedStatement); + if (!hasElse && numberOfStatements == 1) { - var currentBlockStatements = statements[currentIndex]; - if (currentBlockStatements.Count(IsApprovedStatement) < 2) + if (statements.Count > 1 && statements.TrueForAll(x => VisualBasicEquivalenceChecker.AreEquivalent(currentBlockStatements, x))) { - return; + ReportIssue(context, currentBlockStatements, statements[0], constructType); } - + } + else if (numberOfStatements > 1) + { for (var j = 0; j < currentIndex; j++) { if (VisualBasicEquivalenceChecker.AreEquivalent(currentBlockStatements, statements[j])) @@ -97,24 +99,22 @@ private static void CheckStatementsAt(SonarSyntaxNodeReportingContext context, L } } } + } - private static void ReportIssue(SonarSyntaxNodeReportingContext context, SyntaxList statementsToReport, SyntaxList locationProvider, string constructType) + private static void ReportIssue(SonarSyntaxNodeReportingContext context, SyntaxList statementsToReport, SyntaxList locationProvider, string constructType) + { + var firstStatement = statementsToReport.FirstOrDefault(); + if (firstStatement is null) { - var firstStatement = statementsToReport.FirstOrDefault(); - if (firstStatement == null) - { - return; - } - - var lastStatement = statementsToReport.Last(); - - context.ReportIssue(rule, firstStatement.CreateLocation(lastStatement), - locationProvider.First().GetLineNumberToReport().ToString(), constructType); + return; } - private static bool IsApprovedStatement(StatementSyntax statement) - { - return !statement.IsAnyKind(ignoredStatementsInSwitch); - } + var lastStatement = statementsToReport.Last(); + + context.ReportIssue(Rule, firstStatement.CreateLocation(lastStatement), + locationProvider.First().GetLineNumberToReport().ToString(), constructType); } + + private static bool IsApprovedStatement(StatementSyntax statement) => + !statement.IsAnyKind(IgnoredStatementsInSwitch); } diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/ConditionalStructureSameImplementationTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/ConditionalStructureSameImplementationTest.cs index 1b4ed051ffe..bd5a43fc235 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/ConditionalStructureSameImplementationTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/ConditionalStructureSameImplementationTest.cs @@ -48,6 +48,9 @@ public void ConditionalStructureSameImplementation_Switch_VisualBasic() => builderVB.AddPaths("ConditionalStructureSameImplementation_Switch.vb").Verify(); #if NET + [TestMethod] + public void ConditionalStructureSameImplementation_Switch_CSharp_Latest() => + builderCS.AddPaths("ConditionalStructureSameImplementation_Switch.Latest.cs").WithOptions(ParseOptionsHelper.CSharpLatest).VerifyNoIssues(); [TestMethod] public void ConditionalStructureSameImplementation_RazorFile_CorrectMessage() => diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_If.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_If.cs index 33ba991eb6b..78fb1d1b50d 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_If.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_If.cs @@ -25,6 +25,11 @@ public void Test_SingleLineBlocks() { DoSomething1(); } + + if (someCondition1) + DoSomething1(); // Compliant, ignore single line blocks + else + DoSomething1(); } public void Test_MultilineBlocks() @@ -93,6 +98,35 @@ public void Test_Overloads() foo = IntExtension.FooInt(foo); } } + + // https://github.com/SonarSource/sonar-dotnet/issues/1255 + public void ExceptionOfException(int a) + { + if (a == 1) + { // Secondary [Exception] + DoSomething1(); + } + else if (a == 2) + { // Noncompliant [Exception] + DoSomething1(); + } + } + + public void Exception(int a) + { + if (a >= 0 && a < 10) + { + DoSomething1(); + } + else if (a >= 10 && a < 20) + { + DoSomething2(); + } + else if (a >= 20 && a < 50) + { + DoSomething1(); + } + } } public static class IntExtension diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_If.vb b/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_If.vb index 7658abcdb23..c2d8e6b1d0c 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_If.vb +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_If.vb @@ -62,5 +62,22 @@ Namespace Tests.TestCases Return Nothing End Function + Public Sub ExceptionOfException(a As Integer) + If a = 1 Then + DoSomething1() + ElseIf a = 2 Then + DoSomething1() ' Noncompliant + End If + End Sub + + Public Sub Exception(a As Integer) + If a >= 0 AndAlso a < 10 Then + DoSomething1() + ElseIf a >= 10 AndAlso a < 20 Then + DoSomething2() + ElseIf a >= 20 AndAlso a < 50 ' Compliant + DoSomething1() + End If + End Sub End Class End Namespace diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_Switch.Latest.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_Switch.Latest.cs new file mode 100644 index 00000000000..0e138ae922e --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_Switch.Latest.cs @@ -0,0 +1,22 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Tests.TestCases +{ + class ConditionalStructureSameCondition_Switch + { + public int SwitchExpression(int a) + { + return a switch + { + 10 => a * 2, + 20 => a * 2, + 50 => a * 2, + }; + } + + } +} diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_Switch.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_Switch.cs index d9853594e7b..930136ff4b6 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_Switch.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_Switch.cs @@ -10,10 +10,10 @@ class ConditionalStructureSameCondition_Switch { public int prop { get; set; } private void doTheRest() { throw new NotSupportedException(); } - private void doSomething() { throw new NotSupportedException(); } - private void doSomething(int i) { throw new NotSupportedException(); } - private void doSomethingDifferent() { throw new NotSupportedException(); } - private void doSomething2() { throw new NotSupportedException(); } + private void DoSomething() { throw new NotSupportedException(); } + private void DoSomething(int i) { throw new NotSupportedException(); } + private void DoSomethingDifferent() { throw new NotSupportedException(); } + private void DoSomething2() { throw new NotSupportedException(); } public void Test_SingleLine() { @@ -21,27 +21,27 @@ public void Test_SingleLine() switch (i) { case 1: - doSomething(prop); + DoSomething(prop); break; case 2: - doSomethingDifferent(); + DoSomethingDifferent(); break; case 3: - doSomething(prop); + DoSomething(prop); break; case 4: { - doSomething2(); + DoSomething2(); } break; case 5: { - doSomething2(); + DoSomething2(); break; } default: - doSomething(prop); + DoSomething(prop); break; } @@ -49,10 +49,10 @@ public void Test_SingleLine() { case 1: case 3: - doSomething(); + DoSomething(); break; case 2: - doSomethingDifferent(); + DoSomethingDifferent(); break; default: doTheRest(); @@ -67,51 +67,51 @@ public void Test_Multiline() { case 1: // Secondary // Secondary@-1 - doSomething(prop); - doSomething(prop); + DoSomething(prop); + DoSomething(prop); break; case 2: - doSomethingDifferent(); + DoSomethingDifferent(); break; case 3: // Noncompliant - doSomething(prop); - doSomething(prop); + DoSomething(prop); + DoSomething(prop); break; case 4: // Secondary { - doSomething2(); - doSomething2(); + DoSomething2(); + DoSomething2(); break; } case 5: // Noncompliant { - doSomething2(); - doSomething2(); + DoSomething2(); + DoSomething2(); break; } case 6: // Secondary; { - doSomething2(); + DoSomething2(); } { - doSomething2(); + DoSomething2(); } break; case 7: // Noncompliant { - doSomething2(); + DoSomething2(); } { - doSomething2(); + DoSomething2(); } break; default: // Noncompliant - doSomething(prop); - doSomething(prop); + DoSomething(prop); + DoSomething(prop); break; } @@ -120,22 +120,22 @@ public void Test_Multiline() { case 1: case 3: - doSomething(); - doSomething(); + DoSomething(); + DoSomething(); break; case 2: - doSomethingDifferent(); - doSomethingDifferent(); + DoSomethingDifferent(); + DoSomethingDifferent(); break; case 4: k++; - doSomething(); - doSomethingDifferent(); + DoSomething(); + DoSomethingDifferent(); break; case 5: k++; - doSomethingDifferent(); - doSomething(); + DoSomethingDifferent(); + DoSomething(); break; default: doTheRest(); @@ -225,6 +225,64 @@ public int SwitchDifferentSameExtensionMethod(object o) return result; } + public void ExceptionOfException(int a) + { + switch (a) + { + case 1: // Secondary [Exception] + DoSomething(); + break; + case 2: // Noncompliant [Exception] + DoSomething(); + break; + } + + switch (a) + { + case 1: + DoSomething(); + break; + case 2: + DoSomething(); + break; + default: + DoSomething(); + break; + } + } + + public void Exception(int a) + { + switch (a) + { + case 10: + DoSomething(); + break; + case 20: + DoSomething2(); + break; + case 50: + DoSomething(); + break; + } + + switch (a) + { + case 10: + DoSomething(); + break; + case 20: + DoSomething2(); + break; + case 50: + DoSomething(); + break; + default: + DoSomething2(); + break; + } + } + } public static class IntExtension diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_Switch.vb b/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_Switch.vb index 28048c250b3..eb621d07cea 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_Switch.vb +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/ConditionalStructureSameImplementation_Switch.vb @@ -37,5 +37,25 @@ Private Function doSomething2() Return Nothing End Function + + Private Function ExceptionOfException(a As Integer) + Select Case a + Case 1 + doSomething2() + Case 2 + doSomething2() ' Noncompliant + End Select + End Function + + Public Sub Exception(a As Integer) + Select Case True + Case a >= 0 AndAlso a < 10 + doSomething2() + Case a >= 10 AndAlso a < 20 + doSomethingDifferent() + Case a >= 20 AndAlso a < 50 + doSomething2() + End Select + End Sub End Class End Namespace