From 9313d7baf45941b31555606ea31194f4737f3be5 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Sat, 6 Jun 2026 19:28:08 +0200 Subject: [PATCH 1/2] Fix DocBlockVar fixer corrupting callable/Closure types The var-annotation fixer splits the type on its first space to separate a trailing comment, assuming types have no internal spaces. A callable/Closure signature such as `\Closure(string): string` has a space after the colon, so the split cut the type in half and the null-append produced invalid output (`\Closure(string):|null string`), which PHPStan then rejects as an unparseable doc type. Skip types containing `(` (callable/Closure signatures) the same way generics and array shapes are already skipped, since the first-space split cannot handle their internal structure. Adds a regression fixture. --- PhpCollective/Sniffs/Commenting/DocBlockVarSniff.php | 6 +++++- tests/_data/DocBlockVar/after.php | 5 +++++ tests/_data/DocBlockVar/before.php | 5 +++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/PhpCollective/Sniffs/Commenting/DocBlockVarSniff.php b/PhpCollective/Sniffs/Commenting/DocBlockVarSniff.php index 7ad77c9..205722b 100644 --- a/PhpCollective/Sniffs/Commenting/DocBlockVarSniff.php +++ b/PhpCollective/Sniffs/Commenting/DocBlockVarSniff.php @@ -283,7 +283,11 @@ protected function handle(File $phpcsFile, int $docBlockEndIndex, int $stackPoin } $content = $tokens[$classNameIndex]['content']; - if (str_contains($content, '{') || str_contains($content, '<')) { + // Skip types whose syntax contains internal spaces/structure that the + // simple first-space split below cannot handle: generics/array-shapes + // (`{`, `<`) and callable/Closure signatures (`(int): string`). Without + // this the fixer would split mid-type and corrupt the annotation. + if (str_contains($content, '{') || str_contains($content, '<') || str_contains($content, '(')) { return; } diff --git a/tests/_data/DocBlockVar/after.php b/tests/_data/DocBlockVar/after.php index 341ff37..13ccc8e 100644 --- a/tests/_data/DocBlockVar/after.php +++ b/tests/_data/DocBlockVar/after.php @@ -70,4 +70,9 @@ class FixMe * @var MyClass[] */ protected array $objects; + + /** + * @var \Closure(string): string + */ + protected ?Closure $transformer = null; } diff --git a/tests/_data/DocBlockVar/before.php b/tests/_data/DocBlockVar/before.php index 341ff37..13ccc8e 100644 --- a/tests/_data/DocBlockVar/before.php +++ b/tests/_data/DocBlockVar/before.php @@ -70,4 +70,9 @@ class FixMe * @var MyClass[] */ protected array $objects; + + /** + * @var \Closure(string): string + */ + protected ?Closure $transformer = null; } From e180427fc5ccaad477c9b4570d87671e776a295b Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Sat, 6 Jun 2026 19:36:52 +0200 Subject: [PATCH 2/2] Only skip structural types, not descriptions with parentheses Split the doc type from its trailing description before the structural check, then test only the type token. The previous check ran on the full string, so a simple type with a parenthetical description (e.g. `@var string (deprecated)`) was wrongly skipped and never got its missing `null` appended. Closure/generic/array-shape types still skip correctly because their opening token sits before the first space. Also use strict `!== false` for the space split so an index-0 match is not treated as "no space". Add a regression fixture for the description-with-parentheses case. --- .../Sniffs/Commenting/DocBlockVarSniff.php | 19 +++++++++++-------- .../Commenting/DocBlockVarSniffTest.php | 2 +- tests/_data/DocBlockVar/after.php | 5 +++++ tests/_data/DocBlockVar/before.php | 5 +++++ 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/PhpCollective/Sniffs/Commenting/DocBlockVarSniff.php b/PhpCollective/Sniffs/Commenting/DocBlockVarSniff.php index 205722b..6a0705e 100644 --- a/PhpCollective/Sniffs/Commenting/DocBlockVarSniff.php +++ b/PhpCollective/Sniffs/Commenting/DocBlockVarSniff.php @@ -283,21 +283,24 @@ protected function handle(File $phpcsFile, int $docBlockEndIndex, int $stackPoin } $content = $tokens[$classNameIndex]['content']; - // Skip types whose syntax contains internal spaces/structure that the - // simple first-space split below cannot handle: generics/array-shapes - // (`{`, `<`) and callable/Closure signatures (`(int): string`). Without - // this the fixer would split mid-type and corrupt the annotation. - if (str_contains($content, '{') || str_contains($content, '<') || str_contains($content, '(')) { - return; - } $appendix = ''; $spaceIndex = strpos($content, ' '); - if ($spaceIndex) { + if ($spaceIndex !== false) { $appendix = substr($content, $spaceIndex); $content = substr($content, 0, $spaceIndex); } + // Skip types whose syntax contains internal structure that the simple + // first-space split above cannot handle: generics/array-shapes (`{`, + // `<`) and callable/Closure signatures (`(int): string`). Their opening + // token sits before the first space, so checking the split-off type + // catches them while leaving a trailing description like `Foo (note)` + // alone. Without this the fixer would split mid-type and corrupt it. + if (str_contains($content, '{') || str_contains($content, '<') || str_contains($content, '(')) { + return; + } + if (!$content) { $error = 'Doc Block type for property annotation @var missing'; if ($defaultValueType) { diff --git a/tests/PhpCollective/Sniffs/Commenting/DocBlockVarSniffTest.php b/tests/PhpCollective/Sniffs/Commenting/DocBlockVarSniffTest.php index 929cb45..f348126 100644 --- a/tests/PhpCollective/Sniffs/Commenting/DocBlockVarSniffTest.php +++ b/tests/PhpCollective/Sniffs/Commenting/DocBlockVarSniffTest.php @@ -17,7 +17,7 @@ class DocBlockVarSniffTest extends TestCase */ public function testDocBlockConstSniffer(): void { - $this->assertSnifferFindsErrors(new DocBlockVarSniff(), 1); + $this->assertSnifferFindsErrors(new DocBlockVarSniff(), 2); } /** diff --git a/tests/_data/DocBlockVar/after.php b/tests/_data/DocBlockVar/after.php index 13ccc8e..8b9c10f 100644 --- a/tests/_data/DocBlockVar/after.php +++ b/tests/_data/DocBlockVar/after.php @@ -75,4 +75,9 @@ class FixMe * @var \Closure(string): string */ protected ?Closure $transformer = null; + + /** + * @var string|null (deprecated) + */ + protected ?string $note; } diff --git a/tests/_data/DocBlockVar/before.php b/tests/_data/DocBlockVar/before.php index 13ccc8e..a404432 100644 --- a/tests/_data/DocBlockVar/before.php +++ b/tests/_data/DocBlockVar/before.php @@ -75,4 +75,9 @@ class FixMe * @var \Closure(string): string */ protected ?Closure $transformer = null; + + /** + * @var string (deprecated) + */ + protected ?string $note; }