Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor the TooFewArguments check to start checking with named arguments #7348

Merged
merged 3 commits into from Jan 9, 2022
Merged

refactor the TooFewArguments check to start checking with named arguments #7348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c94e25f
refactor the TooFewArguments check to start checking with named argum…
orklah Jan 8, 2022
80f617d
fix test
orklah Jan 8, 2022
9877517
add test
orklah Jan 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
165 changes: 101 additions & 64 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php
View file
Open in desktop
Expand Up @@ -38,20 +38,27 @@
use Psalm\Type;
use Psalm\Type\Atomic\TArray;
use Psalm\Type\Atomic\TCallable;
use Psalm\Type\Atomic\TCallableArray;
use Psalm\Type\Atomic\TCallableKeyedArray;
use Psalm\Type\Atomic\TCallableList;
use Psalm\Type\Atomic\TClosure;
use Psalm\Type\Atomic\TKeyedArray;
use Psalm\Type\Atomic\TList;
use Psalm\Type\Atomic\TLiteralString;
use Psalm\Type\Atomic\TNonEmptyArray;
use Psalm\Type\Atomic\TNonEmptyList;
use Psalm\Type\Atomic\TTemplateParam;
use Psalm\Type\Union;
use UnexpectedValueException;

use function array_map;
use function array_reverse;
use function array_slice;
use function count;
use function in_array;
use function is_string;
use function max;
use function min;
use function reset;
use function strpos;
use function strtolower;
Expand Down Expand Up @@ -561,43 +568,12 @@ public static function checkArgumentsMatch(

$has_packed_var = false;

$packed_var_definite_args = 0;

foreach ($args as $arg) {
if ($arg->unpack) {
$arg_value_type = $statements_analyzer->node_data->getType($arg->value);

if (!$arg_value_type
|| !$arg_value_type->isSingle()
|| !$arg_value_type->hasArray()
) {
$has_packed_var = true;
break;
}

foreach ($arg_value_type->getAtomicTypes() as $atomic_arg_type) {
if (!$atomic_arg_type instanceof TKeyedArray) {
$has_packed_var = true;
break 2;
}

$packed_var_definite_args = 0;

foreach ($atomic_arg_type->properties as $property_type) {
if ($property_type->possibly_undefined) {
$has_packed_var = true;
} else {
$packed_var_definite_args++;
}
}
}
$has_packed_var = true;
}
}

if (!$has_packed_var) {
$packed_var_definite_args = max(0, $packed_var_definite_args - 1);
}

$last_param = $function_params
? $function_params[count($function_params) - 1]
: null;
Expand Down Expand Up @@ -955,9 +931,7 @@ public static function checkArgumentsMatch(
$in_call_map,
$method_id,
$cased_method_id,
$code_location,
$has_packed_var,
$packed_var_definite_args
$code_location
);

return null;
Expand Down Expand Up @@ -1469,9 +1443,7 @@ private static function checkArgCount(
bool $in_call_map,
$method_id,
?string $cased_method_id,
CodeLocation $code_location,
bool $has_packed_var,
int $packed_var_definite_args
CodeLocation $code_location
): void {
if (!$is_variadic
&& count($args) > count($function_params)
Expand All @@ -1495,48 +1467,113 @@ private static function checkArgCount(
return;
}

if (!$has_packed_var && count($args) < count($function_params)) {
if ($function_storage) {
$expected_param_count = $function_storage->required_param_count;
} else {
for ($i = 0, $j = count($function_params); $i < $j; ++$i) {
$param = $function_params[$i];
if (count($args) < count($function_params)) {
//we're gonna loop over given args and unset them from the function_params.
// If some mandatory params are left at the end, we'll throw an error
foreach ($args as $arg) {
// when the argument is not named, we can remove the params in order
if ($arg->name === null) {
// if we're unpacking, we try to unset the exact number of params, if we can't we give up and return
if ($arg->unpack) {
$arg_value_type = $statements_analyzer->node_data->getType($arg->value);

if (!$arg_value_type || !$arg_value_type->hasArray()) {
return;
}

if ($param->is_optional || $param->is_variadic) {
break;
if ($arg_value_type->isSingle()
&& ($atomic_arg_type = $arg_value_type->getSingleAtomic())
&& $atomic_arg_type instanceof TKeyedArray
&& !$atomic_arg_type->is_list
) {
//if we have a single shape, we'll check param names
foreach ($atomic_arg_type->properties as $property_name => $_property_type) {
foreach ($function_params as $k => $param) {
if ($param->name === $property_name) {
unset($function_params[$k]);
}
}
}
continue;
}

foreach ($arg_value_type->getAtomicTypes() as $atomic_arg_type) {
$packed_var_definite_args_tmp = [];
if ($atomic_arg_type instanceof TCallableArray ||
$atomic_arg_type instanceof TCallableList ||
$atomic_arg_type instanceof TCallableKeyedArray
) {
$packed_var_definite_args_tmp[] = 2;
} elseif ($atomic_arg_type instanceof TKeyedArray) {
if (!$atomic_arg_type->sealed) {
return;
}

foreach ($atomic_arg_type->properties as $property_type) {
if ($property_type->possibly_undefined) {
return;
}
}
//we did not return. The number of packed params is the number of properties
$packed_var_definite_args_tmp[] = count($atomic_arg_type->properties);
} elseif ($atomic_arg_type instanceof TNonEmptyArray ||
$atomic_arg_type instanceof TNonEmptyList
) {
if ($atomic_arg_type->count === null) {
return;
}

$packed_var_definite_args_tmp[] = $atomic_arg_type->count;
} elseif ($atomic_arg_type instanceof TArray
&& $atomic_arg_type->type_params[1]->isEmpty()
) {
$packed_var_definite_args_tmp[] = 0;
} else {
return;
}


if (min($packed_var_definite_args_tmp) === max($packed_var_definite_args_tmp)) {
//we have a stable number of params
$packed_var_definite_args = $packed_var_definite_args_tmp[0];
} else {
return;
}
}
} else {
//if we're not unpacking, we remove the first param
$packed_var_definite_args = 1;
}

$function_params = array_slice($function_params, $packed_var_definite_args);
continue;
}

$expected_param_count = $i;
foreach ($function_params as $k => $param) {
if ($param->name === $arg->name->name) {
unset($function_params[$k]);
continue;
}
}
}

for ($i = count($args) + $packed_var_definite_args, $j = count($function_params); $i < $j; ++$i) {
$param = $function_params[$i];

if (!$param->is_optional
&& !$param->is_variadic
&& ($in_call_map
|| !$function_storage instanceof MethodStorage
|| $function_storage->is_static
|| ($method_id instanceof MethodIdentifier
&& $method_id->method_name === '__construct'))
) {
//we're now left with an array of params that were not passed.
// If they're mandatory, throw an error. Otherwise, we compute the default value
foreach ($function_params as $i => $param) {
if (!$param->is_optional && !$param->is_variadic) {
IssueBuffer::maybeAdd(
new TooFewArguments(
'Too few arguments for ' . $cased_method_id
. ' - expecting ' . $expected_param_count
. ' but saw ' . (count($args) + $packed_var_definite_args),
. ' - expecting ' . $param->name . ' to be passed',
$code_location,
(string)$method_id
),
$statements_analyzer->getSuppressedIssues()
);

break;
continue;
}

if ($param->is_optional
&& $param->type
if ($param->type
&& $param->default_type
&& !$param->is_variadic
&& $template_result
Expand Down
18 changes: 18 additions & 0 deletions tests/ArgTest.php
View file
Open in desktop
Expand Up @@ -699,6 +699,24 @@ function takesObject($_o): void {}
',
'error_message' => 'ArgumentTypeCoercion',
],
'MissingMandatoryParamWithNamedParams' => [
'<?php
class User
{
public function __construct(
protected string $name,
protected string $problematicOne,
protected string $id = "",
){}
}

new User(
name: "John",
id: "asd",
);
',
'error_message' => 'TooFewArguments',
],
];
}
}
2 changes: 1 addition & 1 deletion tests/FunctionCallTest.php
View file
Open in desktop
Expand Up @@ -2160,7 +2160,7 @@ function bar($s) : void {
'tooFewArgsAccurateCount' => [
'<?php
preg_match(\'/adsf/\');',
'error_message' => 'TooFewArguments - src' . DIRECTORY_SEPARATOR . 'somefile.php:2:21 - Too few arguments for preg_match - expecting 2 but saw 1',
'error_message' => 'TooFewArguments - src' . DIRECTORY_SEPARATOR . 'somefile.php:2:21 - Too few arguments for preg_match - expecting subject to be passed',
],
'compactUndefinedVariable' => [
'<?php
Expand Down