Skip to content

Commit

Permalink
FunctionCallParametersCheck - argument errors reported on more precis…
Browse files Browse the repository at this point in the history 
…e lines
  • Loading branch information
@ondrejmirtes
ondrejmirtes committed Nov 16, 2020
1 parent 8827841 commit c507ae2
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 25 deletions.
11 changes: 5 additions & 6 deletions src/DependencyInjection/ContainerFactory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,12 @@ public function create(

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

// @phpstan-ignore-next-line
BetterReflection::populate(
$container->getService('betterReflectionSourceLocator'),
$container->getService('betterReflectionClassReflector'),
$container->getService('betterReflectionFunctionReflector'),
$container->getService('betterReflectionConstantReflector'),
$container->getService('phpParserDecorator'),
$container->getService('betterReflectionSourceLocator'), // @phpstan-ignore-line
$container->getService('betterReflectionClassReflector'), // @phpstan-ignore-line
$container->getService('betterReflectionFunctionReflector'), // @phpstan-ignore-line
$container->getService('betterReflectionConstantReflector'), // @phpstan-ignore-line
$container->getService('phpParserDecorator'), // @phpstan-ignore-line
$container->getByType(PhpStormStubsSourceStubber::class)
);

Expand Down
39 changes: 28 additions & 11 deletions src/Rules/FunctionCallParametersCheck.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,20 @@ public function check(
$functionParametersMaxCount = -1;
}

/** @var array<int, array{Expr, Type, bool, string|null}> $arguments */
/** @var array<int, array{Expr, Type, bool, string|null, int}> $arguments */
$arguments = [];
/** @var array<int, \PhpParser\Node\Arg> $args */
$args = $funcCall->args;
$hasNamedArguments = false;
$errors = [];
foreach ($args as $i => $arg) {
$type = $scope->getType($arg->value);
if ($hasNamedArguments && $arg->name === null) {
if ($hasNamedArguments && $arg->name === null && !$arg->unpack) {
$errors[] = RuleErrorBuilder::message('Named argument cannot be followed by a positional argument.')->line($arg->getLine())->nonIgnorable()->build();
}
if ($hasNamedArguments && $arg->unpack) {
$errors[] = RuleErrorBuilder::message('Named argument cannot be followed by an unpacked (...) argument.')->line($arg->getLine())->nonIgnorable()->build();
}
$argumentName = null;
if ($arg->name !== null) {
$hasNamedArguments = true;
Expand All @@ -111,22 +114,35 @@ public function check(

for ($j = 0; $j < $minKeys; $j++) {
$types = [];
$commonKey = null;
foreach ($arrays as $constantArray) {
$types[] = $constantArray->getValueTypes()[$j];
$keyType = $constantArray->getKeyTypes()[$j];
if ($commonKey === null) {
$commonKey = $keyType->getValue();
} elseif ($commonKey !== $keyType->getValue()) {
$commonKey = false;
}
}
$keyArgumentName = null;
if (is_string($commonKey)) {
$keyArgumentName = $commonKey;
}
$arguments[] = [
$arg->value,
TypeCombinator::union(...$types),
false,
$argumentName,
$keyArgumentName,
$arg->getLine(),
];
}
} else {
$arguments[] = [
$arg->value,
$type->getIterableValueType(),
true,
$argumentName,
null,
$arg->getLine(),
];
}
continue;
Expand All @@ -137,6 +153,7 @@ public function check(
$type,
false,
$argumentName,
$arg->getLine(),
];
}

Expand Down Expand Up @@ -195,7 +212,7 @@ public function check(
$parameters = $parametersAcceptor->getParameters();
$alreadyPassedParameters = [];

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

Expand All @@ -234,15 +251,15 @@ static function (Type $type): bool {
} elseif (array_key_exists($argumentName, $parametersByName)) {
$parameter = $parametersByName[$argumentName];
} else {
$errors[] = RuleErrorBuilder::message(sprintf($messages[11], $argumentName))->build();
$errors[] = RuleErrorBuilder::message(sprintf($messages[11], $argumentName))->line($argumentLine)->build();
continue;
}

if (
$hasNamedArguments
&& array_key_exists($parameter->getName(), $alreadyPassedParameters)
) {
$errors[] = RuleErrorBuilder::message(sprintf('Argument for parameter $%s has already been passed.', $parameter->getName()))->build();
$errors[] = RuleErrorBuilder::message(sprintf('Argument for parameter $%s has already been passed.', $parameter->getName()))->line($argumentLine)->build();
continue;
}

Expand All @@ -265,7 +282,7 @@ static function (Type $type): bool {
) : $parameterDescription,
$parameterType->describe($verbosityLevel),
$argumentValueType->describe($verbosityLevel)
))->build();
))->line($argumentLine)->build();
}

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

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

if ($hasNamedArguments) {
Expand Down
24 changes: 16 additions & 8 deletions tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,15 +442,15 @@ public function testCallMethods(): void
],
[
'Only iterables can be unpacked, array<int>|null given in argument #5.',
1456,
1459,
],
[
'Only iterables can be unpacked, int given in argument #6.',
1456,
1460,
],
[
'Only iterables can be unpacked, string given in argument #7.',
1456,
1461,
],
[
'Parameter #1 $s of method Test\ClassStringWithUpperBounds::doFoo() expects class-string<Exception>, string given.',
Expand Down Expand Up @@ -1626,27 +1626,27 @@ public function testNamedArguments(): void
],
[
'Argument for parameter $i has already been passed.',
24,
26,
],
[
'Argument for parameter $i has already been passed.',
30,
32,
],
[
'Missing parameter $k (int) in call to method NamedArgumentsMethod\Foo::doFoo().',
37,
],
[
'Unknown parameter $z in call to method NamedArgumentsMethod\Foo::doFoo().',
42,
46,
],
[
'Parameter #1 $i of method NamedArgumentsMethod\Foo::doFoo() expects int, string given.',
49,
50,
],
[
'Parameter $j of method NamedArgumentsMethod\Foo::doFoo() expects int, string given.',
55,
57,
],
[
'Parameter $i of method NamedArgumentsMethod\Foo::doBaz() is passed by reference, so it expects variables only.',
Expand All @@ -1656,6 +1656,14 @@ public function testNamedArguments(): void
'Parameter $i of method NamedArgumentsMethod\Foo::doBaz() is passed by reference, so it expects variables only.',
71,
],
[
'Named argument cannot be followed by an unpacked (...) argument.',
73,
],
[
'Parameter $j of method NamedArgumentsMethod\Foo::doFoo() expects int, string given.',
75,
],
]);
}

Expand Down
4 changes: 4 additions & 0 deletions tests/PHPStan/Rules/Methods/data/named-arguments.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ public function doLorem(?\stdClass $foo): void
{
$this->doBaz(i: 1);
$this->doBaz(i: $foo?->bar);

$this->doFoo(i: 1, ...['j' => 2, 'k' => 3]);

$this->doFoo(...['k' => 3, 'i' => 1, 'j' => 'str']);
}

}

0 comments on commit c507ae2

Please sign in to comment.