1
0
mirror of https://github.com/danog/psalm.git synced 2024-11-30 04:39:00 +01:00

Use more accurate arguments count

This commit is contained in:
Matt Brown 2020-10-14 18:51:15 -04:00 committed by Daniil Gentili
parent c00bc4ee51
commit 8d1de7757d
Signed by: danog
GPG Key ID: 8C1BE3B34B230CA7
9 changed files with 225 additions and 152 deletions

View File

@ -45,6 +45,7 @@ class TemplateAnalyzer extends Psalm\Internal\Analyzer\FileAnalyzer
throw new \InvalidArgumentException('Could not interpret doc comment correctly'); throw new \InvalidArgumentException('Could not interpret doc comment correctly');
} }
/** @psalm-suppress ArgumentTypeCoercion */
$method_id = new \Psalm\Internal\MethodIdentifier(...explode('::', $matches[1])); $method_id = new \Psalm\Internal\MethodIdentifier(...explode('::', $matches[1]));
$this_params = $this->checkMethod($method_id, $first_stmt, $codebase); $this_params = $this->checkMethod($method_id, $first_stmt, $codebase);

View File

@ -42,6 +42,7 @@ class FunctionCasingChecker implements AfterFunctionCallAnalysisInterface, After
} }
try { try {
/** @psalm-suppress ArgumentTypeCoercion */
$method_id = new \Psalm\Internal\MethodIdentifier(...explode('::', $declaring_method_id)); $method_id = new \Psalm\Internal\MethodIdentifier(...explode('::', $declaring_method_id));
$function_storage = $codebase->methods->getStorage($method_id); $function_storage = $codebase->methods->getStorage($method_id);

View File

@ -902,6 +902,7 @@ class Codebase
if (strpos($symbol, '()')) { if (strpos($symbol, '()')) {
$symbol = substr($symbol, 0, -2); $symbol = substr($symbol, 0, -2);
/** @psalm-suppress ArgumentTypeCoercion */
$method_id = new \Psalm\Internal\MethodIdentifier(...explode('::', $symbol)); $method_id = new \Psalm\Internal\MethodIdentifier(...explode('::', $symbol));
$declaring_method_id = $this->methods->getDeclaringMethodId($method_id); $declaring_method_id = $this->methods->getDeclaringMethodId($method_id);
@ -987,6 +988,7 @@ class Codebase
if (strpos($symbol, '()')) { if (strpos($symbol, '()')) {
$symbol = substr($symbol, 0, -2); $symbol = substr($symbol, 0, -2);
/** @psalm-suppress ArgumentTypeCoercion */
$method_id = new \Psalm\Internal\MethodIdentifier(...explode('::', $symbol)); $method_id = new \Psalm\Internal\MethodIdentifier(...explode('::', $symbol));
$declaring_method_id = $this->methods->getDeclaringMethodId($method_id); $declaring_method_id = $this->methods->getDeclaringMethodId($method_id);
@ -1162,6 +1164,7 @@ class Codebase
public function getSignatureInformation(string $function_symbol) : ?\LanguageServerProtocol\SignatureInformation public function getSignatureInformation(string $function_symbol) : ?\LanguageServerProtocol\SignatureInformation
{ {
if (strpos($function_symbol, '::') !== false) { if (strpos($function_symbol, '::') !== false) {
/** @psalm-suppress ArgumentTypeCoercion */
$method_id = new \Psalm\Internal\MethodIdentifier(...explode('::', $function_symbol)); $method_id = new \Psalm\Internal\MethodIdentifier(...explode('::', $function_symbol));
$declaring_method_id = $this->methods->getDeclaringMethodId($method_id); $declaring_method_id = $this->methods->getDeclaringMethodId($method_id);

View File

@ -60,6 +60,7 @@ class ArgumentAnalyzer
CodeLocation $function_call_location, CodeLocation $function_call_location,
?FunctionLikeParameter $function_param, ?FunctionLikeParameter $function_param,
int $argument_offset, int $argument_offset,
int $unpacked_argument_offset,
bool $allow_named_args, bool $allow_named_args,
PhpParser\Node\Arg $arg, PhpParser\Node\Arg $arg,
?Type\Union $arg_value_type, ?Type\Union $arg_value_type,
@ -175,6 +176,7 @@ class ArgumentAnalyzer
$allow_named_args, $allow_named_args,
$arg_value_type, $arg_value_type,
$argument_offset, $argument_offset,
$unpacked_argument_offset,
$arg, $arg,
$context, $context,
$class_generic_params, $class_generic_params,
@ -205,6 +207,7 @@ class ArgumentAnalyzer
bool $allow_named_args, bool $allow_named_args,
Type\Union $arg_type, Type\Union $arg_type,
int $argument_offset, int $argument_offset,
int $unpacked_argument_offset,
PhpParser\Node\Arg $arg, PhpParser\Node\Arg $arg,
Context $context, Context $context,
?array $class_generic_params, ?array $class_generic_params,
@ -413,18 +416,19 @@ class ArgumentAnalyzer
$unpacked_atomic_array = $arg_type->getAtomicTypes()['array']; $unpacked_atomic_array = $arg_type->getAtomicTypes()['array'];
if ($unpacked_atomic_array instanceof Type\Atomic\TKeyedArray) { if ($unpacked_atomic_array instanceof Type\Atomic\TKeyedArray) {
if ($codebase->php_major_version >= 8 if ($function_param->is_variadic) {
$arg_type = $unpacked_atomic_array->getGenericValueType();
} elseif ($codebase->php_major_version >= 8
&& $allow_named_args && $allow_named_args
&& isset($unpacked_atomic_array->properties[$function_param->name]) && isset($unpacked_atomic_array->properties[$function_param->name])
) { ) {
$arg_type = clone $unpacked_atomic_array->properties[$function_param->name]; $arg_type = clone $unpacked_atomic_array->properties[$function_param->name];
} elseif ($unpacked_atomic_array->is_list } elseif ($unpacked_atomic_array->is_list
&& $argument_offset === 0 && isset($unpacked_atomic_array->properties[$unpacked_argument_offset])
&& isset($unpacked_atomic_array->properties[$argument_offset])
) { ) {
$arg_type = clone $unpacked_atomic_array->properties[$argument_offset]; $arg_type = clone $unpacked_atomic_array->properties[$unpacked_argument_offset];
} else { } else {
$arg_type = $unpacked_atomic_array->getGenericValueType(); $arg_type = Type::getMixed();
} }
} elseif ($unpacked_atomic_array instanceof Type\Atomic\TList) { } elseif ($unpacked_atomic_array instanceof Type\Atomic\TList) {
$arg_type = $unpacked_atomic_array->type_param; $arg_type = $unpacked_atomic_array->type_param;

View File

@ -498,8 +498,41 @@ class ArgumentsAnalyzer
$has_packed_var = false; $has_packed_var = false;
$packed_var_definite_args = 0;
foreach ($args as $arg) { foreach ($args as $arg) {
$has_packed_var = $has_packed_var || $arg->unpack; 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++;
}
}
}
}
}
if (!$has_packed_var) {
$packed_var_definite_args = \max(0, $packed_var_definite_args - 1);
} }
$last_param = $function_params $last_param = $function_params
@ -600,6 +633,7 @@ class ArgumentsAnalyzer
$code_location, $code_location,
$function_params[$i], $function_params[$i],
$i, $i,
$i,
$function_storage ? $function_storage->allow_named_arg_calls : true, $function_storage ? $function_storage->allow_named_arg_calls : true,
new PhpParser\Node\Arg( new PhpParser\Node\Arg(
StubsGenerator::getExpressionFromType($function_params[$i]->default_type) StubsGenerator::getExpressionFromType($function_params[$i]->default_type)
@ -620,17 +654,21 @@ class ArgumentsAnalyzer
} }
foreach ($args as $argument_offset => $arg) { foreach ($args as $argument_offset => $arg) {
$function_param = null; $arg_function_params = [];
if ($arg->name && $function_storage && $function_storage->allow_named_arg_calls) { if ($arg->unpack && $function_param_count > $argument_offset) {
for ($i = $argument_offset; $i < $function_param_count; $i++) {
$arg_function_params[] = $function_params[$i];
}
} elseif ($arg->name && $function_storage && $function_storage->allow_named_arg_calls) {
foreach ($function_params as $candidate_param) { foreach ($function_params as $candidate_param) {
if ($candidate_param->name === $arg->name->name) { if ($candidate_param->name === $arg->name->name) {
$function_param = $candidate_param; $arg_function_params = [$candidate_param];
break; break;
} }
} }
if (!$function_param) { if (!$arg_function_params) {
if (IssueBuffer::accepts( if (IssueBuffer::accepts(
new InvalidNamedArgument( new InvalidNamedArgument(
'Parameter $' . $arg->name->name . ' does not exist on function ' 'Parameter $' . $arg->name->name . ' does not exist on function '
@ -644,13 +682,13 @@ class ArgumentsAnalyzer
} }
} }
} elseif ($function_param_count > $argument_offset) { } elseif ($function_param_count > $argument_offset) {
$function_param = $function_params[$argument_offset]; $arg_function_params = [$function_params[$argument_offset]];
} elseif ($last_param && $last_param->is_variadic) { } elseif ($last_param && $last_param->is_variadic) {
$function_param = $last_param; $arg_function_params = [$last_param];
} }
if ($function_param if ($arg_function_params
&& $function_param->by_ref && $arg_function_params[0]->by_ref
&& $method_id !== 'extract' && $method_id !== 'extract'
) { ) {
if (self::handlePossiblyMatchingByRefParam( if (self::handlePossiblyMatchingByRefParam(
@ -671,24 +709,27 @@ class ArgumentsAnalyzer
$arg_value_type = $statements_analyzer->node_data->getType($arg->value); $arg_value_type = $statements_analyzer->node_data->getType($arg->value);
if (ArgumentAnalyzer::checkArgumentMatches( foreach ($arg_function_params as $i => $function_param) {
$statements_analyzer, if (ArgumentAnalyzer::checkArgumentMatches(
$cased_method_id, $statements_analyzer,
$self_fq_class_name, $cased_method_id,
$static_fq_class_name, $self_fq_class_name,
$code_location, $static_fq_class_name,
$function_param, $code_location,
$argument_offset, $function_param,
$function_storage ? $function_storage->allow_named_arg_calls : true, $argument_offset + $i,
$arg, $i,
$arg_value_type, $function_storage ? $function_storage->allow_named_arg_calls : true,
$context, $arg,
$class_generic_params, $arg_value_type,
$template_result, $context,
$function_storage ? $function_storage->specialize_call : true, $class_generic_params,
$in_call_map $template_result,
) === false) { $function_storage ? $function_storage->specialize_call : true,
return false; $in_call_map
) === false) {
return false;
}
} }
} }
@ -767,7 +808,7 @@ class ArgumentsAnalyzer
$expected_param_count = $i; $expected_param_count = $i;
} }
for ($i = count($args), $j = count($function_params); $i < $j; ++$i) { for ($i = count($args) + $packed_var_definite_args, $j = count($function_params); $i < $j; ++$i) {
$param = $function_params[$i]; $param = $function_params[$i];
if (!$param->is_optional if (!$param->is_optional

View File

@ -153,10 +153,6 @@ class ScalarTypeComparator
} }
if ($container_type_part instanceof TDependentGetDebugType) { if ($container_type_part instanceof TDependentGetDebugType) {
if ($container_type_part instanceof TClassString || $container_type_part instanceof TLiteralClassString) {
return true;
}
return $input_type_part instanceof TString; return $input_type_part instanceof TString;
} }

View File

@ -12,6 +12,94 @@ class ArgTest extends TestCase
public function providerValidCodeParse(): iterable public function providerValidCodeParse(): iterable
{ {
return [ return [
'argumentUnpackingLiteral' => [
'<?php
function add(int $a, int $b, int $c) : int {
return $a + $b + $c;
}
echo add(1, ...[2, 3]);',
],
'arrayPushArgumentUnpackingWithGoodArg' => [
'<?php
$a = ["foo"];
$b = ["foo", "bar"];
array_push($a, ...$b);',
'assertions' => [
'$a' => 'non-empty-list<string>',
],
],
'arrayMergeArgumentUnpacking' => [
'<?php
$a = [[1, 2]];
$b = array_merge([], ...$a);',
'assertions' => [
'$b' => 'array{0: int, 1: int}',
],
],
'preserveTypesWhenUnpacking' => [
'<?php
/**
* @return array<int,array<int,string>>
*/
function getData(): array
{
return [
["a", "b"],
["c", "d"]
];
}
/**
* @return array<int,string>
*/
function f1(): array
{
$data = getData();
return array_merge($data[0], $data[1]);
}
/**
* @return array<int,string>
*/
function f2(): array
{
$data = getData();
return array_merge(...$data);
}
/**
* @return array<int,string>
*/
function f3(): array
{
$data = getData();
return array_merge([], ...$data);
}',
],
'unpackArg' => [
'<?php
function Foo(string $a, string ...$b) : void {}
/** @return array<int, string> */
function Baz(string ...$c) {
Foo(...$c);
return $c;
}',
],
'unpackByRefArg' => [
'<?php
function example (int &...$x): void {}
$y = 0;
example($y);
$z = [0];
example(...$z);',
'assertions' => [
'$y' => 'int',
'$z' => 'array<int, int>',
],
],
'callMapClassOptionalArg' => [ 'callMapClassOptionalArg' => [
'<?php '<?php
class Hello {} class Hello {}
@ -180,6 +268,16 @@ class ArgTest extends TestCase
public function providerInvalidCodeParse(): iterable public function providerInvalidCodeParse(): iterable
{ {
return [ return [
'arrayPushArgumentUnpackingWithBadArg' => [
'<?php
$a = [];
$b = "hello";
$a[] = "foo";
array_push($a, ...$b);',
'error_message' => 'InvalidArgument',
],
'possiblyInvalidArgument' => [ 'possiblyInvalidArgument' => [
'<?php '<?php
$foo = [ $foo = [
@ -291,6 +389,49 @@ class ArgTest extends TestCase
takesArguments(age: 5, name: "hello");', takesArguments(age: 5, name: "hello");',
'error_message' => 'InvalidScalarArgument' 'error_message' => 'InvalidScalarArgument'
], ],
'arrayWithoutAllNamedParameters' => [
'<?php
class User {
public function __construct(
public int $id,
public string $name,
public int $age
) {}
}
/**
* @param array{id: int, name: string} $data
*/
function processUserDataInvalid(array $data) : User {
return new User(...$data);
}',
'error_message' => 'MixedArgument',
[],
false,
'8.0'
],
'arrayWithoutAllNamedParametersSuppressMixed' => [
'<?php
class User {
public function __construct(
public int $id,
public string $name,
public int $age
) {}
}
/**
* @param array{id: int, name: string} $data
*/
function processUserDataInvalid(array $data) : User {
/** @psalm-suppress MixedArgument */
return new User(...$data);
}',
'error_message' => 'TooFewArguments',
[],
false,
'8.0'
],
]; ];
} }
} }

View File

@ -4,7 +4,6 @@ namespace Psalm\Tests;
class Php56Test extends TestCase class Php56Test extends TestCase
{ {
use Traits\ValidCodeAnalysisTestTrait; use Traits\ValidCodeAnalysisTestTrait;
use Traits\InvalidCodeAnalysisTestTrait;
/** /**
* @return iterable<string,array{string,assertions?:array<string,string>,error_levels?:string[]}> * @return iterable<string,array{string,assertions?:array<string,string>,error_levels?:string[]}>
@ -70,101 +69,6 @@ class Php56Test extends TestCase
'$two' => 'int', '$two' => 'int',
], ],
], ],
'argumentUnpacking' => [
'<?php
/**
* @return int
* @param int $a
* @param int $b
* @param int $c
*/
function add($a, $b, $c) {
return $a + $b + $c;
}
$operators = [2, 3];
echo add(1, ...$operators);',
],
'arrayPushArgumentUnpackingWithGoodArg' => [
'<?php
$a = ["foo"];
$b = ["foo", "bar"];
array_push($a, ...$b);',
'assertions' => [
'$a' => 'non-empty-list<string>',
],
],
'arrayMergeArgumentUnpacking' => [
'<?php
$a = [[1, 2]];
$b = array_merge([], ...$a);',
'assertions' => [
'$b' => 'array{0: int, 1: int}',
],
],
'preserveTypesWhenUnpacking' => [
'<?php
/**
* @return array<int,array<int,string>>
*/
function getData(): array
{
return [
["a", "b"],
["c", "d"]
];
}
/**
* @return array<int,string>
*/
function f1(): array
{
$data = getData();
return array_merge($data[0], $data[1]);
}
/**
* @return array<int,string>
*/
function f2(): array
{
$data = getData();
return array_merge(...$data);
}
/**
* @return array<int,string>
*/
function f3(): array
{
$data = getData();
return array_merge([], ...$data);
}',
],
'unpackArg' => [
'<?php
function Foo(string $a, string ...$b) : void {}
/** @return array<int, string> */
function Baz(string ...$c) {
Foo(...$c);
return $c;
}',
],
'unpackByRefArg' => [
'<?php
function example (int &...$x): void {}
$y = 0;
example($y);
$z = [0];
example(...$z);',
'assertions' => [
'$y' => 'int',
'$z' => 'array<int, int>',
],
],
'exponentiation' => [ 'exponentiation' => [
'<?php '<?php
$a = 2; $a = 2;
@ -251,23 +155,4 @@ class Php56Test extends TestCase
], ],
]; ];
} }
/**
* @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
*/
public function providerInvalidCodeParse(): iterable
{
return [
'arrayPushArgumentUnpackingWithBadArg' => [
'<?php
$a = [];
$b = "hello";
$a[] = "foo";
array_push($a, ...$b);',
'error_message' => 'InvalidArgument',
],
];
}
} }

View File

@ -2184,6 +2184,7 @@ class UnusedVariableTest extends TestCase
'funcGetArgs' => [ 'funcGetArgs' => [
'<?php '<?php
function validate(bool $b, bool $c) : void { function validate(bool $b, bool $c) : void {
/** @psalm-suppress MixedArgument */
print_r(...func_get_args()); print_r(...func_get_args());
}' }'
], ],