From 7ee508d15219206a492c76413bc92589173a088e Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Sun, 11 Oct 2026 16:40:35 +0200 Subject: [PATCH] Fix three autofixes that produced broken or wrong output PreferCastOverFunction rewrote the first-class callable `strval(...)` to `(string)(...)`, which is a parse error. The callable form is now skipped. DocBlockParamAllowDefaultValue did not treat `non-empty-array`, `non-empty-list` and a bare `list` as covering an `array` typehint and appended `|array` to the param type. DocBlockThrows wrote a class from the current namespace as if it were global, so `throw new LocalException()` in namespace `App` produced `\LocalException`. Unimported names are now resolved against the namespace the method sits in, and a qualified relative name such as `Sub\NestedException`, which was skipped before, is resolved as well. --- .../DocBlockParamAllowDefaultValueSniff.php | 7 +- .../Sniffs/Commenting/DocBlockThrowsSniff.php | 75 +++++++++++++++++-- .../PHP/PreferCastOverFunctionSniff.php | 9 +++ .../Commenting/DocBlockThrowsSniffTest.php | 4 +- .../DocBlockParamAllowDefaultValue/after.php | 13 ++++ .../DocBlockParamAllowDefaultValue/before.php | 13 ++++ tests/_data/DocBlockThrows/after.php | 23 ++++++ tests/_data/DocBlockThrows/before.php | 20 +++++ tests/_data/PreferCastOverFunction/after.php | 11 +++ tests/_data/PreferCastOverFunction/before.php | 11 +++ 10 files changed, 177 insertions(+), 9 deletions(-) diff --git a/PhpCollective/Sniffs/Commenting/DocBlockParamAllowDefaultValueSniff.php b/PhpCollective/Sniffs/Commenting/DocBlockParamAllowDefaultValueSniff.php index 63e9aaa..db46d4d 100644 --- a/PhpCollective/Sniffs/Commenting/DocBlockParamAllowDefaultValueSniff.php +++ b/PhpCollective/Sniffs/Commenting/DocBlockParamAllowDefaultValueSniff.php @@ -237,7 +237,12 @@ protected function isPrimitiveGenerics(string $type, array $parts): bool return false; } - foreach ($iterableTypes as $iterableType) { + $docBlockIterableTypes = [...$iterableTypes, 'non-empty-array', 'non-empty-list']; + if (array_intersect($docBlockIterableTypes, $parts)) { + return true; + } + + foreach ($docBlockIterableTypes as $iterableType) { if ($this->containsTypeArray($parts, $iterableType)) { return true; } diff --git a/PhpCollective/Sniffs/Commenting/DocBlockThrowsSniff.php b/PhpCollective/Sniffs/Commenting/DocBlockThrowsSniff.php index b087849..0878f8f 100644 --- a/PhpCollective/Sniffs/Commenting/DocBlockThrowsSniff.php +++ b/PhpCollective/Sniffs/Commenting/DocBlockThrowsSniff.php @@ -153,7 +153,7 @@ protected function extractExceptions(File $phpcsFile, int $stackPointer): array // Handle the case where there's no namespace separator, string token, or fully qualified name token // (e.g., for parenthesis after 'new') - if (!$this->isGivenKind([T_NS_SEPARATOR, T_STRING, T_NAME_FULLY_QUALIFIED], $tokens[$contentIndex])) { + if (!$this->isGivenKind([T_NS_SEPARATOR, T_STRING, T_NAME_FULLY_QUALIFIED, T_NAME_QUALIFIED], $tokens[$contentIndex])) { continue; } @@ -248,12 +248,13 @@ protected function extractException(File $phpcsFile, int $contentIndex): array $fullClass = $tokens[$position]['content']; ++$position; } else { - while ($this->isGivenKind([T_NS_SEPARATOR, T_STRING], $tokens[$position])) { + while ($this->isGivenKind([T_NS_SEPARATOR, T_STRING, T_NAME_QUALIFIED], $tokens[$position])) { $fullClass .= $tokens[$position]['content']; ++$position; } } + $fullyQualified = str_starts_with($fullClass, '\\'); $class = $fullClass = ltrim($fullClass, '\\'); $lastSeparator = strrpos($class, '\\'); if ($lastSeparator !== false) { @@ -265,6 +266,7 @@ protected function extractException(File $phpcsFile, int $contentIndex): array 'end' => $position - 1, 'fullClass' => $fullClass, 'class' => $class, + 'fullyQualified' => $fullyQualified, ]; } @@ -283,6 +285,7 @@ protected function compareExceptionsAndAnnotations( int $docBlockEndIndex, ): void { $useStatements = $this->getUseStatements($phpcsFile); + $namespace = $this->getNamespaceAt($phpcsFile, $docBlockEndIndex); foreach ($annotations as $annotation) { if ($this->isInCode($annotation, $exceptions, $useStatements)) { @@ -307,7 +310,7 @@ protected function compareExceptionsAndAnnotations( $exceptionsToAdd = []; foreach ($exceptions as $exception) { - $exception = $this->normalizeClassName($exception, $useStatements); + $exception = $this->normalizeClassName($exception, $useStatements, $namespace); if (empty($exception['fullClass'])) { // We skip for complex scenarios $phpcsFile->addError('Doc Block @throw annotation missing', $docBlockEndIndex, 'ThrowMissingManual'); @@ -333,19 +336,79 @@ protected function compareExceptionsAndAnnotations( $this->addAnnotationLines($phpcsFile, $exceptionsToAdd, $docBlockEndIndex); } + /** + * Namespace the given position is declared in; a file can hold several namespace blocks. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile + * @param int $index + * + * @return string + */ + protected function getNamespaceAt(File $phpcsFile, int $index): string + { + $tokens = $phpcsFile->getTokens(); + + $namespaceIndex = $index; + do { + $namespaceIndex = $phpcsFile->findPrevious(T_NAMESPACE, $namespaceIndex - 1); + if ($namespaceIndex === false) { + return ''; + } + $nextIndex = $phpcsFile->findNext(Tokens::$emptyTokens, $namespaceIndex + 1, null, true); + // `namespace\Foo` is the relative-name operator, not a declaration + } while ($nextIndex !== false && $tokens[$nextIndex]['code'] === T_NS_SEPARATOR); + + if (isset($tokens[$namespaceIndex]['scope_closer']) && $tokens[$namespaceIndex]['scope_closer'] < $index) { + return ''; + } + + $endIndex = $phpcsFile->findNext([T_SEMICOLON, T_OPEN_CURLY_BRACKET], $namespaceIndex + 1); + if ($endIndex === false) { + return ''; + } + + $namespace = ''; + for ($i = $namespaceIndex + 1; $i < $endIndex; $i++) { + if (!in_array($tokens[$i]['code'], Tokens::$emptyTokens, true)) { + $namespace .= $tokens[$i]['content']; + } + } + + return $namespace; + } + /** * @param array $exception * @param array> $useStatements + * @param string $namespace Namespace of the file, to resolve names that are not imported. * * @return array Exception */ - protected function normalizeClassName(array $exception, array $useStatements): array + protected function normalizeClassName(array $exception, array $useStatements, string $namespace = ''): array { + if (!empty($exception['fullyQualified']) || empty($exception['fullClass'])) { + return $exception; + } + + /** @var string $fullClass */ + $fullClass = $exception['fullClass']; + $segments = explode('\\', $fullClass); + $first = array_shift($segments); + foreach ($useStatements as $useStatement) { - if ($useStatement['alias'] === $exception['class'] || $useStatement['shortName'] === $exception['class']) { + if ($useStatement['alias'] !== $first && $useStatement['shortName'] !== $first) { + continue; + } + if (!$segments) { $exception['class'] = $useStatement['shortName']; - $exception['fullClass'] = $useStatement['fullName']; } + $exception['fullClass'] = implode('\\', [$useStatement['fullName'], ...$segments]); + + return $exception; + } + + if ($namespace !== '') { + $exception['fullClass'] = $namespace . '\\' . $fullClass; } return $exception; diff --git a/PhpCollective/Sniffs/PHP/PreferCastOverFunctionSniff.php b/PhpCollective/Sniffs/PHP/PreferCastOverFunctionSniff.php index 3ba6829..13ab079 100644 --- a/PhpCollective/Sniffs/PHP/PreferCastOverFunctionSniff.php +++ b/PhpCollective/Sniffs/PHP/PreferCastOverFunctionSniff.php @@ -60,6 +60,15 @@ public function process(File $phpcsFile, $stackPtr): void $closingBraceIndex = $tokens[$openingBraceIndex]['parenthesis_closer']; + // First-class callable syntax `strval(...)` is not a call and has no cast equivalent + $firstArgumentIndex = $phpcsFile->findNext(T_WHITESPACE, $openingBraceIndex + 1, $closingBraceIndex, true); + if ($firstArgumentIndex !== false && $tokens[$firstArgumentIndex]['code'] === T_ELLIPSIS) { + $afterEllipsisIndex = $phpcsFile->findNext(T_WHITESPACE, $firstArgumentIndex + 1, $closingBraceIndex, true); + if ($afterEllipsisIndex === false) { + return; + } + } + // We must ignore when commas are encountered if ($this->contains($phpcsFile, 'T_COMMA', $openingBraceIndex + 1, $closingBraceIndex - 1)) { return; diff --git a/tests/PhpCollective/Sniffs/Commenting/DocBlockThrowsSniffTest.php b/tests/PhpCollective/Sniffs/Commenting/DocBlockThrowsSniffTest.php index e8b9bdb..1e746eb 100644 --- a/tests/PhpCollective/Sniffs/Commenting/DocBlockThrowsSniffTest.php +++ b/tests/PhpCollective/Sniffs/Commenting/DocBlockThrowsSniffTest.php @@ -17,7 +17,7 @@ class DocBlockThrowsSniffTest extends TestCase */ public function testDocBlockThrowsSniffer(): void { - $this->assertSnifferFindsErrors(new DocBlockThrowsSniff(), 6); + $this->assertSnifferFindsErrors(new DocBlockThrowsSniff(), 9); } /** @@ -25,6 +25,6 @@ public function testDocBlockThrowsSniffer(): void */ public function testDocBlockThrowsFixer(): void { - $this->assertSnifferCanFixErrors(new DocBlockThrowsSniff(), 6); + $this->assertSnifferCanFixErrors(new DocBlockThrowsSniff(), 9); } } diff --git a/tests/_data/DocBlockParamAllowDefaultValue/after.php b/tests/_data/DocBlockParamAllowDefaultValue/after.php index fba7856..e9a668d 100644 --- a/tests/_data/DocBlockParamAllowDefaultValue/after.php +++ b/tests/_data/DocBlockParamAllowDefaultValue/after.php @@ -99,4 +99,17 @@ public function arrayShape(array $array = null): void public function partialDocBlock(array $row, int $accountId, array &$errors): void { } + + /** + * Narrowed array types already cover the `array` typehint. + * + * @param non-empty-array $block + * @param non-empty-list $names + * @param non-empty-array $rows + * @param list $items + * @return void + */ + public function narrowedArrays(array $block, array $names, array $rows, array $items): void + { + } } diff --git a/tests/_data/DocBlockParamAllowDefaultValue/before.php b/tests/_data/DocBlockParamAllowDefaultValue/before.php index c644f32..68b934f 100644 --- a/tests/_data/DocBlockParamAllowDefaultValue/before.php +++ b/tests/_data/DocBlockParamAllowDefaultValue/before.php @@ -99,4 +99,17 @@ public function arrayShape(array $array = null): void public function partialDocBlock(array $row, int $accountId, array &$errors): void { } + + /** + * Narrowed array types already cover the `array` typehint. + * + * @param non-empty-array $block + * @param non-empty-list $names + * @param non-empty-array $rows + * @param list $items + * @return void + */ + public function narrowedArrays(array $block, array $names, array $rows, array $items): void + { + } } diff --git a/tests/_data/DocBlockThrows/after.php b/tests/_data/DocBlockThrows/after.php index f6a627b..4ca40f3 100644 --- a/tests/_data/DocBlockThrows/after.php +++ b/tests/_data/DocBlockThrows/after.php @@ -71,4 +71,27 @@ public function staticCall(): void new Parser(); } + + /** + * @throws \PhpCollective\LocalException + * @return void + */ + public function sameNamespace(): void + { + throw new LocalException(); + } + + /** + * @throws \Foo\Baz\Exception\DeeperException + * @throws \PhpCollective\Sub\NestedException + * @return void + */ + public function relativeNames(): void + { + if ($this->something()) { + throw new Sub\NestedException(); + } + + throw new SomeAliasedException\DeeperException(); + } } diff --git a/tests/_data/DocBlockThrows/before.php b/tests/_data/DocBlockThrows/before.php index 071b354..dade113 100644 --- a/tests/_data/DocBlockThrows/before.php +++ b/tests/_data/DocBlockThrows/before.php @@ -65,4 +65,24 @@ public function staticCall(): void new Parser(); } + + /** + * @return void + */ + public function sameNamespace(): void + { + throw new LocalException(); + } + + /** + * @return void + */ + public function relativeNames(): void + { + if ($this->something()) { + throw new Sub\NestedException(); + } + + throw new SomeAliasedException\DeeperException(); + } } diff --git a/tests/_data/PreferCastOverFunction/after.php b/tests/_data/PreferCastOverFunction/after.php index 3e3847a..63c13f4 100644 --- a/tests/_data/PreferCastOverFunction/after.php +++ b/tests/_data/PreferCastOverFunction/after.php @@ -16,4 +16,15 @@ public function namespacedFunctionForms($value): array return [$plain, $fullyQualified, $qualified, $relative]; } + + /** + * First-class callable syntax must stay as is. + */ + public function firstClassCallable(array $values): array + { + $strings = array_map(strval(...), $values); + $ints = array_map(\intval( ... ), $values); + + return [$strings, $ints]; + } } diff --git a/tests/_data/PreferCastOverFunction/before.php b/tests/_data/PreferCastOverFunction/before.php index 7a3c1dd..cda9000 100644 --- a/tests/_data/PreferCastOverFunction/before.php +++ b/tests/_data/PreferCastOverFunction/before.php @@ -16,4 +16,15 @@ public function namespacedFunctionForms($value): array return [$plain, $fullyQualified, $qualified, $relative]; } + + /** + * First-class callable syntax must stay as is. + */ + public function firstClassCallable(array $values): array + { + $strings = array_map(strval(...), $values); + $ints = array_map(\intval( ... ), $values); + + return [$strings, $ints]; + } }