Skip to content

Commit

Permalink
Merge branch 'feature/psr2-switch-declaration-various-fixes' of https…
Browse files Browse the repository at this point in the history 
…://github.com/jrfnl/PHP_CodeSniffer
  • Loading branch information
@gsherwood
gsherwood committed May 27, 2021
2 parents 10da08b + 5391139 commit e5ceaa1
Show file tree
Hide file tree
Showing 4 changed files with 546 additions and 21 deletions.
78 changes: 57 additions & 21 deletions src/Standards/PSR2/Sniffs/ControlStructures/SwitchDeclarationSniff.php
View file
Expand Up @@ -107,13 +107,13 @@ public function process(File $phpcsFile, $stackPtr)
}
}

$next = $phpcsFile->findNext(T_WHITESPACE, ($opener + 1), null, true);
if ($tokens[$next]['line'] === $tokens[$opener]['line']
&& ($tokens[$next]['code'] === T_COMMENT
|| isset(Tokens::$phpcsCommentTokens[$tokens[$next]['code']]) === true)
) {
// Skip comments on the same line.
$next = $phpcsFile->findNext(T_WHITESPACE, ($next + 1), null, true);
for ($next = ($opener + 1); $next < $nextCloser; $next++) {
if (isset(Tokens::$emptyTokens[$tokens[$next]['code']]) === false
|| (isset(Tokens::$commentTokens[$tokens[$next]['code']]) === true
&& $tokens[$next]['line'] !== $tokens[$opener]['line'])
) {
break;
}
}

if ($tokens[$next]['line'] !== ($tokens[$opener]['line'] + 1)) {
Expand All @@ -126,17 +126,21 @@ public function process(File $phpcsFile, $stackPtr)
} else {
$phpcsFile->fixer->beginChangeset();
for ($i = ($opener + 1); $i < $next; $i++) {
if ($tokens[$i]['line'] === $tokens[$opener]['line']) {
// Ignore trailing comments.
continue;
}

if ($tokens[$i]['line'] === $tokens[$next]['line']) {
break;
}

$phpcsFile->fixer->replaceToken($i, '');
}

$phpcsFile->fixer->addNewLineBefore($i);
$phpcsFile->fixer->endChangeset();
}
}
}//end if
}//end if

if ($tokens[$nextCloser]['scope_condition'] === $nextCase) {
Expand Down Expand Up @@ -250,22 +254,25 @@ private function findNestedTerminator($phpcsFile, $stackPtr, $end)

if ($tokens[$lastToken]['code'] === T_CLOSE_CURLY_BRACKET) {
// We found a closing curly bracket and want to check if its block
// belongs to a SWITCH, IF, ELSEIF or ELSE clause. If yes, we
// continue searching for a terminating statement within that
// belongs to a SWITCH, IF, ELSEIF or ELSE, TRY, CATCH OR FINALLY 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/switch statement has a terminating statement.
// For a try/catch/finally statement, either the finally block has
// to have a terminating statement or every try/catch block has to have one.
$currentCloser = $lastToken;
$hasElseBlock = false;
$hasCatchWithoutTerminator = false;
do {
$scopeOpener = $tokens[$currentCloser]['scope_opener'];
$scopeCloser = $tokens[$currentCloser]['scope_closer'];

$prevToken = $phpcsFile->findPrevious(T_WHITESPACE, ($scopeOpener - 1), $stackPtr, true);
$prevToken = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($scopeOpener - 1), $stackPtr, true);
if ($prevToken === false) {
return false;
}

// SWITCH, IF and ELSEIF clauses possess a condition we have to account for.
// SWITCH, IF, ELSEIF, CATCH clauses possess a condition we have to account for.
if ($tokens[$prevToken]['code'] === T_CLOSE_PARENTHESIS) {
$prevToken = $tokens[$prevToken]['parenthesis_owner'];
}
Expand All @@ -288,10 +295,39 @@ private function findNestedTerminator($phpcsFile, $stackPtr, $end)
return false;
}

$currentCloser = $phpcsFile->findPrevious(T_WHITESPACE, ($prevToken - 1), $stackPtr, true);
$currentCloser = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prevToken - 1), $stackPtr, true);
if ($tokens[$prevToken]['code'] === T_ELSE) {
$hasElseBlock = true;
}
} else if ($tokens[$prevToken]['code'] === T_FINALLY) {
// If we find a terminating statement within this block,
// the whole try/catch/finally statement is covered.
$hasTerminator = $this->findNestedTerminator($phpcsFile, ($scopeOpener + 1), $scopeCloser);
if ($hasTerminator !== false) {
return $hasTerminator;
}

// Otherwise, we continue with the previous TRY or CATCH clause.
$currentCloser = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prevToken - 1), $stackPtr, true);
} else if ($tokens[$prevToken]['code'] === T_TRY) {
// If we've seen CATCH blocks without terminator statement and
// have not seen a FINALLY *with* a terminator statement, we
// don't even need to bother checking the TRY.
if ($hasCatchWithoutTerminator === true) {
return false;
}

return $this->findNestedTerminator($phpcsFile, ($scopeOpener + 1), $scopeCloser);
} else if ($tokens[$prevToken]['code'] === T_CATCH) {
// Keep track of seen catch statements without terminating statement,
// but don't bow out yet as there may still be a FINALLY clause
// with a terminating statement before the CATCH.
$hasTerminator = $this->findNestedTerminator($phpcsFile, ($scopeOpener + 1), $scopeCloser);
if ($hasTerminator === false) {
$hasCatchWithoutTerminator = true;
}

$currentCloser = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prevToken - 1), $stackPtr, true);
} else if ($tokens[$prevToken]['code'] === T_SWITCH) {
$hasDefaultBlock = false;
$endOfSwitch = $tokens[$prevToken]['scope_closer'];
Expand All @@ -305,7 +341,7 @@ private function findNestedTerminator($phpcsFile, $stackPtr, $end)

$opener = $tokens[$nextCase]['scope_opener'];

$nextCode = $phpcsFile->findNext(T_WHITESPACE, ($opener + 1), $endOfSwitch, true);
$nextCode = $phpcsFile->findNext(Tokens::$emptyTokens, ($opener + 1), $endOfSwitch, true);
if ($tokens[$nextCode]['code'] === T_CASE || $tokens[$nextCode]['code'] === T_DEFAULT) {
// This case statement has no content, so skip it.
continue;
Expand Down Expand Up @@ -339,15 +375,15 @@ private function findNestedTerminator($phpcsFile, $stackPtr, $end)
// We found the last statement of the CASE. Now we want to
// check whether it is a terminating one.
$terminators = [
T_RETURN,
T_BREAK,
T_CONTINUE,
T_THROW,
T_EXIT,
T_RETURN => T_RETURN,
T_BREAK => T_BREAK,
T_CONTINUE => T_CONTINUE,
T_THROW => T_THROW,
T_EXIT => T_EXIT,
];

$terminator = $phpcsFile->findStartOfStatement(($lastToken - 1));
if (in_array($tokens[$terminator]['code'], $terminators, true) === true) {
if (isset($terminators[$tokens[$terminator]['code']]) === true) {
return $terminator;
}
}//end if
Expand Down
244 changes: 244 additions & 0 deletions src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc
View file
Expand Up @@ -340,3 +340,247 @@ switch ($foo) {
case 3:
return 2;
}

// Issue 3352.
switch ( $test ) {
case 2: // comment followed by empty line

break;

case 3: /* phpcs:ignore Stnd.Cat.SniffName -- Verify correct handling of ignore comments. */



break;

case 4: /** inline docblock */



break;

case 5: /* checking how it handles */ /* two trailing comments */

break;

case 6:
// Comment as first content of the body.

break;

case 7:
/* phpcs:ignore Stnd.Cat.SniffName -- Verify correct handling of ignore comments at start of body. */

break;

case 8:
/** inline docblock */

break;
}

// Handle comments correctly.
switch ($foo) {
case 1:
if ($bar > 0) {
doSomething();
}
// Comment
else {
return 1;
}
case 2:
return 2;
}

switch ($foo) {
case 1:
if ($bar > 0) /*comment*/ {
return doSomething();
}
else {
return 1;
}
case 2:
return 2;
}

// Issue #3297.
// Okay - finally will always be executed, so all branches are covered by the `return` in finally.
switch ( $a ) {
case 1:
try {
doSomething();
} catch (Exception $e) {
doSomething();
} catch (AnotherException $e) {
doSomething();
} finally {
return true;
}
default:
$other = $code;
break;
}

// Okay - all - non-finally - branches have a terminating statement.
switch ( $a ) {
case 1:
try {
return false;
} catch (Exception $e) /*comment*/ {
return true;
}
// Comment
catch (AnotherException $e) {
return true;
} finally {
doSomething();
}
default:
$other = $code;
break;
}

// Okay - finally will always be executed, so all branches are covered by the `return` in finally.
// Non-standard structure order.
switch ( $a ) {
case 1:
try {
doSomething();
} catch (Exception $e) {
doSomething();
} finally {
return true;
} catch (AnotherException $e) {
doSomething();
}
default:
$other = $code;
break;
}

// Okay - all - non-finally - branches have a terminating statement.
// Non-standard structure order.
switch ( $a ) {
case 1:
try {
return false;
} finally {
doSomething();
} catch (MyException $e) {
return true;
} catch (AnotherException $e) {
return true;
}
default:
$other = $code;
break;
}

// All okay, no finally. Any exception still uncaught will terminate the case anyhow, so we're good.
switch ( $a ) {
case 1:
try {
return false;
} catch (MyException $e) {
return true;
} catch (AnotherException $e) {
return true;
}
default:
$other = $code;
break;
}

// All okay, no catch
switch ( $a ) {
case 1:
try {
return true;
} finally {
doSomething();
}
case 2:
$other = $code;
break;
}

// All okay, try-catch nested in if.
switch ( $a ) {
case 1:
if ($a) {
try {
return true;
} catch (MyException $e) {
throw new Exception($e->getMessage());
}
} else {
return true;
}
case 2:
$other = $code;
break;
}

// Missing fall-through comment.
switch ( $a ) {
case 1:
try {
doSomething();
} finally {
doSomething();
}
case 2:
$other = $code;
break;
}

// Missing fall-through comment. One of the catches does not have a terminating statement.
switch ( $a ) {
case 1:
try {
return false;
} catch (Exception $e) {
doSomething();
} catch (AnotherException $e) {
return true;
} finally {
doSomething();
}
default:
$other = $code;
break;
}

// Missing fall-through comment. Try does not have a terminating statement.
switch ( $a ) {
case 1:
try {
doSomething();
} finally {
doSomething();
} catch (Exception $e) {
return true;
} catch (AnotherException $e) {
return true;
}
default:
$other = $code;
break;
}

// Missing fall-through comment. One of the catches does not have a terminating statement.
switch ( $a ) {
case 1:
try {
return false;
} catch (Exception $e) {
doSomething();
} catch (AnotherException $e) {
return true;
}
default:
$other = $code;
break;
}

0 comments on commit e5ceaa1

Please sign in to comment.