From eca1a926207d4700f8bbff1e89f5476d963575f2 Mon Sep 17 00:00:00 2001 From: Alex Skrypnyk Date: Mon, 24 Nov 2025 16:36:55 +1100 Subject: [PATCH] Fixed snake_case not being updated in the docblocks. --- .../ParameterSnakeCaseSniff.php | 113 ++++++++++++++++++ tests/Fixtures/ParameterDocblockMismatch.php | 105 ++++++++++++++++ .../ParameterSnakeCaseSniffFunctionalTest.php | 48 ++++++++ tests/Unit/ParameterSnakeCaseSniffTest.php | 78 ++++++++++++ 4 files changed, 344 insertions(+) create mode 100644 tests/Fixtures/ParameterDocblockMismatch.php diff --git a/src/DrevOps/Sniffs/NamingConventions/ParameterSnakeCaseSniff.php b/src/DrevOps/Sniffs/NamingConventions/ParameterSnakeCaseSniff.php index 9f82bc5..eec7265 100644 --- a/src/DrevOps/Sniffs/NamingConventions/ParameterSnakeCaseSniff.php +++ b/src/DrevOps/Sniffs/NamingConventions/ParameterSnakeCaseSniff.php @@ -62,9 +62,122 @@ public function process(File $phpcsFile, $stackPtr): void { // and Fixer). Unit tests only check for error detection, not fixing. if ($fix === TRUE) { $phpcsFile->fixer->replaceToken($stackPtr, '$' . $suggestion); + + // Also fix the parameter name in the docblock @param tag if present. + $this->fixDocblockParam($phpcsFile, $stackPtr, $var_name, $suggestion); } // @codeCoverageIgnoreEnd } } + /** + * Find the docblock for a function/method. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile + * The file being scanned. + * @param int $functionPtr + * The position of the function token. + * + * @return int|false + * The position of the docblock open tag, or FALSE if not found. + */ + private function findFunctionDocblock(File $phpcsFile, int $functionPtr): int|false { + $tokens = $phpcsFile->getTokens(); + + // Search backwards for docblock, skipping whitespace, comments, attributes, + // visibility modifiers, and other function modifiers. + $skip_tokens = [ + T_WHITESPACE, + T_COMMENT, + T_ATTRIBUTE, + T_ATTRIBUTE_END, + T_PUBLIC, + T_PROTECTED, + T_PRIVATE, + T_STATIC, + T_FINAL, + T_ABSTRACT, + ]; + + $search = $phpcsFile->findPrevious($skip_tokens, $functionPtr - 1, NULL, TRUE); + + if ($search !== FALSE && $tokens[$search]['code'] === T_DOC_COMMENT_CLOSE_TAG) { + // Found a docblock close tag, return its opener. + return $tokens[$search]['comment_opener'] ?? FALSE; + } + + return FALSE; + } + + /** + * Fix the parameter name in the docblock @param tag. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile + * The file being scanned. + * @param int $stackPtr + * The position of the parameter variable token. + * @param string $oldName + * The old parameter name (without $). + * @param string $newName + * The new parameter name (without $). + * + * @codeCoverageIgnore + */ + private function fixDocblockParam(File $phpcsFile, int $stackPtr, string $oldName, string $newName): void { + $tokens = $phpcsFile->getTokens(); + + // Find the enclosing function. + $function_ptr = $this->findEnclosingFunction($phpcsFile, $stackPtr); + if ($function_ptr === FALSE) { + return; + } + + // Find the function's docblock. + $docblock_start = $this->findFunctionDocblock($phpcsFile, $function_ptr); + if ($docblock_start === FALSE) { + return; + } + + $docblock_end = $tokens[$docblock_start]['comment_closer'] ?? FALSE; + if ($docblock_end === FALSE) { + return; + } + + // Search for @param tags in the docblock. + for ($i = $docblock_start; $i < $docblock_end; $i++) { + if ($tokens[$i]['code'] === T_DOC_COMMENT_TAG && $tokens[$i]['content'] === '@param') { + // Found a @param tag. Look for the parameter name in the following + // tokens. + // The format is typically: @param type $paramName description. + for ($j = $i + 1; $j < $docblock_end; $j++) { + $token = $tokens[$j]; + + // Check for the parameter variable name. + if ($token['code'] === T_DOC_COMMENT_STRING) { + $content = $token['content']; + + // Check if this string contains the parameter name. + // Parameter names in docblocks are prefixed with $. + if (preg_match('/\$' . preg_quote($oldName, '/') . '(?![a-zA-Z0-9_])/', $content) === 1) { + // Replace the parameter name in the string. + $new_content = preg_replace( + '/\$' . preg_quote($oldName, '/') . '(?![a-zA-Z0-9_])/', + '$' . $newName, + $content + ); + $phpcsFile->fixer->replaceToken($j, $new_content); + // Stop searching after finding the parameter for this @param tag. + break; + } + } + + // Stop if we hit another @tag (moved to next tag). + if ($token['code'] === T_DOC_COMMENT_TAG) { + break; + } + } + } + } + } + } diff --git a/tests/Fixtures/ParameterDocblockMismatch.php b/tests/Fixtures/ParameterDocblockMismatch.php new file mode 100644 index 0000000..9a7d631 --- /dev/null +++ b/tests/Fixtures/ParameterDocblockMismatch.php @@ -0,0 +1,105 @@ + $invalidParam + * Complex array type. + * @param \DateTime|null $optionalInvalid + * Optional parameter with union type. + * + * @return void + */ + public function complexTypes(array $invalidParam, ?\DateTime $optionalInvalid = NULL): void { + // Implementation. + } + + /** + * Method with no docblock tags. + * + * This method has a docblock but no @param tags. + */ + public function noParamTags(string $invalidParam): void { + echo $invalidParam; + } + + /** + * Method with extra docblock content. + * + * @see SomeClass + * @param string $invalidParam + * The parameter. + * @throws \Exception + * When something goes wrong. + * + * @return void + */ + public function extraTags(string $invalidParam): void { + echo $invalidParam; + } + +} + +/** + * Standalone function with docblock. + * + * @param string $invalidParam + * The invalid parameter. + * @param int $anotherInvalid + * Another invalid parameter. + * + * @return string + */ +function standaloneFunctionWithDocblock(string $invalidParam, int $anotherInvalid): string { + return $invalidParam . $anotherInvalid; +} diff --git a/tests/Functional/ParameterSnakeCaseSniffFunctionalTest.php b/tests/Functional/ParameterSnakeCaseSniffFunctionalTest.php index 1094446..b1371fa 100644 --- a/tests/Functional/ParameterSnakeCaseSniffFunctionalTest.php +++ b/tests/Functional/ParameterSnakeCaseSniffFunctionalTest.php @@ -80,4 +80,52 @@ public function testPropertiesAreNotFlagged(): void { ); } + /** + * Test that phpcbf fixes both parameter names and docblock @param tags. + */ + public function testPhpcbfFixesDocblockParams(): void { + $fixture_file = static::$fixtures . DIRECTORY_SEPARATOR . 'ParameterDocblockMismatch.php'; + $this->assertFileExists($fixture_file, 'Fixture file must exist'); + + // Create a temporary copy to fix. + $temp_file = static::$tmp . DIRECTORY_SEPARATOR . 'test_' . uniqid() . '.php'; + copy($fixture_file, $temp_file); + + // Run phpcbf to fix the file. + $phpcbf_bin = __DIR__ . '/../../vendor/bin/phpcbf'; + $this->assertFileExists($phpcbf_bin, 'PHPCBF binary must exist'); + + $this->processRun( + $phpcbf_bin, + ['--standard=DrevOps', '--sniffs=DrevOps.NamingConventions.ParameterSnakeCase', '-q', $temp_file], + timeout: 120 + ); + + // Read the fixed file. + $fixed_content = file_get_contents($temp_file); + $this->assertIsString($fixed_content, 'Fixed file should be readable'); + + // Verify that parameter names in signatures are fixed. + $this->assertStringContainsString('function methodWithDocblock(string $invalid_param', $fixed_content, 'Parameter signature should be fixed'); + $this->assertStringContainsString('int $another_invalid', $fixed_content, 'Second parameter signature should be fixed'); + + // Verify that parameter names in docblocks are also fixed. + $this->assertStringContainsString('@param string $invalid_param', $fixed_content, 'Docblock @param should be fixed'); + $this->assertStringContainsString('@param int $another_invalid', $fixed_content, 'Docblock @param should be fixed for second parameter'); + + // Verify old parameter names are gone from signatures and docblocks. + // Note: Parameter usages in method bodies are NOT fixed by this sniff - + // that's the job of LocalVariableSnakeCaseSniff. + $this->assertStringNotContainsString('function methodWithDocblock(string $invalidParam', $fixed_content, 'Old parameter name should not exist in signature'); + $this->assertStringNotContainsString('@param string $invalidParam', $fixed_content, 'Old parameter name should not exist in docblock'); + $this->assertStringNotContainsString('@param int $anotherInvalid', $fixed_content, 'Old parameter name should not exist in docblock'); + + // Verify complex types are preserved. + $this->assertStringContainsString('array $invalid_param', $fixed_content, 'Complex array type should be preserved'); + $this->assertStringContainsString('\DateTime|null $optional_invalid', $fixed_content, 'Union type should be preserved'); + + // Verify multiline descriptions are preserved. + $this->assertStringContainsString('This is a parameter with a very long description', $fixed_content, 'Multiline descriptions should be preserved'); + } + } diff --git a/tests/Unit/ParameterSnakeCaseSniffTest.php b/tests/Unit/ParameterSnakeCaseSniffTest.php index 0e39f33..3bfdd3b 100644 --- a/tests/Unit/ParameterSnakeCaseSniffTest.php +++ b/tests/Unit/ParameterSnakeCaseSniffTest.php @@ -114,4 +114,82 @@ public static function providerProcessCases(): array { ]; } + /** + * Test findFunctionDocblock method finds docblock for a function. + * + * @param string $code + * PHP code to test. + * @param bool $should_find_docblock + * Whether a docblock should be found. + */ + #[DataProvider('providerFindFunctionDocblock')] + public function testFindFunctionDocblock(string $code, bool $should_find_docblock): void { + $file = $this->processCode($code); + $sniff = new ParameterSnakeCaseSniff(); + + // Use reflection to access the private method. + $reflection = new \ReflectionClass($sniff); + $method = $reflection->getMethod('findFunctionDocblock'); + + // Find the function token. + $function_token = $this->findFunctionToken($file); + $this->assertNotFalse($function_token, 'Function token should be found'); + + // Call the method. + $result = $method->invoke($sniff, $file, $function_token); + + if ($should_find_docblock) { + $this->assertNotFalse($result, 'Docblock should be found'); + $this->assertIsInt($result); + } + else { + $this->assertFalse($result, 'Docblock should not be found'); + } + } + + /** + * Data provider for findFunctionDocblock tests. + * + * @return array> + * Test cases. + */ + public static function providerFindFunctionDocblock(): array { + return [ + 'function_with_docblock' => [ + ' [ + ' [ + ' [ + '