Skip to content

Commit

Permalink
PSR2/SwitchDeclaration: bug fix - don't remove trailing comments
Browse files Browse the repository at this point in the history
As reported in 3352, the `PSR2.ControlStructures.SwitchDeclaration.BodyOnNextLineCASE` fixer could inadvertently remove trailing comments.

This commit improves on the fix which was put in place in response to 683, by making the detection of what should be considered the start of the body more precise:
* Any amount of trailing comments on the same line as the `case` will be ignored.
* Any type of comment will be taken into consideration, not just PHPCS annotations and `T_COMMENT` tokens.
* Comments _not_ on the same line as the `case` and non-empty tokens on the same line as the `case` will both be considered as the "start of the body".

As for the fixer. The "body starts on same line" fixer was already okay. The "empty lines between case and body" fixer, however, will now ignore anything on the same line as the `case`, which prevents us having to account for comments which already include a new line.

Includes unit tests.

Fixes 3352
  • Loading branch information
jrfnl committed May 13, 2021
1 parent fde816c commit 478d4e5
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 9 deletions.
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -340,3 +340,41 @@ 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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -343,3 +343,33 @@ 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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ public function getErrorList()
260 => 1,
300 => 1,
311 => 1,
346 => 1,
350 => 1,
356 => 1,
362 => 1,
];

}//end getErrorList()
Expand Down

0 comments on commit 478d4e5

Please sign in to comment.