[cleanup] Remove three unreachable PHPStan type mappers - #8230
Merged
Conversation
PHPStanStaticTypeMapper picks a mapper with a linear is_a() scan in registration order, so a mapper for a child type must be registered before the mapper for its parent type. Three were registered after theirs and could never be reached: GenericClassStringTypeMapper shadowed by ClassStringTypeMapper SelfObjectTypeMapper shadowed by ObjectTypeMapper ThisTypeMapper shadowed by StaticTypeMapper None of them is injected anywhere; they only appeared in the container list, so removing them does not change behavior. Add a test that walks the registered mappers and fails when one is shadowed by an earlier one, which is what let these three go unnoticed.
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.
PHPStanStaticTypeMapperpicks a mapper with a linearis_a()scan in registration order:So a mapper for a child type has to be registered before the mapper for its parent type. Three were registered after theirs, and can never be reached:
GenericClassStringTypeMapperGenericClassStringTypeClassStringTypeMapperSelfObjectTypeMapperSelfObjectTypeObjectTypeMapperThisTypeMapperThisTypeStaticTypeMapperNone of the three is injected anywhere — they appeared only in the
LazyContainerFactorylist — so no call path reaches them and removing them changes no behavior.They were also lying about what happens
Two of them describe an outcome that differs from what actually runs today. Feeding each type to the dead mapper and to the mapper that really wins:
The live behavior looks like the correct one in both cases (fully qualified name in a docblock,
staticfor a@return $this), so this is stale code rather than a bug — but it is the kind of stale code that reads as authoritative.Guard
Added
TypeMapperOrderTest, which walks the registered mappers and fails when one is shadowed by an earlier one. Reverting any single deletion makes it fail with:That ordering constraint was previously enforced by nothing, which is how these three went unnoticed.