Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>` where `T` is the generic object type. When the generic type is a union (e.g., `class-string<Car|Bike|Boat>`), it's a single `GenericClassStringType` with an inner `UnionType`. This is distinct from `class-string<Car>|class-string<Bike>|class-string<Boat>` 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<FinalA|FinalB>` 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:
Expand Down
23 changes: 20 additions & 3 deletions src/Type/Generic/GenericClassStringType.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
}
Expand Down
32 changes: 32 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-9534.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php // lint >= 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<FinalCar|FinalBike|FinalBoat> $string
*/
function narrowing(string $string): void {
assertType('class-string<Bug9534Nsrt\FinalBike|Bug9534Nsrt\FinalBoat|Bug9534Nsrt\FinalCar>', $string);

if ($string === FinalCar::class) {
assertType("'Bug9534Nsrt\\\\FinalCar'", $string);
return;
}

assertType('class-string<Bug9534Nsrt\FinalBike|Bug9534Nsrt\FinalBoat>', $string);

if ($string === FinalBike::class) {
assertType("'Bug9534Nsrt\\\\FinalBike'", $string);
return;
}

assertType("class-string<Bug9534Nsrt\\FinalBoat>", $string);
}
30 changes: 30 additions & 0 deletions tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<T of Bug12998\C|Bug12998\D>',
37,
],
]);
}

#[RequiresPhp('>= 8.0')]
public function testBug9534(): void
{
$this->analyse([__DIR__ . '/data/bug-9534.php'], [
[
'Match expression does not handle remaining value: class-string<Bug9534\Bike|Bug9534\Boat|Bug9534\Car>',
21,
],
[
'Match expression does not handle remaining value: class-string<Bug9534\FinalBike|Bug9534\FinalBoat>',
47,
],
[
'Match expression does not handle remaining value: class-string<Bug9534\Bike|Bug9534\Car>',
58,
],
]);
}

}
41 changes: 41 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-12998.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php // lint >= 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<T> $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<T> $class
*/
function fooNonFinal(string $class): string
{
return match ($class) {
C::class => 'c',
D::class => 'd',
};
}
61 changes: 61 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-9534.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php // lint >= 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<Car|Bike|Boat> $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<FinalCar|FinalBike|FinalBoat> $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<FinalCar|FinalBike|FinalBoat> $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<Car|Bike> $string
*/
function partial_match(string $string): void {
match ($string) { // error on this line
Car::class => 'car',
};
}
Loading