From e4478748712b561ee37fdb22e696dc1e737321f5 Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Fri, 8 Nov 2024 16:06:14 +0100 Subject: [PATCH] Use named argument in error for variadic types When a type is variadic, and multiple named arguments are used, it's often hard to tell for which argument the error is coming. Since we have the named argument, we could use it in this situation. --- src/Rules/AttributesCheck.php | 6 +++--- src/Rules/Classes/InstantiationRule.php | 6 +++--- src/Rules/FunctionCallParametersCheck.php | 15 ++++++++++----- src/Rules/Functions/CallCallablesRule.php | 6 +++--- .../Functions/CallToFunctionParametersRule.php | 6 +++--- src/Rules/Functions/CallUserFuncRule.php | 6 +++--- src/Rules/Methods/CallMethodsRule.php | 6 +++--- src/Rules/Methods/CallStaticMethodsRule.php | 6 +++--- .../PHPStan/Rules/Methods/CallMethodsRuleTest.php | 10 +++++++++- .../Rules/Methods/data/named-arguments.php | 1 + 10 files changed, 41 insertions(+), 27 deletions(-) diff --git a/src/Rules/AttributesCheck.php b/src/Rules/AttributesCheck.php index 8633178d9f..e12f906c00 100644 --- a/src/Rules/AttributesCheck.php +++ b/src/Rules/AttributesCheck.php @@ -144,14 +144,14 @@ public function check( 'Attribute class ' . $attributeClassName . ' constructor invoked with %d parameters, at least %d required.', 'Attribute class ' . $attributeClassName . ' constructor invoked with %d parameter, %d-%d required.', 'Attribute class ' . $attributeClassName . ' constructor invoked with %d parameters, %d-%d required.', - 'Parameter %s of attribute class ' . $attributeClassName . ' constructor expects %s, %s given.', + '%s of attribute class ' . $attributeClassName . ' constructor expects %s, %s given.', '', // constructor does not have a return type - 'Parameter %s of attribute class ' . $attributeClassName . ' constructor is passed by reference, so it expects variables only', + '%s of attribute class ' . $attributeClassName . ' constructor is passed by reference, so it expects variables only', 'Unable to resolve the template type %s in instantiation of attribute class ' . $attributeClassName, 'Missing parameter $%s in call to ' . $attributeClassName . ' constructor.', 'Unknown parameter $%s in call to ' . $attributeClassName . ' constructor.', 'Return type of call to ' . $attributeClassName . ' constructor contains unresolvable type.', - 'Parameter %s of attribute class ' . $attributeClassName . ' constructor contains unresolvable type.', + '%s of attribute class ' . $attributeClassName . ' constructor contains unresolvable type.', 'Attribute class ' . $attributeClassName . ' constructorinvoked with %s, but it\'s not allowed because of @no-named-arguments.', ); diff --git a/src/Rules/Classes/InstantiationRule.php b/src/Rules/Classes/InstantiationRule.php index 604038d1fa..7dd65488a0 100644 --- a/src/Rules/Classes/InstantiationRule.php +++ b/src/Rules/Classes/InstantiationRule.php @@ -209,14 +209,14 @@ private function checkClassName(string $class, bool $isName, Node $node, Scope $ 'Class ' . $classDisplayName . ' constructor invoked with %d parameters, at least %d required.', 'Class ' . $classDisplayName . ' constructor invoked with %d parameter, %d-%d required.', 'Class ' . $classDisplayName . ' constructor invoked with %d parameters, %d-%d required.', - 'Parameter %s of class ' . $classDisplayName . ' constructor expects %s, %s given.', + '%s of class ' . $classDisplayName . ' constructor expects %s, %s given.', '', // constructor does not have a return type - 'Parameter %s of class ' . $classDisplayName . ' constructor is passed by reference, so it expects variables only', + ' %s of class ' . $classDisplayName . ' constructor is passed by reference, so it expects variables only', 'Unable to resolve the template type %s in instantiation of class ' . $classDisplayName, 'Missing parameter $%s in call to ' . $classDisplayName . ' constructor.', 'Unknown parameter $%s in call to ' . $classDisplayName . ' constructor.', 'Return type of call to ' . $classDisplayName . ' constructor contains unresolvable type.', - 'Parameter %s of class ' . $classDisplayName . ' constructor contains unresolvable type.', + '%s of class ' . $classDisplayName . ' constructor contains unresolvable type.', 'Class ' . $classDisplayName . ' constructor invoked with %s, but it\'s not allowed because of @no-named-arguments.', )); } diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index eb714ab171..10e4e50212 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -30,6 +30,7 @@ use function array_key_exists; use function count; use function implode; +use function is_int; use function is_string; use function max; use function sprintf; @@ -331,7 +332,7 @@ public function check( $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($parameterType, $argumentValueType); $errors[] = RuleErrorBuilder::message(sprintf( $wrongArgumentTypeMessage, - $this->describeParameter($parameter, $argumentName === null ? $i + 1 : null), + $this->describeParameter($parameter, $argumentName ?? $i + 1), $parameterType->describe($verbosityLevel), $argumentValueType->describe($verbosityLevel), )) @@ -409,7 +410,7 @@ public function check( } $errors[] = RuleErrorBuilder::message(sprintf( - 'Parameter %s is passed by reference so it does not accept %s.', + '%s is passed by reference so it does not accept %s.', $this->describeParameter($parameter, $argumentName === null ? $i + 1 : null), $propertyDescription, ))->identifier('argument.byRef')->line($argumentLine)->build(); @@ -625,11 +626,15 @@ private function processArguments( return [$errors, $newArguments]; } - private function describeParameter(ParameterReflection $parameter, ?int $position): string + private function describeParameter(ParameterReflection $parameter, int|string|null $positionOrNamed): string { $parts = []; - if ($position !== null) { - $parts[] = '#' . $position; + if (is_int($positionOrNamed)) { + $parts[] = 'Parameter #' . $positionOrNamed; + } elseif ($parameter->isVariadic() && is_string($positionOrNamed)) { + $parts[] = 'Named argument ' . $positionOrNamed . ' for variadic parameter'; + } else { + $parts[] = 'Parameter'; } $name = $parameter->getName(); diff --git a/src/Rules/Functions/CallCallablesRule.php b/src/Rules/Functions/CallCallablesRule.php index 15c0bfb9ca..162894e266 100644 --- a/src/Rules/Functions/CallCallablesRule.php +++ b/src/Rules/Functions/CallCallablesRule.php @@ -126,14 +126,14 @@ public function processNode( ucfirst($callableDescription) . ' invoked with %d parameters, at least %d required.', ucfirst($callableDescription) . ' invoked with %d parameter, %d-%d required.', ucfirst($callableDescription) . ' invoked with %d parameters, %d-%d required.', - 'Parameter %s of ' . $callableDescription . ' expects %s, %s given.', + '%s of ' . $callableDescription . ' expects %s, %s given.', 'Result of ' . $callableDescription . ' (void) is used.', - 'Parameter %s of ' . $callableDescription . ' is passed by reference, so it expects variables only.', + '%s of ' . $callableDescription . ' is passed by reference, so it expects variables only.', 'Unable to resolve the template type %s in call to ' . $callableDescription, 'Missing parameter $%s in call to ' . $callableDescription . '.', 'Unknown parameter $%s in call to ' . $callableDescription . '.', 'Return type of call to ' . $callableDescription . ' contains unresolvable type.', - 'Parameter %s of ' . $callableDescription . ' contains unresolvable type.', + '%s of ' . $callableDescription . ' contains unresolvable type.', ucfirst($callableDescription) . ' invoked with %s, but it\'s not allowed because of @no-named-arguments.', ), ); diff --git a/src/Rules/Functions/CallToFunctionParametersRule.php b/src/Rules/Functions/CallToFunctionParametersRule.php index 39b4ee650f..63e1b93c7a 100644 --- a/src/Rules/Functions/CallToFunctionParametersRule.php +++ b/src/Rules/Functions/CallToFunctionParametersRule.php @@ -57,14 +57,14 @@ public function processNode(Node $node, Scope $scope): array 'Function ' . $functionName . ' invoked with %d parameters, at least %d required.', 'Function ' . $functionName . ' invoked with %d parameter, %d-%d required.', 'Function ' . $functionName . ' invoked with %d parameters, %d-%d required.', - 'Parameter %s of function ' . $functionName . ' expects %s, %s given.', + '%s of function ' . $functionName . ' expects %s, %s given.', 'Result of function ' . $functionName . ' (void) is used.', - 'Parameter %s of function ' . $functionName . ' is passed by reference, so it expects variables only.', + '%s of function ' . $functionName . ' is passed by reference, so it expects variables only.', 'Unable to resolve the template type %s in call to function ' . $functionName, 'Missing parameter $%s in call to function ' . $functionName . '.', 'Unknown parameter $%s in call to function ' . $functionName . '.', 'Return type of call to function ' . $functionName . ' contains unresolvable type.', - 'Parameter %s of function ' . $functionName . ' contains unresolvable type.', + '%s of function ' . $functionName . ' contains unresolvable type.', 'Function ' . $functionName . ' invoked with %s, but it\'s not allowed because of @no-named-arguments.', ); } diff --git a/src/Rules/Functions/CallUserFuncRule.php b/src/Rules/Functions/CallUserFuncRule.php index 2ea1fb2777..5eb21c2128 100644 --- a/src/Rules/Functions/CallUserFuncRule.php +++ b/src/Rules/Functions/CallUserFuncRule.php @@ -73,14 +73,14 @@ public function processNode(Node $node, Scope $scope): array ucfirst($callableDescription) . ' invoked with %d parameters, at least %d required.', ucfirst($callableDescription) . ' invoked with %d parameter, %d-%d required.', ucfirst($callableDescription) . ' invoked with %d parameters, %d-%d required.', - 'Parameter %s of ' . $callableDescription . ' expects %s, %s given.', + '%s of ' . $callableDescription . ' expects %s, %s given.', 'Result of ' . $callableDescription . ' (void) is used.', - 'Parameter %s of ' . $callableDescription . ' is passed by reference, so it expects variables only.', + '%s of ' . $callableDescription . ' is passed by reference, so it expects variables only.', 'Unable to resolve the template type %s in call to ' . $callableDescription, 'Missing parameter $%s in call to ' . $callableDescription . '.', 'Unknown parameter $%s in call to ' . $callableDescription . '.', 'Return type of call to ' . $callableDescription . ' contains unresolvable type.', - 'Parameter %s of ' . $callableDescription . ' contains unresolvable type.', + '%s of ' . $callableDescription . ' contains unresolvable type.', ucfirst($callableDescription) . ' invoked with %s, but it\'s not allowed because of @no-named-arguments.', ); } diff --git a/src/Rules/Methods/CallMethodsRule.php b/src/Rules/Methods/CallMethodsRule.php index b0d522f439..19bc52fe80 100644 --- a/src/Rules/Methods/CallMethodsRule.php +++ b/src/Rules/Methods/CallMethodsRule.php @@ -63,14 +63,14 @@ public function processNode(Node $node, Scope $scope): array 'Method ' . $messagesMethodName . ' invoked with %d parameters, at least %d required.', 'Method ' . $messagesMethodName . ' invoked with %d parameter, %d-%d required.', 'Method ' . $messagesMethodName . ' invoked with %d parameters, %d-%d required.', - 'Parameter %s of method ' . $messagesMethodName . ' expects %s, %s given.', + '%s of method ' . $messagesMethodName . ' expects %s, %s given.', 'Result of method ' . $messagesMethodName . ' (void) is used.', - 'Parameter %s of method ' . $messagesMethodName . ' is passed by reference, so it expects variables only.', + '%s of method ' . $messagesMethodName . ' is passed by reference, so it expects variables only.', 'Unable to resolve the template type %s in call to method ' . $messagesMethodName, 'Missing parameter $%s in call to method ' . $messagesMethodName . '.', 'Unknown parameter $%s in call to method ' . $messagesMethodName . '.', 'Return type of call to method ' . $messagesMethodName . ' contains unresolvable type.', - 'Parameter %s of method ' . $messagesMethodName . ' contains unresolvable type.', + '%s of method ' . $messagesMethodName . ' contains unresolvable type.', 'Method ' . $messagesMethodName . ' invoked with %s, but it\'s not allowed because of @no-named-arguments.', )); } diff --git a/src/Rules/Methods/CallStaticMethodsRule.php b/src/Rules/Methods/CallStaticMethodsRule.php index 0f98eca2d9..e6b2c9dbb5 100644 --- a/src/Rules/Methods/CallStaticMethodsRule.php +++ b/src/Rules/Methods/CallStaticMethodsRule.php @@ -71,14 +71,14 @@ public function processNode(Node $node, Scope $scope): array $displayMethodName . ' invoked with %d parameters, at least %d required.', $displayMethodName . ' invoked with %d parameter, %d-%d required.', $displayMethodName . ' invoked with %d parameters, %d-%d required.', - 'Parameter %s of ' . $lowercasedMethodName . ' expects %s, %s given.', + '%s of ' . $lowercasedMethodName . ' expects %s, %s given.', 'Result of ' . $lowercasedMethodName . ' (void) is used.', - 'Parameter %s of ' . $lowercasedMethodName . ' is passed by reference, so it expects variables only.', + '%s of ' . $lowercasedMethodName . ' is passed by reference, so it expects variables only.', 'Unable to resolve the template type %s in call to method ' . $lowercasedMethodName, 'Missing parameter $%s in call to ' . $lowercasedMethodName . '.', 'Unknown parameter $%s in call to ' . $lowercasedMethodName . '.', 'Return type of call to ' . $lowercasedMethodName . ' contains unresolvable type.', - 'Parameter %s of ' . $lowercasedMethodName . ' contains unresolvable type.', + '%s of ' . $lowercasedMethodName . ' contains unresolvable type.', $displayMethodName . ' invoked with %s, but it\'s not allowed because of @no-named-arguments.', )); diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php index cc614cef95..a883c768d9 100644 --- a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php @@ -1907,7 +1907,7 @@ public function testNamedArguments(): void 91, ], [ - 'Parameter ...$args of method NamedArgumentsMethod\Foo::doIpsum() expects string, int given.', + 'Named argument foo for variadic parameter ...$args of method NamedArgumentsMethod\Foo::doIpsum() expects string, int given.', 91, ], [ @@ -1922,6 +1922,14 @@ public function testNamedArguments(): void 'Unpacked argument (...) cannot be followed by a non-unpacked argument.', 94, ], + [ + 'Named argument foo for variadic parameter ...$args of method NamedArgumentsMethod\Foo::doIpsum() expects string, int given.', + 95, + ], + [ + 'Named argument bar for variadic parameter ...$args of method NamedArgumentsMethod\Foo::doIpsum() expects string, int given.', + 95, + ], ]); } diff --git a/tests/PHPStan/Rules/Methods/data/named-arguments.php b/tests/PHPStan/Rules/Methods/data/named-arguments.php index f5c779f171..25d9ef362b 100644 --- a/tests/PHPStan/Rules/Methods/data/named-arguments.php +++ b/tests/PHPStan/Rules/Methods/data/named-arguments.php @@ -92,6 +92,7 @@ public function doDolor(): void $this->doIpsum(...['a' => 1, 'foo' => 'foo']); $this->doIpsum(...['b' => 1, 'foo' => 'foo']); $this->doIpsum(...[1, 2], 'foo'); + $this->doIpsum(1, 2, foo: 1, bar: 2); } }