Skip to content

Commit

Permalink
Restrict templates
Browse files Browse the repository at this point in the history
  • Loading branch information
@muglug
muglug committed May 5, 2019
1 parent dd40987 commit 06a81ec
Show file tree
Hide file tree
Showing 10 changed files with 395 additions and 127 deletions.
5 changes: 5 additions & 0 deletions src/Psalm/Internal/Analyzer/MethodAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,11 @@ public static function compareMethods(
$template_types,
$codebase
);

$guide_method_storage_return_type->replaceTemplateTypesWithArgTypes(
$template_types,
$codebase
);
}

// treat void as null when comparing against docblock implementer
Expand Down
1 change: 1 addition & 0 deletions src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1010,6 +1010,7 @@ public static function fleshOutType(
$fleshed_out_type->possibly_undefined = $return_type->possibly_undefined;
$fleshed_out_type->by_ref = $return_type->by_ref;
$fleshed_out_type->initialized = $return_type->initialized;
$fleshed_out_type->had_template = $return_type->had_template;

return $fleshed_out_type;
}
Expand Down
31 changes: 31 additions & 0 deletions src/Psalm/Internal/Analyzer/TypeAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1605,6 +1605,37 @@ private static function isMatchingTypeContainedBy(
$allow_interface_equality
)) {
$all_types_contain = false;
} elseif ($input_type_part->value !== 'Generator'
&& $container_type_part->value !== 'Generator'
&& !$container_param->had_template
&& !$input_param->had_template
&& !$container_param->hasTemplate()
&& !$input_param->hasTemplate()
&& !$input_param->hasLiteralValue()
) {
// Make sure types are basically the same
if (!self::isContainedBy(
$codebase,
$container_param,
$input_param,
$container_param->ignore_nullable_issues,
$container_param->ignore_falsable_issues,
$has_scalar_match,
$type_coerced,
$type_coerced_from_mixed,
$to_string_cast,
$type_coerced_from_scalar,
$allow_interface_equality
) || $type_coerced
) {
if ($container_param->hasMixed() || $container_param->isArrayKey()) {
$type_coerced_from_mixed = true;
} else {
$all_types_contain = false;
}

$type_coerced = false;
}
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/Psalm/Internal/Codebase/Populator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,7 @@ private function convertPhpStormGenericToPsalmGeneric(Type\Union $candidate, $is
{
$atomic_types = $candidate->getTypes();

if (isset($atomic_types['array']) && count($atomic_types) > 1) {
if (isset($atomic_types['array']) && count($atomic_types) > 1 && !isset($atomic_types['null'])) {
$iterator_name = null;
$generic_params = null;

Expand Down Expand Up @@ -947,6 +947,7 @@ private function convertPhpStormGenericToPsalmGeneric(Type\Union $candidate, $is
}

$candidate->removeType('array');
$candidate->removeType($iterator_name);
$candidate->addType($generic_iterator);
}
}
Expand Down
288 changes: 165 additions & 123 deletions src/Psalm/Internal/Type/TypeCombination.php
View file

Large diffs are not rendered by default.

19 changes: 19 additions & 0 deletions src/Psalm/Type/Atomic/TGenericObject.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,25 @@ public function __construct($value, array $type_params)
$this->type_params = $type_params;
}

/**
* @return string
*/
public function getKey()
{
$s = '';
foreach ($this->type_params as $type_param) {
$s .= $type_param->getKey() . ', ';
}

$extra_types = '';

if ($this instanceof TNamedObject && $this->extra_types) {
$extra_types = '&' . implode('&', $this->extra_types);
}

return $this->value . '<' . substr($s, 0, -2) . '>' . $extra_types;
}

/**
* @return bool
*/
Expand Down
68 changes: 67 additions & 1 deletion src/Psalm/Type/Union.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ class Union
*/
public $possibly_undefined_from_try = false;

/**
* Whether or not this union had a template, since replaced
* @var bool
*/
public $had_template = false;

/**
* @var array<string, TLiteralString>
*/
Expand Down Expand Up @@ -264,6 +270,47 @@ public function __toString()
return substr($s, 0, -1) ?: '';
}

public function getKey() : string
{
if (empty($this->types)) {
return '';
}
$s = '';

$printed_int = false;
$printed_float = false;
$printed_string = false;

foreach ($this->types as $type) {
if ($type instanceof TLiteralFloat) {
if ($printed_float) {
continue;
}

$s .= 'float|';
$printed_float = true;
} elseif ($type instanceof TLiteralString) {
if ($printed_string) {
continue;
}

$s .= 'string|';
$printed_string = true;
} elseif ($type instanceof TLiteralInt) {
if ($printed_int) {
continue;
}

$s .= 'int|';
$printed_int = true;
} else {
$s .= $type->getKey() . '|';
}
}

return substr($s, 0, -1) ?: '';
}

/**
* @return string
*/
Expand Down Expand Up @@ -785,7 +832,9 @@ public function isNever()
*/
public function isGenerator()
{
return count($this->types) === 1 && isset($this->types['Generator']);
return count($this->types) === 1
&& (($single_type = reset($this->types)) instanceof TNamedObject)
&& ($single_type->value === 'Generator');
}

/**
Expand Down Expand Up @@ -898,6 +947,10 @@ public function replaceTemplateTypesWithStandins(
$keys_to_unset = [];

foreach ($this->types as $key => $atomic_type) {
if ($bracket_pos = strpos($key, '<')) {
$key = substr($key, 0, $bracket_pos);
}

if ($atomic_type instanceof Type\Atomic\TTemplateParam
&& isset($template_types[$key][$atomic_type->defining_class ?: ''])
) {
Expand Down Expand Up @@ -925,6 +978,8 @@ public function replaceTemplateTypesWithStandins(
}
}

$this->had_template = true;

if ($input_type) {
$generic_param = clone $input_type;
$generic_param->setFromDocblock();
Expand Down Expand Up @@ -971,6 +1026,8 @@ public function replaceTemplateTypesWithStandins(

$this->types[$class_string->getKey()] = $class_string;

$this->had_template = true;

if ($input_type) {
$valid_input_atomic_types = [];

Expand Down Expand Up @@ -1017,6 +1074,10 @@ public function replaceTemplateTypesWithStandins(

if ($input_type && $codebase) {
foreach ($input_type->types as $input_key => $atomic_input_type) {
if ($bracket_pos = strpos($input_key, '<')) {
$input_key = substr($input_key, 0, $bracket_pos);
}

if ($input_key === $key) {
$matching_atomic_type = $atomic_input_type;
break;
Expand Down Expand Up @@ -1352,6 +1413,11 @@ public function allStringLiterals() : bool
return true;
}

public function hasLiteralValue() : bool
{
return $this->literal_int_types || $this->literal_string_types || $this->literal_float_types;
}

/**
* @return bool true if this is a int literal with only one possible value
*/
Expand Down
1 change: 1 addition & 0 deletions tests/Template/TemplateExtendsTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1547,6 +1547,7 @@ protected function createFrom(array $elements)
'<?php
/**
* @template E
* @template-extends \IteratorAggregate<int, E>
*/
interface Collection extends \IteratorAggregate
{
Expand Down
81 changes: 81 additions & 0 deletions tests/Template/TemplateTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2098,6 +2098,56 @@ public function __construct(array $elements = [])
'$c' => 'ArrayCollection<array-key, mixed>',
],
],
'doNotCombineTypes' => [
'<?php
class A {}
class B {}
/**
* @template T
*/
class C {
/**
* @var T
*/
private $t;
/**
* @param T $t
*/
public function __construct($t) {
$this->t = $t;
}
/**
* @return T
*/
public function get() {
return $this->t;
}
}
/**
* @param C<A> $a
* @param C<B> $b
* @return C<A>|C<B>
*/
function randomCollection(C $a, C $b) : C {
if (rand(0, 1)) {
return $a;
}
return $b;
}
$random_collection = randomCollection(new C(new A), new C(new B));
$a_or_b = $random_collection->get();',
[
'$random_collection' => 'C<A>|C<B>',
'$a_or_b' => 'B|A',
],
],
];
}

