Skip to content

Commit c507ae2

Browse files
ondrejmirtesondrejmirtes
committed
FunctionCallParametersCheck - argument errors reported on more precise lines
1 parent 8827841 commit c507ae2

4 files changed

Lines changed: 53 additions & 25 deletions

File tree

‎src/DependencyInjection/ContainerFactory.php‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,13 +104,12 @@ public function create(
104104

105105
BetterReflection::$phpVersion = $container->getByType(PhpVersion::class)->getVersionId();
106106

107-
// @phpstan-ignore-next-line
108107
BetterReflection::populate(
109-
$container->getService('betterReflectionSourceLocator'),
110-
$container->getService('betterReflectionClassReflector'),
111-
$container->getService('betterReflectionFunctionReflector'),
112-
$container->getService('betterReflectionConstantReflector'),
113-
$container->getService('phpParserDecorator'),
108+
$container->getService('betterReflectionSourceLocator'), // @phpstan-ignore-line
109+
$container->getService('betterReflectionClassReflector'), // @phpstan-ignore-line
110+
$container->getService('betterReflectionFunctionReflector'), // @phpstan-ignore-line
111+
$container->getService('betterReflectionConstantReflector'), // @phpstan-ignore-line
112+
$container->getService('phpParserDecorator'), // @phpstan-ignore-line
114113
$container->getByType(PhpStormStubsSourceStubber::class)
115114
);
116115

‎src/Rules/FunctionCallParametersCheck.php‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -80,17 +80,20 @@ public function check(
8080
$functionParametersMaxCount = -1;
8181
}
8282

83-
/** @var array<int, array{Expr, Type, bool, string|null}> $arguments */
83+
/** @var array<int, array{Expr, Type, bool, string|null, int}> $arguments */
8484
$arguments = [];
8585
/** @var array<int, \PhpParser\Node\Arg> $args */
8686
$args = $funcCall->args;
8787
$hasNamedArguments = false;
8888
$errors = [];
8989
foreach ($args as $i => $arg) {
9090
$type = $scope->getType($arg->value);
91-
if ($hasNamedArguments && $arg->name === null) {
91+
if ($hasNamedArguments && $arg->name === null && !$arg->unpack) {
9292
$errors[] = RuleErrorBuilder::message('Named argument cannot be followed by a positional argument.')->line($arg->getLine())->nonIgnorable()->build();
9393
}
94+
if ($hasNamedArguments && $arg->unpack) {
95+
$errors[] = RuleErrorBuilder::message('Named argument cannot be followed by an unpacked (...) argument.')->line($arg->getLine())->nonIgnorable()->build();
96+
}
9497
$argumentName = null;
9598
if ($arg->name !== null) {
9699
$hasNamedArguments = true;
@@ -111,22 +114,35 @@ public function check(
111114

112115
for ($j = 0; $j < $minKeys; $j++) {
113116
$types = [];
117+
$commonKey = null;
114118
foreach ($arrays as $constantArray) {
115119
$types[] = $constantArray->getValueTypes()[$j];
120+
$keyType = $constantArray->getKeyTypes()[$j];
121+
if ($commonKey === null) {
122+
$commonKey = $keyType->getValue();
123+
} elseif ($commonKey !== $keyType->getValue()) {
124+
$commonKey = false;
125+
}
126+
}
127+
$keyArgumentName = null;
128+
if (is_string($commonKey)) {
129+
$keyArgumentName = $commonKey;
116130
}
117131
$arguments[] = [
118132
$arg->value,
119133
TypeCombinator::union(...$types),
120134
false,
121-
$argumentName,
135+
$keyArgumentName,
136+
$arg->getLine(),
122137
];
123138
}
124139
} else {
125140
$arguments[] = [
126141
$arg->value,
127142
$type->getIterableValueType(),
128143
true,
129-
$argumentName,
144+
null,
145+
$arg->getLine(),
130146
];
131147
}
132148
continue;
@@ -137,6 +153,7 @@ public function check(
137153
$type,
138154
false,
139155
$argumentName,
156+
$arg->getLine(),
140157
];
141158
}
142159

@@ -195,7 +212,7 @@ public function check(
195212
$parameters = $parametersAcceptor->getParameters();
196213
$alreadyPassedParameters = [];
197214

198-
foreach ($arguments as $i => [$argumentValue, $argumentValueType, $unpack, $argumentName]) {
215+
foreach ($arguments as $i => [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine]) {
199216
if ($this->checkArgumentTypes && $unpack) {
200217
$iterableTypeResult = $this->ruleLevelHelper->findTypeToCheck(
201218
$scope,
@@ -214,7 +231,7 @@ static function (Type $type): bool {
214231
'Only iterables can be unpacked, %s given in argument #%d.',
215232
$iterableTypeResultType->describe(VerbosityLevel::typeOnly()),
216233
$i + 1
217-
))->build();
234+
))->line($argumentLine)->build();
218235
}
219236
}
220237

@@ -234,15 +251,15 @@ static function (Type $type): bool {
234251
} elseif (array_key_exists($argumentName, $parametersByName)) {
235252
$parameter = $parametersByName[$argumentName];
236253
} else {
237-
$errors[] = RuleErrorBuilder::message(sprintf($messages[11], $argumentName))->build();
254+
$errors[] = RuleErrorBuilder::message(sprintf($messages[11], $argumentName))->line($argumentLine)->build();
238255
continue;
239256
}
240257

241258
if (
242259
$hasNamedArguments
243260
&& array_key_exists($parameter->getName(), $alreadyPassedParameters)
244261
) {
245-
$errors[] = RuleErrorBuilder::message(sprintf('Argument for parameter $%s has already been passed.', $parameter->getName()))->build();
262+
$errors[] = RuleErrorBuilder::message(sprintf('Argument for parameter $%s has already been passed.', $parameter->getName()))->line($argumentLine)->build();
246263
continue;
247264
}
248265

@@ -265,7 +282,7 @@ static function (Type $type): bool {
265282
) : $parameterDescription,
266283
$parameterType->describe($verbosityLevel),
267284
$argumentValueType->describe($verbosityLevel)
268-
))->build();
285+
))->line($argumentLine)->build();
269286
}
270287

