From 249ba483cf40bf9c2e8c915ceba14980a3553619 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Sun, 25 Oct 2026 14:55:19 +0100 Subject: [PATCH] Fix three fixer loops that kept phpcbf from converging InlineDocBlock treated everything after the type as the variable name, so an inline @var with a description (`@var bool $x Some text`) was reported as wrong order and the fixer moved one word to the end on every pass, mangling the comment. It now only swaps a reversed `$var Type` pair and leaves any description in place. Other content is still reported, but no longer "fixed". The single-space-before-`*/` check only accepted an alphanumeric last character, so a description ending in `.` gained a space on every pass; it now accepts any non-space character. ConsistentIndent stopped its backward scan at the `;` inside a multi-line `for (...)` header and dedented the second and third expressions, which PSR12 ControlStructureSpacing then indented again. Tokens inside parentheses now count as a multi-line construct. AttributeAndTargetSpacing (without allowOnSameLine) moved the target of an inline parameter attribute such as `#[SensitiveParameter] array $x` onto its own tab-indented line, turning a single-line signature multi-line, so the trailing-comma sniffs added and removed a comma forever. Attributes on the same line as their target are now allowed. On the 206 files of the cakephp PER-CS PR, phpcbf previously failed to fix 5 files; all now converge and lint clean. --- .../Sniffs/Commenting/InlineDocBlockSniff.php | 32 +++++++++++++++---- .../WhiteSpace/ConsistentIndentSniff.php | 5 +++ PhpCollective/ruleset.xml | 6 +++- .../Commenting/InlineDocBlockSniffTest.php | 2 +- tests/_data/ConsistentIndent/after.php | 11 +++++++ tests/_data/ConsistentIndent/before.php | 11 +++++++ tests/_data/InlineDocBlock/after.php | 9 ++++++ tests/_data/InlineDocBlock/before.php | 9 ++++++ 8 files changed, 77 insertions(+), 8 deletions(-) diff --git a/PhpCollective/Sniffs/Commenting/InlineDocBlockSniff.php b/PhpCollective/Sniffs/Commenting/InlineDocBlockSniff.php index a69b582..03b61cb 100644 --- a/PhpCollective/Sniffs/Commenting/InlineDocBlockSniff.php +++ b/PhpCollective/Sniffs/Commenting/InlineDocBlockSniff.php @@ -139,12 +139,13 @@ protected function checkInlineComments(File $phpcsFile, int $startIndex, int $en $comment = $tokens[$contentTag]['content']; - if (isset($errors['space-before-end']) || isset($errors['end'])) { - $comment .= ' '; + if (isset($errors['space-before-end'])) { + $comment = rtrim($comment) . ' '; } if (isset($errors['order'])) { - $comment = preg_replace('|^(.+?)\s+(.+?)\s*$|', '\2 \1 ', $comment); + // Swap only `$var Type`, a trailing description stays where it is + $comment = (string)preg_replace('|^(\S+)(\s+)(\S+)|', '\3\2\1', $comment, 1); } $phpcsFile->fixer->replaceToken($contentTag, $comment); @@ -217,17 +218,36 @@ protected function findErrors(File $phpcsFile, int $contentIndex, bool $isSingle $errors = []; - if ($isSingleLine && !preg_match('|([a-z0-9]) $|i', $comment)) { + if ($isSingleLine && !preg_match('|\S $|', $comment)) { $errors['space-before-end'] = 'Expected single space before ´*/´'; } - if (!preg_match('|^\$[a-z0-9_]+$|i', $contentMatches[3])) { - $errors['order'] = 'Expected `{Type} ${var}`, got `' . $contentMatches[1] . $contentMatches[2] . $contentMatches[3] . '`'; + [$first, $second] = preg_split('|\s+|', trim($comment), 3) ?: ['', '']; + if ($this->isVariable($second)) { + return $errors; } + if ($this->isVariable($first)) { + $errors['order'] = 'Expected `{Type} ${var}`, got `' . $first . ' ' . $second . '`'; + + return $errors; + } + + $phpcsFile->addError('Invalid Inline Doc Block content, expected `{Type} ${var}` style', $contentIndex, 'ContentInvalid'); + return $errors; } + /** + * @param string $word + * + * @return bool + */ + protected function isVariable(string $word): bool + { + return (bool)preg_match('|^\$[a-z0-9_]+$|i', $word); + } + /** * @param \PHP_CodeSniffer\Files\File $phpcsFile * @param int $contentIndex diff --git a/PhpCollective/Sniffs/WhiteSpace/ConsistentIndentSniff.php b/PhpCollective/Sniffs/WhiteSpace/ConsistentIndentSniff.php index 3ae3ed1..5cec314 100644 --- a/PhpCollective/Sniffs/WhiteSpace/ConsistentIndentSniff.php +++ b/PhpCollective/Sniffs/WhiteSpace/ConsistentIndentSniff.php @@ -634,6 +634,11 @@ protected function isPreviousLineComplete(int $prevToken, array $tokens): bool */ protected function isInMultiLineConstruct(File $phpcsFile, int $stackPtr, array $tokens): bool { + // The backward scan below stops at `;`, which would miss the parts of a `for (...)` header + if (!empty($tokens[$stackPtr]['nested_parenthesis'])) { + return true; + } + // Check if we have unmatched parentheses on previous lines $currentLine = $tokens[$stackPtr]['line']; $openParens = 0; diff --git a/PhpCollective/ruleset.xml b/PhpCollective/ruleset.xml index e5a84ee..c126abe 100644 --- a/PhpCollective/ruleset.xml +++ b/PhpCollective/ruleset.xml @@ -70,7 +70,11 @@ - + + + + + diff --git a/tests/PhpCollective/Sniffs/Commenting/InlineDocBlockSniffTest.php b/tests/PhpCollective/Sniffs/Commenting/InlineDocBlockSniffTest.php index feb272b..7bfee53 100644 --- a/tests/PhpCollective/Sniffs/Commenting/InlineDocBlockSniffTest.php +++ b/tests/PhpCollective/Sniffs/Commenting/InlineDocBlockSniffTest.php @@ -17,7 +17,7 @@ class InlineDocBlockSniffTest extends TestCase */ public function testDocBlockConstSniffer(): void { - $this->assertSnifferFindsErrors(new InlineDocBlockSniff(), 1); + $this->assertSnifferFindsErrors(new InlineDocBlockSniff(), 2); } /** diff --git a/tests/_data/ConsistentIndent/after.php b/tests/_data/ConsistentIndent/after.php index c551819..d82943a 100644 --- a/tests/_data/ConsistentIndent/after.php +++ b/tests/_data/ConsistentIndent/after.php @@ -141,4 +141,15 @@ public function defaultProvider(): string }, ); } + + public function forHeader(array $list, int $lastIndex): void + { + for ( + $changeIndex = $lastIndex; + $list[$changeIndex] === 0 && $changeIndex > 0; + $changeIndex-- + ) { + $list[$changeIndex] = 1; + } + } } diff --git a/tests/_data/ConsistentIndent/before.php b/tests/_data/ConsistentIndent/before.php index a246723..cb4b2d3 100644 --- a/tests/_data/ConsistentIndent/before.php +++ b/tests/_data/ConsistentIndent/before.php @@ -141,4 +141,15 @@ public function defaultProvider(): string }, ); } + + public function forHeader(array $list, int $lastIndex): void + { + for ( + $changeIndex = $lastIndex; + $list[$changeIndex] === 0 && $changeIndex > 0; + $changeIndex-- + ) { + $list[$changeIndex] = 1; + } + } } diff --git a/tests/_data/InlineDocBlock/after.php b/tests/_data/InlineDocBlock/after.php index 2782e6a..5854255 100644 --- a/tests/_data/InlineDocBlock/after.php +++ b/tests/_data/InlineDocBlock/after.php @@ -32,5 +32,14 @@ public function test() /** @var object{user_id: int, home_id: int} $row */ $row = new \stdClass(); $user_id = $row->user_id; + + /** @var bool $deprecation Expand type for psalm */ + $deprecation = false; + + /** @var string $dotted Ends with a dot. */ + $dotted = ''; + + /** @var string $reversed Some description */ + $reversed = ''; } } diff --git a/tests/_data/InlineDocBlock/before.php b/tests/_data/InlineDocBlock/before.php index 6e5a561..0756cb7 100644 --- a/tests/_data/InlineDocBlock/before.php +++ b/tests/_data/InlineDocBlock/before.php @@ -32,5 +32,14 @@ public function test() /** @var object{user_id: int, home_id: int} $row */ $row = new \stdClass(); $user_id = $row->user_id; + + /** @var bool $deprecation Expand type for psalm */ + $deprecation = false; + + /** @var string $dotted Ends with a dot. */ + $dotted = ''; + + /** @var $reversed string Some description */ + $reversed = ''; } }