Skip to content

Commit f71da02

Browse files
takaramondrejmirtes
authored andcommitted
Improve error messages on unnamed parameters
Closes phpstan/phpstan#10814
1 parent 074de75 commit f71da02

File tree

4 files changed

+47
-22
lines changed

4 files changed

+47
-22
lines changed

src/Rules/FunctionCallParametersCheck.php

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use function array_fill;
2727
use function array_key_exists;
2828
use function count;
29+
use function implode;
2930
use function is_string;
3031
use function max;
3132
use function sprintf;
@@ -260,14 +261,9 @@ public function check(
260261

261262
if (!$accepts->result) {
262263
$verbosityLevel = VerbosityLevel::getRecommendedLevelByType($parameterType, $argumentValueType);
263-
$parameterDescription = sprintf('%s$%s', $parameter->isVariadic() ? '...' : '', $parameter->getName());
264264
$errors[] = RuleErrorBuilder::message(sprintf(
265265
$messages[6],
266-
$argumentName === null ? sprintf(
267-
'#%d %s',
268-
$i + 1,
269-
$parameterDescription,
270-
) : $parameterDescription,
266+
$this->describeParameter($parameter, $argumentName === null ? $i + 1 : null),
271267
$parameterType->describe($verbosityLevel),
272268
$argumentValueType->describe($verbosityLevel),
273269
))->line($argumentLine)->acceptsReasonsTip($accepts->reasons)->build();
@@ -280,14 +276,9 @@ public function check(
280276
&& !$this->unresolvableTypeHelper->containsUnresolvableType($originalParameter->getType())
281277
&& $this->unresolvableTypeHelper->containsUnresolvableType($parameterType)
282278
) {
283-
$parameterDescription = sprintf('%s$%s', $parameter->isVariadic() ? '...' : '', $parameter->getName());
284279
$errors[] = RuleErrorBuilder::message(sprintf(
285280
$messages[13],
286-
$argumentName === null ? sprintf(
287-
'#%d %s',
288-
$i + 1,
289-
$parameterDescription,
290-
) : $parameterDescription,
281+
$this->describeParameter($parameter, $argumentName === null ? $i + 1 : null),
291282
))->line($argumentLine)->build();
292283
}
293284
}
@@ -300,10 +291,9 @@ public function check(
300291
}
301292

302293
if ($this->nullsafeCheck->containsNullSafe($argumentValue)) {
303-
$parameterDescription = sprintf('%s$%s', $parameter->isVariadic() ? '...' : '', $parameter->getName());
304294
$errors[] = RuleErrorBuilder::message(sprintf(
305295
$messages[8],
306-
$argumentName === null ? sprintf('#%d %s', $i + 1, $parameterDescription) : $parameterDescription,
296+
$this->describeParameter($parameter, $argumentName === null ? $i + 1 : null),
307297
))->line($argumentLine)->build();
308298
continue;
309299
}
@@ -327,10 +317,9 @@ public function check(
327317
$propertyDescription = sprintf('readonly property %s::$%s', $propertyReflection->getDeclaringClass()->getDisplayName(), $propertyReflection->getName());
328318
}
329319

330-
$parameterDescription = sprintf('%s$%s', $parameter->isVariadic() ? '...' : '', $parameter->getName());
331320
$errors[] = RuleErrorBuilder::message(sprintf(
332321
'Parameter %s is passed by reference so it does not accept %s.',
333-
$argumentName === null ? sprintf('#%d %s', $i + 1, $parameterDescription) : $parameterDescription,
322+
$this->describeParameter($parameter, $argumentName === null ? $i + 1 : null),
334323
$propertyDescription,
335324
))->line($argumentLine)->build();
336325
}
@@ -343,10 +332,9 @@ public function check(
343332
continue;
344333
}
345334

346-
$parameterDescription = sprintf('%s$%s', $parameter->isVariadic() ? '...' : '', $parameter->getName());
347335
$errors[] = RuleErrorBuilder::message(sprintf(
348336
$messages[8],
349-
$argumentName === null ? sprintf('#%d %s', $i + 1, $parameterDescription) : $parameterDescription,
337+
$this->describeParameter($parameter, $argumentName === null ? $i + 1 : null),
350338
))->line($argumentLine)->build();
351339
}
352340

@@ -526,4 +514,19 @@ private function processArguments(
526514
return [$errors, $newArguments];
527515
}
528516

517+
private function describeParameter(ParameterReflection $parameter, ?int $position): string
518+
{
519+
$parts = [];
520+
if ($position !== null) {
521+
$parts[] = '#' . $position;
522+
}
523+
524+
$name = $parameter->getName();
525+
if ($name !== '') {
526+
$parts[] = ($parameter->isVariadic() ? '...$' : '$') . $name;
527+
}
528+
529+
return implode(' ', $parts);
530+
}
531+
529532
}

tests/PHPStan/Levels/data/callableVariance-5.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[
22
{
3-
"message": "Parameter #1 $ of callable callable(Levels\\CallableVariance\\B): void expects Levels\\CallableVariance\\B, Levels\\CallableVariance\\A given.",
3+
"message": "Parameter #1 of callable callable(Levels\\CallableVariance\\B): void expects Levels\\CallableVariance\\B, Levels\\CallableVariance\\A given.",
44
"line": 14,
55
"ignorable": true
66
},
@@ -39,4 +39,4 @@
3939
"line": 85,
4040
"ignorable": true
4141
}
42-
]
42+
]

tests/PHPStan/Rules/Functions/CallCallablesRuleTest.php

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ public function dataBug3566(): array
190190
true,
191191
[
192192
[
193-
'Parameter #1 $ of closure expects int, TMemberType given.',
193+
'Parameter #1 of closure expects int, TMemberType given.',
194194
29,
195195
],
196196
],
@@ -280,7 +280,7 @@ public function testBug6485(): void
280280
{
281281
$this->analyse([__DIR__ . '/data/bug-6485.php'], [
282282
[
283-
'Parameter #1 $ of closure expects never, TBlockType of Bug6485\Block given.',
283+
'Parameter #1 of closure expects never, TBlockType of Bug6485\Block given.',
284284
33,
285285
],
286286
]);
@@ -306,4 +306,14 @@ public function testBug9614(): void
306306
$this->analyse([__DIR__ . '/data/bug-9614.php'], []);
307307
}
308308

309+
public function testBug10814(): void
310+
{
311+
$this->analyse([__DIR__ . '/data/bug-10814.php'], [
312+
[
313+
'Parameter #1 of closure expects DateTime, DateTimeImmutable given.',
314+
10,
315+
],
316+
]);
317+
}
318+
309319
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug10814;
4+
5+
class HelloWorld
6+
{
7+
/** @param \Closure(\DateTime): void $fx */
8+
public function foo($fx): void
9+
{
10+
$fx(new \DateTimeImmutable());
11+
}
12+
}

0 commit comments

Comments
 (0)