Fix sealed class-string match exhaustiveness for ::class comparisons

[mhert]

Summary

match($foo::class) on a @phpstan-sealed hierarchy falsely reported "Match expression does not handle remaining value" even when all allowed subtypes were covered.

Closes phpstan/phpstan#12241

Root Cause

GenericClassStringType::tryRemove() only handled final classes — it had no awareness of @phpstan-sealed hierarchies. Removing a class-string constant for a non-final sealed subtype was a no-op, so the type was never fully exhausted to never.

Fix

Extended GenericClassStringType::tryRemove() to check ClassReflection::getAllowedSubTypes() and delegate to TypeCombinator::remove(), which already handles sealed subtraction via ObjectType::changeSubtractedType().

Test Plan

• MatchExpressionRuleTest::testBug12241 — exhaustive match on sealed hierarchy produces no error
• All existing tests pass — no regressions

[phpstan-bot]

You've opened the pull request against the latest branch 2.2.x. PHPStan 2.2 is not going to be released for months. If your code is relevant on 2.1.x and you want it to be released sooner, please rebase your pull request and change its target to 2.1.x.

[phpstan-bot]

This pull request has been marked as ready for review.

[staabm]

this looks like a very big AI generated PR.

@mhert did review it yourself and double checked whether it makes sense to you?
(I just want to make sure, that we look into something at least one human already made sense of)

[mhert]

this looks like a very big AI generated PR.

@mhert did review it yourself and double checked whether it makes sense to you?
(I just want to make sure, that we look into something at least one human already made sense of)

I'm not an expert in phpstan's internals, but I did my best, to try to understand what's happening, and I think the AI did a good job. I also fixed one possible performance bottleneck (unnecessary foreach in the generated code) by myself.

I also did let the AI do some reviews and let the upgraded phpstan run against some repositories I work with. I didn't notice any problems in correctness or performance.

So I'm sure that this PR is okay. Thanks for looking in 😊

[ondrejmirtes]

I can tell it’s wrong just be looking at the diff size, sorry 😊

[mhert]

I can tell it’s wrong just be looking at the diff size, sorry 😊

As there are two fixes, should I create two pull requests, to make it easier to review (the commits are separated by topic already)? And if the diff size is too big, should I add less tests? It's "only" +130 without tests.

And if you think the implementation is wrong, could you please give me a hint of what to do instead, to resolve phpstan/phpstan#12241 and the narrowing of a generic union with ::class. I would love to get this implemented and I will do my very best, to make it possible!

Edit: @ondrejmirtes Looks like your comment made Claude to overthink the implmentation. Let me update my PR. Thanks for the hint!

[ondrejmirtes]

This is all the needed change in src/ to implement this:

diff --git a/src/Type/Generic/GenericClassStringType.php b/src/Type/Generic/GenericClassStringType.php
index b4ac3eff88..3123e533d5 100644
--- a/src/Type/Generic/GenericClassStringType.php
+++ b/src/Type/Generic/GenericClassStringType.php
@@ -219,6 +219,18 @@ class GenericClassStringType extends ClassStringType
 					if ($classReflection->isFinal() && $genericObjectClassNames[0] === $typeToRemove->getValue()) {
 						return new NeverType();
 					}
+
+					if ($classReflection->getAllowedSubTypes() !== null) {
+						$objectTypeToRemove = new ObjectType($typeToRemove->getValue());
+						$remainingType = TypeCombinator::remove($generic, $objectTypeToRemove);
+						if ($remainingType instanceof NeverType) {
+							return new NeverType();
+						}
+
+						if (!$remainingType->equals($generic)) {
+							return new self($remainingType);
+						}
+					}
 				}
 			} elseif (count($genericObjectClassNames) > 1) {
 				$objectTypeToRemove = new ObjectType($typeToRemove->getValue());

This is a valid regression test for MatchExpressionRule:

<?php declare(strict_types = 1);

namespace Bug12241;

/**
 * @phpstan-sealed Bar|Baz
 */
abstract class Foo{}

final class Bar extends Foo{}
final class Baz extends Foo{}

function (Foo $foo): string {
	return match ($foo::class) {
		Bar::class => 'Bar',
		Baz::class => 'Baz',
	};
};

I'll accept the PR if you update it like this. The classes need to be final, otherwise the $foo::class === Bar::class comparisons do not work.

[mhert]

Thanks for the review! I tried applying your suggested patch, but it doesn't cover generic sealed types.

The problem is that TypeCombinator::remove() relies on isSuperTypeOf() returning Yes, but GenericObjectType::isSuperTypeOf(plain ObjectType) returns Maybe because the plain ObjectType has no generic parameters to match against:

/**
 * @template T
 * @phpstan-sealed Bar|Baz
 */
abstract class Foo {}

/** @template T @extends Foo<T> */
final class Bar extends Foo {}

/** @template T @extends Foo<T> */
final class Baz extends Foo {}

/** @param Foo<string> $foo */
function test(Foo $foo): string {
    return match ($foo::class) {
        Bar::class => 'bar',
        Baz::class => 'baz',
    };
    // Still reports: Match expression does not handle remaining value:
    // class-string<Foo<string>>&literal-string
}

Inside tryRemove(), the $generic is GenericObjectType('Foo', [StringType]). Calling TypeCombinator::remove($generic, new ObjectType('Bar')) reaches ObjectType::tryRemove() which checks $this->isSuperTypeOf(ObjectType('Bar'))->yes() — that returns Maybe rather than Yes, so tryRemove() returns null and the removal is silently skipped.

I was able to keep your approach by stripping the generic parameters before delegating to TypeCombinator::remove() — this way the existing changeSubtractedType() sealed logic handles everything correctly:

diff --git a/src/Type/Generic/GenericClassStringType.php b/src/Type/Generic/GenericClassStringType.php
index b4ac3eff88..3a1b2c5e4f 100644
--- a/src/Type/Generic/GenericClassStringType.php
+++ b/src/Type/Generic/GenericClassStringType.php
@@ -219,6 +219,19 @@ class GenericClassStringType extends ClassStringType
 					if ($classReflection->isFinal() && $genericObjectClassNames[0] === $typeToRemove->getValue()) {
 						return new NeverType();
 					}
+
+					if ($classReflection->getAllowedSubTypes() !== null) {
+						$objectTypeToRemove = new ObjectType($typeToRemove->getValue());
+						$baseType = new ObjectType($genericObjectClassNames[0],
+							$generic instanceof SubtractableType ? $generic->getSubtractedType() : null);
+						$remainingType = TypeCombinator::remove($baseType, $objectTypeToRemove);
+						if ($remainingType instanceof NeverType) {
+							return new NeverType();
+						}
+
+						if (!$remainingType->equals($baseType)) {
+							return new self($remainingType);
+						}
+					}
 				}
 			} elseif (count($genericObjectClassNames) > 1) {
 				$objectTypeToRemove = new ObjectType($typeToRemove->getValue());

If this is okay for you, I'll update the PR with this simplified version.

---

The PR also includes a second commit that addresses a related issue we ran into: when narrowing a generic union via ::class comparison, the generic type parameters get lost. For example:

/** @template T @extends Animal<T> */
class Cat extends Animal {}
/** @template T @extends Animal<T> */
class Dog extends Animal {}

/** @param Cat<string>|Dog<string> $a */
function test(Animal $a): void {
    if ($a::class === Cat::class) {
        // Currently: $a becomes Cat (generic <string> lost)
        // With fix:  $a becomes Cat<string>
        echo $a->value(); // should be string, not mixed
    }
}

This happens because resolveNormalizedIdentical() constructs a plain ObjectType without considering the variable's known generic parameters. The fix consolidates the duplicated $a::class === 'Foo' / 'Foo' === $a::class handling into a single helper and adds generic-aware narrowing. I kept it in a separate commit since it's a distinct concern. Do you prefer we split it into its own PR?

[ondrejmirtes]

What happens if you make the @template in your example @template-covariant? Or if you update /** @param Foo<string> $foo */ to /** @param Foo<covariant string> $foo */?

[mhert]

Thanks for your proposal! But I tested both, neither @template-covariant nor @param Foo<covariant string> resolves it.

Here are the tests I ran. Except the first they fail with your original patch and pass with the generic-stripping approach:

// --- a: non-generic ---

/** @phpstan-sealed Bar|Baz */
abstract class Foo{}

final class Bar extends Foo{}
final class Baz extends Foo{}

function (Foo $foo): string {
	return match ($foo::class) {
		Bar::class => 'Bar',
		Baz::class => 'Baz',
	};
};

// --- b: @template-covariant ---

/**
 * @template-covariant T
 * @phpstan-sealed BarCov|BazCov
 */
abstract class FooCov {}

/** @template-covariant T @extends FooCov<T> */
final class BarCov extends FooCov {}

/** @template-covariant T @extends FooCov<T> */
final class BazCov extends FooCov {}

/** @param FooCov<string> $foo */
function testTemplateCovariant(FooCov $foo): string {
	return match ($foo::class) {
		BarCov::class => 'bar',
		BazCov::class => 'baz',
	};
}

// --- c: @param Foo<covariant string> ---

/**
 * @template T
 * @phpstan-sealed BarInv|BazInv
 */
abstract class FooInv {}

/** @template T @extends FooInv<T> */
final class BarInv extends FooInv {}

/** @template T @extends FooInv<T> */
final class BazInv extends FooInv {}

/** @param FooInv<covariant string> $foo */
function testCovariantParam(FooInv $foo): string {
	return match ($foo::class) {
		BarInv::class => 'bar',
		BazInv::class => 'baz',
	};
}

[ondrejmirtes]

Please update the PR to the fix I pasted. And then we can open a new issue specific to generics.

[phpstan-bot]

This pull request has been marked as ready for review.

[mhert]

I updated this PR and will create a two new ones for generics: one for sealed + generic class-string exhaustiveness and one for generic parameter preservation during ::class narrowing.

[ondrejmirtes]

Thank you.