Return null from ArgumentsNormalizer::reorderArgs() when positional args after named args create holes beyond parameter count#5637
Conversation
…al args after named args create holes beyond parameter count - In `reorderArgs()`, positional arguments keep their original index from `$callArgs`. When a named argument appears between positional arguments (which is invalid PHP), the named arg is removed from the positional sequence, creating a hole in the `$reorderedArgs` array at an index beyond the parameter signature length. - The hole-filling loop at line 401 threw `ShouldNotHappenException` when encountering a hole index not present in `$signatureParameters`. - Replace the throw with `return null`, which tells all callers (func calls, method calls, static calls, `new`, `ParametersAcceptorSelector`) that argument reordering failed, and they fall back gracefully. - The fix applies to all callable types (functions, methods, static methods, constructors) since they all share `reorderArgs()`. - Added two test cases to `ArgumentsNormalizerTest::dataReorderInvalid`: one with a variadic parameter and one without.
staabm
left a comment
There was a problem hiding this comment.
since this fixes a crash-bug it needs a test in AnalyserIntegrationTest
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Done. Added an integration test in |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ntax The test data file defines a class in the Bug14596 namespace, which gets indexed by Composer's classmap autoloader. Since the file also contains `positional argument after named argument` (a PHP compile-time error), any autoload attempt crashes the process — including PHPStan's parallel workers. Move class and function definitions to a separate bug-14596-definition.php file with valid PHP syntax, keeping only the invalid calls in the main test file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…d PHP syntax" This reverts commit 1845afd.
| // method call | ||
| $obj = new Foo(1, 2, 3); | ||
| $obj->bar(1, 2, 3, d: 'foo', 5); | ||
|
|
||
| // static method call | ||
| Foo::baz(1, 2, 3, d: 'foo', 5); |
There was a problem hiding this comment.
@phpstan-bot cover this example also in CallMethodsRuleTest, CallStaticMethodsRuleTest and assert expected errors
There was a problem hiding this comment.
Pushed. Added testBug14596() to both CallMethodsRuleTest and CallStaticMethodsRuleTest, with a shared test data file at tests/PHPStan/Rules/Methods/data/bug-14596.php that exercises method calls and static method calls with positional arguments after named arguments. Both tests assert the expected "Named argument cannot be followed by a positional argument." error. All 12048 tests pass and make phpstan reports no errors.
Assert that positional arguments after named arguments on method calls and static method calls report the expected error instead of crashing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…al args after named args create holes beyond parameter count (phpstan#5637) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Markus Staab <markus.staab@redaxo.de>
…al args after named args create holes beyond parameter count (#5637) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Markus Staab <markus.staab@redaxo.de>
Summary
When calling a function with positional arguments after a named argument (e.g.
\PHPStan\dumpType(1, 2, 3, d: 'foo', 5)), PHPStan crashed with an internal error "Parameter signatures cannot have holes" instead of reporting proper errors about the invalid argument order.Changes
src/Analyser/ArgumentsNormalizer.php: Replacethrow new ShouldNotHappenException(...)withreturn nullin the hole-filling loop ofreorderArgs()when a hole index exceeds the parameter signature length.tests/PHPStan/Analyser/ArgumentsNormalizerTest.php: Added two test cases todataReorderInvalid:Root cause
In
ArgumentsNormalizer::reorderArgs(), positional arguments use their original index from the$callArgsarray as keys in$reorderedArgs. When a named argument sits between positional arguments (invalid PHP: "Cannot use positional argument after named argument"), the named arg is handled separately (placed inadditionalNamedArgsorappendArgs), but the positional arg following it keeps its original index. This creates a gap in the$reorderedArgsarray at an index that has no corresponding entry in$signatureParameters, causing the hole-filling loop to throw.The fix returns
nullinstead, which all callers already handle gracefully — they either fall back to the original (unreordered) arguments or skip the check. The normal argument validation rules then correctly report "Named argument cannot be followed by a positional argument."Since all callable types (functions, methods, static methods, constructors,
call_user_func) share the samereorderArgs()method, the fix covers all of them automatically. Verified with manual testing ofFuncCall,MethodCall,StaticCall,New_, andcall_user_func.Test
ArgumentsNormalizerTest::dataReorderInvalidthat reproduce the crash and verifyreorderArgs()returnsnullfor these invalid argument patterns.Fixes phpstan/phpstan#14596