Skip to content

Implement getNamedArgumentsVariants() on UnionTypeMethodReflection and IntersectionTypeMethodReflection to combine parameters by name#5727

Open
phpstan-bot wants to merge 3 commits into
phpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-wfs4ewg
Open

Implement getNamedArgumentsVariants() on UnionTypeMethodReflection and IntersectionTypeMethodReflection to combine parameters by name#5727
phpstan-bot wants to merge 3 commits into
phpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-wfs4ewg

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

When calling a method with named arguments on a union type A|B where both classes have the same method with parameters in a different order, PHPStan falsely reported "Unknown parameter $target". This happened because UnionTypeMethodReflection::getNamedArgumentsVariants() returned null, forcing the system to use the positional-combined acceptor from getVariants() which produced compound parameter names like other|target that couldn't be matched by the named argument normalizer.

Changes

  • Added ParametersAcceptorSelector::combineAcceptorsByParameterName() — a new public method analogous to combineAcceptors() but groups parameters by name instead of position (src/Reflection/ParametersAcceptorSelector.php)
  • Implemented UnionTypeMethodReflection::getNamedArgumentsVariants() using combineAcceptorsByParameterName() (src/Reflection/Type/UnionTypeMethodReflection.php)
  • Implemented IntersectionTypeMethodReflection::getNamedArgumentsVariants() using the same approach (src/Reflection/Type/IntersectionTypeMethodReflection.php)
  • Modified the early-return optimization in ParametersAcceptorSelector::selectFromArgs() to skip when the acceptor has compound parameter names (from union type positional combining) and named arguments are present, allowing the existing named-arguments flow to handle them correctly
  • Added acceptorHasCompoundParameterNames() private helper to detect names containing |

Analogous cases probed

  • Static method calls on union types: confirmed fixed — they reuse UnionTypeMethodReflection, so the fix applies automatically. Added test in CallStaticMethodsRuleTest.
  • Intersection types: fixed IntersectionTypeMethodReflection::getNamedArgumentsVariants() with the same pattern.
  • Constructor calls on union types: not affected — InstantiationRule iterates per-class, doesn't go through UnionTypeMethodReflection.
  • Callable calls: not affected — uses its own callable-specific reflection.

Root cause

combineAcceptors() merges parameters by position index, producing compound names like other|target when parameters at the same position have different names across union members. Neither ArgumentsNormalizer::reorderArgs() nor FunctionCallParametersCheck::processArguments() could match a named argument like target: against a parameter named other|target. The fix provides a separate named-arguments-specific variant combined by parameter name.

Test

  • tests/PHPStan/Rules/Methods/data/bug-14661.php — regression test for instance method calls on A|B with named arguments in different parameter order. Covers: single named arg, both args in both orders, different parameter types, and three-way union.
  • tests/PHPStan/Rules/Methods/data/bug-14661-static.php — analogous test for static method calls on class-string<A>|class-string<B>.
  • Both tests expect zero errors (false positive fix).

Fixes phpstan/phpstan#14661

…` and `IntersectionTypeMethodReflection` to combine parameters by name

- Add `combineAcceptorsByParameterName()` to `ParametersAcceptorSelector` that
  groups parameters by name instead of position, so named argument matching
  works when union/intersection members have the same parameters in different order
- Implement `getNamedArgumentsVariants()` on `UnionTypeMethodReflection` and
  `IntersectionTypeMethodReflection` using the new combiner (previously returned null)
- Skip the early return in `selectFromArgs()` when the acceptor has compound
  parameter names (from positional combining) and named arguments are present,
  allowing the named-arguments flow at line 539 to handle them correctly
- Add `acceptorHasCompoundParameterNames()` helper to detect compound names
  created by `combineAcceptors()` (e.g. `other|target`)

Closes phpstan/phpstan#14661
Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I miss some test expected error failures to demonstrate that differentTypes method cannot be called without named arguments.

…erent parameter order

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Pushed. Added test cases for differentTypes called with wrong named argument types on the A|B union — differentTypes(a: 'hello', b: 1) and differentTypes(b: 1, a: 'hello') both correctly produce errors because $a expects int and $b expects string (consistent across both union members when matched by name). This demonstrates that the combineAcceptorsByParameterName() path correctly enforces parameter types, catching type mismatches that the positional combined acceptor (int|string) cannot.

Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I want to test that $obj->differentTypes('hello', 1); is wrong. Without named arguments.

…ent parameter names

When calling a method on a union type where parameters at the same position
have different names (e.g., A::foo(int $a, string $b) vs B::foo(string $b, int $a)),
the combined positional acceptor produces compound names like $a|$b with union
types int|string that accept too broadly. This adds per-member checking in
CallMethodsRule for positional-only calls when compound parameter names are
detected, so type errors against individual union members are reported.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Pushed. The changes address the reviewer's request by adding per-member checking for positional calls on union type methods where parameter names differ at the same position.

What changed:

  • CallMethodsRule: When the combined positional acceptor has compound parameter names (like $a|b) and the call uses only positional arguments, each union member's method is checked individually instead of using the over-permissive combined acceptor. Extracted checkMethodParameters() helper to avoid duplicating the 15 message template strings.

  • Test data (bug-14661.php): Added differentTypesPositional(A|B $obj) that calls $obj->differentTypes('hello', 1) and $obj->differentTypes(1, 'hello') without named arguments.

  • Test expectations: 4 new expected errors demonstrating that positional calls produce type errors against the union member that doesn't accept the argument order (A for ('hello', 1), B for (1, 'hello')).

@ondrejmirtes
Copy link
Copy Markdown
Member

I don't like this direction, doesn't work well. Keeping open so that someone can pick this up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants