Accept bidirectionally equivalent types in invariant template variance check#5709
Open
phpstan-bot wants to merge 1 commit into
Open
Accept bidirectionally equivalent types in invariant template variance check#5709phpstan-bot wants to merge 1 commit into
phpstan-bot wants to merge 1 commit into
Conversation
…e check - In `TemplateTypeVariance::isValidVariance()`, the invariant check used `equals()` which is a structural comparison. `StaticType` and `ObjectType` are structurally different PHP classes, so `equals()` returned false even when both represent the same type (e.g. `static(FinalClass)` vs `FinalClass`). - After `equals()` fails, now check bidirectional `isSuperTypeOf()`: if both `$a->isSuperTypeOf($b)->yes()` and `$b->isSuperTypeOf($a)->yes()`, the types are semantically equivalent and accepted for invariant templates. - This fixes false positives like `Collection<static(Foo)>` not matching `Collection<Foo>` when `Foo` is a final class and the template is invariant. - Probed analogous cases: `ThisType` in final classes (requires broader `ThisType::isSuperTypeOf` changes that affect intersection simplification for enums — left for a separate fix), covariant/contravariant templates (already handled by `isSuperTypeOf` directly), parameter types (already resolved by `transformStaticType`).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
For final classes,
static(Foo)andFooare semantically identical since the class cannot be extended. However, PHPStan's invariant template variance check usedequals()(a structural comparison) which returned false becauseStaticTypeandObjectTypeare different PHP classes. This caused false positives like:Changes
src/Type/Generic/TemplateTypeVariance.php: In the invariant branch ofisValidVariance(), after theequals()check fails, added a bidirectionalisSuperTypeOf()check. If$a->isSuperTypeOf($b)->yes()AND$b->isSuperTypeOf($a)->yes(), the types are semantically equivalent and accepted. The existingisSuperTypeOfcall for the "not covariant" hint is reused (stored in a local variable) to avoid redundant computation.Root cause
The invariant template variance check in
TemplateTypeVariance::isValidVariance()used$a->equals($b)to determine type equality.equals()is a structural comparison —ObjectType::equals()checksget_class($type) !== static::class, so it rejectsStaticTypeeven when the types are semantically identical.For a final class
Foo,StaticType(Foo)andObjectType(Foo)represent the same set of values (since no subclass ofFooexists). BothisSuperTypeOfdirections correctly returnyesfor this case. The fix uses this bidirectional supertype relationship as the semantic equality check.Analogous cases probed
ThisTypein final classes:$this(FinalFoo)vsFinalFooin invariant generics. This requires changes toThisType::isSuperTypeOf(currently returnsmaybeforObjectTypeeven for final classes). A broaderThisTypechange was tested but reverted because it affected intersection type simplification for enums (which are inherently final). Left for a separate fix.isSuperTypeOfdirectly, notequals().TransformStaticTypeTraverserwhich resolvesstaticto the concrete type for final classes in declared method signatures.Test
tests/PHPStan/Rules/Methods/ReturnTypeRuleTest::testBug14647with test data reproducing the original issue: a final class with@return Collection<static>returningnew Collection([new static()]).tests/PHPStan/Analyser/nsrt/bug-14647.phpverifying inferred types forcollect(),childCollect(),boxed(), andstaticBoxed()on final classes.staticcase and a static method variant withBox<static>.Fixes phpstan/phpstan#14647