Expand Down Expand Up @@ -2654,6 +2704,37 @@ public function __construct()
}',
'error_message' => 'InvalidPropertyAssignmentValue'
],
'preventDogCatSnafu' => [
'<?php
class Animal {}
class Dog extends Animal {}
class Cat extends Animal {}
/**
* @template T
*/
class Collection {
/**
* @param T $t
*/
public function add($t) : void {}
}
/**
* @param Collection<Animal> $list
*/
function addAnimal(Collection $list) : void {
$list->add(new Cat());
}
/**
* @param Collection<Dog> $list
*/
function takesDogList(Collection $list) : void {
addAnimal($list); // this should be an error
}',
'error_message' => 'InvalidArgument',
],
];
}
}
25 changes: 23 additions & 2 deletions tests/TypeCombinationTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ public function providerTestValidTypeCombination()
],
],
'arrayObjectAndParamsWithEmptyArray' => [
'ArrayObject<int, string>|array<empty, empty>',
'array<empty, empty>|ArrayObject<int, string>',
[
'ArrayObject<int, string>',
'array<empty, empty>',
Expand Down Expand Up @@ -316,7 +316,7 @@ public function providerTestValidTypeCombination()
],
],
'aAndAOfB' => [
'A',
'A<B>|A',
[
'A',
'A<B>',
Expand Down Expand Up @@ -379,6 +379,27 @@ public function providerTestValidTypeCombination()
'string',
],
],
'traversableAorB' => [
'Traversable<mixed, A|B>',
[
'Traversable<A>',
'Traversable<B>',
],
],
'iterableAorB' => [
'iterable<mixed, A|B>',
[
'iterable<A>',
'iterable<B>',
],
],
'FooAorB' => [
'Foo<A>|Foo<B>',
[
'Foo<A>',
'Foo<B>',
],
],
];
}

Expand Down

0 comments on commit 06a81ec

Please sign in to comment.