From 9cc8bbbc9a4f247a589faec1255eec7b670f427c Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Tue, 18 Aug 2026 23:49:02 +0200 Subject: [PATCH 1/5] Compare the element type of a variadic by-ref parameter against its out type The out type of a variadic by-ref parameter describes a single argument: that is how NodeScopeResolver applies it at the call site, writing the out type - or the declared type when there is no @param-out - back to each argument individually. Inside the body the variable holds the packed array of those arguments, so comparing the packed array against the out type reported the array as the wrong type and, in the too-wide rules, claimed the parameter never gets the values it does get. The element type is the side to compare, both in the level-dependent filtering and in the comparison itself. Rebinding the packed variable to something that is no longer an array leaves nothing to compare: the references it held are discarded, so PHP writes nothing back to any caller. Closes https://github.com/phpstan/phpstan/issues/15066 Co-Authored-By: Claude Opus 5 (1M context) --- .../TooWideParameterOutTypeCheck.php | 8 ++ src/Rules/Variables/ParameterOutTypeCheck.php | 24 +++++- src/Rules/VariadicByRefParameterOutType.php | 35 ++++++++ ...ooWideFunctionParameterOutTypeRuleTest.php | 19 +++++ .../TooWideMethodParameterOutTypeRuleTest.php | 13 +++ .../Rules/TooWideTypehints/data/bug-15066.php | 65 +++++++++++++++ .../ParameterOutAssignedTypeRuleTest.php | 23 +++++ .../ParameterOutExecutionEndTypeRuleTest.php | 12 +++ .../Rules/Variables/data/bug-15066.php | 83 +++++++++++++++++++ 9 files changed, 281 insertions(+), 1 deletion(-) create mode 100644 src/Rules/VariadicByRefParameterOutType.php create mode 100644 tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php create mode 100644 tests/PHPStan/Rules/Variables/data/bug-15066.php diff --git a/src/Rules/TooWideTypehints/TooWideParameterOutTypeCheck.php b/src/Rules/TooWideTypehints/TooWideParameterOutTypeCheck.php index 4fd41dc37b1..25c1a1ff39d 100644 --- a/src/Rules/TooWideTypehints/TooWideParameterOutTypeCheck.php +++ b/src/Rules/TooWideTypehints/TooWideParameterOutTypeCheck.php @@ -9,6 +9,7 @@ use PHPStan\Node\ReturnStatement; use PHPStan\Reflection\ExtendedParameterReflection; use PHPStan\Rules\IdentifierRuleError; +use PHPStan\Rules\VariadicByRefParameterOutType; use function lcfirst; use function sprintf; @@ -94,6 +95,13 @@ private function processSingleParameter( $variableExpr = new Variable($parameter->getName()); $variableType = $scope->getType($variableExpr); + if ($parameter->isVariadic()) { + $variableType = VariadicByRefParameterOutType::elementType($variableType); + if ($variableType === null) { + return []; + } + } + return $this->tooWideTypeCheck->checkParameterOutType( $outType, $variableType, diff --git a/src/Rules/Variables/ParameterOutTypeCheck.php b/src/Rules/Variables/ParameterOutTypeCheck.php index b2ddbf64f02..7065eb14c4f 100644 --- a/src/Rules/Variables/ParameterOutTypeCheck.php +++ b/src/Rules/Variables/ParameterOutTypeCheck.php @@ -11,6 +11,7 @@ use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Rules\RuleLevelHelper; +use PHPStan\Rules\VariadicByRefParameterOutType; use PHPStan\Type\ErrorType; use PHPStan\Type\Type; use PHPStan\Type\VerbosityLevel; @@ -22,6 +23,9 @@ * The promise is either an explicit `@param-out` or, in its absence, the parameter's own type. * Which one it is only shows in the error message, so callers report it via $isParamOutType. * + * For a variadic parameter the promise describes a single argument while the variable holds the + * packed array of them, so the two sides are reconciled through VariadicByRefParameterOutType. + * * @internal */ #[AutowiredService] @@ -47,17 +51,35 @@ public function check( bool $isParamOutType, ): array { + $isVariadic = $parameter->isVariadic(); + $typeResult = $this->ruleLevelHelper->findTypeToCheck( $scope, $checkedExpr, '', - static fn (Type $type): bool => $outType->isSuperTypeOf($type)->yes(), + static function (Type $type) use ($outType, $isVariadic): bool { + if ($isVariadic) { + $type = VariadicByRefParameterOutType::elementType($type); + if ($type === null) { + return false; + } + } + + return $outType->isSuperTypeOf($type)->yes(); + }, ); if ($typeResult->getType() instanceof ErrorType) { return $typeResult->getUnknownClassErrors(); } $assignedExprType = $scope->getType($checkedExpr); + if ($isVariadic) { + $assignedExprType = VariadicByRefParameterOutType::elementType($assignedExprType); + if ($assignedExprType === null) { + return []; + } + } + if ($outType->isSuperTypeOf($assignedExprType)->yes()) { return []; } diff --git a/src/Rules/VariadicByRefParameterOutType.php b/src/Rules/VariadicByRefParameterOutType.php new file mode 100644 index 00000000000..7fb0020ee32 --- /dev/null +++ b/src/Rules/VariadicByRefParameterOutType.php @@ -0,0 +1,35 @@ +isArray()->yes()) { + return null; + } + + return $packedType->getIterableValueType(); + } + +} diff --git a/tests/PHPStan/Rules/TooWideTypehints/TooWideFunctionParameterOutTypeRuleTest.php b/tests/PHPStan/Rules/TooWideTypehints/TooWideFunctionParameterOutTypeRuleTest.php index ddc91cbc001..591d77f09ee 100644 --- a/tests/PHPStan/Rules/TooWideTypehints/TooWideFunctionParameterOutTypeRuleTest.php +++ b/tests/PHPStan/Rules/TooWideTypehints/TooWideFunctionParameterOutTypeRuleTest.php @@ -5,6 +5,7 @@ use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule as TRule; use PHPStan\Testing\RuleTestCase; +use PHPUnit\Framework\Attributes\RequiresPhp; /** * @extends RuleTestCase @@ -50,4 +51,22 @@ public function testNestedTooWideType(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testBug15066(): void + { + $this->analyse([__DIR__ . '/data/bug-15066.php'], [ + [ + 'Function Bug15066\\variadicNeverNull() never assigns null to &$refs so it can be removed from the by-ref type.', + 24, + 'You can narrow the parameter out type with @param-out PHPDoc tag.', + ], + // rebinding the packed variable to a non-array is silent, to an array is not - see the fixture + [ + 'Function Bug15066\\variadicRebindOnlyString() never assigns null to &$refs so it can be removed from the by-ref type.', + 62, + 'You can narrow the parameter out type with @param-out PHPDoc tag.', + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/TooWideTypehints/TooWideMethodParameterOutTypeRuleTest.php b/tests/PHPStan/Rules/TooWideTypehints/TooWideMethodParameterOutTypeRuleTest.php index d8dc023a57e..56824b30bc1 100644 --- a/tests/PHPStan/Rules/TooWideTypehints/TooWideMethodParameterOutTypeRuleTest.php +++ b/tests/PHPStan/Rules/TooWideTypehints/TooWideMethodParameterOutTypeRuleTest.php @@ -5,6 +5,7 @@ use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule as TRule; use PHPStan\Testing\RuleTestCase; +use PHPUnit\Framework\Attributes\RequiresPhp; /** * @extends RuleTestCase @@ -144,4 +145,16 @@ public function testNestedTooWideType(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testBug15066(): void + { + $this->analyse([__DIR__ . '/data/bug-15066.php'], [ + [ + 'Method Bug15066\\Foo::variadicNeverNull() never assigns null to &$refs so it can be removed from the by-ref type.', + 45, + 'You can narrow the parameter out type with @param-out PHPDoc tag.', + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php b/tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php new file mode 100644 index 00000000000..fecc2dbb9af --- /dev/null +++ b/tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php @@ -0,0 +1,65 @@ += 8.0 + +namespace Bug15066; + +function variadicByRef(string|null &...$refs): void +{ + foreach ($refs as &$ref) { + $ref = $ref === null ? null : trim($ref); + } +} + +function singleByRef(string|null &$ref): void +{ + $ref = $ref === null ? null : trim($ref); +} + +function variadicByIndex(string|null &...$refs): void +{ + foreach ($refs as $key => $value) { + $refs[$key] = $value === null ? null : trim($value); + } +} + +function variadicNeverNull(string|null &...$refs): void +{ + foreach ($refs as $key => $value) { + $refs[$key] = 'foo'; + } +} + +function variadicNeverWritten(string|null &...$refs): void +{ +} + +class Foo +{ + + public function variadicByIndex(string|null &...$refs): void + { + foreach ($refs as $key => $value) { + $refs[$key] = $value === null ? null : trim($value); + } + } + + public function variadicNeverNull(string|null &...$refs): void + { + foreach ($refs as $key => $value) { + $refs[$key] = 'foo'; + } + } + +} + +// Rebinding the packed variable discards the references it held, so PHP writes nothing back and +// there is no out value left to check. An array is still compared, because a write through an +// offset reaches the caller and leaves the variable as an array too. +function variadicRebindNonArray(string|null &...$refs): void +{ + $refs = 42; +} + +function variadicRebindOnlyString(string|null &...$refs): void +{ + $refs = ['ok']; +} diff --git a/tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php b/tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php index abba16bcb0e..ed7fb791ddb 100644 --- a/tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php +++ b/tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php @@ -117,4 +117,27 @@ public function testCatchVariable(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testBug15066(): void + { + $this->analyse([__DIR__ . '/data/bug-15066.php'], [ + [ + 'Parameter &$refs by-ref type of function Bug15066Variables\\variadicWrongType() expects string|null, int|string|null given.', + 22, + 'You can change the parameter out type with @param-out PHPDoc tag.', + ], + [ + 'Parameter &$refs by-ref type of method Bug15066Variables\\Foo::variadicWrongType() expects string|null, int|string|null given.', + 52, + 'You can change the parameter out type with @param-out PHPDoc tag.', + ], + // rebinding the packed variable to a non-array is silent, to an array is not - see the fixture + [ + 'Parameter &$refs by-ref type of function Bug15066Variables\\variadicRebindWrongArray() expects string|null, int given.', + 69, + 'You can change the parameter out type with @param-out PHPDoc tag.', + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/ParameterOutExecutionEndTypeRuleTest.php b/tests/PHPStan/Rules/Variables/ParameterOutExecutionEndTypeRuleTest.php index 311ffea2114..bd3a5258348 100644 --- a/tests/PHPStan/Rules/Variables/ParameterOutExecutionEndTypeRuleTest.php +++ b/tests/PHPStan/Rules/Variables/ParameterOutExecutionEndTypeRuleTest.php @@ -5,6 +5,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleLevelHelper; use PHPStan\Testing\RuleTestCase; +use PHPUnit\Framework\Attributes\RequiresPhp; /** * @extends RuleTestCase @@ -74,4 +75,15 @@ public function testBug12330(): void $this->analyse([__DIR__ . '/data/bug-12330.php'], []); } + #[RequiresPhp('>= 8.0.0')] + public function testBug15066(): void + { + $this->analyse([__DIR__ . '/data/bug-15066.php'], [ + [ + 'Parameter &$refs @param-out type of function Bug15066Variables\\variadicParamOutNeverWritten() expects string, string|null given.', + 35, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/data/bug-15066.php b/tests/PHPStan/Rules/Variables/data/bug-15066.php new file mode 100644 index 00000000000..9558061f86e --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-15066.php @@ -0,0 +1,83 @@ += 8.0 + +namespace Bug15066Variables; + +function variadicByRef(string|null &...$refs): void +{ + foreach ($refs as &$ref) { + $ref = $ref === null ? null : trim($ref); + } +} + +function variadicByIndex(string|null &...$refs): void +{ + foreach ($refs as $key => $value) { + $refs[$key] = $value === null ? null : trim($value); + } +} + +function variadicWrongType(string|null &...$refs): void +{ + foreach ($refs as $key => $value) { + $refs[$key] = 42; + } +} + +/** @param-out string|null $refs */ +function variadicParamOut(string|null &...$refs): void +{ + foreach ($refs as $key => $value) { + $refs[$key] = $value === null ? null : trim($value); + } +} + +/** @param-out string $refs */ +function variadicParamOutNeverWritten(string|null &...$refs): void +{ +} + +class Foo +{ + + public function variadicByIndex(string|null &...$refs): void + { + foreach ($refs as $key => $value) { + $refs[$key] = $value === null ? null : trim($value); + } + } + + public function variadicWrongType(string|null &...$refs): void + { + foreach ($refs as $key => $value) { + $refs[$key] = 42; + } + } + +} + +// Rebinding the packed variable discards the references it held, so PHP writes nothing back to any +// caller. Nothing is reported for a non-array, because no out value is left to check. An array is +// still reported: a write through an offset reaches the caller and leaves the variable as an array +// too, so the two cannot be told apart here, and the offset write is the case that matters. +function variadicRebindNonArray(string|null &...$refs): void +{ + $refs = 42; +} + +function variadicRebindWrongArray(string|null &...$refs): void +{ + $refs = [42]; +} + +function variadicRebindOkArray(string|null &...$refs): void +{ + $refs = ['ok']; +} + +// The packed variable may end up only maybe holding an array. There is then no element type to +// speak of, so the comparison is skipped rather than run against a nonexistent one. +function variadicRebindMaybeArray(string|null &...$refs): void +{ + $refs = rand(0, 1) === 1 ? [42] : null; +} + From 8fa6e1cd82f8e387dec50449371b5e90dfa38368 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Wed, 19 Aug 2026 14:47:38 +0200 Subject: [PATCH 2/5] Drop the variadic branch from the level-filtering callback The callback only decides which members of the observed type findTypeToCheck() keeps, and this check uses that result for nothing but its ErrorType test - which the callback cannot influence, since filtering everything out falls back to the unfiltered type. Unpacking the element type in there therefore changed nothing: output is identical at levels 3, 5, 7, 8 and 9 on nullable, union, mixed and object-typed variadic by-ref parameters, which are the shapes where the callback is consulted at all. Co-Authored-By: Claude Opus 5 (1M context) --- src/Rules/Variables/ParameterOutTypeCheck.php | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/Rules/Variables/ParameterOutTypeCheck.php b/src/Rules/Variables/ParameterOutTypeCheck.php index 7065eb14c4f..6d30d3fe196 100644 --- a/src/Rules/Variables/ParameterOutTypeCheck.php +++ b/src/Rules/Variables/ParameterOutTypeCheck.php @@ -57,16 +57,7 @@ public function check( $scope, $checkedExpr, '', - static function (Type $type) use ($outType, $isVariadic): bool { - if ($isVariadic) { - $type = VariadicByRefParameterOutType::elementType($type); - if ($type === null) { - return false; - } - } - - return $outType->isSuperTypeOf($type)->yes(); - }, + static fn (Type $type): bool => $outType->isSuperTypeOf($type)->yes(), ); if ($typeResult->getType() instanceof ErrorType) { return $typeResult->getUnknownClassErrors(); From c209234173d1d61b63ecacdc50d0df0dc5151c24 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Mon, 28 Sep 2026 11:23:34 +0200 Subject: [PATCH 3/5] Inline the variadic element type into the two parameter-out checks The helper class held three lines of logic. Both checks now unpack the element type themselves, and the reason for it moves to the docblock of ParameterOutTypeCheck. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../TooWideParameterOutTypeCheck.php | 6 ++-- src/Rules/Variables/ParameterOutTypeCheck.php | 12 ++++--- src/Rules/VariadicByRefParameterOutType.php | 35 ------------------- 3 files changed, 10 insertions(+), 43 deletions(-) delete mode 100644 src/Rules/VariadicByRefParameterOutType.php diff --git a/src/Rules/TooWideTypehints/TooWideParameterOutTypeCheck.php b/src/Rules/TooWideTypehints/TooWideParameterOutTypeCheck.php index 25c1a1ff39d..765f2477fc4 100644 --- a/src/Rules/TooWideTypehints/TooWideParameterOutTypeCheck.php +++ b/src/Rules/TooWideTypehints/TooWideParameterOutTypeCheck.php @@ -9,7 +9,6 @@ use PHPStan\Node\ReturnStatement; use PHPStan\Reflection\ExtendedParameterReflection; use PHPStan\Rules\IdentifierRuleError; -use PHPStan\Rules\VariadicByRefParameterOutType; use function lcfirst; use function sprintf; @@ -95,11 +94,12 @@ private function processSingleParameter( $variableExpr = new Variable($parameter->getName()); $variableType = $scope->getType($variableExpr); + // a variadic out type describes one argument - see ParameterOutTypeCheck if ($parameter->isVariadic()) { - $variableType = VariadicByRefParameterOutType::elementType($variableType); - if ($variableType === null) { + if (!$variableType->isArray()->yes()) { return []; } + $variableType = $variableType->getIterableValueType(); } return $this->tooWideTypeCheck->checkParameterOutType( diff --git a/src/Rules/Variables/ParameterOutTypeCheck.php b/src/Rules/Variables/ParameterOutTypeCheck.php index 6d30d3fe196..ebddd7bc691 100644 --- a/src/Rules/Variables/ParameterOutTypeCheck.php +++ b/src/Rules/Variables/ParameterOutTypeCheck.php @@ -11,7 +11,6 @@ use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Rules\RuleLevelHelper; -use PHPStan\Rules\VariadicByRefParameterOutType; use PHPStan\Type\ErrorType; use PHPStan\Type\Type; use PHPStan\Type\VerbosityLevel; @@ -23,8 +22,11 @@ * The promise is either an explicit `@param-out` or, in its absence, the parameter's own type. * Which one it is only shows in the error message, so callers report it via $isParamOutType. * - * For a variadic parameter the promise describes a single argument while the variable holds the - * packed array of them, so the two sides are reconciled through VariadicByRefParameterOutType. + * For a variadic parameter the promise describes a single argument, as NodeScopeResolver applies it at + * the call site, while the variable holds the packed array of them, so its element type is compared. + * Once the variable no longer holds an array, rebinding it has discarded the references and nothing + * reaches a caller. A write through an offset, `$refs[0] = ...`, does reach the caller and leaves an + * array, so an array is always compared. * * @internal */ @@ -65,10 +67,10 @@ public function check( $assignedExprType = $scope->getType($checkedExpr); if ($isVariadic) { - $assignedExprType = VariadicByRefParameterOutType::elementType($assignedExprType); - if ($assignedExprType === null) { + if (!$assignedExprType->isArray()->yes()) { return []; } + $assignedExprType = $assignedExprType->getIterableValueType(); } if ($outType->isSuperTypeOf($assignedExprType)->yes()) { diff --git a/src/Rules/VariadicByRefParameterOutType.php b/src/Rules/VariadicByRefParameterOutType.php deleted file mode 100644 index 7fb0020ee32..00000000000 --- a/src/Rules/VariadicByRefParameterOutType.php +++ /dev/null @@ -1,35 +0,0 @@ -isArray()->yes()) { - return null; - } - - return $packedType->getIterableValueType(); - } - -} From a6cf832fa269aeb7c492d67eb2e2f7d5cd83214e Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Wed, 30 Sep 2026 22:22:24 +0200 Subject: [PATCH 4/5] Cover the non-array rebind of a variadic by-ref parameter with an iterable Rebinding the packed variable to 42 was already silent without the guard, because an int has no element type. An ArrayIterator has one, so without the guard the too-wide check reported it. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php b/tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php index fecc2dbb9af..48717b131db 100644 --- a/tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php +++ b/tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php @@ -63,3 +63,10 @@ function variadicRebindOnlyString(string|null &...$refs): void { $refs = ['ok']; } + +// An iterable that is not an array still has an element type, but rebinding to it discards the +// references just the same, so it stays silent too. +function variadicRebindTraversable(string|null &...$refs): void +{ + $refs = new \ArrayIterator(['ok']); +} From 84adb60f2017eb1f73f8eb115ca923226521e24f Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Thu, 1 Oct 2026 07:06:25 +0200 Subject: [PATCH 5/5] Cover a maybe-array rebind of a variadic by-ref parameter Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php b/tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php index 48717b131db..d62db2c76ac 100644 --- a/tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php +++ b/tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php @@ -70,3 +70,9 @@ function variadicRebindTraversable(string|null &...$refs): void { $refs = new \ArrayIterator(['ok']); } + +// Maybe an array is not compared either, even when the other types also have an element type. +function variadicRebindMaybeArray(string|null &...$refs): void +{ + $refs = rand(0, 1) === 1 ? ['ok'] : new \ArrayIterator(['ok']); +}