Merge branch 'nestedSwitch' of https://github.com/VincentLanglet/PHP_…

…CodeSniffer
squizlabs · Feb 1, 2021 · 15afb2f · 15afb2f
2 parents e73f14b + 46dc37a
commit 15afb2f
Show file tree

Hide file tree

Showing 5 changed files with 143 additions and 3 deletions.
diff --git a/src/Files/File.php b/src/Files/File.php
@@ -2313,6 +2313,11 @@ public function findStartOfStatement($start, $ignore=null)
                 && $this->tokens[$i]['code'] !== T_CLOSE_PARENTHESIS
                 && $this->tokens[$i]['code'] !== T_END_NOWDOC
                 && $this->tokens[$i]['code'] !== T_END_HEREDOC
+                && $this->tokens[$i]['code'] !== T_BREAK
+                && $this->tokens[$i]['code'] !== T_RETURN
+                && $this->tokens[$i]['code'] !== T_CONTINUE
+                && $this->tokens[$i]['code'] !== T_THROW
+                && $this->tokens[$i]['code'] !== T_EXIT
             ) {
                 // Found the end of the previous scope block.
                 return $lastNotEmpty;

diff --git a/src/Standards/PSR2/Sniffs/ControlStructures/SwitchDeclarationSniff.php b/src/Standards/PSR2/Sniffs/ControlStructures/SwitchDeclarationSniff.php
@@ -251,8 +251,8 @@ private function findNestedTerminator($phpcsFile, $stackPtr, $end)
         $lastToken = $phpcsFile->findPrevious(T_WHITESPACE, ($end - 1), $stackPtr, true);
         if ($lastToken !== false) {
             if ($tokens[$lastToken]['code'] === T_CLOSE_CURLY_BRACKET) {
-                // We found a closing curly bracket and want to check if its
-                // block belongs to an IF, ELSEIF or ELSE clause. If yes, we
+                // We found a closing curly bracket and want to check if its block
+                // belongs to a SWITCH or an IF, ELSEIF or ELSE clause. If yes, we
                 // continue searching for a terminating statement within that
                 // block. Note that we have to make sure that every block of
                 // the entire if/else statement has a terminating statement.
@@ -267,7 +267,7 @@ private function findNestedTerminator($phpcsFile, $stackPtr, $end)
                         return false;
                     }
 
-                    // IF and ELSEIF clauses possess a condition we have to account for.
+                    // SWITCH, IF and ELSEIF clauses possess a condition we have to account for.
                     if ($tokens[$prevToken]['code'] === T_CLOSE_PARENTHESIS) {
                         $prevToken = $tokens[$prevToken]['parenthesis_owner'];
                     }
@@ -294,6 +294,39 @@ private function findNestedTerminator($phpcsFile, $stackPtr, $end)
                         if ($tokens[$prevToken]['code'] === T_ELSE) {
                             $hasElseBlock = true;
                         }
+                    } else if ($tokens[$prevToken]['code'] === T_SWITCH) {
+                        $hasDefaultBlock = false;
+                        $endOfSwitch     = $tokens[$prevToken]['scope_closer'];
+                        $nextCase        = $prevToken;
+
+                        // We look for a terminating statement within every blocks.
+                        while (($nextCase = $this->findNextCase($phpcsFile, ($nextCase + 1), $endOfSwitch)) !== false) {
+                            if ($tokens[$nextCase]['code'] === T_DEFAULT) {
+                                $hasDefaultBlock = true;
+                            }
+
+                            $opener = $tokens[$nextCase]['scope_opener'];
+
+                            $nextCode = $phpcsFile->findNext(T_WHITESPACE, ($opener + 1), $endOfSwitch, true);
+                            if ($tokens[$nextCode]['code'] === T_CASE || $tokens[$nextCode]['code'] === T_DEFAULT) {
+                                // This case statement has no content. We skip it.
+                                continue;
+                            }
+
+                            $nextCode = $this->findNextCase($phpcsFile, ($opener + 1), $endOfSwitch);
+                            if ($nextCode === false) {
+                                $nextCode = $endOfSwitch;
+                            }
+
+                            $hasTerminator = $this->findNestedTerminator($phpcsFile, ($opener + 1), $nextCode);
+                            if ($hasTerminator === false) {
+                                return false;
+                            }
+                        }//end while
+
+                        // If we have not encountered a DEFAULT block by now, we cannot
+                        // be sure that the whole statement terminates in every case.
+                        return $hasDefaultBlock;
                     } else {
                         return false;
                     }//end if

diff --git a/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc
@@ -281,3 +281,53 @@ case Foo::ARRAY:
     echo '1';
     return self::VALUE;
 }
+
+// OK: Every clause terminates
+switch ($foo) {
+    case 1:
+        switch ($bar) {
+            case 1:
+                return 1;
+            default:
+                return 3;
+        }
+    case 2:
+        return 2;
+}
+
+// KO: Not every clause terminates
+switch ($foo) {
+    case 1:
+        switch ($bar) {
+            case 1:
+                return;
+        }
+    case 2:
+        return 2;
+}
+
+// KO: Not every clause terminates
+switch ($foo) {
+    case 1:
+        switch ($bar) {
+            case 1:
+                return;
+            default:
+                $a = 1;
+        }
+    case 2:
+        return 2;
+}
+
+// OK: Every clause terminates
+switch ($foo) {
+    case 1:
+        switch ($bar) {
+            case 1:
+                return 1;
+            default:
+                throw new \Exception();
+        }
+    case 2:
+        return 2;
+}
diff --git a/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed
@@ -284,3 +284,53 @@ case Foo::ARRAY:
     echo '1';
     return self::VALUE;
 }
+
+// OK: Every clause terminates
+switch ($foo) {
+    case 1:
+        switch ($bar) {
+            case 1:
+                return 1;
+            default:
+                return 3;
+        }
+    case 2:
+        return 2;
+}
+
+// KO: Not every clause terminates
+switch ($foo) {
+    case 1:
+        switch ($bar) {
+            case 1:
+                return;
+        }
+    case 2:
+        return 2;
+}
+
+// KO: Not every clause terminates
+switch ($foo) {
+    case 1:
+        switch ($bar) {
+            case 1:
+                return;
+            default:
+                $a = 1;
+        }
+    case 2:
+        return 2;
+}
+
+// OK: Every clause terminates
+switch ($foo) {
+    case 1:
+        switch ($bar) {
+            case 1:
+                return 1;
+            default:
+                throw new \Exception();
+        }
+    case 2:
+        return 2;
+}
diff --git a/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.php b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.php
@@ -47,6 +47,8 @@ public function getErrorList()
             224 => 1,
             236 => 1,
             260 => 1,
+            300 => 1,
+            311 => 1,
         ];
 
     }//end getErrorList()