Skip to content

Commit 0b4157d

Browse files
staabmstaabm
authored
Fix @phpstan-assert not working correctly with union types (#4920)
1 parent 200a203 commit 0b4157d

File tree

8 files changed

+286
-12
lines changed

8 files changed

+286
-12
lines changed

‎CLAUDE.md‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -399,10 +399,6 @@ When adding or editing PHPDoc comments in this codebase, follow these guidelines
399399
- Use imperative voice without "Returns the..." preambles when a brief note suffices. Prefer `/** Replaces unresolved TemplateTypes with their bounds. */` over a multi-line block.
400400
- Preserve `@api` and type tags on their own lines, with no redundant description alongside them.
401401

402-
### UnionTypeMethodReflection and IntersectionTypeMethodReflection parity
403-
404-
When methods are called on union types (`Foo|Bar`), the resolved method reflection is a `UnionTypeMethodReflection` that wraps the individual method reflections. Similarly, `IntersectionTypeMethodReflection` handles intersection types. These two classes must maintain feature parity for things like `getAsserts()`, `getSelfOutType()`, etc. When one class correctly combines member data (e.g. `IntersectionTypeMethodReflection::getAsserts()` iterating over methods and calling `intersectWith()`), the other should do the same rather than returning empty/null. The `Assertions::intersectWith()` method merges assertion tag lists from multiple sources.
405-
406402
## Important dependencies
407403

408404
- `nikic/php-parser` ^5.7.0 - PHP AST parsing

‎src/Reflection/Assertions.php‎

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55
use PHPStan\PhpDoc\ResolvedPhpDocBlock;
66
use PHPStan\PhpDoc\Tag\AssertTag;
77
use PHPStan\Type\Type;
8+
use PHPStan\Type\TypeCombinator;
89
use function array_filter;
910
use function array_map;
1011
use function array_merge;
1112
use function count;
13+
use function sprintf;
1214

1315
/**
1416
* Collection of @phpstan-assert annotations on a function or method.
@@ -91,12 +93,76 @@ public function mapTypes(callable $callable): self
9193
{
9294
$assertTagsCallback = static fn (AssertTag $tag): AssertTag => $tag->withType($callable($tag->getType()));
9395

94-
return new self(array_map($assertTagsCallback, $this->asserts));
96+
return self::create(array_map($assertTagsCallback, $this->asserts));
9597
}
9698

99+
/**
100+
* @deprecated use union() or intersect() instead
101+
*/
97102
public function intersectWith(Assertions $other): self
98103
{
99-
return new self(array_merge($this->getAll(), $other->getAll()));
104+
return $this->union($other);
105+
}
106+
107+
public function union(Assertions $other): self
108+
{
109+
if ($this === self::$empty) {
110+
return $other;
111+
}
112+
if ($other === self::$empty) {
113+
return $this;
114+
}
115+
116+
return self::create(array_merge($this->getAll(), $other->getAll()));
117+
}
118+
119+
public function intersect(Assertions $other): self
120+
{
121+
if ($this === self::$empty) {
122+
return $other;
123+
}
124+
if ($other === self::$empty) {
125+
return $this;
126+
}
127+
128+
$otherAsserts = $other->getAll();
129+
$thisAsserts = $this->getAll();
130+
131+
$merged = [];
132+
foreach ($thisAsserts as $thisAssert) {
133+
$key = self::getAssertKey($thisAssert);
134+
135+
foreach ($otherAsserts as $otherAssert) {
136+
if (self::getAssertKey($otherAssert) !== $key) {
137+
continue;
138+
}
139+
140+
$merged[] = $thisAssert->withType(TypeCombinator::union($thisAssert->getType(), $otherAssert->getType()));
141+
}
142+
}
143+
144+
return self::create($merged);
145+
}
146+
147+
private static function getAssertKey(AssertTag $assert): string
148+
{
149+
return sprintf(
150+
'%s-%s-%s',
151+
$assert->getParameter()->describe(),
152+
$assert->getIf(),
153+
$assert->isNegated() ? '1' : '0',
154+
);
155+
}
156+
157+
/**
158+
* @param AssertTag[] $asserts
159+
*/
160+
private static function create(array $asserts): self
161+
{
162+
if (count($asserts) === 0) {
163+
return self::createEmpty();
164+
}
165+
return new self($asserts);
100166
}
101167

102168
public static function createEmpty(): self
@@ -116,11 +182,8 @@ public static function createEmpty(): self
116182
public static function createFromResolvedPhpDocBlock(ResolvedPhpDocBlock $phpDocBlock): self
117183
{
118184
$tags = $phpDocBlock->getAssertTags();
119-
if (count($tags) === 0) {
120-
return self::createEmpty();
121-
}
122185

123-
return new self($tags);
186+
return self::create($tags);
124187
}
125188

126189
}

‎src/Reflection/Type/IntersectionTypeMethodReflection.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ public function getAsserts(): Assertions
198198
$assertions = Assertions::createEmpty();
199199

200200
foreach ($this->methods as $method) {
201-
$assertions = $assertions->intersectWith($method->getAsserts());
201+
$assertions = $assertions->union($method->getAsserts());
202202
}
203203

204204
return $assertions;

