Skip to content

Commit

Permalink
Refactored levels that report named arguments errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@ondrejmirtes
ondrejmirtes committed Nov 17, 2020
1 parent e0a7c7e commit 1e42295
Show file tree
Hide file tree
Showing 4 changed files with 178 additions and 135 deletions.
173 changes: 108 additions & 65 deletions src/Rules/FunctionCallParametersCheck.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,7 @@ public function check(
{
$functionParametersMinCount = 0;
$functionParametersMaxCount = 0;
$parametersByName = [];
$unusedParametersByName = [];
foreach ($parametersAcceptor->getParameters() as $parameter) {
$parametersByName[$parameter->getName()] = $parameter;

if (!$parameter->isVariadic()) {
$unusedParametersByName[$parameter->getName()] = $parameter;
}
if (!$parameter->isOptional()) {
$functionParametersMinCount++;
}
Expand Down Expand Up @@ -215,14 +208,16 @@ public function check(
$errors[] = RuleErrorBuilder::message($messages[7])->build();
}

[$addedErrors, $argumentsWithParameters] = $this->processArguments($parametersAcceptor, $arguments, $hasNamedArguments, $messages[10], $messages[11]);
foreach ($addedErrors as $error) {
$errors[] = $error;
}

if (!$this->checkArgumentTypes && !$this->checkArgumentsPassedByReference) {
return $errors;
}

$parameters = $parametersAcceptor->getParameters();
$namedArgumentAlreadyOccurred = false;

foreach ($arguments as $i => [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine]) {
foreach ($argumentsWithParameters as $i => [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine, $parameter]) {
if ($this->checkArgumentTypes && $unpack) {
$iterableTypeResult = $this->ruleLevelHelper->findTypeToCheck(
$scope,
Expand All @@ -245,54 +240,10 @@ static function (Type $type): bool {
}
}

if ($namedArgumentAlreadyOccurred && $argumentName === null && !$unpack) {
$errors[] = RuleErrorBuilder::message('Named argument cannot be followed by a positional argument.')->line($argumentLine)->nonIgnorable()->build();
if ($parameter === null) {
continue;
}

if ($argumentName === null) {
if (!isset($parameters[$i])) {
if (!$parametersAcceptor->isVariadic() || count($parameters) === 0) {
break;
}

$parameter = $parameters[count($parameters) - 1];
if (!$parameter->isVariadic()) {
break; // func_get_args
}
} else {
$parameter = $parameters[$i];
}
} elseif (array_key_exists($argumentName, $parametersByName)) {
$namedArgumentAlreadyOccurred = true;
$parameter = $parametersByName[$argumentName];
} else {
$namedArgumentAlreadyOccurred = true;

$parametersCount = count($parameters);
if (
count($unusedParametersByName) !== 0
|| !$parametersAcceptor->isVariadic()
|| $parametersCount <= 0
) {
$errors[] = RuleErrorBuilder::message(sprintf($messages[11], $argumentName))->line($argumentLine)->build();
continue;
}

$parameter = $parameters[$parametersCount - 1];
}

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

unset($unusedParametersByName[$parameter->getName()]);

$parameterType = $parameter->getType();
if (
$this->checkArgumentTypes
Expand Down Expand Up @@ -343,27 +294,119 @@ static function (Type $type): bool {
))->line($argumentLine)->build();
}

if ($hasNamedArguments) {
foreach ($unusedParametersByName as $parameter) {
if ($parameter->isOptional()) {
if ($this->checkMissingTypehints) {
foreach ($parametersAcceptor->getResolvedTemplateTypeMap()->getTypes() as $name => $type) {
if (!($type instanceof ErrorType) && !($type instanceof NeverType)) {
continue;
}

$errors[] = RuleErrorBuilder::message(sprintf($messages[10], sprintf('%s (%s)', $parameter->getName(), $parameter->getType()->describe(VerbosityLevel::typeOnly()))))->build();
$errors[] = RuleErrorBuilder::message(sprintf($messages[9], $name))->build();
}
}

if ($this->checkMissingTypehints) {
foreach ($parametersAcceptor->getResolvedTemplateTypeMap()->getTypes() as $name => $type) {
if (!($type instanceof ErrorType) && !($type instanceof NeverType)) {
return $errors;
}

/**
* @param ParametersAcceptor $parametersAcceptor
* @param array<int, array{Expr, Type, bool, string|null, int}> $arguments
* @param bool $hasNamedArguments
* @param string $missingParameterMessage
* @param string $unknownParameterMessage
* @return array{RuleError[], array<int, array{Expr, Type, bool, string|null, int, \PHPStan\Reflection\ParameterReflection|null}>}
*/
private function processArguments(
ParametersAcceptor $parametersAcceptor,
array $arguments,
bool $hasNamedArguments,
string $missingParameterMessage,
string $unknownParameterMessage
): array
{
$parameters = $parametersAcceptor->getParameters();
$parametersByName = [];
$unusedParametersByName = [];
$errors = [];
foreach ($parametersAcceptor->getParameters() as $parameter) {
$parametersByName[$parameter->getName()] = $parameter;

if ($parameter->isVariadic()) {
continue;
}

$unusedParametersByName[$parameter->getName()] = $parameter;
}

$newArguments = [];

$namedArgumentAlreadyOccurred = false;
foreach ($arguments as $i => [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine]) {
if ($argumentName === null) {
if (!isset($parameters[$i])) {
if (!$parametersAcceptor->isVariadic() || count($parameters) === 0) {
$newArguments[$i] = [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine, null];
break;
}

$parameter = $parameters[count($parameters) - 1];
if (!$parameter->isVariadic()) {
$newArguments[$i] = [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine, $parameter];
break; // func_get_args
}
} else {
$parameter = $parameters[$i];
}
} elseif (array_key_exists($argumentName, $parametersByName)) {
$namedArgumentAlreadyOccurred = true;
$parameter = $parametersByName[$argumentName];
} else {
$namedArgumentAlreadyOccurred = true;

$parametersCount = count($parameters);
if (
count($unusedParametersByName) !== 0
|| !$parametersAcceptor->isVariadic()
|| $parametersCount <= 0
) {
$errors[] = RuleErrorBuilder::message(sprintf($unknownParameterMessage, $argumentName))->line($argumentLine)->build();
$newArguments[$i] = [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine, null];
continue;
}

$errors[] = RuleErrorBuilder::message(sprintf($messages[9], $name))->build();
$parameter = $parameters[$parametersCount - 1];
}

if ($namedArgumentAlreadyOccurred && $argumentName === null && !$unpack) {
$errors[] = RuleErrorBuilder::message('Named argument cannot be followed by a positional argument.')->line($argumentLine)->nonIgnorable()->build();
$newArguments[$i] = [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine, null];
continue;
}

$newArguments[$i] = [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine, $parameter];

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

unset($unusedParametersByName[$parameter->getName()]);
}

return $errors;
if ($hasNamedArguments) {
foreach ($unusedParametersByName as $parameter) {
if ($parameter->isOptional()) {
continue;
}

$errors[] = RuleErrorBuilder::message(sprintf($missingParameterMessage, sprintf('%s (%s)', $parameter->getName(), $parameter->getType()->describe(VerbosityLevel::typeOnly()))))->build();
}
}

return [$errors, $newArguments];
}

}
35 changes: 35 additions & 0 deletions tests/PHPStan/Levels/data/namedArguments-0.json
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,42 @@
[
{
"message": "Missing parameter $j (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 19,
"ignorable": true
},
{
"message": "Missing parameter $k (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 19,
"ignorable": true
},
{
"message": "Named argument cannot be followed by a positional argument.",
"line": 21,
"ignorable": false
},
{
"message": "Named argument cannot be followed by a positional argument.",
"line": 22,
"ignorable": false
},
{
"message": "Missing parameter $k (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 24,
"ignorable": true
},
{
"message": "Named argument cannot be followed by an unpacked (...) argument.",
"line": 38,
"ignorable": false
},
{
"message": "Missing parameter $j (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 40,
"ignorable": true
},
{
"message": "Named argument cannot be followed by a positional argument.",
"line": 40,
"ignorable": false
}
]
35 changes: 35 additions & 0 deletions tests/PHPStan/Levels/data/namedArguments-2.json
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,42 @@
[
{
"message": "Missing parameter $j (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 45,
"ignorable": true
},
{
"message": "Missing parameter $k (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 45,
"ignorable": true
},
{
"message": "Named argument cannot be followed by a positional argument.",
"line": 47,
"ignorable": false
},
{
"message": "Named argument cannot be followed by a positional argument.",
"line": 48,
"ignorable": false
},
{
"message": "Missing parameter $k (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 50,
"ignorable": true
},
{
"message": "Named argument cannot be followed by an unpacked (...) argument.",
"line": 64,
"ignorable": false
},
{
"message": "Missing parameter $j (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 66,
"ignorable": true
},
{
"message": "Named argument cannot be followed by a positional argument.",
"line": 66,
"ignorable": false
}
]
70 changes: 0 additions & 70 deletions tests/PHPStan/Levels/data/namedArguments-5.json
View file
Original file line number Diff line number Diff line change
@@ -1,29 +1,4 @@
[
{
"message": "Missing parameter $j (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 19,
"ignorable": true
},
{
"message": "Missing parameter $k (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 19,
"ignorable": true
},
{
"message": "Named argument cannot be followed by a positional argument.",
"line": 21,
"ignorable": false
},
{
"message": "Named argument cannot be followed by a positional argument.",
"line": 22,
"ignorable": false
},
{
"message": "Missing parameter $k (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 24,
"ignorable": true
},
{
"message": "Parameter #1 $i of method NamedArgumentsIntegrationTest\\Foo::doFoo() expects int, string given.",
"line": 29,
Expand All @@ -39,41 +14,6 @@
"line": 39,
"ignorable": true
},
{
"message": "Missing parameter $j (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 40,
"ignorable": true
},
{
"message": "Named argument cannot be followed by a positional argument.",
"line": 40,
"ignorable": false
},
{
"message": "Missing parameter $j (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 45,
"ignorable": true
},
{
"message": "Missing parameter $k (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 45,
"ignorable": true
},
{
"message": "Named argument cannot be followed by a positional argument.",
"line": 47,
"ignorable": false
},
{
"message": "Named argument cannot be followed by a positional argument.",
"line": 48,
"ignorable": false
},
{
"message": "Missing parameter $k (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 50,
"ignorable": true
},
{
"message": "Parameter #1 $i of method NamedArgumentsIntegrationTest\\Foo::doFoo() expects int, string given.",
"line": 55,
Expand All @@ -88,15 +28,5 @@
"message": "Parameter $j of method NamedArgumentsIntegrationTest\\Foo::doFoo() expects int, string given.",
"line": 65,
"ignorable": true
},
{
"message": "Missing parameter $j (int) in call to method NamedArgumentsIntegrationTest\\Foo::doFoo().",
"line": 66,
"ignorable": true
},
{
"message": "Named argument cannot be followed by a positional argument.",
"line": 66,
"ignorable": false
}
]

0 comments on commit 1e42295

Please sign in to comment.