From b47de299b79654af3618a37734db5ecb7847a78f Mon Sep 17 00:00:00 2001 From: ondrejmirtes <104888+ondrejmirtes@users.noreply.github.com> Date: Tue, 14 Apr 2026 07:40:05 +0000 Subject: [PATCH 1/2] Do not traverse into `GenericObjectType` type arguments in `RuleLevelHelper::transformAcceptedType()` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - When `checkNullables` is false (levels 0-7), `transformAcceptedType()` removes null from types via `TypeCombinator::removeNull()`. The traversal descended into `GenericObjectType`'s type arguments, removing null from them. This caused invariant template type checking (which uses `equals()`) to fail when comparing identical types like `Container` vs `Container`, because the accepted type's argument was transformed to `string` while the accepting type's argument remained `string|null`. - The fix stops traversal at `GenericObjectType` boundaries, preserving type arguments as-is. This is correct because generic type arguments represent template bindings whose identity must be preserved for invariant template comparison. - Verified the same bug affected method call parameters, property assignments, and closure return types — all use the same `RuleLevelHelper::accepts()` path and are fixed by this single change. --- src/Rules/RuleLevelHelper.php | 5 ++ .../Functions/ClosureReturnTypeRuleTest.php | 10 ++- .../Rules/Functions/data/bug-12490.php | 36 +++++++++ .../Rules/Methods/CallMethodsRuleTest.php | 8 ++ .../Rules/Methods/ReturnTypeRuleTest.php | 10 ++- .../Rules/Methods/data/bug-12490-call.php | 75 +++++++++++++++++++ .../PHPStan/Rules/Methods/data/bug-12490.php | 63 ++++++++++++++++ .../TypesAssignedToPropertiesRuleTest.php | 10 ++- .../Rules/Properties/data/bug-12490.php | 39 ++++++++++ 9 files changed, 253 insertions(+), 3 deletions(-) create mode 100644 tests/PHPStan/Rules/Functions/data/bug-12490.php create mode 100644 tests/PHPStan/Rules/Methods/data/bug-12490-call.php create mode 100644 tests/PHPStan/Rules/Methods/data/bug-12490.php create mode 100644 tests/PHPStan/Rules/Properties/data/bug-12490.php diff --git a/src/Rules/RuleLevelHelper.php b/src/Rules/RuleLevelHelper.php index 9c055399356..6712967ce40 100644 --- a/src/Rules/RuleLevelHelper.php +++ b/src/Rules/RuleLevelHelper.php @@ -11,6 +11,7 @@ use PHPStan\Type\CallableType; use PHPStan\Type\ClosureType; use PHPStan\Type\ErrorType; +use PHPStan\Type\Generic\GenericObjectType; use PHPStan\Type\Generic\TemplateMixedType; use PHPStan\Type\IntersectionType; use PHPStan\Type\MixedType; @@ -126,6 +127,10 @@ private function transformAcceptedType(Type $acceptingType, Type $acceptedType): ); } + if ($acceptedType instanceof GenericObjectType) { + return $acceptedType; + } + if ( !$this->checkNullables && !$acceptingType instanceof NullType diff --git a/tests/PHPStan/Rules/Functions/ClosureReturnTypeRuleTest.php b/tests/PHPStan/Rules/Functions/ClosureReturnTypeRuleTest.php index c363c194d36..2053954522b 100644 --- a/tests/PHPStan/Rules/Functions/ClosureReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Functions/ClosureReturnTypeRuleTest.php @@ -13,12 +13,14 @@ class ClosureReturnTypeRuleTest extends RuleTestCase { + private bool $checkNullables = true; + protected function getRule(): Rule { return new ClosureReturnTypeRule(new FunctionReturnTypeCheck( new RuleLevelHelper( self::createReflectionProvider(), - checkNullables: true, + checkNullables: $this->checkNullables, checkThisOnly: false, checkUnionTypes: true, checkExplicitMixed: false, @@ -149,4 +151,10 @@ public function testBug13964(): void $this->analyse([__DIR__ . '/data/bug-13964.php'], []); } + public function testBug12490(): void + { + $this->checkNullables = false; + $this->analyse([__DIR__ . '/data/bug-12490.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/bug-12490.php b/tests/PHPStan/Rules/Functions/data/bug-12490.php new file mode 100644 index 00000000000..8af52ee64dd --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-12490.php @@ -0,0 +1,36 @@ +value = $value; + } +} + +class Foo +{ + public function test(): void + { + /** @return Container */ + $closure = function (?string $val): Container { + return new Container($val); + }; + + /** @return Container */ + $closure2 = function (?int $val): Container { + return new Container($val); + }; + } +} diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php index 597b4154b57..28d003b4a97 100644 --- a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php @@ -4009,4 +4009,12 @@ public function testBug10422(): void $this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-10422.php'], []); } + public function testBug12490(): void + { + $this->checkThisOnly = false; + $this->checkNullables = false; + $this->checkUnionTypes = true; + $this->analyse([__DIR__ . '/data/bug-12490-call.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php index 1f3465af5d8..59513455c05 100644 --- a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php @@ -16,6 +16,8 @@ class ReturnTypeRuleTest extends RuleTestCase { + private bool $checkNullables = true; + private bool $checkExplicitMixed = false; private bool $checkUnionTypes = true; @@ -27,7 +29,7 @@ protected function getRule(): Rule return new ReturnTypeRule(new FunctionReturnTypeCheck( new RuleLevelHelper( self::createReflectionProvider(), - checkNullables: true, + checkNullables: $this->checkNullables, checkThisOnly: false, checkUnionTypes: $this->checkUnionTypes, checkExplicitMixed: $this->checkExplicitMixed, @@ -1331,4 +1333,10 @@ public function testBug11430(): void $this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-11430.php'], []); } + public function testBug12490(): void + { + $this->checkNullables = false; + $this->analyse([__DIR__ . '/data/bug-12490.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/bug-12490-call.php b/tests/PHPStan/Rules/Methods/data/bug-12490-call.php new file mode 100644 index 00000000000..f747a1e0532 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-12490-call.php @@ -0,0 +1,75 @@ +value = $value; + } +} + +class Foo +{ + /** + * @param Container $container + */ + public function acceptsNullableString(Container $container): void + { + } + + /** + * @param Container $container + */ + public function acceptsNullableInt(Container $container): void + { + } + + /** + * @param Container $container + */ + public function acceptsNullableFloat(Container $container): void + { + } + + /** + * @return Container + */ + public function createNullableString(): Container + { + return new Container(null); + } + + /** + * @return Container + */ + public function createNullableInt(): Container + { + return new Container(null); + } + + /** + * @return Container + */ + public function createNullableFloat(): Container + { + return new Container(null); + } + + public function test(): void + { + $this->acceptsNullableString($this->createNullableString()); + $this->acceptsNullableInt($this->createNullableInt()); + $this->acceptsNullableFloat($this->createNullableFloat()); + } +} diff --git a/tests/PHPStan/Rules/Methods/data/bug-12490.php b/tests/PHPStan/Rules/Methods/data/bug-12490.php new file mode 100644 index 00000000000..4bd93a76285 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-12490.php @@ -0,0 +1,63 @@ +): TGet)|null */ + public $get; + + /** @var (callable(TSet, array): mixed)|null*/ + public $set; + + /** + * @param (callable(mixed, array): TGet)|null $get + * @param (callable(TSet, array): mixed)|null $set + */ + public function __construct(?callable $get = null, ?callable $set = null) + { + $this->get = $get; + $this->set = $set; + } + + /** + * @template T + * @param callable(mixed, array): T $get + * @return Attribute + */ + public static function get(callable $get): self + { + return new self($get); + } +} + + +class Foo +{ + public ?int $id = null; + public ?string $surveyable_type = null; + + /** + * @return Attribute + */ + protected function surveyedLink(): Attribute + { + return Attribute::get(fn () => $this->surveyable_type); + } + + /** @return Attribute */ + protected function packageWeightCalculated(): Attribute + { + return Attribute::get(fn () => $this->id === null ? null : round(50 * .15, 2)); + } + + /** @return Attribute */ + protected function durationMs(): Attribute + { + return Attribute::get(fn () => $this->id); + } +} diff --git a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php index 77a9b0e3c93..88512b854f0 100644 --- a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php @@ -13,6 +13,8 @@ class TypesAssignedToPropertiesRuleTest extends RuleTestCase { + private bool $checkNullables = true; + private bool $checkExplicitMixed = false; private bool $checkImplicitMixed = false; @@ -22,7 +24,7 @@ protected function getRule(): Rule return new TypesAssignedToPropertiesRule( new RuleLevelHelper( self::createReflectionProvider(), - checkNullables: true, + checkNullables: $this->checkNullables, checkThisOnly: false, checkUnionTypes: true, checkExplicitMixed: $this->checkExplicitMixed, @@ -1066,4 +1068,10 @@ public function testBug10924(): void $this->analyse([__DIR__ . '/../Methods/data/bug-10924.php'], []); } + public function testBug12490(): void + { + $this->checkNullables = false; + $this->analyse([__DIR__ . '/data/bug-12490.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/bug-12490.php b/tests/PHPStan/Rules/Properties/data/bug-12490.php new file mode 100644 index 00000000000..aafb261f5f9 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-12490.php @@ -0,0 +1,39 @@ +value = $value; + } +} + +class Foo +{ + /** @var Container */ + public Container $stringContainer; + + /** @var Container */ + public Container $intContainer; + + /** + * @param Container $s + * @param Container $i + */ + public function test(Container $s, Container $i): void + { + $this->stringContainer = $s; + $this->intContainer = $i; + } +} From 7d6a5c4a35e2bfdcf0c412dd99ac00aebfe5e622 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Tue, 14 Apr 2026 08:19:54 +0000 Subject: [PATCH 2/2] Use traverseSimultaneously to properly handle null removal in GenericObjectType type arguments Instead of completely blocking traversal into GenericObjectType, use traverseSimultaneously to pair up accepted and accepting type arguments. Only remove null from accepted type arguments where the corresponding accepting type argument does not contain null. This ensures that: - Container accepted where Container is expected does not produce a false positive (null not removed when accepting also has null) - Container accepted where Container is expected correctly has null removed (accepted matches after transformation) Co-Authored-By: Claude Opus 4.6 --- src/Rules/RuleLevelHelper.php | 10 +++++++ .../PHPStan/Rules/Methods/data/bug-12490.php | 26 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/Rules/RuleLevelHelper.php b/src/Rules/RuleLevelHelper.php index 6712967ce40..25104ea8515 100644 --- a/src/Rules/RuleLevelHelper.php +++ b/src/Rules/RuleLevelHelper.php @@ -128,6 +128,16 @@ private function transformAcceptedType(Type $acceptingType, Type $acceptedType): } if ($acceptedType instanceof GenericObjectType) { + if (!$this->checkNullables && $acceptingType instanceof GenericObjectType) { + return $acceptedType->traverseSimultaneously($acceptingType, static function (Type $acceptedInner, Type $acceptingInner): Type { + if (TypeCombinator::containsNull($acceptingInner)) { + return $acceptedInner; + } + + return TypeCombinator::removeNull($acceptedInner); + }); + } + return $acceptedType; } diff --git a/tests/PHPStan/Rules/Methods/data/bug-12490.php b/tests/PHPStan/Rules/Methods/data/bug-12490.php index 4bd93a76285..4706e0d7e47 100644 --- a/tests/PHPStan/Rules/Methods/data/bug-12490.php +++ b/tests/PHPStan/Rules/Methods/data/bug-12490.php @@ -61,3 +61,29 @@ protected function durationMs(): Attribute return Attribute::get(fn () => $this->id); } } + +/** + * @template T + */ +class Container +{ + /** @var T */ + public $value; + + /** + * @param T $value + */ + public function __construct($value) + { + $this->value = $value; + } +} + +class Bar +{ + /** @return Container */ + public function test(?string $val): Container + { + return new Container($val); + } +}