From bee1fb1cef1ecd109a95d3c79d3b4e70c16c1d5c Mon Sep 17 00:00:00 2001 From: mscherer Date: Thu, 27 Nov 2025 13:47:40 +0100 Subject: [PATCH] Fix param order when adding missing `@param` annotations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a docblock has only some `@param` annotations (e.g., the middle parameter), the fixer was adding missing params at the wrong position. For example, with method `foo($a, $b, $c)` and only `@param ... $b` documented, the fixer would add `$a` and `$c` after `$b` instead of in the correct order. This fix ensures params are inserted in the correct positions: - Params before the first existing `@param` are inserted before it - Params after an existing `@param` are inserted after it Adds test case for this scenario. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../Sniffs/Commenting/DocBlockParamSniff.php | 135 ++++++++++++++++-- .../Commenting/DocBlockParamSniffTest.php | 8 +- tests/_data/DocBlockParam/after.php | 12 ++ tests/_data/DocBlockParam/before.php | 10 ++ 4 files changed, 154 insertions(+), 11 deletions(-) diff --git a/PhpCollective/Sniffs/Commenting/DocBlockParamSniff.php b/PhpCollective/Sniffs/Commenting/DocBlockParamSniff.php index 2d8604a..fff1677 100644 --- a/PhpCollective/Sniffs/Commenting/DocBlockParamSniff.php +++ b/PhpCollective/Sniffs/Commenting/DocBlockParamSniff.php @@ -269,20 +269,139 @@ protected function canAddMissingParams(File $phpcsFile, int $docBlockStartIndex, if ($fix === true) { $phpcsFile->fixer->beginChangeset(); - // Build list of existing param variables - $existingVars = []; + // Build map of existing param variables to their docblock info + $existingParamsByVar = []; foreach ($docBlockParams as $param) { - $existingVars[] = $param['variable']; + $existingParamsByVar[$param['variable']] = $param; } - // Add missing params + // Build ordered list of what the @param section should look like + $orderedParams = []; foreach ($methodSignature as $methodParam) { $variable = $tokens[$methodParam['variableIndex']]['content']; - if (!in_array($variable, $existingVars, true)) { - $indent = $this->getIndentForParam($phpcsFile, $docBlockStartIndex, $docBlockEndIndex); - $paramLine = "\n" . $indent . '* @param ' . $methodParam['typehintFull'] . ' ' . $variable; + if (isset($existingParamsByVar[$variable])) { + // Use existing param's type (preserve user's more specific type) + $orderedParams[] = [ + 'type' => $existingParamsByVar[$variable]['type'], + 'variable' => $variable, + 'appendix' => $existingParamsByVar[$variable]['appendix'], + 'existing' => true, + ]; + } else { + // Add new param with method signature type + $orderedParams[] = [ + 'type' => $methodParam['typehintFull'], + 'variable' => $variable, + 'appendix' => ' ' . $variable, + 'existing' => false, + ]; + } + } + + // Now we need to add the missing params in the correct positions + // Strategy: find each existing param in the docblock and add missing ones around it + + // Build a map of existing @param tag positions by variable + $paramTagsByVar = []; + for ($i = $docBlockStartIndex + 1; $i < $docBlockEndIndex; $i++) { + if ($tokens[$i]['type'] === 'T_DOC_COMMENT_TAG' && $tokens[$i]['content'] === '@param') { + $classNameIndex = $i + 2; + if (isset($tokens[$classNameIndex]) && $tokens[$classNameIndex]['type'] === 'T_DOC_COMMENT_STRING') { + $content = $tokens[$classNameIndex]['content']; + $spacePos = strpos($content, ' '); + if ($spacePos) { + $appendix = substr($content, $spacePos); + preg_match('/\$[^\s]+/', $appendix, $matches); + if ($matches) { + $paramTagsByVar[$matches[0]] = [ + 'tagIndex' => $i, + 'stringIndex' => $classNameIndex, + ]; + } + } + } + } + } + + $indent = $this->getIndentForParam($phpcsFile, $docBlockStartIndex, $docBlockEndIndex); + + // Process ordered params - insert missing ones at appropriate positions + $pendingInserts = []; // Params to insert before the first existing one + $lastExistingTagIndex = null; + + foreach ($orderedParams as $param) { + if ($param['existing']) { + // This param exists - first flush any pending inserts before it + if ($pendingInserts !== [] && isset($paramTagsByVar[$param['variable']])) { + $tagInfo = $paramTagsByVar[$param['variable']]; + // Find the whitespace/newline before this tag + $insertBeforeIndex = $tagInfo['tagIndex']; + for ($j = $tagInfo['tagIndex'] - 1; $j > $docBlockStartIndex; $j--) { + if ($tokens[$j]['type'] === 'T_DOC_COMMENT_WHITESPACE' && $tokens[$j]['content'] !== ' ') { + // This is the indentation whitespace, insert before it + $insertBeforeIndex = $j; + + break; + } + } + + foreach ($pendingInserts as $pendingParam) { + $paramLine = $indent . '* @param ' . $pendingParam['type'] . ' ' . $pendingParam['variable'] . "\n"; + $phpcsFile->fixer->addContentBefore($insertBeforeIndex, $paramLine); + } + $pendingInserts = []; + } + $lastExistingTagIndex = $paramTagsByVar[$param['variable']]['tagIndex'] ?? null; + } else { + // This param needs to be added + if ($lastExistingTagIndex === null) { + // No existing param seen yet - queue it + $pendingInserts[] = $param; + } else { + // Insert after the last existing param's line + // Find the start of the next line (the whitespace token after the newline) + $insertBeforeIndex = null; + $foundNewline = false; + for ($j = $lastExistingTagIndex + 1; $j < $docBlockEndIndex; $j++) { + if ($tokens[$j]['content'] === "\n") { + $foundNewline = true; + + continue; + } + if ($foundNewline && $tokens[$j]['type'] === 'T_DOC_COMMENT_WHITESPACE') { + $insertBeforeIndex = $j; + + break; + } + } + + if ($insertBeforeIndex !== null) { + $paramLine = $indent . '* @param ' . $param['type'] . ' ' . $param['variable'] . "\n"; + $phpcsFile->fixer->addContentBefore($insertBeforeIndex, $paramLine); + } + // Update lastExistingTagIndex to track where we just inserted + // (This is tricky - the token indices don't change, but logically we've added after) + } + } + } + + // If there are still pending inserts (all params come before existing ones, or no existing params) + if ($pendingInserts !== []) { + // Find the whitespace before the closing tag + $insertBeforeIndex = null; + for ($j = $docBlockEndIndex - 1; $j > $docBlockStartIndex; $j--) { + if ($tokens[$j]['type'] === 'T_DOC_COMMENT_WHITESPACE' && $tokens[$j]['content'] !== "\n" && strpos($tokens[$j]['content'], "\n") === false) { + $insertBeforeIndex = $j; - $phpcsFile->fixer->addContentBefore($insertPosition, $paramLine); + break; + } + } + + if ($insertBeforeIndex !== null) { + foreach ($pendingInserts as $pendingParam) { + $paramLine = $indent . '* @param ' . $pendingParam['type'] . ' ' . $pendingParam['variable'] . "\n"; + $phpcsFile->fixer->addContentBefore($insertBeforeIndex, $paramLine); + } } } diff --git a/tests/PhpCollective/Sniffs/Commenting/DocBlockParamSniffTest.php b/tests/PhpCollective/Sniffs/Commenting/DocBlockParamSniffTest.php index 8337be3..e5a99f2 100644 --- a/tests/PhpCollective/Sniffs/Commenting/DocBlockParamSniffTest.php +++ b/tests/PhpCollective/Sniffs/Commenting/DocBlockParamSniffTest.php @@ -26,7 +26,8 @@ public function testDocBlockParamSniffer(): void // Line 74: ExtraParam - no params but has @param (fixable - can remove param) // Line 84: MissingType - missing type in @param (not fixable - needs manual type) // Line 87: SignatureMismatch - params don't match after missing type (not fixable) - $this->assertSnifferFindsErrors(new DocBlockParamSniff(), 8); + // Line 169: SignatureMismatch - middle param documented, need to add before and after (fixable) + $this->assertSnifferFindsErrors(new DocBlockParamSniff(), 9); } /** @@ -34,10 +35,11 @@ public function testDocBlockParamSniffer(): void */ public function testDocBlockParamFixer(): void { - // 3 fixable errors: + // 4 fixable errors: // Line 37: Can add missing @param // Line 64: Can remove extra @param // Line 74: Can remove @param when no params - $this->assertSnifferCanFixErrors(new DocBlockParamSniff(), 3); + // Line 169: Can add missing @param before and after existing param (order preserved) + $this->assertSnifferCanFixErrors(new DocBlockParamSniff(), 4); } } diff --git a/tests/_data/DocBlockParam/after.php b/tests/_data/DocBlockParam/after.php index 8cd6f19..1d97b7b 100644 --- a/tests/_data/DocBlockParam/after.php +++ b/tests/_data/DocBlockParam/after.php @@ -161,4 +161,16 @@ public function nestedMultiLineGenerics(array $nested): void { // Should not error - deeply nested multi-line type } + + /** + * Middle param documented - should add before and after + * + * @param Node $parent + * @param array $lines + * @param int $indent + */ + public function middleParamDocumented(Node $parent, array $lines, int $indent): void + { + // Should error: missing @param for $parent (before) and $indent (after) + } } \ No newline at end of file diff --git a/tests/_data/DocBlockParam/before.php b/tests/_data/DocBlockParam/before.php index f6f94e7..e8487f5 100644 --- a/tests/_data/DocBlockParam/before.php +++ b/tests/_data/DocBlockParam/before.php @@ -162,4 +162,14 @@ public function nestedMultiLineGenerics(array $nested): void { // Should not error - deeply nested multi-line type } + + /** + * Middle param documented - should add before and after + * + * @param array $lines + */ + public function middleParamDocumented(Node $parent, array $lines, int $indent): void + { + // Should error: missing @param for $parent (before) and $indent (after) + } } \ No newline at end of file