[TypeDeclaration] Drop AstResolver from NarrowObjectReturnTypeRector parent check#8100
Merged
Merged
Conversation
…parent check The parent-method lookup only needs the parent's native return type and whether its @return is a generic type. PHPStan's ExtendedMethodReflection exposes both via getNativeReturnType() and getReturnType(), so resolve the parent method through reflection instead of re-parsing its source file with AstResolver.
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.
What
NarrowObjectReturnTypeRector::isNarrowingValidFromParent()resolved each ancestor's method into a full AST (AstResolver->resolveClassMethod()) only to read two things:@returnis a generic type (e.g.Collection<int, Foo>), which blocks narrowing.Both are available from PHPStan's
ExtendedMethodReflectionwithout re-parsing the source file:getNativeReturnType()→ replaces theIdentifier/FullyQualifiednode check. We keep onlyObjectType(single bare class); object-keyword, union, nullable, scalar all map to non-ObjectTypeand are skipped exactly as before.getReturnType() instanceof GenericObjectType→ replaces reading the parent node's docblock viaPhpDocInfoFactory. PHPStan resolves@return Collection<int, Foo>to aGenericObjectType, and@return T(template) to a non-generic type — matching the previous behaviour.Why
AstResolverre-parses the declaring file of every ancestor on every matching method. The data needed here is pure type metadata that reflection already holds, so the parse was overhead. The earlier "docblock = AST-only" assumption only holds for raw docblock text; the semantic generic-type check is fully available viagetReturnType().Tests
All existing fixtures pass unchanged, including the parent-path cases (
narrow_with_generic_parent,narrow_parent_has_specific_return_type,narrow_from_abstract_class,narrow_from_parent_to_child).vendor/bin/phpunit rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector→ 14/14 green. ECS + PHPStan level 8 clean.