Skip to content

Commit b47de29

Browse files
ondrejmirtesphpstan-bot
ondrejmirtes
authored and
phpstan-bot
committed
Do not traverse into GenericObjectType type arguments in RuleLevelHelper::transformAcceptedType()
- 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<string|null>` vs `Container<string|null>`, 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.
1 parent 03834ec commit b47de29

File tree

9 files changed

+253
-3
lines changed

9 files changed

+253
-3
lines changed

‎src/Rules/RuleLevelHelper.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use PHPStan\Type\CallableType;
1212
use PHPStan\Type\ClosureType;
1313
use PHPStan\Type\ErrorType;
14+
use PHPStan\Type\Generic\GenericObjectType;
1415
use PHPStan\Type\Generic\TemplateMixedType;
1516
use PHPStan\Type\IntersectionType;
1617
use PHPStan\Type\MixedType;
@@ -126,6 +127,10 @@ private function transformAcceptedType(Type $acceptingType, Type $acceptedType):
126127
);
127128
}
128129

130+
if ($acceptedType instanceof GenericObjectType) {
131+
return $acceptedType;
132+
}
133+
129134
if (
130135
!$this->checkNullables
131136
&& !$acceptingType instanceof NullType

‎tests/PHPStan/Rules/Functions/ClosureReturnTypeRuleTest.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,14 @@
1313
class ClosureReturnTypeRuleTest extends RuleTestCase
1414
{
1515

16+
private bool $checkNullables = true;
17+
1618
protected function getRule(): Rule
1719
{
1820
return new ClosureReturnTypeRule(new FunctionReturnTypeCheck(
1921
new RuleLevelHelper(
2022
self::createReflectionProvider(),
21-
checkNullables: true,
23+
checkNullables: $this->checkNullables,
2224
checkThisOnly: false,
2325
checkUnionTypes: true,
2426
checkExplicitMixed: false,
@@ -149,4 +151,10 @@ public function testBug13964(): void
149151
$this->analyse([__DIR__ . '/data/bug-13964.php'], []);
150152
}
151153

154+
public function testBug12490(): void
155+
{
156+
$this->checkNullables = false;
157+
$this->analyse([__DIR__ . '/data/bug-12490.php'], []);
158+
}
159+
152160
}

‎tests/PHPStan/Rules/Functions/data/bug-12490.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug12490Closure;
4+
5+
/**
6+
* @template T
7+
*/
8+
class Container
9+
{
10+
/** @var T */
11+
public $value;
12+
13+
/**
14+
* @param T $value
15+
*/
16+
public function __construct($value)
17+
{
18+
$this->value = $value;
19+
}
20+
}
21+
22+
class Foo
23+
{
24+
public function test(): void
25+
{
26+
/** @return Container<string|null> */
27+
$closure = function (?string $val): Container {
28+
return new Container($val);
29+
};
30+
31+
/** @return Container<int|null> */
32+
$closure2 = function (?int $val): Container {
33+
return new Container($val);
34+
};
35+
}
36+
}

‎tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4009,4 +4009,12 @@ public function testBug10422(): void
40094009
$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-10422.php'], []);
40104010
}
40114011

4012+
public function testBug12490(): void
4013+
{
4014+
$this->checkThisOnly = false;
4015+
$this->checkNullables = false;
4016+
$this->checkUnionTypes = true;
4017+
$this->analyse([__DIR__ . '/data/bug-12490-call.php'], []);
4018+
}
4019+
40124020
}

‎tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
class ReturnTypeRuleTest extends RuleTestCase
1717
{
1818

19+
private bool $checkNullables = true;
20+
1921
private bool $checkExplicitMixed = false;
2022

2123
private bool $checkUnionTypes = true;
@@ -27,7 +29,7 @@ protected function getRule(): Rule
2729
return new ReturnTypeRule(new FunctionReturnTypeCheck(
2830
new RuleLevelHelper(
2931
self::createReflectionProvider(),
30-
checkNullables: true,
32+
checkNullables: $this->checkNullables,
3133
checkThisOnly: false,
3234
checkUnionTypes: $this->checkUnionTypes,
3335
checkExplicitMixed: $this->checkExplicitMixed,
@@ -1331,4 +1333,10 @@ public function testBug11430(): void
13311333
$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-11430.php'], []);
13321334
}
13331335

1336+
public function testBug12490(): void
1337+
{
1338+
$this->checkNullables = false;
1339+
$this->analyse([__DIR__ . '/data/bug-12490.php'], []);
1340+
}
1341+
13341342
}

‎tests/PHPStan/Rules/Methods/data/bug-12490-call.php‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug12490Call;
4+
5+
/**
6+
* @template T
7+
*/
8+
class Container
9+
{
10+
/** @var T */
11+
public $value;
12+
13+
/**
14+
* @param T $value
15+
*/
16+
public function __construct($value)
17+
{
18+
$this->value = $value;
19+
}
20+
}
21+
22+
class Foo
23+
{
24+
/**
25+
* @param Container<string|null> $container
26+
*/
27+
public function acceptsNullableString(Container $container): void
28+
{
29+
}
30+
31+
/**
32+
* @param Container<int|null> $container
33+
*/
34+
public function acceptsNullableInt(Container $container): void
35+
{
36+
}
37+
38+
/**
39+
* @param Container<float|null> $container
40+
*/
41+
public function acceptsNullableFloat(Container $container): void
42+
{
43+
}
44+
45+
/**
46+
* @return Container<string|null>
47+
*/
48+
public function createNullableString(): Container
49+
{
50+
return new Container(null);
51+
}
52+
53+
/**
54+
* @return Container<int|null>
55+
*/
56+
public function createNullableInt(): Container
57+
{
58+
return new Container(null);
59+
}
60+
61+
/**
62+
* @return Container<float|null>
63+
*/
64+
public function createNullableFloat(): Container
65+
{
66+
return new Container(null);
67+
}
68+
69+
public function test(): void
70+
{
71+
$this->acceptsNullableString($this->createNullableString());
72+
$this->acceptsNullableInt($this->createNullableInt());
73+
$this->acceptsNullableFloat($this->createNullableFloat());
74+
}
75+
}

‎tests/PHPStan/Rules/Methods/data/bug-12490.php‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug12490;
4+
5+
/**
6+
* @template TGet
7+
* @template TSet
8+
*/
9+
class Attribute
10+
{
11+
/** @var (callable(mixed, array<string, mixed>): TGet)|null */
12+
public $get;
13+
14+
/** @var (callable(TSet, array<string, mixed>): mixed)|null*/
15+
public $set;
16+
17+
/**
18+
* @param (callable(mixed, array<string, mixed>): TGet)|null $get
19+
* @param (callable(TSet, array<string, mixed>): mixed)|null $set
20+
*/
21+
public function __construct(?callable $get = null, ?callable $set = null)
22+
{
23+
$this->get = $get;
24+
$this->set = $set;
25+
}
26+
27+
/**
28+
* @template T
29+
* @param callable(mixed, array<string, mixed>): T $get
30+
* @return Attribute<T, never>
31+
*/
32+
public static function get(callable $get): self
33+
{
34+
return new self($get);
35+
}
36+
}
37+
38+
39+
class Foo
40+
{
41+
public ?int $id = null;
42+
public ?string $surveyable_type = null;
43+
44+
/**
45+
* @return Attribute<null|string, never>
46+
*/
47+
protected function surveyedLink(): Attribute
48+
{
49+
return Attribute::get(fn () => $this->surveyable_type);
50+
}
51+
52+
/** @return Attribute<null|float, never> */
53+
protected function packageWeightCalculated(): Attribute
54+
{
55+
return Attribute::get(fn () => $this->id === null ? null : round(50 * .15, 2));
56+
}
57+
58+
/** @return Attribute<?int, never> */
59+
protected function durationMs(): Attribute
60+
{
61+
return Attribute::get(fn () => $this->id);
62+
}
63+
}

‎tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
class TypesAssignedToPropertiesRuleTest extends RuleTestCase
1414
{
1515

16+
private bool $checkNullables = true;
17+
1618
private bool $checkExplicitMixed = false;
1719

1820
private bool $checkImplicitMixed = false;
@@ -22,7 +24,7 @@ protected function getRule(): Rule
2224
return new TypesAssignedToPropertiesRule(
2325
new RuleLevelHelper(
2426
self::createReflectionProvider(),
25-
checkNullables: true,
27+
checkNullables: $this->checkNullables,
2628
checkThisOnly: false,
2729
checkUnionTypes: true,
2830
checkExplicitMixed: $this->checkExplicitMixed,
@@ -1066,4 +1068,10 @@ public function testBug10924(): void
10661068
$this->analyse([__DIR__ . '/../Methods/data/bug-10924.php'], []);
10671069
}
10681070

1071+
public function testBug12490(): void
1072+
{
1073+
$this->checkNullables = false;
1074+
$this->analyse([__DIR__ . '/data/bug-12490.php'], []);
1075+
}
1076+
10691077
}

‎tests/PHPStan/Rules/Properties/data/bug-12490.php‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug12490Property;
4+
5+
/**
6+
* @template T
7+
*/
8+
class Container
9+
{
10+
/** @var T */
11+
public $value;
12+
13+
/**
14+
* @param T $value
15+
*/
16+
public function __construct($value)
17+
{
18+
$this->value = $value;
19+
}
20+
}
21+
22+
class Foo
23+
{
24+
/** @var Container<string|null> */
25+
public Container $stringContainer;
26+
27+
/** @var Container<int|null> */
28+
public Container $intContainer;
29+
30+
/**
31+
* @param Container<string|null> $s
32+
* @param Container<int|null> $i
33+
*/
34+
public function test(Container $s, Container $i): void
35+
{
36+
$this->stringContainer = $s;
37+
$this->intContainer = $i;
38+
}
39+
}

0 commit comments

Comments
 (0)