‎src/Reflection/Type/UnionTypeMethodReflection.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ public function getAsserts(): Assertions
181181
$assertions = Assertions::createEmpty();
182182

183183
foreach ($this->methods as $method) {
184-
$assertions = $assertions->intersectWith($method->getAsserts());
184+
$assertions = $assertions->intersect($method->getAsserts());
185185
}
186186

187187
return $assertions;

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php // lint >= 8.0
2+
3+
namespace Bug14108;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
class Foo
8+
{
9+
public function __construct(private ?string $param)
10+
{
11+
}
12+
13+
public function getParam(): ?string
14+
{
15+
return $this->param;
16+
}
17+
18+
/**
19+
* @phpstan-assert string $this->getParam()
20+
*/
21+
public function narrowGetParam(): void
22+
{
23+
}
24+
}
25+
26+
class Bar
27+
{
28+
public function __construct(private ?int $param)
29+
{
30+
}
31+
32+
public function getParam(): ?int
33+
{
34+
return $this->param;
35+
}
36+
37+
/**
38+
* @phpstan-assert int $this->getParam()
39+
*/
40+
public function narrowGetParam(): void
41+
{
42+
}
43+
}
44+
45+
function test(Foo|Bar $fooOrBar): void
46+
{
47+
assertType('int|string|null', $fooOrBar->getParam());
48+
49+
$fooOrBar->narrowGetParam();
50+
51+
assertType('int|string', $fooOrBar->getParam());
52+
}

‎tests/PHPStan/Analyser/nsrt/bug-14108b.php‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php // lint >= 8.0
2+
3+
namespace Bug14108b;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
class Foo
8+
{
9+
public function __construct(private ?string $param)
10+
{
11+
}
12+
13+
public function getParam(): ?string
14+
{
15+
return $this->param;
16+
}
17+
18+
/**
19+
* @phpstan-assert !null $this->getParam()
20+
*/
21+
public function narrowGetParam(): void
22+
{
23+
}
24+
}
25+
26+
class Bar
27+
{
28+
public function __construct(private ?int $param)
29+
{
30+
}
31+
32+
public function getParam(): ?int
33+
{
34+
return $this->param;
35+
}
36+
37+
/**
38+
* @phpstan-assert int $this->getParam()
39+
*/
40+
public function narrowGetParam(): void
41+
{
42+
}
43+
}
44+
45+
function test(Foo|Bar $fooOrBar): void
46+
{
47+
assertType('int|string|null', $fooOrBar->getParam());
48+
49+
$fooOrBar->narrowGetParam();
50+
51+
assertType('int|string|null', $fooOrBar->getParam()); // could be 'int|string'
52+
}

‎tests/PHPStan/Analyser/nsrt/bug-14108c.php‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php // lint >= 8.0
2+
3+
namespace Bug14108c;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
class Foo
8+
{
9+
public function __construct(private ?string $param)
10+
{
11+
}
12+
13+
public function getParam(): ?string
14+
{
15+
return $this->param;
16+
}
17+
18+
/**
19+
* @phpstan-assert =string $this->getParam()
20+
*/
21+
public function narrowGetParam(): void
22+
{
23+
}
24+
}
25+
26+
class Bar
27+
{
28+
public function __construct(private ?int $param)
29+
{
30+
}
31+
32+
public function getParam(): ?int
33+
{
34+
return $this->param;
35+
}
36+
37+
/**
38+
* @phpstan-assert int $this->getParam()
39+
*/
40+
public function narrowGetParam(): void
41+
{
42+
}
43+
}
44+
45+
function test(Foo|Bar $fooOrBar): void
46+
{
47+
assertType('int|string|null', $fooOrBar->getParam());
48+
49+
$fooOrBar->narrowGetParam();
50+
51+
assertType('int|string', $fooOrBar->getParam());
52+
}

‎tests/PHPStan/Analyser/nsrt/bug-14108d.php‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<?php // lint >= 8.0
2+
3+
namespace Bug14108d;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
class Foo
8+
{
9+
public function __construct(private ?string $param)
10+
{
11+
}
12+
13+
public function getParam(): ?string
14+
{
15+
return $this->param;
16+
}
17+
18+
/**
19+
* @phpstan-assert-if-true string $this->getParam()
20+
*/
21+
public function narrowGetParam(): bool
22+
{
23+
}
24+
}
25+
26+
class Bar
27+
{
28+
public function __construct(private ?int $param)
29+
{
30+
}
31+
32+
public function getParam(): ?int
33+
{
34+
return $this->param;
35+
}
36+
37+
/**
38+
* @phpstan-assert int $this->getParam()
39+
*/
40+
public function narrowGetParam(): void
41+
{
42+
}
43+
}
44+
45+
function test(Foo|Bar $fooOrBar): void
46+
{
47+
assertType('int|string|null', $fooOrBar->getParam());
48+
49+
$fooOrBar->narrowGetParam();
50+
51+
assertType('int|string|null', $fooOrBar->getParam());
52+
53+
if ($fooOrBar->narrowGetParam()) {
54+
assertType('int|string|null', $fooOrBar->getParam()); // could be 'int|string'
55+
} else {
56+
assertType('int|string|null', $fooOrBar->getParam());
57+
}
58+
assertType('int|string|null', $fooOrBar->getParam());
59+
}

0 commit comments

Comments
 (0)