From c1e273a4d0bbb9aa77d41c6768fe9ef5d9890b40 Mon Sep 17 00:00:00 2001 From: staabm <120441+staabm@users.noreply.github.com> Date: Fri, 13 Feb 2026 09:08:28 +0000 Subject: [PATCH 1/8] Fix @phpstan-assert not working correctly with union types - UnionTypeMethodReflection::getAsserts() was using intersectWith() which concatenates assert tags, causing specifyTypesFromAsserts to intersect the asserted types (string & int = never) instead of unioning them - Added Assertions::unionWith() that groups assertions by parameter identity and unions their types via TypeCombinator::union() - Changed UnionTypeMethodReflection to use unionWith() instead of intersectWith() - Assertions without a matching counterpart in the other union member are dropped - New regression test in tests/PHPStan/Analyser/nsrt/bug-14108.php --- CLAUDE.md | 6 +- src/Reflection/Assertions.php | 62 +++++++++++++++++++ .../Type/UnionTypeMethodReflection.php | 2 +- tests/PHPStan/Analyser/nsrt/bug-14108.php | 52 ++++++++++++++++ 4 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-14108.php diff --git a/CLAUDE.md b/CLAUDE.md index f93091704c..945baf87d0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -401,7 +401,11 @@ When adding or editing PHPDoc comments in this codebase, follow these guidelines ### UnionTypeMethodReflection and IntersectionTypeMethodReflection parity -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. +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. + +**Critical distinction for `getAsserts()`**: Intersection types use `Assertions::intersectWith()` which concatenates all assert tags — this is correct because both assertions must hold simultaneously, so `specifyTypesFromAsserts` intersects the sureTypes (e.g., `list & non-empty-array = non-empty-list`). Union types must use `Assertions::unionWith()` which groups assertions by the same parameter/condition and **unions their types** — this is correct because only one union member's assertion applies at runtime (e.g., `string | int` not `string & int = never`). Assertions that don't have a matching counterpart in the other union member are dropped since they cannot be guaranteed. + +The `Assertions::intersectWith()` method merges assertion tag lists by concatenation. The `Assertions::unionWith()` method groups assertions by parameter identity (parameter name + if-condition + negated + equality) and unions the asserted types via `TypeCombinator::union()`. ## Important dependencies diff --git a/src/Reflection/Assertions.php b/src/Reflection/Assertions.php index 30f9106b77..3faa96f689 100644 --- a/src/Reflection/Assertions.php +++ b/src/Reflection/Assertions.php @@ -5,10 +5,12 @@ use PHPStan\PhpDoc\ResolvedPhpDocBlock; use PHPStan\PhpDoc\Tag\AssertTag; use PHPStan\Type\Type; +use PHPStan\Type\TypeCombinator; use function array_filter; use function array_map; use function array_merge; use function count; +use function sprintf; /** * Collection of @phpstan-assert annotations on a function or method. @@ -99,6 +101,66 @@ public function intersectWith(Assertions $other): self return new self(array_merge($this->getAll(), $other->getAll())); } + /** + * Combines assertions from union type members by unioning the asserted types + * for assertions that target the same parameter with the same condition. + */ + public function unionWith(Assertions $other): self + { + $otherAsserts = $other->getAll(); + if (count($otherAsserts) === 0) { + return $this; + } + + $thisAsserts = $this->getAll(); + if (count($thisAsserts) === 0) { + return $other; + } + + $merged = []; + $usedOther = []; + + foreach ($thisAsserts as $thisAssert) { + $key = self::getAssertKey($thisAssert); + $found = false; + + foreach ($otherAsserts as $j => $otherAssert) { + if (isset($usedOther[$j])) { + continue; + } + + if (self::getAssertKey($otherAssert) !== $key) { + continue; + } + + $merged[] = $thisAssert->withType(TypeCombinator::union($thisAssert->getType(), $otherAssert->getType())); + $usedOther[$j] = true; + $found = true; + break; + } + + if ($found) { + continue; + } + + // No matching assertion in other — this assertion cannot be guaranteed + // for the union type, so we drop it + } + + return new self($merged); + } + + private static function getAssertKey(AssertTag $assert): string + { + return sprintf( + '%s-%s-%s-%s', + $assert->getParameter()->describe(), + $assert->getIf(), + $assert->isNegated() ? '1' : '0', + $assert->isEquality() ? '1' : '0', + ); + } + public static function createEmpty(): self { $empty = self::$empty; diff --git a/src/Reflection/Type/UnionTypeMethodReflection.php b/src/Reflection/Type/UnionTypeMethodReflection.php index 135fcfc39d..b381db91cd 100644 --- a/src/Reflection/Type/UnionTypeMethodReflection.php +++ b/src/Reflection/Type/UnionTypeMethodReflection.php @@ -181,7 +181,7 @@ public function getAsserts(): Assertions $assertions = Assertions::createEmpty(); foreach ($this->methods as $method) { - $assertions = $assertions->intersectWith($method->getAsserts()); + $assertions = $assertions->unionWith($method->getAsserts()); } return $assertions; diff --git a/tests/PHPStan/Analyser/nsrt/bug-14108.php b/tests/PHPStan/Analyser/nsrt/bug-14108.php new file mode 100644 index 0000000000..af7667bf16 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14108.php @@ -0,0 +1,52 @@ += 8.0 + +namespace Bug14108; + +use function PHPStan\Testing\assertType; + +class Foo +{ + public function __construct(private ?string $param) + { + } + + public function getParam(): ?string + { + return $this->param; + } + + /** + * @phpstan-assert string $this->getParam() + */ + public function narrowGetParam(): void + { + } +} + +class Bar +{ + public function __construct(private ?int $param) + { + } + + public function getParam(): ?int + { + return $this->param; + } + + /** + * @phpstan-assert int $this->getParam() + */ + public function narrowGetParam(): void + { + } +} + +function test(Foo|Bar $fooOrBar): void +{ + assertType('int|string|null', $fooOrBar->getParam()); + + $fooOrBar->narrowGetParam(); + + assertType('int|string', $fooOrBar->getParam()); +} From c0301ad7380d03754ee9973bfdb439db07d9fbd4 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 Feb 2026 11:21:19 +0100 Subject: [PATCH 2/8] fix names --- src/Reflection/Assertions.php | 10 +++++++++- .../Type/IntersectionTypeMethodReflection.php | 2 +- src/Reflection/Type/UnionTypeMethodReflection.php | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Reflection/Assertions.php b/src/Reflection/Assertions.php index 3faa96f689..73856d4c2d 100644 --- a/src/Reflection/Assertions.php +++ b/src/Reflection/Assertions.php @@ -96,7 +96,15 @@ public function mapTypes(callable $callable): self return new self(array_map($assertTagsCallback, $this->asserts)); } + /** + * @deprecated use union() or intersect() instead + */ public function intersectWith(Assertions $other): self + { + return $this->union($other); + } + + public function union(Assertions $other): self { return new self(array_merge($this->getAll(), $other->getAll())); } @@ -105,7 +113,7 @@ public function intersectWith(Assertions $other): self * Combines assertions from union type members by unioning the asserted types * for assertions that target the same parameter with the same condition. */ - public function unionWith(Assertions $other): self + public function intersect(Assertions $other): self { $otherAsserts = $other->getAll(); if (count($otherAsserts) === 0) { diff --git a/src/Reflection/Type/IntersectionTypeMethodReflection.php b/src/Reflection/Type/IntersectionTypeMethodReflection.php index b1b52f3d33..b31124c17a 100644 --- a/src/Reflection/Type/IntersectionTypeMethodReflection.php +++ b/src/Reflection/Type/IntersectionTypeMethodReflection.php @@ -198,7 +198,7 @@ public function getAsserts(): Assertions $assertions = Assertions::createEmpty(); foreach ($this->methods as $method) { - $assertions = $assertions->intersectWith($method->getAsserts()); + $assertions = $assertions->union($method->getAsserts()); } return $assertions; diff --git a/src/Reflection/Type/UnionTypeMethodReflection.php b/src/Reflection/Type/UnionTypeMethodReflection.php index b381db91cd..89557b4e2c 100644 --- a/src/Reflection/Type/UnionTypeMethodReflection.php +++ b/src/Reflection/Type/UnionTypeMethodReflection.php @@ -181,7 +181,7 @@ public function getAsserts(): Assertions $assertions = Assertions::createEmpty(); foreach ($this->methods as $method) { - $assertions = $assertions->unionWith($method->getAsserts()); + $assertions = $assertions->intersect($method->getAsserts()); } return $assertions; From 2ef6497d1084f224442544166157eb3a98da1c68 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 Feb 2026 11:30:22 +0100 Subject: [PATCH 3/8] simplify --- src/Reflection/Assertions.php | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/Reflection/Assertions.php b/src/Reflection/Assertions.php index 73856d4c2d..3da91d1381 100644 --- a/src/Reflection/Assertions.php +++ b/src/Reflection/Assertions.php @@ -109,10 +109,6 @@ public function union(Assertions $other): self return new self(array_merge($this->getAll(), $other->getAll())); } - /** - * Combines assertions from union type members by unioning the asserted types - * for assertions that target the same parameter with the same condition. - */ public function intersect(Assertions $other): self { $otherAsserts = $other->getAll(); @@ -130,7 +126,6 @@ public function intersect(Assertions $other): self foreach ($thisAsserts as $thisAssert) { $key = self::getAssertKey($thisAssert); - $found = false; foreach ($otherAsserts as $j => $otherAssert) { if (isset($usedOther[$j])) { @@ -143,16 +138,8 @@ public function intersect(Assertions $other): self $merged[] = $thisAssert->withType(TypeCombinator::union($thisAssert->getType(), $otherAssert->getType())); $usedOther[$j] = true; - $found = true; break; } - - if ($found) { - continue; - } - - // No matching assertion in other — this assertion cannot be guaranteed - // for the union type, so we drop it } return new self($merged); From a87fd76c643792790626772c5d11942a415115c3 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 Feb 2026 11:50:57 +0100 Subject: [PATCH 4/8] add more tests --- src/Reflection/Assertions.php | 3 +- tests/PHPStan/Analyser/nsrt/bug-14108b.php | 52 +++++++++++++++++++ tests/PHPStan/Analyser/nsrt/bug-14108c.php | 52 +++++++++++++++++++ tests/PHPStan/Analyser/nsrt/bug-14108d.php | 59 ++++++++++++++++++++++ 4 files changed, 164 insertions(+), 2 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-14108b.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-14108c.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-14108d.php diff --git a/src/Reflection/Assertions.php b/src/Reflection/Assertions.php index 3da91d1381..bf69355f96 100644 --- a/src/Reflection/Assertions.php +++ b/src/Reflection/Assertions.php @@ -148,11 +148,10 @@ public function intersect(Assertions $other): self private static function getAssertKey(AssertTag $assert): string { return sprintf( - '%s-%s-%s-%s', + '%s-%s-%s', $assert->getParameter()->describe(), $assert->getIf(), $assert->isNegated() ? '1' : '0', - $assert->isEquality() ? '1' : '0', ); } diff --git a/tests/PHPStan/Analyser/nsrt/bug-14108b.php b/tests/PHPStan/Analyser/nsrt/bug-14108b.php new file mode 100644 index 0000000000..6c4f87afb4 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14108b.php @@ -0,0 +1,52 @@ += 8.0 + +namespace Bug14108b; + +use function PHPStan\Testing\assertType; + +class Foo +{ + public function __construct(private ?string $param) + { + } + + public function getParam(): ?string + { + return $this->param; + } + + /** + * @phpstan-assert !null $this->getParam() + */ + public function narrowGetParam(): void + { + } +} + +class Bar +{ + public function __construct(private ?int $param) + { + } + + public function getParam(): ?int + { + return $this->param; + } + + /** + * @phpstan-assert int $this->getParam() + */ + public function narrowGetParam(): void + { + } +} + +function test(Foo|Bar $fooOrBar): void +{ + assertType('int|string|null', $fooOrBar->getParam()); + + $fooOrBar->narrowGetParam(); + + assertType('int|string|null', $fooOrBar->getParam()); +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-14108c.php b/tests/PHPStan/Analyser/nsrt/bug-14108c.php new file mode 100644 index 0000000000..f95876d99b --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14108c.php @@ -0,0 +1,52 @@ += 8.0 + +namespace Bug14108c; + +use function PHPStan\Testing\assertType; + +class Foo +{ + public function __construct(private ?string $param) + { + } + + public function getParam(): ?string + { + return $this->param; + } + + /** + * @phpstan-assert =string $this->getParam() + */ + public function narrowGetParam(): void + { + } +} + +class Bar +{ + public function __construct(private ?int $param) + { + } + + public function getParam(): ?int + { + return $this->param; + } + + /** + * @phpstan-assert int $this->getParam() + */ + public function narrowGetParam(): void + { + } +} + +function test(Foo|Bar $fooOrBar): void +{ + assertType('int|string|null', $fooOrBar->getParam()); + + $fooOrBar->narrowGetParam(); + + assertType('int|string', $fooOrBar->getParam()); +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-14108d.php b/tests/PHPStan/Analyser/nsrt/bug-14108d.php new file mode 100644 index 0000000000..7a04985253 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14108d.php @@ -0,0 +1,59 @@ += 8.0 + +namespace Bug14108d; + +use function PHPStan\Testing\assertType; + +class Foo +{ + public function __construct(private ?string $param) + { + } + + public function getParam(): ?string + { + return $this->param; + } + + /** + * @phpstan-assert-if-true string $this->getParam() + */ + public function narrowGetParam(): bool + { + } +} + +class Bar +{ + public function __construct(private ?int $param) + { + } + + public function getParam(): ?int + { + return $this->param; + } + + /** + * @phpstan-assert int $this->getParam() + */ + public function narrowGetParam(): void + { + } +} + +function test(Foo|Bar $fooOrBar): void +{ + assertType('int|string|null', $fooOrBar->getParam()); + + $fooOrBar->narrowGetParam(); + + assertType('int|string|null', $fooOrBar->getParam()); + + if ($fooOrBar->narrowGetParam()) { + assertType('int|string|null', $fooOrBar->getParam()); // could be 'int|string' + } else { + assertType('int|string|null', $fooOrBar->getParam()); + } + assertType('int|string|null', $fooOrBar->getParam()); +} From 646922f1f327d9086556383216a64e2ccea7c10b Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 Feb 2026 11:52:08 +0100 Subject: [PATCH 5/8] Update CLAUDE.md --- CLAUDE.md | 8 -------- 1 file changed, 8 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 945baf87d0..a5849a4edc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -399,14 +399,6 @@ When adding or editing PHPDoc comments in this codebase, follow these guidelines - Use imperative voice without "Returns the..." preambles when a brief note suffices. Prefer `/** Replaces unresolved TemplateTypes with their bounds. */` over a multi-line block. - Preserve `@api` and type tags on their own lines, with no redundant description alongside them. -### UnionTypeMethodReflection and IntersectionTypeMethodReflection parity - -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. - -**Critical distinction for `getAsserts()`**: Intersection types use `Assertions::intersectWith()` which concatenates all assert tags — this is correct because both assertions must hold simultaneously, so `specifyTypesFromAsserts` intersects the sureTypes (e.g., `list & non-empty-array = non-empty-list`). Union types must use `Assertions::unionWith()` which groups assertions by the same parameter/condition and **unions their types** — this is correct because only one union member's assertion applies at runtime (e.g., `string | int` not `string & int = never`). Assertions that don't have a matching counterpart in the other union member are dropped since they cannot be guaranteed. - -The `Assertions::intersectWith()` method merges assertion tag lists by concatenation. The `Assertions::unionWith()` method groups assertions by parameter identity (parameter name + if-condition + negated + equality) and unions the asserted types via `TypeCombinator::union()`. - ## Important dependencies - `nikic/php-parser` ^5.7.0 - PHP AST parsing From bde29eeab0ddb46c230eeda9eee3e3e4e6626701 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 Feb 2026 11:54:29 +0100 Subject: [PATCH 6/8] Update bug-14108b.php --- tests/PHPStan/Analyser/nsrt/bug-14108b.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPStan/Analyser/nsrt/bug-14108b.php b/tests/PHPStan/Analyser/nsrt/bug-14108b.php index 6c4f87afb4..738fcdc5cb 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-14108b.php +++ b/tests/PHPStan/Analyser/nsrt/bug-14108b.php @@ -48,5 +48,5 @@ function test(Foo|Bar $fooOrBar): void $fooOrBar->narrowGetParam(); - assertType('int|string|null', $fooOrBar->getParam()); + assertType('int|string|null', $fooOrBar->getParam()); // could be 'int|string' } From 0357aa9429d93dd612367ecb32f40a0a4694901e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 Feb 2026 12:28:13 +0100 Subject: [PATCH 7/8] simplify --- src/Reflection/Assertions.php | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/Reflection/Assertions.php b/src/Reflection/Assertions.php index bf69355f96..701caefe28 100644 --- a/src/Reflection/Assertions.php +++ b/src/Reflection/Assertions.php @@ -122,23 +122,15 @@ public function intersect(Assertions $other): self } $merged = []; - $usedOther = []; - foreach ($thisAsserts as $thisAssert) { $key = self::getAssertKey($thisAssert); - foreach ($otherAsserts as $j => $otherAssert) { - if (isset($usedOther[$j])) { - continue; - } - + foreach ($otherAsserts as $otherAssert) { if (self::getAssertKey($otherAssert) !== $key) { continue; } $merged[] = $thisAssert->withType(TypeCombinator::union($thisAssert->getType(), $otherAssert->getType())); - $usedOther[$j] = true; - break; } } From 1c4a95f4211a6eacb5d48fd1964254217fa75723 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 Feb 2026 13:43:31 +0100 Subject: [PATCH 8/8] make use of empty-instance --- src/Reflection/Assertions.php | 39 ++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/src/Reflection/Assertions.php b/src/Reflection/Assertions.php index 701caefe28..208b1f4f82 100644 --- a/src/Reflection/Assertions.php +++ b/src/Reflection/Assertions.php @@ -93,7 +93,7 @@ public function mapTypes(callable $callable): self { $assertTagsCallback = static fn (AssertTag $tag): AssertTag => $tag->withType($callable($tag->getType())); - return new self(array_map($assertTagsCallback, $this->asserts)); + return self::create(array_map($assertTagsCallback, $this->asserts)); } /** @@ -106,20 +106,27 @@ public function intersectWith(Assertions $other): self public function union(Assertions $other): self { - return new self(array_merge($this->getAll(), $other->getAll())); + if ($this === self::$empty) { + return $other; + } + if ($other === self::$empty) { + return $this; + } + + return self::create(array_merge($this->getAll(), $other->getAll())); } public function intersect(Assertions $other): self { - $otherAsserts = $other->getAll(); - if (count($otherAsserts) === 0) { + if ($this === self::$empty) { + return $other; + } + if ($other === self::$empty) { return $this; } + $otherAsserts = $other->getAll(); $thisAsserts = $this->getAll(); - if (count($thisAsserts) === 0) { - return $other; - } $merged = []; foreach ($thisAsserts as $thisAssert) { @@ -134,7 +141,7 @@ public function intersect(Assertions $other): self } } - return new self($merged); + return self::create($merged); } private static function getAssertKey(AssertTag $assert): string @@ -147,6 +154,17 @@ private static function getAssertKey(AssertTag $assert): string ); } + /** + * @param AssertTag[] $asserts + */ + private static function create(array $asserts): self + { + if (count($asserts) === 0) { + return self::createEmpty(); + } + return new self($asserts); + } + public static function createEmpty(): self { $empty = self::$empty; @@ -164,11 +182,8 @@ public static function createEmpty(): self public static function createFromResolvedPhpDocBlock(ResolvedPhpDocBlock $phpDocBlock): self { $tags = $phpDocBlock->getAssertTags(); - if (count($tags) === 0) { - return self::createEmpty(); - } - return new self($tags); + return self::create($tags); } }