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..dc7e8c05a7 100644 --- a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php @@ -416,4 +416,34 @@ 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 + { + $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-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', + }; +} 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', + }; +}