Refactor RemoveDuplicatedCaseInSwitchRector and adding test cases (#7175)

Author: mspirkov

rules-tests/DeadCode/Rector/Switch_/RemoveDuplicatedCaseInSwitchRector/Fixture/no_stmts_with_equal_comments.php.inc

@@ -0,0 +1,53 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Switch_\RemoveDuplicatedCaseInSwitchRector\Fixture;
+
+class NoStmtsWithEqualComments
+{
+    public function run($foo)
+    {
+        switch ($foo) {
+            case 'A':
+                // Equal comment
+                break;
+            case 'B':
+            case 'C':
+                // Some comment
+                break;
+            case 'D':
+                $type = 'BAR';
+                break;
+            case 'E':
+            case 'F':
+            default:
+                // Equal comment
+                break;
+        }
+    }
+}
+-----
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Switch_\RemoveDuplicatedCaseInSwitchRector\Fixture;
+
+class NoStmtsWithEqualComments
+{
+    public function run($foo)
+    {
+        switch ($foo) {
+            case 'A':
+            case 'E':
+            case 'F':
+            default:
+                // Equal comment
+                break;
+            case 'B':
+            case 'C':
+                // Some comment
+                break;
+            case 'D':
+                $type = 'BAR';
+                break;
+        }
+    }
+}

rules-tests/DeadCode/Rector/Switch_/RemoveDuplicatedCaseInSwitchRector/Fixture/skip_with_one_case.php.inc

@@ -0,0 +1,14 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Switch_\RemoveDuplicatedCaseInSwitchRector\Fixture;
+
+class SkipWithOneCase
+{
+    public function run($foo)
+    {
+        switch ($foo) {
+            case 'A':
+                return 'a';
+        }
+    }
+}

rules/DeadCode/Rector/Switch_/RemoveDuplicatedCaseInSwitchRector.php

@@ -106,14 +106,14 @@ private function removeDuplicatedCases(Switch_ $switch): void
         $result = [];
 
         /** @var int[] */
-        $processedCasesNumbers = [];
+        $processedCasesKeys = [];
 
         foreach ($switch->cases as $outerCaseKey => $outerCase) {
-            if (in_array($outerCaseKey, $processedCasesNumbers)) {
+            if (in_array($outerCaseKey, $processedCasesKeys)) {
                 continue;
             }
 
-            $processedCasesNumbers[] = $outerCaseKey;
+            $processedCasesKeys[] = $outerCaseKey;
 
             if ($outerCase->stmts === []) {
                 $result[] = $outerCase;
@@ -127,7 +127,7 @@ private function removeDuplicatedCases(Switch_ $switch): void
             $equalCases = [];
 
             foreach ($switch->cases as $innerCaseKey => $innerCase) {
-                if (in_array($innerCaseKey, $processedCasesNumbers)) {
+                if (in_array($innerCaseKey, $processedCasesKeys)) {
                     continue;
                 }
 
@@ -137,30 +137,26 @@ private function removeDuplicatedCases(Switch_ $switch): void
                 }
 
                 if ($this->areSwitchStmtsEqualsAndWithBreak($outerCase, $innerCase)) {
-                    if ($casesWithoutStmts !== []) {
-                        foreach ($casesWithoutStmts as $caseWithoutStmtsKey => $caseWithoutStmts) {
-                            $equalCases[] = $caseWithoutStmts;
-                            $processedCasesNumbers[] = $caseWithoutStmtsKey;
-                        }
-
-                        $casesWithoutStmts = [];
+                    foreach ($casesWithoutStmts as $caseWithoutStmtsKey => $caseWithoutStmts) {
+                        $equalCases[] = $caseWithoutStmts;
+                        $processedCasesKeys[] = $caseWithoutStmtsKey;
                     }
 
                     $innerCase->stmts = [];
                     $equalCases[] = $innerCase;
-                    $processedCasesNumbers[] = $innerCaseKey;
-
-                    $this->hasChanged = true;
-                } else {
-                    $casesWithoutStmts = [];
+                    $processedCasesKeys[] = $innerCaseKey;
                 }
+
+                $casesWithoutStmts = [];
             }
 
             if ($equalCases === []) {
                 $result[] = $outerCase;
                 continue;
             }
 
+            $this->hasChanged = true;
+
             $equalCases[array_key_last($equalCases)]->stmts = $outerCase->stmts;
             $outerCase->stmts = [];
 
@@ -185,11 +181,7 @@ private function areSwitchStmtsEqualsAndWithBreak(Case_ $currentCase, Case_ $nex
         }
 
         foreach ($currentCase->stmts as $stmt) {
-            if ($stmt instanceof Break_) {
-                return true;
-            }
-
-            if ($stmt instanceof Return_) {
+            if ($stmt instanceof Break_ || $stmt instanceof Return_) {
                 return true;
             }
         }
@@ -199,6 +191,9 @@ private function areSwitchStmtsEqualsAndWithBreak(Case_ $currentCase, Case_ $nex
 
     private function areSwitchStmtsEqualsConsideringComments(Case_ $currentCase, Case_ $nextCase): bool
     {
-        return $this->betterStandardPrinter->print($currentCase->stmts) === $this->betterStandardPrinter->print($nextCase->stmts);
+        $currentCasePrintResult = $this->betterStandardPrinter->print($currentCase->stmts);
+        $nextCasePrintResult = $this->betterStandardPrinter->print($nextCase->stmts);
+
+        return $currentCasePrintResult === $nextCasePrintResult;
     }
 }