Fix S1871 FN: Support single line conditional block

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"
+    }
+  ]
+}

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"
     }
   ]
 }

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"
+    }
+  ]
+}

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"
+    }
+  ]
+}

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"
+    }
+  ]
+}

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<SyntaxKind> IgnoredStatementsInSwitch = new HashSet<SyntaxKind>
     {
-        private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);
+        SyntaxKind.BreakStatement,
+        SyntaxKind.ReturnStatement,
+        SyntaxKind.ThrowStatement,
+    };
 
-        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
 
-        private static readonly ISet<SyntaxKind> IgnoredStatementsInSwitch = new HashSet<SyntaxKind>
-        {
-            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<StatementSyntax> GetStatements(SwitchSectionSyntax switchSection) =>
-            Enumerable.Empty<StatementSyntax>()
-                      .Union(switchSection.Statements.OfType<BlockSyntax>().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<StatementSyntax> 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<SyntaxNode> 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<BlockSyntax>().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<SyntaxNode> first, SyntaxList<SyntaxNode> second, SemanticModel model)
+    {
+        var referenceInvocations = first.SelectMany(x => x.DescendantNodes().OfType<InvocationExpressionSyntax>()).ToArray();
+        var candidateInvocations = second.SelectMany(x => x.DescendantNodes().OfType<InvocationExpressionSyntax>()).ToArray();
+        return HaveTheSameInvocations(referenceInvocations, candidateInvocations, model);
+    }
+
+    private static bool HaveTheSameInvocations(SyntaxNode first, SyntaxNode second, SemanticModel model)
+    {
+        var referenceInvocations = first.DescendantNodes().OfType<InvocationExpressionSyntax>().ToArray();
+        var candidateInvocations = second.DescendantNodes().OfType<InvocationExpressionSyntax>().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<SyntaxNode> first, SyntaxList<SyntaxNode> second, SemanticModel model)
+        for (var i = 0; i < referenceInvocations.Length; i++)
         {
-            var referenceInvocations = first.SelectMany(x => x.DescendantNodes().OfType<InvocationExpressionSyntax>()).ToArray();
-            var candidateInvocations = second.SelectMany(x => x.DescendantNodes().OfType<InvocationExpressionSyntax>()).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);
 }

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.";
 }