Skip to content

Commit 5f688c7

Browse files
phpstan-botondrejmirtes
phpstan-bot
authored and
ondrejmirtes
committed
Generalize a template argument resolution when writes widen the object beyond its initial type
- TemplateArgumentSolver::resolveObservation(): when the lower bounds put into an object (`$box->set(...)`) are not covered by the type it was created with, the union of the initial type and the lower bounds is generalized (GeneralizePrecision::lessSpecific()). The observation pass reads the object as its initial type, so a value derived from a read and written back (`$box->set($box->get() + 1)`) is one step of a loop: the second pass read `0|1` and wrote `1|2`, which `Box<0|1>` rejected. - Covers `new`, static generic factories and generic functions creating the object, in straight-line code, loops, closures and arrow functions, and int/string/array values. - Writes that stay within the initial type keep the precise resolution. - Updated two expectations that pinned the literal union after an unrelated write widened a known initial type (now generalized). - Probed: writes to template-typed properties are not observed as lower bounds at all (a separate gap in AssignHandler, unchanged here).
1 parent 4ef9ab7 commit 5f688c7

6 files changed

Lines changed: 219 additions & 3 deletions

File tree

‎src/Analyser/Generics/TemplateArgumentSolver.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use PhpParser\Node\Expr;
66
use PHPStan\Analyser\MutatingScope;
77
use PHPStan\Turbo\ReferencedByTurboExtension;
8+
use PHPStan\Type\GeneralizePrecision;
89
use PHPStan\Type\Generic\TemplateTypeVariance;
910
use PHPStan\Type\Generic\UnresolvedTemplateArgumentType;
1011
use PHPStan\Type\MixedType;
@@ -297,7 +298,16 @@ private function resolveObservation(array $observation): Type
297298
if (TemplateArgumentStats::$enabled) {
298299
TemplateArgumentStats::increment(count($lowerBounds) > 0 ? 'resolvedWithLowerBounds' : 'resolvedToInitial');
299300
}
300-
return TypeCombinator::union(...$parts);
301+
$resolved = TypeCombinator::union(...$parts);
302+
if ($initial !== null && !$initial instanceof NeverType && !$initial->isSuperTypeOf($resolved)->yes()) {
303+
// the body puts values into the object it was not created with: what
304+
// the observation pass read out of it was the initial type alone, so
305+
// a value derived from such a read and put back ($box->set($box->get() + 1))
306+
// is one step of a loop - generalized like a loop's variables
307+
return $resolved->generalize(GeneralizePrecision::lessSpecific());
308+
}
309+
310+
return $resolved;
301311
}
302312

