Skip to content

Commit 4eceb58

Browse files
phpstan-botondrejmirtes
authored andcommitted
Resolve an inferred template argument to its bound when a lower bound refers back to a site still being resolved
- TemplateArgumentSolver: count when substituting a marker reaches a site that is still being resolved (a cycle), and when that happens while substituting a lower bound, resolve the site to the template's bound. `T :> Foo<T>` has no finite solution other than the bound; previously the cycle was broken with the delegate, inferring e.g. `WeakAnalysingMap<WeakAnalysingMap<object>>` for `$map = new WeakAnalysingMap(); $map->set($map);` and then reporting the call. - The counter is restored after each site's resolution, so a cycle settled inside another site (indirect cycle `$a->set($b); $b->set($a);`) does not widen the outer site too. - Also covers the analogous shapes: indirect cycles across two objects, the object wrapped in a list/array argument, a cycle after another lower bound, a constructor-inferred initial type, and contravariant templates. - TemplateArgumentSolver is not shadowed by turbo (only instantiated from C++), so no native mirror change is needed.
1 parent 4ef9ab7 commit 4eceb58

4 files changed

Lines changed: 191 additions & 1 deletion

File tree

‎src/Analyser/Generics/TemplateArgumentSolver.php‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,9 @@ private function removeSelfBounds(Type $type, string $key): Type
134134
/** @var array<string, true> */
135135
private array $resolving = [];
136136

137+
/** How many times a substitution reached a site that was still being resolved. */
138+
private int $cyclesReached = 0;
139+
137140
private function resolveKey(string $key): Type
138141
{
139142
$key = $this->representative($key);
@@ -144,14 +147,18 @@ private function resolveKey(string $key): Type
144147
if (isset($this->resolving[$key])) {
145148
// a site whose inferred argument refers back to itself through another
146149
// site (wrap($x = new Foo($x))): the inferred type stands
150+
$this->cyclesReached++;
147151
return $observation['marker']->getDelegate();
148152
}
149153

150154
$this->resolving[$key] = true;
155+
$cyclesReached = $this->cyclesReached;
151156
try {
152157
return $this->resolutions[$key] = $this->resolveObservation($observation);
153158
} finally {
154159
unset($this->resolving[$key]);
160+
// a cycle closed inside another site's resolution was settled there
161+
$this->cyclesReached = $cyclesReached;
155162
}
156163
}
157164

@@ -208,7 +215,14 @@ private function resolveObservation(array $observation): Type
208215
$template = $observation['marker']->getTemplate();
209216
$lowerBounds = [];
210217
foreach ($observation['lowerBounds'] as $lowerBound) {
211-
$inferred = $template->inferTemplateTypes($this->substituteResolutions($lowerBound))->getType($template->getName());
218+
$cyclesReached = $this->cyclesReached;
219+
$lowerBound = $this->substituteResolutions($lowerBound);
220+
if ($this->cyclesReached !== $cyclesReached) {
221+
// the object is sent into its own argument ($map->set($map)): no
222+
// finite type contains Foo<itself>, only the bound does
223+
return $template->getBound();
224+
}
225+
$inferred = $template->inferTemplateTypes($lowerBound)->getType($template->getName());
212226
if ($inferred === null) {
213227
continue;
214228
}
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug15434Types;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
/**
8+
* @template TOwner of object
9+
*/
10+
class WeakAnalysingMap
11+
{
12+
/**
13+
* @param TOwner $owner
14+
*/
15+
public function set(object $owner): void {}
16+
}
17+
18+
/** @template T */
19+
class Box
20+
{
21+
22+
/** @param T $v */
23+
public function add($v): void
24+
{
25+
}
26+
27+
}
28+
29+
/** @template-contravariant T */
30+
class Sink
31+
{
32+
33+
/** @param T $v */
34+
public function put($v): void
35+
{
36+
}
37+
38+
}
39+
40+
class Test
41+
{
42+
43+
public function testSet(): void
44+
{
45+
$map = new WeakAnalysingMap();
46+
assertType('Bug15434Types\WeakAnalysingMap<object>', $map);
47+
$map->set($map);
48+
assertType('Bug15434Types\WeakAnalysingMap<object>', $map);
49+
}
50+
51+
public function indirectCycle(): void
52+
{
53+
$a = new WeakAnalysingMap();
54+
$b = new WeakAnalysingMap();
55+
$a->set($b);
56+
$b->set($a);
57+
assertType('Bug15434Types\WeakAnalysingMap<Bug15434Types\WeakAnalysingMap<object>>', $a);
58+
assertType('Bug15434Types\WeakAnalysingMap<object>', $b);
59+
}
60+
61+
public function afterAnotherLowerBound(): void
62+
{
63+
$b = new Box();
64+
$b->add(1);
65+
$b->add($b);
66+
assertType('Bug15434Types\Box<mixed>', $b);
67+
}
68+
69+
public function contravariant(): void
70+
{
71+
$s = new Sink();
72+
$s->put($s);
73+
assertType('Bug15434Types\Sink<mixed>', $s);
74+
}
75+
76+
public function notACycle(): void
77+
{
78+
$b = new Box();
79+
$c = new Box();
80+
$b->add($c);
81+
$c->add(1);
82+
assertType('Bug15434Types\Box<1>', $c);
83+
assertType('Bug15434Types\Box<Bug15434Types\Box<1>>', $b);
84+
}
85+
86+
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4616,4 +4616,12 @@ public function testDeadCatch(): void
46164616
]);
46174617
}
46184618

4619+
public function testBug15434(): void
4620+
{
4621+
$this->checkThisOnly = false;
4622+
$this->checkNullables = true;
4623+
$this->checkUnionTypes = true;
4624+
$this->analyse([__DIR__ . '/data/bug-15434.php'], []);
4625+
}
4626+
46194627
}
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug15434;
4+
5+
class Test
6+
{
7+
public function testSet(): void
8+
{
9+
$map = new WeakAnalysingMap();
10+
$map->set($map);
11+
}
12+
}
13+
14+
/**
15+
* @template TOwner of object
16+
*/
17+
class WeakAnalysingMap
18+
{
19+
/**
20+
* @param TOwner $owner
21+
*/
22+
public function set(object $owner): void {}
23+
}
24+
25+
/** @template T */
26+
class Box
27+
{
28+
29+
/** @param T $v */
30+
public function __construct($v = null)
31+
{
32+
}
33+
34+
/** @param T $v */
35+
public function add($v): void
36+
{
37+
}
38+
39+
/** @param list<T> $v */
40+
public function addAll(array $v): void
41+
{
42+
}
43+
44+
}
45+
46+
class Analogous
47+
{
48+
49+
public function indirectCycle(): void
50+
{
51+
$a = new WeakAnalysingMap();
52+
$b = new WeakAnalysingMap();
53+
$a->set($b);
54+
$b->set($a);
55+
}
56+
57+
public function afterAnotherLowerBound(): void
58+
{
59+
$b = new Box();
60+
$b->add(1);
61+
$b->add($b);
62+
}
63+
64+
public function wrappedInList(): void
65+
{
66+
$b = new Box();
67+
$b->addAll([$b]);
68+
}
69+
70+
public function wrappedInArray(): void
71+
{
72+
$b = new Box();
73+
$b->add([$b]);
74+
}
75+
76+
public function withInitialType(): void
77+
{
78+
$b = new Box(1);
79+
$b->add($b);
80+
}
81+
82+
}

0 commit comments

Comments
 (0)