From e1300d30ede78449c4f973de9627ed695ba6ee01 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Tue, 15 Dec 2020 20:54:38 +0100 Subject: [PATCH 1/3] Handle nested switch --- .../SwitchDeclarationSniff.php | 39 +++++++++++++++++-- .../SwitchDeclarationUnitTest.inc | 37 ++++++++++++++++++ .../SwitchDeclarationUnitTest.inc.fixed | 37 ++++++++++++++++++ .../SwitchDeclarationUnitTest.php | 2 + 4 files changed, 112 insertions(+), 3 deletions(-) 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..3dc1064fc3 100644 --- a/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc +++ b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc @@ -281,3 +281,40 @@ case Foo::ARRAY: echo '1'; return self::VALUE; } + +// OK: Every clause terminates +switch ($foo) { + case 1: + switch ($bar) { + case 1: + return; + default: + return; + } + 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; +} diff --git a/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed index e454cd3318..1b38419016 100644 --- a/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed +++ b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed @@ -284,3 +284,40 @@ case Foo::ARRAY: echo '1'; return self::VALUE; } + +// OK: Every clause terminates +switch ($foo) { + case 1: + switch ($bar) { + case 1: + return; + default: + return; + } + 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; +} 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() From 18a0e54735bb9b3850fec266e5f4c50dacf618ea Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sun, 3 Jan 2021 02:56:46 +0100 Subject: [PATCH 2/3] Fix findStartOfStatement inside of switch/case --- src/Files/File.php | 5 +++++ .../SwitchDeclarationUnitTest.inc | 17 +++++++++++++++-- .../SwitchDeclarationUnitTest.inc.fixed | 17 +++++++++++++++-- 3 files changed, 35 insertions(+), 4 deletions(-) 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/Tests/ControlStructures/SwitchDeclarationUnitTest.inc b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc index 3dc1064fc3..8050b636f6 100644 --- a/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc +++ b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc @@ -287,9 +287,9 @@ switch ($foo) { case 1: switch ($bar) { case 1: - return; + return 1; default: - return; + return 3; } case 2: return 2; @@ -318,3 +318,16 @@ switch ($foo) { 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 1b38419016..bebc66619f 100644 --- a/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed +++ b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed @@ -290,9 +290,9 @@ switch ($foo) { case 1: switch ($bar) { case 1: - return; + return 1; default: - return; + return 3; } case 2: return 2; @@ -321,3 +321,16 @@ switch ($foo) { 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; +} From 46dc37a862c2cd54fd8bc56fd29782c815da8d6e Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sun, 3 Jan 2021 03:06:54 +0100 Subject: [PATCH 3/3] Fix tests --- .../Squiz/Tests/Commenting/FileCommentUnitTest.1.inc.fixed | 2 +- .../Squiz/Tests/Commenting/FileCommentUnitTest.1.js.fixed | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Standards/Squiz/Tests/Commenting/FileCommentUnitTest.1.inc.fixed b/src/Standards/Squiz/Tests/Commenting/FileCommentUnitTest.1.inc.fixed index bae8e7f810..5cc4764205 100644 --- a/src/Standards/Squiz/Tests/Commenting/FileCommentUnitTest.1.inc.fixed +++ b/src/Standards/Squiz/Tests/Commenting/FileCommentUnitTest.1.inc.fixed @@ -25,7 +25,7 @@ * @author * @copyright 1997 Squiz Pty Ltd (ABN 77 084 670 600) * @copyright 1994-1997 Squiz Pty Ltd (ABN 77 084 670 600) -* @copyright 2020 Squiz Pty Ltd (ABN 77 084 670 600) +* @copyright 2021 Squiz Pty Ltd (ABN 77 084 670 600) * @license http://www.php.net/license/3_0.txt * @summary An unknown summary tag * diff --git a/src/Standards/Squiz/Tests/Commenting/FileCommentUnitTest.1.js.fixed b/src/Standards/Squiz/Tests/Commenting/FileCommentUnitTest.1.js.fixed index 82d63d9036..b2b071f485 100644 --- a/src/Standards/Squiz/Tests/Commenting/FileCommentUnitTest.1.js.fixed +++ b/src/Standards/Squiz/Tests/Commenting/FileCommentUnitTest.1.js.fixed @@ -25,7 +25,7 @@ * @author * @copyright 1997 Squiz Pty Ltd (ABN 77 084 670 600) * @copyright 1994-1997 Squiz Pty Ltd (ABN 77 084 670 600) -* @copyright 2020 Squiz Pty Ltd (ABN 77 084 670 600) +* @copyright 2021 Squiz Pty Ltd (ABN 77 084 670 600) * @license http://www.php.net/license/3_0.txt * @summary An unknown summary tag *