From f71da02958da0dd6b40193c64fcb6da12daf7227 Mon Sep 17 00:00:00 2001 From: Takuya Aramaki Date: Tue, 9 Apr 2024 23:49:14 +0900 Subject: [PATCH] Improve error messages on unnamed parameters Closes phpstan/phpstan#10814 --- src/Rules/FunctionCallParametersCheck.php | 39 ++++++++++--------- .../Levels/data/callableVariance-5.json | 4 +- .../Rules/Functions/CallCallablesRuleTest.php | 14 ++++++- .../Rules/Functions/data/bug-10814.php | 12 ++++++ 4 files changed, 47 insertions(+), 22 deletions(-) create mode 100644 tests/PHPStan/Rules/Functions/data/bug-10814.php diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index da1f9c32e4..d2ea6b6173 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -26,6 +26,7 @@ use function array_fill; use function array_key_exists; use function count; +use function implode; use function is_string; use function max; use function sprintf; @@ -260,14 +261,9 @@ public function check( if (!$accepts->result) { $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($parameterType, $argumentValueType); - $parameterDescription = sprintf('%s$%s', $parameter->isVariadic() ? '...' : '', $parameter->getName()); $errors[] = RuleErrorBuilder::message(sprintf( $messages[6], - $argumentName === null ? sprintf( - '#%d %s', - $i + 1, - $parameterDescription, - ) : $parameterDescription, + $this->describeParameter($parameter, $argumentName === null ? $i + 1 : null), $parameterType->describe($verbosityLevel), $argumentValueType->describe($verbosityLevel), ))->line($argumentLine)->acceptsReasonsTip($accepts->reasons)->build(); @@ -280,14 +276,9 @@ public function check( && !$this->unresolvableTypeHelper->containsUnresolvableType($originalParameter->getType()) && $this->unresolvableTypeHelper->containsUnresolvableType($parameterType) ) { - $parameterDescription = sprintf('%s$%s', $parameter->isVariadic() ? '...' : '', $parameter->getName()); $errors[] = RuleErrorBuilder::message(sprintf( $messages[13], - $argumentName === null ? sprintf( - '#%d %s', - $i + 1, - $parameterDescription, - ) : $parameterDescription, + $this->describeParameter($parameter, $argumentName === null ? $i + 1 : null), ))->line($argumentLine)->build(); } } @@ -300,10 +291,9 @@ public function check( } if ($this->nullsafeCheck->containsNullSafe($argumentValue)) { - $parameterDescription = sprintf('%s$%s', $parameter->isVariadic() ? '...' : '', $parameter->getName()); $errors[] = RuleErrorBuilder::message(sprintf( $messages[8], - $argumentName === null ? sprintf('#%d %s', $i + 1, $parameterDescription) : $parameterDescription, + $this->describeParameter($parameter, $argumentName === null ? $i + 1 : null), ))->line($argumentLine)->build(); continue; } @@ -327,10 +317,9 @@ public function check( $propertyDescription = sprintf('readonly property %s::$%s', $propertyReflection->getDeclaringClass()->getDisplayName(), $propertyReflection->getName()); } - $parameterDescription = sprintf('%s$%s', $parameter->isVariadic() ? '...' : '', $parameter->getName()); $errors[] = RuleErrorBuilder::message(sprintf( 'Parameter %s is passed by reference so it does not accept %s.', - $argumentName === null ? sprintf('#%d %s', $i + 1, $parameterDescription) : $parameterDescription, + $this->describeParameter($parameter, $argumentName === null ? $i + 1 : null), $propertyDescription, ))->line($argumentLine)->build(); } @@ -343,10 +332,9 @@ public function check( continue; } - $parameterDescription = sprintf('%s$%s', $parameter->isVariadic() ? '...' : '', $parameter->getName()); $errors[] = RuleErrorBuilder::message(sprintf( $messages[8], - $argumentName === null ? sprintf('#%d %s', $i + 1, $parameterDescription) : $parameterDescription, + $this->describeParameter($parameter, $argumentName === null ? $i + 1 : null), ))->line($argumentLine)->build(); } @@ -526,4 +514,19 @@ private function processArguments( return [$errors, $newArguments]; } + private function describeParameter(ParameterReflection $parameter, ?int $position): string + { + $parts = []; + if ($position !== null) { + $parts[] = '#' . $position; + } + + $name = $parameter->getName(); + if ($name !== '') { + $parts[] = ($parameter->isVariadic() ? '...$' : '$') . $name; + } + + return implode(' ', $parts); + } + } diff --git a/tests/PHPStan/Levels/data/callableVariance-5.json b/tests/PHPStan/Levels/data/callableVariance-5.json index 81682ac6e7..c37c7adeb0 100644 --- a/tests/PHPStan/Levels/data/callableVariance-5.json +++ b/tests/PHPStan/Levels/data/callableVariance-5.json @@ -1,6 +1,6 @@ [ { - "message": "Parameter #1 $ of callable callable(Levels\\CallableVariance\\B): void expects Levels\\CallableVariance\\B, Levels\\CallableVariance\\A given.", + "message": "Parameter #1 of callable callable(Levels\\CallableVariance\\B): void expects Levels\\CallableVariance\\B, Levels\\CallableVariance\\A given.", "line": 14, "ignorable": true }, @@ -39,4 +39,4 @@ "line": 85, "ignorable": true } -] \ No newline at end of file +] diff --git a/tests/PHPStan/Rules/Functions/CallCallablesRuleTest.php b/tests/PHPStan/Rules/Functions/CallCallablesRuleTest.php index bc20e208da..bcd1c9ee87 100644 --- a/tests/PHPStan/Rules/Functions/CallCallablesRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallCallablesRuleTest.php @@ -190,7 +190,7 @@ public function dataBug3566(): array true, [ [ - 'Parameter #1 $ of closure expects int, TMemberType given.', + 'Parameter #1 of closure expects int, TMemberType given.', 29, ], ], @@ -280,7 +280,7 @@ public function testBug6485(): void { $this->analyse([__DIR__ . '/data/bug-6485.php'], [ [ - 'Parameter #1 $ of closure expects never, TBlockType of Bug6485\Block given.', + 'Parameter #1 of closure expects never, TBlockType of Bug6485\Block given.', 33, ], ]); @@ -306,4 +306,14 @@ public function testBug9614(): void $this->analyse([__DIR__ . '/data/bug-9614.php'], []); } + public function testBug10814(): void + { + $this->analyse([__DIR__ . '/data/bug-10814.php'], [ + [ + 'Parameter #1 of closure expects DateTime, DateTimeImmutable given.', + 10, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/bug-10814.php b/tests/PHPStan/Rules/Functions/data/bug-10814.php new file mode 100644 index 0000000000..a1c7ed8cbe --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-10814.php @@ -0,0 +1,12 @@ +