danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2024-11-30 04:39:00 +01:00
Code Issues Projects Releases Wiki Activity

Fix #1540 - use correct comparison for callable param types

Browse Source
This commit is contained in:
Matthew Brown 2019-04-12 00:44:10 -04:00
parent a9b8952ea2
commit ea20a2bd04
5 changed files with 205 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php
View File

@ -938,18 +938,21 @@ class CallAnalyzer
$calling_class_storage = $class_storage; $calling_class_storage = $class_storage;
$static_fq_class_name = $fq_class_name;
$self_fq_class_name = $fq_class_name;
if ($method_id && strpos($method_id, '::')) { if ($method_id && strpos($method_id, '::')) {
$declaring_method_id = $codebase->methods->getDeclaringMethodId($method_id); $declaring_method_id = $codebase->methods->getDeclaringMethodId($method_id);
if ($declaring_method_id && $declaring_method_id !== $method_id) { if ($declaring_method_id && $declaring_method_id !== $method_id) {
list($fq_class_name) = explode('::', $declaring_method_id); list($self_fq_class_name) = explode('::', $declaring_method_id);
$class_storage = $codebase->classlike_storage_provider->get($fq_class_name); $class_storage = $codebase->classlike_storage_provider->get($self_fq_class_name);
} }
$appearing_method_id = $codebase->methods->getAppearingMethodId($method_id); $appearing_method_id = $codebase->methods->getAppearingMethodId($method_id);
if ($appearing_method_id && $declaring_method_id !== $appearing_method_id) { if ($appearing_method_id && $declaring_method_id !== $appearing_method_id) {
list($fq_class_name) = explode('::', $appearing_method_id); list($self_fq_class_name) = explode('::', $appearing_method_id);
} }
} }
@ -1044,7 +1047,8 @@ class CallAnalyzer
if (self::checkFunctionLikeArgumentMatches( if (self::checkFunctionLikeArgumentMatches(
$statements_analyzer, $statements_analyzer,
$cased_method_id, $cased_method_id,
$fq_class_name, $self_fq_class_name,
$static_fq_class_name,
$function_param, $function_param,
$argument_offset, $argument_offset,
$arg, $arg,
@ -1158,7 +1162,8 @@ class CallAnalyzer
/** /**
* @param string|null $cased_method_id * @param string|null $cased_method_id
* @param string|null $fq_class_name * @param string|null $self_fq_class_name
* @param string|null $static_fq_class_name
* @param FunctionLikeParameter|null $function_param * @param FunctionLikeParameter|null $function_param
* @param array<string, array<string, array{Type\Union, 1?:int}>> $existing_generic_params * @param array<string, array<string, array{Type\Union, 1?:int}>> $existing_generic_params
* @param array<string, array<string, array{Type\Union, 1?:int}>> $generic_params * @param array<string, array<string, array{Type\Union, 1?:int}>> $generic_params
@ -1168,7 +1173,8 @@ class CallAnalyzer
private static function checkFunctionLikeArgumentMatches( private static function checkFunctionLikeArgumentMatches(
StatementsAnalyzer $statements_analyzer, StatementsAnalyzer $statements_analyzer,
$cased_method_id, $cased_method_id,
$fq_class_name, $self_fq_class_name,
$static_fq_class_name,
$function_param, $function_param,
int $argument_offset, int $argument_offset,
PhpParser\Node\Arg $arg, PhpParser\Node\Arg $arg,
@ -1224,7 +1230,8 @@ class CallAnalyzer
$statements_analyzer, $statements_analyzer,
$codebase, $codebase,
$cased_method_id, $cased_method_id,
$fq_class_name, $self_fq_class_name,
$static_fq_class_name,
$function_param, $function_param,
$arg->value->inferredType, $arg->value->inferredType,
$argument_offset, $argument_offset,
@ -1375,7 +1382,8 @@ class CallAnalyzer
/** /**
* @param string|null $cased_method_id * @param string|null $cased_method_id
* @param string|null $fq_class_name * @param string|null $self_fq_class_name
* @param string|null $static_fq_class_name
* @param array<string, array<string, array{Type\Union, 1?:int}>> $existing_generic_params * @param array<string, array<string, array{Type\Union, 1?:int}>> $existing_generic_params
* @param array<string, array<string, array{Type\Union, 1?:int}>> $generic_params * @param array<string, array<string, array{Type\Union, 1?:int}>> $generic_params
* @param array<string, array<string, array{Type\Union}>> $template_types * @param array<string, array<string, array{Type\Union}>> $template_types
@ -1386,7 +1394,8 @@ class CallAnalyzer
StatementsAnalyzer $statements_analyzer, StatementsAnalyzer $statements_analyzer,
Codebase $codebase, Codebase $codebase,
$cased_method_id, $cased_method_id,
$fq_class_name, $self_fq_class_name,
$static_fq_class_name,
$function_param, $function_param,
Type\Union $arg_type, Type\Union $arg_type,
int $argument_offset, int $argument_offset,
@ -1452,8 +1461,8 @@ class CallAnalyzer
$fleshed_out_type = ExpressionAnalyzer::fleshOutType( $fleshed_out_type = ExpressionAnalyzer::fleshOutType(
$codebase, $codebase,
$param_type, $param_type,
$fq_class_name ?: $context->self, $self_fq_class_name,
$fq_class_name ?: $context->self $static_fq_class_name
); );
if ($arg->unpack) { if ($arg->unpack) {

88
src/Psalm/Internal/Analyzer/TypeAnalyzer.php
View File

@ -1712,58 +1712,66 @@ class TypeAnalyzer
return false; return false;
} }
if ($container_type_part->params !== null) { if ($input_type_part->params !== null && $container_type_part->params !== null) {
foreach ($container_type_part->params as $i => $container_param) { foreach ($input_type_part->params as $i => $input_param) {
if (!isset($input_type_part->params[$i])) { $container_param = null;
if ($container_param->is_optional) {
if (isset($container_type_part->params[$i])) {
$container_param = $container_type_part->params[$i];
} elseif ($container_type_part->params) {
$last_param = end($container_type_part->params);
if ($last_param->is_variadic) {
$container_param = $last_param;
}
}
if (!$container_param) {
if ($input_param->is_optional) {
break; break;
} }
$type_coerced = true; return false;
$type_coerced_from_mixed = true;
$all_types_contain = false;
break;
} }
$input_param = $input_type_part->params[$i]; if ($container_param->type
&& !$container_param->type->hasMixed()
if (!self::isContainedBy( && !self::isContainedBy(
$codebase, $codebase,
$input_param->type ?: Type::getMixed(), $container_param->type,
$container_param->type ?: Type::getMixed(), $input_param->type ?: Type::getMixed(),
false, false,
false, false,
$has_scalar_match, $has_scalar_match,
$type_coerced, $type_coerced,
$type_coerced_from_mixed $type_coerced_from_mixed
) )
) { ) {
$all_types_contain = false; $all_types_contain = false;
} }
} }
}
if (isset($container_type_part->return_type)) { if (isset($container_type_part->return_type)) {
if (!isset($input_type_part->return_type)) { if (!isset($input_type_part->return_type)) {
$type_coerced = true; $type_coerced = true;
$type_coerced_from_mixed = true; $type_coerced_from_mixed = true;
$all_types_contain = false;
} else {
if (!$container_type_part->return_type->isVoid()
&& !self::isContainedBy(
$codebase,
$input_type_part->return_type,
$container_type_part->return_type,
false,
false,
$has_scalar_match,
$type_coerced,
$type_coerced_from_mixed
)
) {
$all_types_contain = false; $all_types_contain = false;
} else {
if (!$container_type_part->return_type->isVoid()
&& !self::isContainedBy(
$codebase,
$input_type_part->return_type,
$container_type_part->return_type,
false,
false,
$has_scalar_match,
$type_coerced,
$type_coerced_from_mixed
)
) {
$all_types_contain = false;
}
} }
} }
} }

2
src/Psalm/Internal/CallMap.php
View File

@ -11303,7 +11303,7 @@ return [
'RegexIterator::setPregFlags' => ['void', 'new_flags'=>'int'], 'RegexIterator::setPregFlags' => ['void', 'new_flags'=>'int'],
'RegexIterator::valid' => ['bool'], 'RegexIterator::valid' => ['bool'],
'register_event_handler' => ['bool', 'event_handler_func'=>'string', 'handler_register_name'=>'string', 'event_type_mask'=>'int'], 'register_event_handler' => ['bool', 'event_handler_func'=>'string', 'handler_register_name'=>'string', 'event_type_mask'=>'int'],
'register_shutdown_function' => ['void', 'function'=>'callable():void', '...parameter='=>'mixed'], 'register_shutdown_function' => ['void', 'function'=>'callable(mixed...):void', '...parameter='=>'mixed'],
'register_tick_function' => ['bool', 'function'=>'callable():void', '...args='=>'mixed'], 'register_tick_function' => ['bool', 'function'=>'callable():void', '...args='=>'mixed'],
'rename' => ['bool', 'old_name'=>'string', 'new_name'=>'string', 'context='=>'resource'], 'rename' => ['bool', 'old_name'=>'string', 'new_name'=>'string', 'context='=>'resource'],
'rename_function' => ['bool', 'original_name'=>'string', 'new_name'=>'string'], 'rename_function' => ['bool', 'original_name'=>'string', 'new_name'=>'string'],

24
src/Psalm/Internal/Scanner/PhpStormMetaScanner.php
View File

@ -90,7 +90,7 @@ class PhpStormMetaScanner
* @return ?Type\Union * @return ?Type\Union
*/ */
function ( function (
StatementsAnalyzer $_statements_analyzer, \Psalm\StatementsSource $_statements_analyzer,
string $fq_classlike_name, string $fq_classlike_name,
string $method_name, string $method_name,
array $call_args, array $call_args,
@ -143,7 +143,7 @@ class PhpStormMetaScanner
* @return ?Type\Union * @return ?Type\Union
*/ */
function ( function (
StatementsAnalyzer $_statements_analyzer, \Psalm\StatementsSource $_statements_analyzer,
string $fq_classlike_name, string $fq_classlike_name,
string $method_name, string $method_name,
array $call_args, array $call_args,
@ -176,7 +176,7 @@ class PhpStormMetaScanner
* @return ?Type\Union * @return ?Type\Union
*/ */
function ( function (
StatementsAnalyzer $_statements_analyzer, \Psalm\StatementsSource $_statements_analyzer,
string $fq_classlike_name, string $fq_classlike_name,
string $method_name, string $method_name,
array $call_args, array $call_args,
@ -229,7 +229,7 @@ class PhpStormMetaScanner
* @param array<PhpParser\Node\Arg> $call_args * @param array<PhpParser\Node\Arg> $call_args
*/ */
function ( function (
StatementsAnalyzer $statements_analyzer, \Psalm\StatementsSource $statements_analyzer,
string $function_id, string $function_id,
array $call_args, array $call_args,
Context $_context, Context $_context,
@ -262,6 +262,10 @@ class PhpStormMetaScanner
} }
} }
if (!$statements_analyzer instanceof StatementsAnalyzer) {
throw new \UnexpectedValueException('This is bad');
}
$storage = $statements_analyzer->getCodebase()->functions->getStorage( $storage = $statements_analyzer->getCodebase()->functions->getStorage(
$statements_analyzer, $statements_analyzer,
$function_id $function_id
@ -277,7 +281,7 @@ class PhpStormMetaScanner
* @param array<PhpParser\Node\Arg> $call_args * @param array<PhpParser\Node\Arg> $call_args
*/ */
function ( function (
StatementsAnalyzer $statements_analyzer, \Psalm\StatementsSource $statements_analyzer,
string $function_id, string $function_id,
array $call_args, array $call_args,
Context $_context, Context $_context,
@ -290,6 +294,10 @@ class PhpStormMetaScanner
return clone $call_arg_type; return clone $call_arg_type;
} }
if (!$statements_analyzer instanceof StatementsAnalyzer) {
throw new \UnexpectedValueException('This is bad');
}
$storage = $statements_analyzer->getCodebase()->functions->getStorage( $storage = $statements_analyzer->getCodebase()->functions->getStorage(
$statements_analyzer, $statements_analyzer,
$function_id $function_id
@ -305,7 +313,7 @@ class PhpStormMetaScanner
* @param array<PhpParser\Node\Arg> $call_args * @param array<PhpParser\Node\Arg> $call_args
*/ */
function ( function (
StatementsAnalyzer $statements_analyzer, \Psalm\StatementsSource $statements_analyzer,
string $function_id, string $function_id,
array $call_args, array $call_args,
Context $_context, Context $_context,
@ -327,6 +335,10 @@ class PhpStormMetaScanner
} }
} }
if (!$statements_analyzer instanceof StatementsAnalyzer) {
throw new \UnexpectedValueException('This is bad');
}
$storage = $statements_analyzer->getCodebase()->functions->getStorage( $storage = $statements_analyzer->getCodebase()->functions->getStorage(
$statements_analyzer, $statements_analyzer,
$function_id $function_id

147
tests/CallableTest.php
View File

@ -364,10 +364,10 @@ class CallableTest extends TestCase
* @param Closure(B):A $f * @param Closure(B):A $f
* @param Closure(C):B $g * @param Closure(C):B $g
* *
* @return Closure(C):A * @return Closure(C2):A
*/ */
function foo(Closure $f, Closure $g) : Closure { function foo(Closure $f, Closure $g) : Closure {
return function (C2 $x) use ($f, $g) : A { return function (C $x) use ($f, $g) : A {
return $f($g($x)); return $f($g($x));
}; };
} }
@ -693,6 +693,56 @@ class CallableTest extends TestCase
], ],
'callableSelfArg' => [ 'callableSelfArg' => [
'<?php '<?php
class C extends B {}
$b = new B();
$c = new C();
$b->func2(function(B $x): void {});
$c->func2(function(B $x): void {});
class A {}
class B extends A {
/**
* @param callable(self) $f
*/
function func2(callable $f): void {
$f($this);
}
}',
],
'callableParentArg' => [
'<?php
class C extends B {}
$b = new B();
$c = new C();
$b->func3(function(A $x): void {});
$c->func3(function(A $x): void {});
class A {}
class B extends A {
/**
* @param callable(parent) $f
*/
function func3(callable $f): void {
$f($this);
}
}',
],
'callableStaticArg' => [
'<?php
class C extends B {}
$b = new B();
$c = new C();
$b->func1(function(B $x): void {});
$c->func1(function(C $x): void {});
class A {} class A {}
class B extends A { class B extends A {
@ -702,31 +752,7 @@ class CallableTest extends TestCase
function func1(callable $f): void { function func1(callable $f): void {
$f($this); $f($this);
} }
}',
/**
* @param callable(self) $f
*/
function func2(callable $f): void {
$f($this);
}
/**
* @param callable(parent) $f
*/
function func3(callable $f): void {
$f($this);
}
}
class C extends B {}
$b = new B();
$c = new C();
$b->func1(function(B $x): void {});
$c->func1(function(C $x): void {});
$b->func2(function(B $x): void {});
$c->func2(function(B $x): void {});',
], ],
'callableSelfReturn' => [ 'callableSelfReturn' => [
'<?php '<?php
@ -844,6 +870,54 @@ class CallableTest extends TestCase
} }
}', }',
], ],
'allowCallableWithNarrowerReturn' => [
'<?php
class A {}
class B extends A {}
/**
* @param Closure():A $x
*/
function accept_closure($x) : void {
$x();
}
accept_closure(
function () : B {
return new B();
}
);'
],
'allowCallableWithWiderParam' => [
'<?php
class A {}
class B extends A {}
/**
* @param Closure(B $a):A $x
*/
function accept_closure($x) : void {
$x(new B());
}
accept_closure(
function (A $a) : A {
return $a;
}
);'
],
'allowCallableWithOptionalArg' => [
'<?php
/**
* @param Closure():int $x
*/
function accept_closure($x) : void {
$x();
}
accept_closure(
function (int $x = 5) : int {
return $x;
}
);'
],
]; ];
} }
@ -1118,10 +1192,10 @@ class CallableTest extends TestCase
* @param Closure(B):A $f * @param Closure(B):A $f
* @param Closure(C):B $g * @param Closure(C):B $g
* *
* @return Closure(C2):A * @return Closure(C):A
*/ */
function foo(Closure $f, Closure $g) : Closure { function foo(Closure $f, Closure $g) : Closure {
return function (C $x) use ($f, $g) : A { return function (C2 $x) use ($f, $g) : A {
return $f($g($x)); return $f($g($x));
}; };
}', }',
@ -1334,6 +1408,21 @@ class CallableTest extends TestCase
}', }',
'error_message' => 'InvalidArgument', 'error_message' => 'InvalidArgument',
], ],
'prohibitCallableWithRequiredArg' => [
'<?php
/**
* @param Closure():int $x
*/
function accept_closure($x) : void {
$x();
}
accept_closure(
function (int $x) : int {
return $x;
}
);',
'error_message' => 'InvalidArgument'
],
]; ];
} }
} }