Skip to content

Commit c9a3b33

Browse files
miguelmeloninnatialmiguelmeloninnatial
committed
Keep type arguments when narrowing a generic type to a subclass
A generic class written without type arguments is a subtype of every parameterization of its ancestors, so `TypeCombinator::intersect()` kept the bare subclass and dropped the arguments: `instanceof Some` narrowed an `Option<int>` to `Some`, and so did `is_a()`, `@phpstan-assert` and `$o::class` comparisons. Since `GenericObjectType::changeSubtractedType()` returns the lone remaining subtype of a sealed hierarchy (#5369), that subtype lost its arguments too: `Result<int, string>` without `Ok` became `Err`. The merge after the branch no longer hid the loss, so a wrong return type was accepted. Both now take the arguments that the generic type implies for the subclass. A subclass stays bare when the generic type does not determine all of its arguments or has call-site variance. In a union, a bare subtracted subclass is read within its type: `Option<int>~Some` together with `Some<int>` gives `Option<int>`.
1 parent 120821a commit c9a3b33

5 files changed

Lines changed: 306 additions & 3 deletions

File tree

‎src/Type/Generic/GenericObjectType.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -445,10 +445,16 @@ public function changeSubtractedType(?Type $subtractedType): Type
445445

446446
// Parent handles sealed type exhaustiveness (returning NeverType when all
447447
// allowed subtypes are subtracted, or a single remaining subtype).
448-
if (!$result instanceof ObjectType || $result->getClassName() !== $this->getClassName()) {
448+
if (!$result instanceof ObjectType) {
449449
return $result;
450450
}
451451

452+
// The remaining subtype comes back as the sealed tag names it, without
453+
// type arguments, and takes the ones this type implies for it.
454+
if ($result->getClassName() !== $this->getClassName()) {
455+
return self::specializeSubclass($this, $result);
456+
}
457+
452458
return new self($this->getClassName(), $this->types, $subtractedType, null, $this->variances);
453459
}
454460

‎src/Type/TypeCombinator.php‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use PHPStan\Type\Constant\ConstantIntegerType;
2525
use PHPStan\Type\Constant\ConstantStringType;
2626
use PHPStan\Type\Generic\GenericClassStringType;
27+
use PHPStan\Type\Generic\GenericObjectType;
2728
use PHPStan\Type\Generic\TemplateArrayType;
2829
use PHPStan\Type\Generic\TemplateBenevolentUnionType;
2930
use PHPStan\Type\Generic\TemplateMixedType;
@@ -902,6 +903,20 @@ private static function intersectWithSubtractedType(
902903
} elseif ($isBAlreadySubtracted->yes()) {
903904
$subtractedType = self::remove($a->getSubtractedType(), $b);
904905

906+
// The subtracted type counts only within $a, where a class written
907+
// without type arguments has the ones $a implies: in Option<int>,
908+
// a subtracted Some is used up by Some<int>.
909+
$subtractedWithinA = GenericObjectType::specializeSubclass(
910+
$a->getTypeWithoutSubtractedType(),
911+
$a->getSubtractedType(),
912+
);
913+
if (
914+
$subtractedWithinA !== $a->getSubtractedType()
915+
&& self::remove($subtractedWithinA, $b) instanceof NeverType
916+
) {
917+
$subtractedType = new NeverType();
918+
}
919+
905920
if (
906921
$subtractedType instanceof NeverType
907922
|| !$subtractedType->isSuperTypeOf($b)->no()
@@ -1870,6 +1885,7 @@ public static function doIntersect(Type ...$types): Type
18701885

18711886
// transform IntegerType & ConstantIntegerType to ConstantIntegerType
18721887
// transform Child & Parent to Child
1888+
// transform Child & Parent<int> to Child<int>
18731889
// transform Object & ~null to Object
18741890
// transform A & A to A
18751891
// transform int[] & string to never
@@ -1887,7 +1903,13 @@ public static function doIntersect(Type ...$types): Type
18871903
$isSuperTypeSubtractableA = $typeWithoutSubtractedTypeA->isSuperTypeOf($types[$i]);
18881904
}
18891905
if ($isSuperTypeSubtractableA->yes()) {
1890-
$types[$i] = self::unionWithSubtractedType($types[$i], $types[$j]->getSubtractedType());
1906+
// A generic class written without type arguments is a subtype of
1907+
// every parameterization of its generic ancestors, so it is kept -
1908+
// with the arguments the dropped ancestor implies for it.
1909+
$types[$i] = self::unionWithSubtractedType(
1910+
GenericObjectType::specializeSubclass($typeWithoutSubtractedTypeA, $types[$i]),
1911+
$types[$j]->getSubtractedType(),
1912+
);
18911913
array_splice($types, $j--, 1);
18921914
$typesCount--;
18931915
continue 1;
@@ -1903,7 +1925,10 @@ public static function doIntersect(Type ...$types): Type
19031925
$isSuperTypeSubtractableB = $typeWithoutSubtractedTypeB->isSuperTypeOf($types[$j]);
19041926
}
19051927
if ($isSuperTypeSubtractableB->yes()) {
1906-
$types[$j] = self::unionWithSubtractedType($types[$j], $types[$i]->getSubtractedType());
1928+
$types[$j] = self::unionWithSubtractedType(
1929+
GenericObjectType::specializeSubclass($typeWithoutSubtractedTypeB, $types[$j]),
1930+
$types[$i]->getSubtractedType(),
1931+
);
19071932
array_splice($types, $i--, 1);
19081933
$typesCount--;
19091934
continue 2;

‎tests/PHPStan/Analyser/nsrt/bug-15289.php‎

Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
<?php // lint >= 8.0
2+
3+
namespace Bug15289;
4+
5+
use LogicException;
6+
use function PHPStan\Testing\assertType;
7+
8+
/**
9+
* @template-covariant T
10+
* @phpstan-sealed Some|None
11+
*/
12+
interface Option {}
13+
14+
/**
15+
* @template-covariant T
16+
* @implements Option<T>
17+
*/
18+
final class Some implements Option {}
19+
20+
/** @implements Option<never> */
21+
final class None implements Option {}
22+
23+
/**
24+
* @template-covariant T
25+
* @template-covariant E
26+
* @phpstan-sealed Ok|Err
27+
*/
28+
interface Result {}
29+
30+
/**
31+
* @template-covariant T
32+
* @implements Result<T, never>
33+
*/
34+
final class Ok implements Result {}
35+
36+
/**
37+
* @template-covariant E
38+
* @implements Result<never, E>
39+
*/
40+
final class Err implements Result {}
41+
42+
/**
43+
* @param Option<int> $o
44+
* @return Option<string>
45+
*/
46+
function afterEmptyBranch(Option $o): Option
47+
{
48+
if ($o instanceof Some) {
49+
}
50+
assertType('Bug15289\None|Bug15289\Some<int>', $o);
51+
52+
return $o; // should be reported
53+
}
54+
55+
/**
56+
* @param Option<int> $o
57+
* @return Option<string>
58+
*/
59+
function inTheBranch(Option $o): Option
60+
{
61+
if ($o instanceof Some) {
62+
assertType('Bug15289\Some<int>', $o);
63+
64+
return $o; // should be reported
65+
}
66+
67+
return $o; // correctly not reported: $o is None here
68+
}
69+
70+
/**
71+
* @param Result<int, string> $r
72+
* @return Err<int>
73+
*/
74+
function elseBranch(Result $r): Err
75+
{
76+
if ($r instanceof Ok) {
77+
throw new LogicException();
78+
}
79+
assertType('Bug15289\Err<string>', $r);
80+
81+
return $r; // should be reported
82+
}
83+
84+
/** @param Option<int> $o */
85+
function exhaustiveMatch(Option $o): int
86+
{
87+
return match (true) {
88+
$o instanceof Some => 1,
89+
$o instanceof None => 2,
90+
};
91+
}
92+
93+
/** @template-covariant T */
94+
interface UnsealedOption {}
95+
96+
/**
97+
* @template-covariant T
98+
* @implements UnsealedOption<T>
99+
*/
100+
final class UnsealedSome implements UnsealedOption {}
101+
102+
/**
103+
* @param UnsealedOption<int> $o
104+
* @return UnsealedOption<string>
105+
*/
106+
function unsealedControl(UnsealedOption $o): UnsealedOption
107+
{
108+
if ($o instanceof UnsealedSome) {
109+
assertType('Bug15289\UnsealedSome<int>', $o);
110+
}
111+
assertType('Bug15289\UnsealedOption<int>', $o);
112+
113+
return $o; // reported
114+
}
115+
116+
/** @param Result<int, string> $r */
117+
function truthyBranchOfEachVariant(Result $r): void
118+
{
119+
if ($r instanceof Err) {
120+
assertType('Bug15289\Err<string>', $r);
121+
} else {
122+
assertType('Bug15289\Ok<int>', $r);
123+
}
124+
}
125+
126+
/**
127+
* @template T
128+
* @param Option<T> $o
129+
*/
130+
function templateArgument(Option $o): void
131+
{
132+
assertType('Bug15289\Option<T (function Bug15289\templateArgument(), argument)>', $o);
133+
if ($o instanceof Some) {
134+
assertType('Bug15289\Some<T (function Bug15289\templateArgument(), argument)>', $o);
135+
}
136+
assertType('Bug15289\None|Bug15289\Some<T (function Bug15289\templateArgument(), argument)>', $o);
137+
}
138+
139+
/** @template T */
140+
interface Invariant {}
141+
142+
/**
143+
* @template T
144+
* @implements Invariant<T>
145+
*/
146+
final class InvariantImpl implements Invariant {}
147+
148+
/** @param Invariant<covariant int> $i */
149+
function callSiteVariance(Invariant $i): void
150+
{
151+
if ($i instanceof InvariantImpl) {
152+
assertType('Bug15289\InvariantImpl', $i);
153+
}
154+
}
155+
156+
/** @phpstan-assert Some $o */
157+
function assertSome(Option $o): void
158+
{
159+
}
160+
161+
/** @param Option<int>|null $o */
162+
function otherNarrowings(?Option $o): void
163+
{
164+
if ($o instanceof Some) {
165+
assertType('Bug15289\Some<int>', $o);
166+
}
167+
168+
if ($o === null) {
169+
return;
170+
}
171+
172+
if (is_a($o, Some::class)) {
173+
assertType('Bug15289\Some<int>', $o);
174+
}
175+
176+
if ($o::class === Some::class) {
177+
assertType('Bug15289\Some<int>', $o);
178+
}
179+
180+
assertSome($o);
181+
assertType('Bug15289\Some<int>', $o);
182+
}
183+
184+
/**
185+
* @template-covariant T
186+
* @phpstan-sealed Plain|Tagged
187+
*/
188+
interface Wrapped {}
189+
190+
/**
191+
* @template-covariant T
192+
* @implements Wrapped<T>
193+
*/
194+
final class Plain implements Wrapped {}
195+
196+
/**
197+
* @template-covariant T
198+
* @template-covariant Tag
199+
* @implements Wrapped<T>
200+
*/
201+
final class Tagged implements Wrapped {}
202+
203+
/** @param Wrapped<int> $w */
204+
function undeterminedArgument(Wrapped $w): void
205+
{
206+
if ($w instanceof Tagged) {
207+
assertType('Bug15289\Tagged', $w);
208+
}
209+
}
210+
211+
/** @param Wrapped<int> $w */
212+
function undeterminedRemainder(Wrapped $w): void
213+
{
214+
if ($w instanceof Plain) {
215+
assertType('Bug15289\Plain<int>', $w);
216+
return;
217+
}
218+
219+
assertType('Bug15289\Tagged', $w);
220+
}
221+
222+
function bareSupertype(UnsealedOption $o): void
223+
{
224+
assertType('Bug15289\UnsealedOption', $o);
225+
if ($o instanceof UnsealedSome) {
226+
assertType('Bug15289\UnsealedSome', $o);
227+
}
228+
}
229+
230+
/**
231+
* @template O of Option<int>
232+
* @param O $o
233+
*/
234+
function templateRemainder(Option $o): void
235+
{
236+
if ($o instanceof None) {
237+
return;
238+
}
239+
240+
assertType('O of Bug15289\Some<int> (function Bug15289\templateRemainder(), argument)', $o);
241+
}

‎tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,12 @@ public function testBug14412(): void
515515
$this->analyse([__DIR__ . '/data/bug-14412.php'], []);
516516
}
517517

518+
#[RequiresPhp('>= 8.0.0')]
519+
public function testBug15289(): void
520+
{
521+
$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-15289.php'], []);
522+
}
523+
518524
#[RequiresPhp('>= 8.0.0')]
519525
public function testClassStringOfGenericClass(): void
520526
{

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,4 +516,29 @@ public function testBug15245(): void
516516
]);
517517
}
518518

519+
#[RequiresPhp('>= 8.0.0')]
520+
public function testBug15289(): void
521+
{
522+
$this->checkNullables = true;
523+
$this->checkExplicitMixed = false;
524+
$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-15289.php'], [
525+
[
526+
'Function Bug15289\afterEmptyBranch() should return Bug15289\Option<string> but returns Bug15289\None|Bug15289\Some<int>.',
527+
52,
528+
],
529+
[
530+
'Function Bug15289\inTheBranch() should return Bug15289\Option<string> but returns Bug15289\Some<int>.',
531+
64,
532+
],
533+
[
534+
'Function Bug15289\elseBranch() should return Bug15289\Err<int> but returns Bug15289\Err<string>.',
535+
81,
536+
],
537+
[
538+
'Function Bug15289\unsealedControl() should return Bug15289\UnsealedOption<string> but returns Bug15289\UnsealedOption<int>.',
539+
113,
540+
],
541+
]);
542+
}
543+
519544
}

0 commit comments

Comments
 (0)