Skip to content

Commit 4e99486

Browse files
ondrejmirtesphpstan-bot
authored andcommitted
Fix false positive for invariant generic types when checkNullables is off
- At rule levels 3-7, RuleLevelHelper strips null from accepted type arguments - This caused Trap<int|null, A|null> to become Trap<int, A> on the accepted side - Invariant template check uses equals(), so int|null ≠ int → false positive - Added fallback in GenericObjectType::isSuperTypeOfInternal() to detect when the only difference is null stripping and accept the type in that case - New regression test in tests/PHPStan/Rules/Properties/data/bug-13876.php Closes phpstan/phpstan#13876
1 parent d32efcb commit 4e99486

File tree

3 files changed

+89
-2
lines changed

3 files changed

+89
-2
lines changed

src/Type/Generic/GenericObjectType.php

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use PHPStan\Type\IsSuperTypeOfResult;
2121
use PHPStan\Type\ObjectType;
2222
use PHPStan\Type\Type;
23+
use PHPStan\Type\TypeCombinator;
2324
use PHPStan\Type\TypeWithClassName;
2425
use PHPStan\Type\UnionType;
2526
use PHPStan\Type\VerbosityLevel;
@@ -186,7 +187,17 @@ private function isSuperTypeOfInternal(Type $type, bool $acceptsContext): IsSupe
186187
if (!$thisVariance->invariant()) {
187188
$results[] = $thisVariance->isValidVariance($templateType, $this->types[$i], $ancestor->types[$i]);
188189
} else {
189-
$results[] = $templateType->isValidVariance($this->types[$i], $ancestor->types[$i]);
190+
$result = $templateType->isValidVariance($this->types[$i], $ancestor->types[$i]);
191+
if ($acceptsContext && !$result->yes()) {
192+
$thisWithoutNull = TypeCombinator::removeNull($this->types[$i]);
193+
if (
194+
$this->types[$i]->isSuperTypeOf($ancestor->types[$i])->yes()
195+
&& $ancestor->types[$i]->isSuperTypeOf($thisWithoutNull)->yes()
196+
) {
197+
$result = IsSuperTypeOfResult::createYes();
198+
}
199+
}
200+
$results[] = $result;
190201
}
191202

192203
$results[] = IsSuperTypeOfResult::createFromBoolean($thisVariance->validPosition($ancestorVariance));

tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php

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

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

1820
private bool $checkImplicitMixed = false;
1921

2022
protected function getRule(): Rule
2123
{
22-
return new TypesAssignedToPropertiesRule(new RuleLevelHelper(self::createReflectionProvider(), true, false, true, $this->checkExplicitMixed, $this->checkImplicitMixed, false, true), new PropertyReflectionFinder());
24+
return new TypesAssignedToPropertiesRule(new RuleLevelHelper(self::createReflectionProvider(), $this->checkNullables, false, true, $this->checkExplicitMixed, $this->checkImplicitMixed, false, true), new PropertyReflectionFinder());
2325
}
2426

2527
public function testTypesAssignedToProperties(): void
@@ -1038,4 +1040,10 @@ public function testBug4525(): void
10381040
$this->analyse([__DIR__ . '/data/bug-4525.php'], []);
10391041
}
10401042

1043+
public function testBug13876(): void
1044+
{
1045+
$this->checkNullables = false;
1046+
$this->analyse([__DIR__ . '/data/bug-13876.php'], []);
1047+
}
1048+
10411049
}
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug13876;
4+
5+
/**
6+
* @template BAIT
7+
* @template PROMISED
8+
*/
9+
class Trap
10+
{
11+
12+
/**
13+
* @var BAIT
14+
*/
15+
private $bait;
16+
17+
/**
18+
* @var \Closure(BAIT):PROMISED
19+
*/
20+
private $switch;
21+
22+
/**
23+
* @param BAIT $bait
24+
* @param \Closure(BAIT):PROMISED $switch
25+
*/
26+
public function __construct($bait, \Closure $switch)
27+
{
28+
$this->bait = $bait;
29+
$this->switch = $switch;
30+
}
31+
32+
/**
33+
* @return PROMISED
34+
*/
35+
public function fall()
36+
{
37+
return ($this->switch)($this->bait);
38+
}
39+
}
40+
41+
class A {}
42+
43+
class B {
44+
45+
/**
46+
* @var Trap<int|null, A|null>
47+
*/
48+
private Trap $b;
49+
50+
public function __construct() {
51+
52+
/**
53+
* @var Trap<int|null, A|null>
54+
*/
55+
$nullPerson = new Trap(null, function (): ?A {
56+
return null;
57+
});
58+
59+
$this->b = $nullPerson;
60+
}
61+
62+
/**
63+
* @return ?A
64+
*/
65+
public function getB() {
66+
return $this->b->fall();
67+
}
68+
}

0 commit comments

Comments
 (0)