From 81427ef74f31b31bfb3a0100b50cc962c321e482 Mon Sep 17 00:00:00 2001 From: ondrejmirtes <104888+ondrejmirtes@users.noreply.github.com> Date: Thu, 12 Feb 2026 14:26:36 +0000 Subject: [PATCH 1/2] Fix match exhaustiveness for class-string with union of final classes - GenericClassStringType::tryRemove() only handled class-string with a single generic type parameter, failing for unions like class-string - Added handling for union generic types: when removing a constant class-string of a final class, remove the corresponding ObjectType from the inner union - New regression tests in both rule test and nsrt for bug #9534 --- CLAUDE.md | 8 +++ src/Type/Generic/GenericClassStringType.php | 23 ++++++- tests/PHPStan/Analyser/nsrt/bug-9534.php | 32 ++++++++++ .../Comparison/MatchExpressionRuleTest.php | 19 ++++++ .../Rules/Comparison/data/bug-9534.php | 61 +++++++++++++++++++ 5 files changed, 140 insertions(+), 3 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-9534.php create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-9534.php diff --git a/CLAUDE.md b/CLAUDE.md index c2df251460..c9eade3bcd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -269,6 +269,14 @@ Loops are a frequent source of false positives because PHPStan must reason about Match expressions in `NodeScopeResolver` (around line 4154) process each arm and merge the resulting scopes. The critical pattern for variable certainty is: when a match is exhaustive (has a `default` arm or an always-true condition), arm body scopes should be merged only with each other (not with the original pre-match scope). This mirrors how if/else merging works — `$finalScope` starts as `null`, and each branch's scope is merged via `$branchScope->mergeWith($finalScope)`. When the match is NOT exhaustive, the original scope must also participate in the merge (via `$scope->mergeWith($armBodyFinalScope)`) because execution may skip all arms and throw `UnhandledMatchError`. The `mergeVariableHolders()` method in `MutatingScope` uses `ExpressionTypeHolder::createMaybe()` for variables present in only one scope, so merging an arm scope that defines `$x` with the original scope that lacks `$x` degrades certainty to "maybe" — this is the root cause of false "might not be defined" reports for exhaustive match expressions. +### GenericClassStringType narrowing and tryRemove + +`GenericClassStringType` represents `class-string` where `T` is the generic object type. When the generic type is a union (e.g., `class-string`), it's a single `GenericClassStringType` with an inner `UnionType`. This is distinct from `class-string|class-string|class-string` which is a `UnionType` of individual `GenericClassStringType`s. + +The `tryRemove()` method handles removing a `ConstantStringType` (e.g., `'Car'`) from the class-string type. It must check whether the class is final — only for final classes can exact class-string removal be performed, since non-final classes could have subclasses whose class-strings would still be valid values. When the inner generic type is a union, `TypeCombinator::remove()` is used to remove the corresponding `ObjectType` from the inner union. + +This affects match expression exhaustiveness: `class-string` matched against `FinalA::class` and `FinalB::class` is exhaustive only because both classes are final. + ### PHPDoc inheritance PHPDoc types (`@return`, `@param`, `@throws`, `@property`) are inherited through class hierarchies. Bugs arise when: diff --git a/src/Type/Generic/GenericClassStringType.php b/src/Type/Generic/GenericClassStringType.php index b39d98cb85..b4ac3eff88 100644 --- a/src/Type/Generic/GenericClassStringType.php +++ b/src/Type/Generic/GenericClassStringType.php @@ -211,10 +211,27 @@ public function tryRemove(Type $typeToRemove): ?Type $generic = $this->getGenericType(); $genericObjectClassNames = $generic->getObjectClassNames(); + $reflectionProvider = ReflectionProviderStaticAccessor::getInstance(); + if (count($genericObjectClassNames) === 1) { - $classReflection = ReflectionProviderStaticAccessor::getInstance()->getClass($genericObjectClassNames[0]); - if ($classReflection->isFinal() && $genericObjectClassNames[0] === $typeToRemove->getValue()) { - return new NeverType(); + if ($reflectionProvider->hasClass($genericObjectClassNames[0])) { + $classReflection = $reflectionProvider->getClass($genericObjectClassNames[0]); + if ($classReflection->isFinal() && $genericObjectClassNames[0] === $typeToRemove->getValue()) { + return new NeverType(); + } + } + } elseif (count($genericObjectClassNames) > 1) { + $objectTypeToRemove = new ObjectType($typeToRemove->getValue()); + if ($reflectionProvider->hasClass($typeToRemove->getValue())) { + $classReflection = $reflectionProvider->getClass($typeToRemove->getValue()); + if ($classReflection->isFinal()) { + $remainingType = TypeCombinator::remove($generic, $objectTypeToRemove); + if ($remainingType instanceof NeverType) { + return new NeverType(); + } + + return new self($remainingType); + } } } } diff --git a/tests/PHPStan/Analyser/nsrt/bug-9534.php b/tests/PHPStan/Analyser/nsrt/bug-9534.php new file mode 100644 index 0000000000..22b72414ca --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-9534.php @@ -0,0 +1,32 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug9534Nsrt; + +use function PHPStan\Testing\assertType; + +final class FinalCar {} +final class FinalBike {} +final class FinalBoat {} + +/** + * @param class-string $string + */ +function narrowing(string $string): void { + assertType('class-string', $string); + + if ($string === FinalCar::class) { + assertType("'Bug9534Nsrt\\\\FinalCar'", $string); + return; + } + + assertType('class-string', $string); + + if ($string === FinalBike::class) { + assertType("'Bug9534Nsrt\\\\FinalBike'", $string); + return; + } + + assertType("class-string", $string); +} diff --git a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php index 917f016c4a..d03fde159b 100644 --- a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php @@ -416,4 +416,23 @@ public function testBug13303(): void ]); } + #[RequiresPhp('>= 8.0')] + public function testBug9534(): void + { + $this->analyse([__DIR__ . '/data/bug-9534.php'], [ + [ + 'Match expression does not handle remaining value: class-string', + 21, + ], + [ + 'Match expression does not handle remaining value: class-string', + 47, + ], + [ + 'Match expression does not handle remaining value: class-string', + 58, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-9534.php b/tests/PHPStan/Rules/Comparison/data/bug-9534.php new file mode 100644 index 0000000000..7615296262 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-9534.php @@ -0,0 +1,61 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug9534; + +class Car {} +class Bike {} +class Boat {} + +final class FinalCar {} +final class FinalBike {} +final class FinalBoat {} + +/** + * Non-final classes: match can't be exhaustive because subclasses may exist + * + * @param class-string $string + */ +function accept_class(string $string): void { + match ($string) { // error on this line + Car::class => 'car', + Bike::class => 'bike', + Boat::class => 'boat', + }; +} + +/** + * Final classes: match IS exhaustive because no subclasses can exist + * + * @param class-string $string + */ +function accept_final_class(string $string): void { + match ($string) { // no error + FinalCar::class => 'car', + FinalBike::class => 'bike', + FinalBoat::class => 'boat', + }; +} + +/** + * Partial match with final classes: should report remaining value + * + * @param class-string $string + */ +function partial_final_match(string $string): void { + match ($string) { // error on this line + FinalCar::class => 'car', + }; +} + +/** + * Partial match with non-final classes + * + * @param class-string $string + */ +function partial_match(string $string): void { + match ($string) { // error on this line + Car::class => 'car', + }; +} From 597a11b13f6b68c2fdd1587933a2a29f01c34b36 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Thu, 12 Feb 2026 14:36:05 +0000 Subject: [PATCH 2/2] Add regression test for #12998 Closes https://github.com/phpstan/phpstan/issues/12998 --- .../Comparison/MatchExpressionRuleTest.php | 11 +++++ .../Rules/Comparison/data/bug-12998.php | 41 +++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-12998.php diff --git a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php index d03fde159b..dc7e8c05a7 100644 --- a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php @@ -416,6 +416,17 @@ public function testBug13303(): void ]); } + #[RequiresPhp('>= 8.0')] + public function testBug12998(): void + { + $this->analyse([__DIR__ . '/data/bug-12998.php'], [ + [ + 'Match expression does not handle remaining value: class-string', + 37, + ], + ]); + } + #[RequiresPhp('>= 8.0')] public function testBug9534(): void { diff --git a/tests/PHPStan/Rules/Comparison/data/bug-12998.php b/tests/PHPStan/Rules/Comparison/data/bug-12998.php new file mode 100644 index 0000000000..9dc745ce5f --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-12998.php @@ -0,0 +1,41 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug12998; + +final class A {} +final class B {} + +class C {} +class D {} + +/** + * Final classes with template: match IS exhaustive + * + * @template T of A|B + * + * @param class-string $class + */ +function fooFinal(string $class): string +{ + return match ($class) { + A::class => 'a', + B::class => 'b', + }; +} + +/** + * Non-final classes with template: match can't be exhaustive + * + * @template T of C|D + * + * @param class-string $class + */ +function fooNonFinal(string $class): string +{ + return match ($class) { + C::class => 'c', + D::class => 'd', + }; +}