Skip to content

Commit

Permalink
Fix #1845 - prevent string return when expecting template
Browse files Browse the repository at this point in the history
  • Loading branch information
@muglug
muglug committed Jun 25, 2019
1 parent 6cb52d2 commit 16bf5f1
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 34 deletions.
36 changes: 19 additions & 17 deletions src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php
View file
Expand Up @@ -165,25 +165,27 @@ public static function analyze(
if ($storage instanceof \Psalm\Storage\MethodStorage) {
list($fq_class_name, $method_name) = explode('::', $cased_method_id);

if ($fq_class_name !== $context->self) {
$class_storage = $codebase->classlike_storage_provider->get($fq_class_name);

$found_generic_params = MethodCallAnalyzer::getClassTemplateParams(
$codebase,
$class_storage,
$fq_class_name,
strtolower($method_name),
null,
null
);
$class_storage = $codebase->classlike_storage_provider->get($fq_class_name);

$found_generic_params = MethodCallAnalyzer::getClassTemplateParams(
$codebase,
$class_storage,
$fq_class_name,
strtolower($method_name),
null,
null
);

if ($found_generic_params) {
foreach ($found_generic_params as $template_name => $_) {
unset($found_generic_params[$template_name][$fq_class_name]);
}

if ($found_generic_params) {
$local_return_type = clone $local_return_type;
$local_return_type = clone $local_return_type;

$local_return_type->replaceTemplateTypesWithArgTypes(
$found_generic_params
);
}
$local_return_type->replaceTemplateTypesWithArgTypes(
$found_generic_params
);
}
}

Expand Down
56 changes: 49 additions & 7 deletions src/Psalm/Internal/Analyzer/TypeAnalyzer.php
View file
Expand Up @@ -612,10 +612,36 @@ public static function isAtomicContainedBy(
&$to_string_cast = null,
&$type_coerced_from_scalar = null
) {
if ($container_type_part instanceof TTemplateParam && $input_type_part instanceof TTemplateParam) {
if ($container_type_part->param_name !== $input_type_part->param_name
|| (string)$container_type_part->defining_class !== (string)$input_type_part->defining_class
) {
if ($container_type_part->defining_class
&& $input_type_part->defining_class
) {
$input_class_storage = $codebase->classlike_storage_provider->get(
$input_type_part->defining_class
);

if (isset($input_class_storage->template_type_extends
[$container_type_part->defining_class]
[$container_type_part->param_name])
) {
return true;
}
}

return false;
}

return true;
}

if ($container_type_part instanceof TMixed
|| ($container_type_part instanceof TTemplateParam
&& $container_type_part->as->isMixed()
&& !$container_type_part->extra_types)
&& !$container_type_part->extra_types
&& $input_type_part instanceof TMixed)
) {
if (get_class($container_type_part) === TEmptyMixed::class
&& get_class($input_type_part) === TMixed::class
Expand Down Expand Up @@ -675,7 +701,8 @@ public static function isAtomicContainedBy(

if ($input_type_part->shallowEquals($container_type_part)
|| (($input_type_part instanceof TNamedObject
|| $input_type_part instanceof TTemplateParam
|| ($input_type_part instanceof TTemplateParam
&& $input_type_part->as->hasObjectType())
|| $input_type_part instanceof TIterable)
&& ($container_type_part instanceof TNamedObject
|| ($container_type_part instanceof TTemplateParam
Expand All @@ -702,10 +729,6 @@ public static function isAtomicContainedBy(
}

if ($container_type_part instanceof TTemplateParam && $input_type_part instanceof TTemplateParam) {
if ($container_type_part->param_name !== $input_type_part->param_name) {
return false;
}

return self::isContainedBy(
$codebase,
$input_type_part->as,
Expand Down Expand Up @@ -735,7 +758,7 @@ public static function isAtomicContainedBy(
$to_string_cast,
$type_coerced_from_scalar
)) {
if ($allow_interface_equality || !$input_type_part instanceof TNamedObject) {
if ($allow_interface_equality || $input_type_part instanceof TArray) {
return true;
}
}
Expand All @@ -745,6 +768,25 @@ public static function isAtomicContainedBy(
}

if ($input_type_part instanceof TTemplateParam) {
if ($input_type_part->extra_types) {
foreach ($input_type_part->extra_types as $extra_type) {
if (self::isAtomicContainedBy(
$codebase,
$extra_type,
$container_type_part,
$allow_interface_equality,
$allow_float_int_equality,
$has_scalar_match,
$type_coerced,
$type_coerced_from_mixed,
$to_string_cast,
$type_coerced_from_scalar
)) {
return true;
}
}
}

foreach ($input_type_part->as->getTypes() as $input_as_type_part) {
if ($input_as_type_part instanceof TNull && $container_type_part instanceof TNull) {
continue;
Expand Down
7 changes: 7 additions & 0 deletions src/Psalm/Internal/Codebase/Populator.php
View file
Expand Up @@ -493,6 +493,13 @@ private function populateDataFromParentClass(
= $template_type[0];
}
}

if ($parent_storage->template_type_extends) {
$storage->template_type_extends = array_merge(
$storage->template_type_extends,
$parent_storage->template_type_extends
);
}
}
} elseif ($parent_storage->template_type_extends) {
$storage->template_type_extends = array_merge(
Expand Down
34 changes: 25 additions & 9 deletions tests/Template/ClassTemplateExtendsTest.php
View file
Expand Up @@ -663,6 +663,23 @@ function getFooCollection() : Collection {
$foo->bar();
}',
],
'constructExtendedArrayIteratorWithTemplateExtends' => [
'<?php
/**
* @template TKey as array-key
* @template TValue
* @template-extends ArrayIterator<TKey, TValue>
*/
class Collection1 extends ArrayIterator{}
class Collection2 extends Collection1{}
class Collection3 extends Collection2{}
$a = new Collection1(["a" => "b"]);
$a = new Collection2(["a" => "b"]);
$a = new Collection3(["a" => "b"]);',
],
'iterateOverExtendedArrayObjectWithoutParam' => [
'<?php
class O {}
Expand Down Expand Up @@ -2395,23 +2412,25 @@ class GrandChildContainer extends ChildContainer {}
'extendsTwiceSameNameLastDoesNotExtend' => [
'<?php
/**
* @template T
* @template T1
*/
class Container
{
/**
* @var T
* @var T1
*/
private $v;
/**
* @param T $v
* @param T1 $v
*/
public function __construct($v)
{
$this->v = $v;
}
/**
* @return T
* @return T1
*/
public function getValue()
{
Expand All @@ -2420,14 +2439,11 @@ public function getValue()
}
/**
* @template T
* @template-extends Container<T>
* @template T2
* @template-extends Container<T2>
*/
class ChildContainer extends Container {}
/**
* @template T
*/
class GrandChildContainer extends ChildContainer {}
$fc = new GrandChildContainer(5);
Expand Down
16 changes: 15 additions & 1 deletion tests/Template/FunctionTemplateTest.php
View file
Expand Up @@ -526,7 +526,9 @@ function call(callable $gen) : array {
if ($return instanceof Generator) {
return [$gen->getReturn()];
}
return [$gen];
/** @var array<int, TReturn> */
$wrapped_gen = [$gen];
return $wrapped_gen;
}
$arr = call(
Expand Down Expand Up @@ -1045,6 +1047,18 @@ function foo($x)
}',
'error_message' => 'InvalidReturnStatement'
],
'preventReturningString' => [
'<?php
/**
* @template T
* @psalm-param T $t
* @return T
*/
function mirror($t) {
return "string";
}',
'error_message' => 'InvalidReturnStatement'
],
];
}
}

0 comments on commit 16bf5f1

Please sign in to comment.