303313
/**

‎tests/PHPStan/Analyser/Generics/TemplateArgumentResolverTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ public function testLowerBoundsUnionWithTheInitialUnlessASendWins(): void
128128
$constraints = $constraints->merge((new TemplateArgumentObserver())->collectArgument(new CallableType([], $marker, false), new CallableType([], new StringType(), false)));
129129
$frame = (new TemplateArgumentResolver())->resolve($constraints, null, []);
130130

131-
$this->assertSame("1|2|'a'|null", self::describe($frame->resolve($site, 'T')), 'callable parameters are contravariant and contribute nothing');
131+
$this->assertSame('int|string|null', self::describe($frame->resolve($site, 'T')), 'callable parameters are contravariant and contribute nothing');
132132

133133
[$constraints, $site, $ofMarker] = self::constraintsWithA(new ConstantIntegerType(1));
134134
$constraints = $constraints->merge((new TemplateArgumentObserver())->collectArgument($ofMarker->getTypes()[0], new ConstantStringType('a')));

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

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
<?php // lint >= 8.0
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug15430;
6+
7+
use function PHPStan\Testing\assertType;
8+
9+
/** @template T */
10+
final class Box
11+
{
12+
/** @param T $value */
13+
public function __construct(private mixed $value)
14+
{
15+
}
16+
17+
/** @return T */
18+
public function get(): mixed
19+
{
20+
return $this->value;
21+
}
22+
23+
/** @param T $value */
24+
public function set(mixed $value): void
25+
{
26+
$this->value = $value;
27+
}
28+
29+
/**
30+
* @template U
31+
* @param U $value
32+
* @return self<U>
33+
*/
34+
public static function of(mixed $value): self
35+
{
36+
return new self($value);
37+
}
38+
}
39+
40+
/** @param callable(): void $callback */
41+
function repeat(int $times, callable $callback): void
42+
{
43+
for ($i = 0; $i < $times; $i++) {
44+
$callback();
45+
}
46+
}
47+
48+
/**
49+
* @template U
50+
* @param U $value
51+
* @return Box<U>
52+
*/
53+
function wrap(mixed $value): Box
54+
{
55+
return new Box($value);
56+
}
57+
58+
function test(): void
59+
{
60+
$count = new Box(0);
61+
62+
repeat(5, function () use ($count): void {
63+
$count->set($count->get() + 1);
64+
});
65+
66+
assertType('Bug15430\Box<int>', $count);
67+
68+
if ($count->get() === 5) {
69+
echo 'reached at runtime';
70+
}
71+
}
72+
73+
function straight(): void
74+
{
75+
$count = new Box(0);
76+
$count->set($count->get() + 1);
77+
assertType('Bug15430\Box<int>', $count);
78+
}
79+
80+
function loop(): void
81+
{
82+
$count = new Box(0);
83+
for ($i = 0; $i < 5; $i++) {
84+
$count->set($count->get() + 1);
85+
}
86+
assertType('Bug15430\Box<int>', $count);
87+
}
88+
89+
function arrowFunction(): void
90+
{
91+
$count = new Box(0);
92+
repeat(5, fn () => $count->set($count->get() + 1));
93+
assertType('Bug15430\Box<int>', $count);
94+
}
95+
96+
function staticFactory(): void
97+
{
98+
$count = Box::of(0);
99+
repeat(5, function () use ($count): void {
100+
$count->set($count->get() + 1);
101+
});
102+
assertType('Bug15430\Box<int>', $count);
103+
}
104+
105+
function genericFunction(): void
106+
{
107+
$count = wrap(0);
108+
repeat(5, function () use ($count): void {
109+
$count->set($count->get() + 1);
110+
});
111+
assertType('Bug15430\Box<int>', $count);
112+
}
113+
114+
function concat(): void
115+
{
116+
$s = new Box('a');
117+
repeat(5, function () use ($s): void {
118+
$s->set($s->get() . 'b');
119+
});
120+
assertType('Bug15430\Box<string>', $s);
121+
}
122+
123+
function append(): void
124+
{
125+
$s = new Box([]);
126+
repeat(5, function () use ($s): void {
127+
$a = $s->get();
128+
$a[] = 1;
129+
$s->set($a);
130+
});
131+
assertType('Bug15430\Box<list<int>>', $s);
132+
}
133+
134+
function shape(): void
135+
{
136+
$s = new Box(['a' => 1]);
137+
repeat(5, function () use ($s): void {
138+
$s->set(['a' => $s->get()['a'] + 1]);
139+
});
140+
assertType('Bug15430\Box<non-empty-array<string, int>>', $s);
141+
}
142+
143+
function writtenBackUnchanged(): void
144+
{
145+
$b = new Box(0);
146+
$b->set($b->get());
147+
assertType('Bug15430\Box<0>', $b);
148+
}

‎tests/PHPStan/Analyser/nsrt/template-argument-multiple-sites.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ function covariantIntoInvariantInferred(): void
205205
$z = collectBoxes($a, $b);
206206
$z->add(3);
207207
assertType('TemplateArgumentMultipleSites\Box<1>', $a);
208-
assertType('TemplateArgumentMultipleSites\Coll<1|2|3>', $z);
208+
assertType('TemplateArgumentMultipleSites\Coll<int>', $z);
209209
}
210210

211211
function invariantIntoCovariant(): void

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4560,6 +4560,15 @@ public function testBug15251(): void
45604560
$this->analyse([__DIR__ . '/data/bug-15251.php'], []);
45614561
}
45624562

4563+
#[RequiresPhp('>= 8.0.0')]
4564+
public function testBug15430(): void
4565+
{
4566+
$this->checkThisOnly = false;
4567+
$this->checkNullables = true;
4568+
$this->checkUnionTypes = true;
4569+
$this->analyse([__DIR__ . '/data/bug-15430.php'], []);
4570+
}
4571+
45634572
#[RequiresPhp('>= 8.0.0')]
45644573
public function testBug12820(): void
45654574
{

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php // lint >= 8.0
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug15430Rule;
6+
7+
8+
/** @template T */
9+
final class Box
10+
{
11+
/** @param T $value */
12+
public function __construct(private mixed $value)
13+
{
14+
}
15+
16+
/** @return T */
17+
public function get(): mixed
18+
{
19+
return $this->value;
20+
}
21+
22+
/** @param T $value */
23+
public function set(mixed $value): void
24+
{
25+
$this->value = $value;
26+
}
27+
}
28+
29+
/** @param callable(): void $callback */
30+
function repeat(int $times, callable $callback): void
31+
{
32+
for ($i = 0; $i < $times; $i++) {
33+
$callback();
34+
}
35+
}
36+
37+
function test(): void
38+
{
39+
$count = new Box(0);
40+
41+
repeat(5, function () use ($count): void {
42+
$count->set($count->get() + 1);
43+
});
44+
45+
46+
if ($count->get() === 5) {
47+
echo 'reached at runtime';
48+
}
49+
}

0 commit comments

Comments
 (0)