Skip to content

Commit 478d4e5

Browse files
committed
PSR2/SwitchDeclaration: bug fix - don't remove trailing comments
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
1 parent fde816c commit 478d4e5

File tree

4 files changed

+85
-9
lines changed

4 files changed

+85
-9
lines changed

src/Standards/PSR2/Sniffs/ControlStructures/SwitchDeclarationSniff.php

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,13 @@ public function process(File $phpcsFile, $stackPtr)
107107
}
108108
}
109109

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

119119
if ($tokens[$next]['line'] !== ($tokens[$opener]['line'] + 1)) {
@@ -126,17 +126,21 @@ public function process(File $phpcsFile, $stackPtr)
126126
} else {
127127
$phpcsFile->fixer->beginChangeset();
128128
for ($i = ($opener + 1); $i < $next; $i++) {
129+
if ($tokens[$i]['line'] === $tokens[$opener]['line']) {
130+
// Ignore trailing comments.
131+
continue;
132+
}
133+
129134
if ($tokens[$i]['line'] === $tokens[$next]['line']) {
130135
break;
131136
}
132137

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

136-
$phpcsFile->fixer->addNewLineBefore($i);
137141
$phpcsFile->fixer->endChangeset();
138142
}
139-
}
143+
}//end if
140144
}//end if
141145

142146
if ($tokens[$nextCloser]['scope_condition'] === $nextCase) {

src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,3 +340,41 @@ switch ($foo) {
340340
case 3:
341341
return 2;
342342
}
343+
344+
// Issue 3352.
345+
switch ( $test ) {
346+
case 2: // comment followed by empty line
347+
348+
break;
349+
350+
case 3: /* phpcs:ignore Stnd.Cat.SniffName -- Verify correct handling of ignore comments. */
351+
352+
353+
354+
break;
355+
356+
case 4: /** inline docblock */
357+
358+
359+
360+
break;
361+
362+
case 5: /* checking how it handles */ /* two trailing comments */
363+
364+
break;
365+
366+
case 6:
367+
// Comment as first content of the body.
368+
369+
break;
370+
371+
case 7:
372+
/* phpcs:ignore Stnd.Cat.SniffName -- Verify correct handling of ignore comments at start of body. */
373+
374+
break;
375+
376+
case 8:
377+
/** inline docblock */
378+
379+
break;
380+
}

src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,3 +343,33 @@ switch ($foo) {
343343
case 3:
344344
return 2;
345345
}
346+
347+
// Issue 3352.
348+
switch ( $test ) {
349+
case 2: // comment followed by empty line
350+
break;
351+
352+
case 3: /* phpcs:ignore Stnd.Cat.SniffName -- Verify correct handling of ignore comments. */
353+
break;
354+
355+
case 4: /** inline docblock */
356+
break;
357+
358+
case 5: /* checking how it handles */ /* two trailing comments */
359+
break;
360+
361+
case 6:
362+
// Comment as first content of the body.
363+
364+
break;
365+
366+
case 7:
367+
/* phpcs:ignore Stnd.Cat.SniffName -- Verify correct handling of ignore comments at start of body. */
368+
369+
break;
370+
371+
case 8:
372+
/** inline docblock */
373+
374+
break;
375+
}

src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ public function getErrorList()
4949
260 => 1,
5050
300 => 1,
5151
311 => 1,
52+
346 => 1,
53+
350 => 1,
54+
356 => 1,
55+
362 => 1,
5256
];
5357

5458
}//end getErrorList()

0 commit comments

Comments
 (0)