diff --git a/src/Files/File.php b/src/Files/File.php index c5d581f28d..a14bd0bdd8 100644 --- 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 index 0db0cd98b8..6b05227c15 100644 --- 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 index 572e6ada05..8050b636f6 100644 --- 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 index e454cd3318..bebc66619f 100644 --- 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 index 9c1735563f..97a68704f5 100644 --- 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()