Skip to content

Commit 2936d15

Browse files
committed
PEAR/FunctionDeclaration: bug fix - block comment handling
When a block comment is multi-line, the first `T_COMMENT` token will start with the `/*`, but `T_COMMENT` tokens which are part of the block comment on subsequent lines will include the indentation in the `T_COMMENT` token. This makes these tokens more complex to deal with in sniffs, especially for indentation calculations. The `PEAR.Functions.FunctionDeclaration` sniff did not take the possibility of a multi-line block comment being part of the signature of a multi-line function declaration into account (at all). This also affected, by extension, the `Squiz.Functions.MultiLineFunctionDeclaration` sniff and would cause a fixer conflict of the sniff with itself. This commit fixes this. Includes extensive tests. Note - there are two caveats to the fix: 1. If subsequent lines in a block comment are not prefixed with a `*`, the indentation fix will not be precise, but will be a best-effort "indent needs to be the same or larger than the expected indent". 2. Like for all sniffs, lines containing PHPCS annotations will be ignored unless the `--ignore-annotations` CLI flag is used. That also applies to these lines when they are part of a multi-line block comment. Fixes 1368
1 parent 2108e8b commit 2936d15

File tree

4 files changed

+152
-0
lines changed

4 files changed

+152
-0
lines changed

src/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,30 @@ public function processArgumentList(File $phpcsFile, int $stackPtr, int $indent,
468468
} elseif ($tokens[$i]['code'] === T_DOC_COMMENT_WHITESPACE) {
469469
$foundIndent = $tokens[$i]['length'];
470470
++$expectedIndent;
471+
} elseif (($tokens[$i]['code'] === T_COMMENT
472+
|| isset(Tokens::PHPCS_ANNOTATION_TOKENS[$tokens[$i]['code']]) === true)
473+
&& (($tokens[($i - 1)]['code'] === T_COMMENT
474+
|| isset(Tokens::PHPCS_ANNOTATION_TOKENS[$tokens[($i - 1)]['code']]) === true)
475+
&& $tokens[($i - 1)]['line'] === $lastLine)
476+
) {
477+
// This is the second line of a multi-line comment.
478+
// This token _may_ include indentation whitespace if this is part of a block comment.
479+
$trimmedContent = ltrim($tokens[$i]['content']);
480+
$trimmedLength = strlen($trimmedContent);
481+
if ($trimmedLength === 0) {
482+
// This is a blank comment line, so indenting it is pointless.
483+
$lastLine = $tokens[$i]['line'];
484+
continue;
485+
}
486+
487+
$foundIndent = (strlen($tokens[$i]['content']) - $trimmedLength);
488+
if ($trimmedContent[0] === '*') {
489+
++$expectedIndent;
490+
} elseif ($foundIndent >= $expectedIndent) {
491+
// Multi-line block comment, not star-aligned. These may contain extra indent.
492+
$lastLine = $tokens[$i]['line'];
493+
continue;
494+
}
471495
}
472496

