From 5f6d57c8055bb52286f8360b96f30f0d0804b943 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Tue, 18 Aug 2026 23:49:02 +0200 Subject: [PATCH 1/2] 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 6766421089237cb194d7429f011ff8abb31fd072 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Wed, 19 Aug 2026 14:47:38 +0200 Subject: [PATCH 2/2] 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();