From 579e874321cae0329d42bc5e0ae25d59dd06104c Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Mon, 2 Dec 2024 16:25:37 +0100 Subject: [PATCH 1/6] Support named arguments after unpacking --- src/Php/PhpVersions.php | 5 +++ src/Rules/FunctionCallParametersCheck.php | 35 +++++++++++++------ .../CallToFunctionParametersRuleTest.php | 27 ++++++++++++++ .../Rules/Functions/data/bug-11418.php | 9 +++++ .../data/named-arguments-after-unpacking.php | 14 ++++++++ 5 files changed, 80 insertions(+), 10 deletions(-) create mode 100755 tests/PHPStan/Rules/Functions/data/bug-11418.php create mode 100755 tests/PHPStan/Rules/Functions/data/named-arguments-after-unpacking.php diff --git a/src/Php/PhpVersions.php b/src/Php/PhpVersions.php index 229dccb72d..96bf233209 100644 --- a/src/Php/PhpVersions.php +++ b/src/Php/PhpVersions.php @@ -38,4 +38,9 @@ public function supportsNamedArguments(): TrinaryLogic return IntegerRangeType::fromInterval(80000, null)->isSuperTypeOf($this->phpVersions)->result; } + public function supportsNamedArgumentAfterUnpackedArgument(): TrinaryLogic + { + return IntegerRangeType::fromInterval(80100, null)->isSuperTypeOf($this->phpVersions)->result; + } + } diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index 50371ab23c..d2f5ef0b0f 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -27,6 +27,7 @@ use PHPStan\Type\VerbosityLevel; use function array_fill; use function array_key_exists; +use function array_slice; use function count; use function implode; use function in_array; @@ -81,12 +82,15 @@ public function check( { $functionParametersMinCount = 0; $functionParametersMaxCount = 0; + $functionParameterNames = []; foreach ($parametersAcceptor->getParameters() as $parameter) { if (!$parameter->isOptional()) { $functionParametersMinCount++; } $functionParametersMaxCount++; + + $functionParameterNames[] = $parameter->getName(); } if ($parametersAcceptor->isVariadic()) { @@ -101,6 +105,12 @@ public function check( $hasUnpackedArgument = false; $errors = []; foreach ($args as $arg) { + $argumentName = null; + if ($arg->name !== null) { + $hasNamedArguments = true; + $argumentName = $arg->name->toString(); + } + if ($hasNamedArguments && $arg->unpack) { $errors[] = RuleErrorBuilder::message('Named argument cannot be followed by an unpacked (...) argument.') ->identifier('argument.unpackAfterNamed') @@ -109,20 +119,25 @@ public function check( ->build(); } if ($hasUnpackedArgument && !$arg->unpack) { - $errors[] = RuleErrorBuilder::message('Unpacked argument (...) cannot be followed by a non-unpacked argument.') - ->identifier('argument.nonUnpackAfterUnpacked') - ->line($arg->getStartLine()) - ->nonIgnorable() - ->build(); + if ($argumentName !== null && $scope->getPhpVersion()->supportsNamedArgumentAfterUnpackedArgument()->yes()) { + if (in_array($argumentName, array_slice($functionParameterNames, 0, count($arguments)), true)) { + $errors[] = RuleErrorBuilder::message(sprintf('Named parameter cannot overwrite already unpacked argument $%s.', $argumentName)) + ->identifier('argument.namedOverwriteAfterUnpacked') + ->line($arg->getStartLine()) + ->nonIgnorable() + ->build(); + } + } else { + $errors[] = RuleErrorBuilder::message('Unpacked argument (...) cannot be followed by a non-unpacked argument.') + ->identifier('argument.nonUnpackAfterUnpacked') + ->line($arg->getStartLine()) + ->nonIgnorable() + ->build(); + } } if ($arg->unpack) { $hasUnpackedArgument = true; } - $argumentName = null; - if ($arg->name !== null) { - $hasNamedArguments = true; - $argumentName = $arg->name->toString(); - } if ($arg->unpack) { $type = $scope->getType($arg->value); $arrays = $type->getConstantArrays(); diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index a6513f03cb..82f5036ce3 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -499,6 +499,24 @@ public function testNamedArguments(): void $this->analyse([__DIR__ . '/data/named-arguments.php'], $errors); } + public function testNamedArgumentsAfterUnpacking(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + + $this->analyse([__DIR__ . '/data/named-arguments-after-unpacking.php'], [ + [ + 'Named parameter cannot overwrite already unpacked argument $b.', + 14, + ], + [ + 'Argument for parameter $b has already been passed.', + 14, + ], + ]); + } + public function testBug4514(): void { $this->analyse([__DIR__ . '/data/bug-4514.php'], []); @@ -1936,4 +1954,13 @@ public function testBug12051(): void $this->analyse([__DIR__ . '/data/bug-12051.php'], []); } + public function testBug11418(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + + $this->analyse([__DIR__ . '/data/bug-11418.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/bug-11418.php b/tests/PHPStan/Rules/Functions/data/bug-11418.php new file mode 100755 index 0000000000..8172892d95 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-11418.php @@ -0,0 +1,9 @@ + 2, 'a' => 1], d: 40)); // 46 + +var_dump(foo(...[1, 2], b: 20)); // Fatal error. Named parameter $b overwrites previous argument From b7b5f84f89a14faa59efa3085c77394869ba94c0 Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Fri, 20 Dec 2024 15:14:27 +0100 Subject: [PATCH 2/6] Handle new error inside processArguments --- src/Rules/FunctionCallParametersCheck.php | 42 ++++++++++--------- .../CallToFunctionParametersRuleTest.php | 4 -- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index d2f5ef0b0f..5f7ebe9f27 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -27,7 +27,6 @@ use PHPStan\Type\VerbosityLevel; use function array_fill; use function array_key_exists; -use function array_slice; use function count; use function implode; use function in_array; @@ -82,22 +81,19 @@ public function check( { $functionParametersMinCount = 0; $functionParametersMaxCount = 0; - $functionParameterNames = []; foreach ($parametersAcceptor->getParameters() as $parameter) { if (!$parameter->isOptional()) { $functionParametersMinCount++; } $functionParametersMaxCount++; - - $functionParameterNames[] = $parameter->getName(); } if ($parametersAcceptor->isVariadic()) { $functionParametersMaxCount = -1; } - /** @var array $arguments */ + /** @var array $arguments */ $arguments = []; /** @var array $args */ $args = $funcCall->getArgs(); @@ -111,6 +107,8 @@ public function check( $argumentName = $arg->name->toString(); } + $nonUnpackAfterUnpacked = false; + if ($hasNamedArguments && $arg->unpack) { $errors[] = RuleErrorBuilder::message('Named argument cannot be followed by an unpacked (...) argument.') ->identifier('argument.unpackAfterNamed') @@ -119,15 +117,9 @@ public function check( ->build(); } if ($hasUnpackedArgument && !$arg->unpack) { - if ($argumentName !== null && $scope->getPhpVersion()->supportsNamedArgumentAfterUnpackedArgument()->yes()) { - if (in_array($argumentName, array_slice($functionParameterNames, 0, count($arguments)), true)) { - $errors[] = RuleErrorBuilder::message(sprintf('Named parameter cannot overwrite already unpacked argument $%s.', $argumentName)) - ->identifier('argument.namedOverwriteAfterUnpacked') - ->line($arg->getStartLine()) - ->nonIgnorable() - ->build(); - } - } else { + $nonUnpackAfterUnpacked = true; + + if ($argumentName === null || !$scope->getPhpVersion()->supportsNamedArgumentAfterUnpackedArgument()->yes()) { $errors[] = RuleErrorBuilder::message('Unpacked argument (...) cannot be followed by a non-unpacked argument.') ->identifier('argument.nonUnpackAfterUnpacked') ->line($arg->getStartLine()) @@ -191,6 +183,7 @@ public function check( false, $keyArgumentName, $arg->getStartLine(), + $nonUnpackAfterUnpacked, ]; } } else { @@ -200,6 +193,7 @@ public function check( true, null, $arg->getStartLine(), + $nonUnpackAfterUnpacked, ]; } continue; @@ -211,6 +205,7 @@ public function check( false, $argumentName, $arg->getStartLine(), + $nonUnpackAfterUnpacked, ]; } @@ -562,7 +557,7 @@ private function processArguments( $newArguments = []; $namedArgumentAlreadyOccurred = false; - foreach ($arguments as $i => [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine]) { + foreach ($arguments as $i => [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine, $nonUnpackAfterUnpacked]) { if ($argumentName === null) { if (!isset($parameters[$i])) { if (!$parametersAcceptor->isVariadic() || count($parameters) === 0) { @@ -622,10 +617,19 @@ private function processArguments( && !$parameter->isVariadic() && !array_key_exists($parameter->getName(), $unusedParametersByName) ) { - $errors[] = RuleErrorBuilder::message(sprintf('Argument for parameter $%s has already been passed.', $parameter->getName())) - ->identifier('argument.duplicate') - ->line($argumentLine) - ->build(); + if ($nonUnpackAfterUnpacked) { + $errors[] = RuleErrorBuilder::message(sprintf('Named parameter cannot overwrite already unpacked argument $%s.', $argumentName)) + ->identifier('argument.namedOverwriteAfterUnpacked') + ->line($argumentLine) + ->nonIgnorable() + ->build(); + } else { + $errors[] = RuleErrorBuilder::message(sprintf('Argument for parameter $%s has already been passed.', $parameter->getName())) + ->identifier('argument.duplicate') + ->line($argumentLine) + ->build(); + } + continue; } diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 82f5036ce3..b8b7769d04 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -510,10 +510,6 @@ public function testNamedArgumentsAfterUnpacking(): void 'Named parameter cannot overwrite already unpacked argument $b.', 14, ], - [ - 'Argument for parameter $b has already been passed.', - 14, - ], ]); } From e1a314c4b96e5d671c26932839a451d32a33217a Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Fri, 20 Dec 2024 15:22:50 +0100 Subject: [PATCH 3/6] more consistency in way parameter name is retrieved --- src/Rules/FunctionCallParametersCheck.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index 5f7ebe9f27..ffc086f539 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -618,7 +618,7 @@ private function processArguments( && !array_key_exists($parameter->getName(), $unusedParametersByName) ) { if ($nonUnpackAfterUnpacked) { - $errors[] = RuleErrorBuilder::message(sprintf('Named parameter cannot overwrite already unpacked argument $%s.', $argumentName)) + $errors[] = RuleErrorBuilder::message(sprintf('Named parameter cannot overwrite already unpacked argument $%s.', $parameter->getName())) ->identifier('argument.namedOverwriteAfterUnpacked') ->line($argumentLine) ->nonIgnorable() From 63aad5379a85d4f361df3fdfe2be8a73fa5f96d7 Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Sat, 4 Jan 2025 14:56:35 +0100 Subject: [PATCH 4/6] Simplify by not using non-ignorable error --- src/Rules/FunctionCallParametersCheck.php | 28 ++++++----------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index ffc086f539..5c492c88b6 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -93,7 +93,7 @@ public function check( $functionParametersMaxCount = -1; } - /** @var array $arguments */ + /** @var array $arguments */ $arguments = []; /** @var array $args */ $args = $funcCall->getArgs(); @@ -107,8 +107,6 @@ public function check( $argumentName = $arg->name->toString(); } - $nonUnpackAfterUnpacked = false; - if ($hasNamedArguments && $arg->unpack) { $errors[] = RuleErrorBuilder::message('Named argument cannot be followed by an unpacked (...) argument.') ->identifier('argument.unpackAfterNamed') @@ -117,8 +115,6 @@ public function check( ->build(); } if ($hasUnpackedArgument && !$arg->unpack) { - $nonUnpackAfterUnpacked = true; - if ($argumentName === null || !$scope->getPhpVersion()->supportsNamedArgumentAfterUnpackedArgument()->yes()) { $errors[] = RuleErrorBuilder::message('Unpacked argument (...) cannot be followed by a non-unpacked argument.') ->identifier('argument.nonUnpackAfterUnpacked') @@ -183,7 +179,6 @@ public function check( false, $keyArgumentName, $arg->getStartLine(), - $nonUnpackAfterUnpacked, ]; } } else { @@ -193,7 +188,6 @@ public function check( true, null, $arg->getStartLine(), - $nonUnpackAfterUnpacked, ]; } continue; @@ -205,7 +199,6 @@ public function check( false, $argumentName, $arg->getStartLine(), - $nonUnpackAfterUnpacked, ]; } @@ -557,7 +550,7 @@ private function processArguments( $newArguments = []; $namedArgumentAlreadyOccurred = false; - foreach ($arguments as $i => [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine, $nonUnpackAfterUnpacked]) { + foreach ($arguments as $i => [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine]) { if ($argumentName === null) { if (!isset($parameters[$i])) { if (!$parametersAcceptor->isVariadic() || count($parameters) === 0) { @@ -617,18 +610,11 @@ private function processArguments( && !$parameter->isVariadic() && !array_key_exists($parameter->getName(), $unusedParametersByName) ) { - if ($nonUnpackAfterUnpacked) { - $errors[] = RuleErrorBuilder::message(sprintf('Named parameter cannot overwrite already unpacked argument $%s.', $parameter->getName())) - ->identifier('argument.namedOverwriteAfterUnpacked') - ->line($argumentLine) - ->nonIgnorable() - ->build(); - } else { - $errors[] = RuleErrorBuilder::message(sprintf('Argument for parameter $%s has already been passed.', $parameter->getName())) - ->identifier('argument.duplicate') - ->line($argumentLine) - ->build(); - } + $errors[] = RuleErrorBuilder::message(sprintf('Named parameter cannot overwrite already unpacked argument $%s.', $parameter->getName())) + ->identifier('argument.namedOverwriteAfterUnpacked') + ->line($argumentLine) + ->nonIgnorable() + ->build(); continue; } From 68e6c971026c98474889574a6992dd103923771b Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Sat, 4 Jan 2025 15:00:16 +0100 Subject: [PATCH 5/6] Add another regression test --- .../Functions/CallToFunctionParametersRuleTest.php | 9 +++++++++ tests/PHPStan/Rules/Functions/data/bug-8046.php | 11 +++++++++++ 2 files changed, 20 insertions(+) create mode 100644 tests/PHPStan/Rules/Functions/data/bug-8046.php diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index b8b7769d04..2e8afa35a4 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -1950,6 +1950,15 @@ public function testBug12051(): void $this->analyse([__DIR__ . '/data/bug-12051.php'], []); } + public function testBug8046(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + + $this->analyse([__DIR__ . '/data/bug-8046.php'], []); + } + public function testBug11418(): void { if (PHP_VERSION_ID < 80100) { diff --git a/tests/PHPStan/Rules/Functions/data/bug-8046.php b/tests/PHPStan/Rules/Functions/data/bug-8046.php new file mode 100644 index 0000000000..368f656341 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-8046.php @@ -0,0 +1,11 @@ + 7]; + +var_dump(add(...$args, b: 8)); From c333f9cf84a6739237cfb62f05a59c3c661526cd Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Sat, 4 Jan 2025 15:01:55 +0100 Subject: [PATCH 6/6] Fix --- src/Rules/FunctionCallParametersCheck.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index 5c492c88b6..bd637913bc 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -610,12 +610,10 @@ private function processArguments( && !$parameter->isVariadic() && !array_key_exists($parameter->getName(), $unusedParametersByName) ) { - $errors[] = RuleErrorBuilder::message(sprintf('Named parameter cannot overwrite already unpacked argument $%s.', $parameter->getName())) - ->identifier('argument.namedOverwriteAfterUnpacked') + $errors[] = RuleErrorBuilder::message(sprintf('Argument for parameter $%s has already been passed.', $parameter->getName())) + ->identifier('argument.duplicate') ->line($argumentLine) - ->nonIgnorable() ->build(); - continue; }