[6.x] Resolve self/static template args against implementing class before bound check#11833
Open
alies-dev wants to merge 3 commits into
Open
[6.x] Resolve self/static template args against implementing class before bound check#11833alies-dev wants to merge 3 commits into
alies-dev wants to merge 3 commits into
Conversation
…ound check
`ClassLikeAnalyzer::checkTemplateParams` was comparing the raw `$extended_type`
against the parent's template bound without first running it through
`TypeExpander::expandUnion`. As a result `self`/`static` survived as an
unresolved `TNamedObject('self'|'static')` and `UnionTypeComparator::isContainedBy`
correctly returned false against any class bound, producing a false-positive
`InvalidTemplateParam` on annotations like
/** @implements Holder<self> */
final class C extends Base implements Holder {}
even when the implementing class clearly satisfied the bound. Replacing `self`
with the FQN made the error go away. The fix runs `expandUnion` against the
implementing class up-front, matching the call shape in
`Codebase\ClassLikes::1078` and `Codebase\Methods::750`.
Covers all four `@implements`/`@extends`/`@template-implements`/
`@template-extends` surfaces, plus trait `@use` (which routes through the same
`checkTemplateParams`). Resolves vimeo#11199.
alies-dev
changed the title
…* coverage Matches the more cautious precedent in `Codebase\Methods::750`. For final classes, `static` and `self` are equivalent and the resulting type should not carry `is_static = true`; passing `$storage->final` preserves that. Adds 5 regression tests: - nestedStaticInGenericTemplateArgFinalClass (Holder<Box<static>> on a final class) - nestedStaticInGenericTemplateArgNonFinalClass - nestedSelfInGenericTemplateArg - staticAsTemplateArgInTemplateImplementsRespectsParentBound - staticAsTemplateArgInTemplateExtendsRespectsParentBound The nested-generic cases were already accepted by the previous patch (the expansion recurses through `TGenericObject::type_params`); they are committed as documentation that the bound check stays correct in those shapes.
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
1 participant
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.
Fixes #11199.
Currently
@implements I<self>errors withExtended template param ... type self given, even when the implementing class clearly satisfies the bound. Replacingselfwith the FQN makes the error go away, and PHPStan accepts both forms.The cause:
ClassLikeAnalyzer::checkTemplateParamswas passing$extended_typestraight fromtemplate_extended_paramsintoUnionTypeComparator::isContainedBy, without first running it throughTypeExpander::expandUnion. Soself/staticsurvived as a rawTNamedObjectand the bound check had no way to know it should resolve to the implementing class. Other call sites (Codebase\ClassLikes:1078,Codebase\Methods:750) already do this expansion; this site was just missing it.The fix is the missing
expandUnioncall. Same code path is used for@extends,@template-implements,@template-extends, and trait@use, so all of those are covered too.Tests exercise the full matrix (self/static across the four annotation forms plus trait
@use) plus two negative cases proving the bound check still fires when it should.