Resolve component names from combined |-separated parameter names for named argument lookup on union types#5726
Closed
phpstan-bot wants to merge 1 commit into
Closed
Conversation
…or named argument lookup on union types - When `combineAcceptors()` merges parameters from union type method variants by position, parameters at the same position with different names get a combined name like `other|target`. Named argument lookup then fails because it searches for `target` but only finds `other|target` in the parameters-by-name map. - Add `mapCombinedParameterNames()` to `FunctionCallParametersCheck` to split combined parameter names and register each component name in the lookup maps, using a primary-names-first strategy to ensure each component maps to a unique parameter. - Add `mapCombinedParameterPositions()` to `ArgumentsNormalizer` with the same strategy for the argument positions map used during argument reordering. - Fix covers instance method calls, static method calls, constructor calls, and function calls since they all funnel through the same `FunctionCallParametersCheck::check()` and `ArgumentsNormalizer::reorderArgs()` code paths. - Intersection types use a different approach (selecting the method with most parameters) and are not affected.
Member
|
This needs more thought I think, the issue is really crazy and breaks a lot of assumptions. |
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
3 participants
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
When calling a method on a union type (
A|B) with named arguments, PHPStan incorrectly reports "Unknown parameter" if the parameter exists in all variants but at different positions. For example,A::mixedOrder($other, $target)andB::mixedOrder($target, $other)— calling$obj->mixedOrder(target: 'value')reportedUnknown parameter $targeteven though both classes have a$targetparameter.Changes
Added
FunctionCallParametersCheck::mapCombinedParameterNames()(src/Rules/FunctionCallParametersCheck.php) — splits|-separated combined parameter names (e.g.,other|target) into individual component names and registers each in the$parametersByNameand$originalParametersByNamelookup maps. Uses a two-pass strategy: first maps primary names (first component of each combined name), then fills remaining components, ensuring each component name maps to a unique parameter even when names overlap across positions.Added
ArgumentsNormalizer::mapCombinedParameterPositions()(src/Analyser/ArgumentsNormalizer.php) — same strategy for the$argumentPositionsmap used during named argument reordering.Analogous cases probed:
InstantiationRule) — covered by the fixCallToFunctionParametersRule) — covered by the fixIntersectionTypeMethodReflectionwhich selects the method with the most parameters rather than combining by positionRoot cause
ParametersAcceptorSelector::combineAcceptors()merges parameters from union type method variants by position index. When different variants have different parameter names at the same position, it creates a combined name with|separator (e.g.,other|target). BothFunctionCallParametersCheck::processArguments()andArgumentsNormalizer::reorderArgs()build name-to-parameter lookup maps keyed by the full combined name — so looking uptargetfails because onlyother|targetexists as a key.Test
tests/PHPStan/Rules/Methods/data/bug-14661.php— regression test for instance method calls with named arguments onA|Bunion type where parameters have different order across variants. Tests: single named arg, both named args, both orders, different types, and unknown parameter (should still error).tests/PHPStan/Rules/Methods/data/bug-14661-static.php— same test for static method calls on union types.Fixes phpstan/phpstan#14661