473497
if ($expectedIndent !== $foundIndent) {
@@ -480,8 +504,13 @@ public function processArgumentList(File $phpcsFile, int $stackPtr, int $indent,
480504
$fix = $phpcsFile->addFixableError($error, $i, 'Indent', $data);
481505
if ($fix === true) {
482506
$spaces = str_repeat(' ', $expectedIndent);
507+
483508
if ($foundIndent === 0) {
484509
$phpcsFile->fixer->addContentBefore($i, $spaces);
510+
} elseif ($tokens[$i]['code'] === T_COMMENT
511+
|| isset(Tokens::PHPCS_ANNOTATION_TOKENS[$tokens[$i]['code']]) === true
512+
) {
513+
$phpcsFile->fixer->replaceToken($i, $spaces . ltrim($tokens[$i]['content']));
485514
} else {
486515
$phpcsFile->fixer->replaceToken($i, $spaces);
487516
}

src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.1.inc

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,3 +488,55 @@ function foo(
488488
$param2
489489
) : // comment.
490490
\Package\Sub\SomeClass {}
491+
492+
class MultiLineCommentsBetweenParamsIndentOK
493+
{
494+
public function something(
495+
/*
496+
* Some comment for whatever reason
497+
* phpcs:disable Stnd.Cat -- for reasons
498+
*/
499+
string $paramA,
500+
// phpcs:enable Stnd.Cat
501+
// Comment line 2
502+
// Comment line 3
503+
#[NotBlank]
504+
int $paramB,
505+
) {
506+
}
507+
}
508+
509+
class MultiLineCommentsBetweenParamsIncorrectIndent
510+
{
511+
public function something(
512+
/*
513+
* Some comment for whatever reason
514+
* phpcs:disable Stnd.CatA -- this line will only be flagged & fixed when `--ignore-annotations` is used.
515+
*/
516+
string $paramA,
517+
/*
518+
* Some comment for whatever reason
519+
* phpcs:disable Stnd.CatB -- this line will only be flagged & fixed when `--ignore-annotations` is used.
520+
*/
521+
string $paramB,
522+
523+
// phpcs:enable Stnd.CatA,Stnd.CatB -- this line will only be flagged & fixed when `--ignore-annotations` is used.
524+
// Comment line 2
525+
// Comment line 3
526+
#[NotBlank]
527+
int $paramC,
528+
/*
529+
530+
Some comment surrounded by blank lines (deliberately!)
531+
532+
*/
533+
?string $paramD,
534+
/*
535+
536+
Some comment surrounded by blank lines (deliberately!)
537+
538+
*/
539+
?string $paramE,
540+
) {
541+
}
542+
}

src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.1.inc.fixed

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,3 +485,54 @@ function foo(
485485
$param2
486486
) : // comment.
487487
\Package\Sub\SomeClass {}
488+
489+
class MultiLineCommentsBetweenParamsIndentOK
490+
{
491+
public function something(
492+
/*
493+
* Some comment for whatever reason
494+
* phpcs:disable Stnd.Cat -- for reasons
495+
*/
496+
string $paramA,
497+
// phpcs:enable Stnd.Cat
498+
// Comment line 2
499+
// Comment line 3
500+
#[NotBlank]
501+
int $paramB,
502+
) {
503+
}
504+
}
505+
506+
class MultiLineCommentsBetweenParamsIncorrectIndent
507+
{
508+
public function something(
509+
/*
510+
* Some comment for whatever reason
511+
* phpcs:disable Stnd.CatA -- this line will only be flagged & fixed when `--ignore-annotations` is used.
512+
*/
513+
string $paramA,
514+
/*
515+
* Some comment for whatever reason
516+
* phpcs:disable Stnd.CatB -- this line will only be flagged & fixed when `--ignore-annotations` is used.
517+
*/
518+
string $paramB,
519+
// phpcs:enable Stnd.CatA,Stnd.CatB -- this line will only be flagged & fixed when `--ignore-annotations` is used.
520+
// Comment line 2
521+
// Comment line 3
522+
#[NotBlank]
523+
int $paramC,
524+
/*
525+
526+
Some comment surrounded by blank lines (deliberately!)
527+
528+
*/
529+
?string $paramD,
530+
/*
531+
532+
Some comment surrounded by blank lines (deliberately!)
533+
534+
*/
535+
?string $paramE,
536+
) {
537+
}
538+
}

src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,26 @@ public function getErrorList($testFile = '')
109109
475 => 1,
110110
483 => 1,
111111
490 => 2,
112+
512 => 1,
113+
513 => 1,
114+
515 => 1,
115+
516 => 1,
116+
517 => 1,
117+
518 => 1,
118+
520 => 1,
119+
521 => 1,
120+
522 => 1,
121+
524 => 1,
122+
525 => 1,
123+
526 => 1,
124+
527 => 1,
125+
528 => 1,
126+
532 => 1,
127+
533 => 1,
128+
534 => 1,
129+
536 => 1,
130+
538 => 1,
131+
539 => 1,
112132
];
113133

114134
case 'FunctionDeclarationUnitTest.4.inc':

0 commit comments

Comments
 (0)