271288
if (
@@ -280,7 +297,7 @@ static function (Type $type): bool {
280297
$errors[] = RuleErrorBuilder::message(sprintf(
281298
$messages[8],
282299
$argumentName === null ? sprintf('#%d %s', $i + 1, $parameterDescription) : $parameterDescription
283-
))->build();
300+
))->line($argumentLine)->build();
284301
continue;
285302
}
286303

@@ -295,7 +312,7 @@ static function (Type $type): bool {
295312
$errors[] = RuleErrorBuilder::message(sprintf(
296313
$messages[8],
297314
$argumentName === null ? sprintf('#%d %s', $i + 1, $parameterDescription) : $parameterDescription
298-
))->build();
315+
))->line($argumentLine)->build();
299316
}
300317

301318
if ($hasNamedArguments) {

‎tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -442,15 +442,15 @@ public function testCallMethods(): void
442442
],
443443
[
444444
'Only iterables can be unpacked, array<int>|null given in argument #5.',
445-
1456,
445+
1459,
446446
],
447447
[
448448
'Only iterables can be unpacked, int given in argument #6.',
449-
1456,
449+
1460,
450450
],
451451
[
452452
'Only iterables can be unpacked, string given in argument #7.',
453-
1456,
453+
1461,
454454
],
455455
[
456456
'Parameter #1 $s of method Test\ClassStringWithUpperBounds::doFoo() expects class-string<Exception>, string given.',
@@ -1626,27 +1626,27 @@ public function testNamedArguments(): void
16261626
],
16271627
[
16281628
'Argument for parameter $i has already been passed.',
1629-
24,
1629+
26,
16301630
],
16311631
[
16321632
'Argument for parameter $i has already been passed.',
1633-
30,
1633+
32,
16341634
],
16351635
[
16361636
'Missing parameter $k (int) in call to method NamedArgumentsMethod\Foo::doFoo().',
16371637
37,
16381638
],
16391639
[
16401640
'Unknown parameter $z in call to method NamedArgumentsMethod\Foo::doFoo().',
1641-
42,
1641+
46,
16421642
],
16431643
[
16441644
'Parameter #1 $i of method NamedArgumentsMethod\Foo::doFoo() expects int, string given.',
1645-
49,
1645+
50,
16461646
],
16471647
[
16481648
'Parameter $j of method NamedArgumentsMethod\Foo::doFoo() expects int, string given.',
1649-
55,
1649+
57,
16501650
],
16511651
[
16521652
'Parameter $i of method NamedArgumentsMethod\Foo::doBaz() is passed by reference, so it expects variables only.',
@@ -1656,6 +1656,14 @@ public function testNamedArguments(): void
16561656
'Parameter $i of method NamedArgumentsMethod\Foo::doBaz() is passed by reference, so it expects variables only.',
16571657
71,
16581658
],
1659+
[
1660+
'Named argument cannot be followed by an unpacked (...) argument.',
1661+
73,
1662+
],
1663+
[
1664+
'Parameter $j of method NamedArgumentsMethod\Foo::doFoo() expects int, string given.',
1665+
75,
1666+
],
16591667
]);
16601668
}
16611669

‎tests/PHPStan/Rules/Methods/data/named-arguments.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ public function doLorem(?\stdClass $foo): void
6969
{
7070
$this->doBaz(i: 1);
7171
$this->doBaz(i: $foo?->bar);
72+
73+
$this->doFoo(i: 1, ...['j' => 2, 'k' => 3]);
74+
75+
$this->doFoo(...['k' => 3, 'i' => 1, 'j' => 'str']);
7276
}
7377

7478
}

0 commit comments

Comments
 (0)