Skip to content

Commit

Permalink
Fix #1603 - prevent invalid covariant template classes from being passed
Browse files Browse the repository at this point in the history
  • Loading branch information
@muglug
muglug committed May 6, 2019
1 parent 57a5852 commit 751253d
Show file tree
Hide file tree
Showing 19 changed files with 566 additions and 164 deletions.
2 changes: 1 addition & 1 deletion src/Psalm/DocComment.php
View file
Expand Up @@ -128,7 +128,7 @@ public static function parse($docblock, $line_number = null, $preserve_format =
$special_key, $special_key,
[ [
'return', 'param', 'template', 'var', 'type', 'return', 'param', 'template', 'var', 'type',
'property', 'method', 'template-covariant', 'property', 'method',
'assert', 'assert-if-true', 'assert-if-false', 'suppress', 'assert', 'assert-if-true', 'assert-if-false', 'suppress',
'ignore-nullable-return', 'override-property-visibility', 'ignore-nullable-return', 'override-property-visibility',
'override-method-visibility', 'seal-properties', 'seal-methods', 'override-method-visibility', 'seal-properties', 'seal-methods',
Expand Down
84 changes: 77 additions & 7 deletions src/Psalm/Internal/Analyzer/CommentAnalyzer.php
View file
Expand Up @@ -401,15 +401,52 @@ public static function extractFunctionDocblockInfo($comment, $line_number)
foreach ($all_templates as $template_line) { foreach ($all_templates as $template_line) {
$template_type = preg_split('/[\s]+/', $template_line); $template_type = preg_split('/[\s]+/', $template_line);


if (count($template_type) > 2 $template_name = array_shift($template_type);
&& in_array(strtolower($template_type[1]), ['as', 'super', 'of'], true)
if (count($template_type) > 1
&& in_array(strtolower($template_type[0]), ['as', 'super', 'of'], true)
) { ) {
$template_modifier = strtolower(array_shift($template_type));
$info->templates[] = [ $info->templates[] = [
$template_type[0], $template_name,
strtolower($template_type[1]), $template_type[2] $template_modifier,
implode(' ', $template_type),
false
]; ];
} else { } else {
$info->templates[] = [$template_type[0]]; $info->templates[] = [$template_name, null, null, false];
}
}
}

if (isset($comments['specials']['template-covariant'])
|| isset($comments['specials']['psalm-template-covariant'])
) {
$all_templates =
(isset($comments['specials']['template-covariant'])
? $comments['specials']['template-covariant']
: [])
+ (isset($comments['specials']['psalm-template-covariant'])
? $comments['specials']['psalm-template-covariant']
: []);

foreach ($all_templates as $template_line) {
$template_type = preg_split('/[\s]+/', $template_line);

$template_name = array_shift($template_type);

if (count($template_type) > 1
&& in_array(strtolower($template_type[0]), ['as', 'super', 'of'], true)
) {
$template_modifier = strtolower(array_shift($template_type));
$info->templates[] = [
$template_name,
$template_modifier,
implode(' ', $template_type),
true
];
} else {
$info->templates[] = [$template_name, null, null, true];
} }
} }
} }
Expand Down Expand Up @@ -549,10 +586,43 @@ public static function extractClassLikeDocblockInfo(\PhpParser\Node $node, $comm
$info->templates[] = [ $info->templates[] = [
$template_name, $template_name,
$template_modifier, $template_modifier,
implode(' ', $template_type) implode(' ', $template_type),
false
];
} else {
$info->templates[] = [$template_name, null, null, false];
}
}
}

if (isset($comments['specials']['template-covariant'])
|| isset($comments['specials']['psalm-template-covariant'])
) {
$all_templates =
(isset($comments['specials']['template-covariant'])
? $comments['specials']['template-covariant']
: [])
+ (isset($comments['specials']['psalm-template-covariant'])
? $comments['specials']['psalm-template-covariant']
: []);

foreach ($all_templates as $template_line) {
$template_type = preg_split('/[\s]+/', $template_line);

$template_name = array_shift($template_type);

if (count($template_type) > 1
&& in_array(strtolower($template_type[0]), ['as', 'super', 'of'], true)
) {
$template_modifier = strtolower(array_shift($template_type));
$info->templates[] = [
$template_name,
$template_modifier,
implode(' ', $template_type),
true
]; ];
} else { } else {
$info->templates[] = [$template_name]; $info->templates[] = [$template_name, null, null, true];
} }
} }
} }
Expand Down
5 changes: 5 additions & 0 deletions src/Psalm/Internal/Analyzer/MethodAnalyzer.php
View file
Expand Up @@ -653,6 +653,11 @@ public static function compareMethods(
$template_types, $template_types,
$codebase $codebase
); );

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


// treat void as null when comparing against docblock implementer // 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
Expand Up @@ -1010,6 +1010,7 @@ public static function fleshOutType(
$fleshed_out_type->possibly_undefined = $return_type->possibly_undefined; $fleshed_out_type->possibly_undefined = $return_type->possibly_undefined;
$fleshed_out_type->by_ref = $return_type->by_ref; $fleshed_out_type->by_ref = $return_type->by_ref;
$fleshed_out_type->initialized = $return_type->initialized; $fleshed_out_type->initialized = $return_type->initialized;
$fleshed_out_type->had_template = $return_type->had_template;


return $fleshed_out_type; return $fleshed_out_type;
} }
Expand Down
34 changes: 34 additions & 0 deletions src/Psalm/Internal/Analyzer/TypeAnalyzer.php
View file
Expand Up @@ -1605,6 +1605,40 @@ private static function isMatchingTypeContainedBy(
$allow_interface_equality $allow_interface_equality
)) { )) {
$all_types_contain = false; $all_types_contain = false;
} elseif (!$input_type_part instanceof TIterable
&& !$container_param->had_template
&& !$input_param->had_template
&& !$container_param->hasTemplate()
&& !$input_param->hasTemplate()
&& !$input_param->hasLiteralValue()
) {
$input_storage = $codebase->classlike_storage_provider->get($input_type_part->value);

if (!($input_storage->template_covariants[$i] ?? false)) {
// 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
Expand Up @@ -905,7 +905,7 @@ private function convertPhpStormGenericToPsalmGeneric(Type\Union $candidate, $is
{ {
$atomic_types = $candidate->getTypes(); $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; $iterator_name = null;
$generic_params = null; $generic_params = null;


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


$candidate->removeType('array'); $candidate->removeType('array');
$candidate->removeType($iterator_name);
$candidate->addType($generic_iterator); $candidate->addType($generic_iterator);
} }
} }
Expand Down
4 changes: 2 additions & 2 deletions src/Psalm/Internal/LanguageServer/Server/TextDocument.php
View file
Expand Up @@ -160,7 +160,7 @@ public function didClose(TextDocumentIdentifier $textDocument)
* *
* @param TextDocumentIdentifier $textDocument The text document * @param TextDocumentIdentifier $textDocument The text document
* @param Position $position The position inside the text document * @param Position $position The position inside the text document
* @psalm-return Promise<Location|Hover> * @psalm-return Promise<Location>|Promise<Hover>
*/ */
public function definition(TextDocumentIdentifier $textDocument, Position $position): Promise public function definition(TextDocumentIdentifier $textDocument, Position $position): Promise
{ {
Expand Down Expand Up @@ -244,7 +244,7 @@ public function hover(TextDocumentIdentifier $textDocument, Position $position):
* *
* @param TextDocumentIdentifier The text document * @param TextDocumentIdentifier The text document
* @param Position $position The position * @param Position $position The position
* @psalm-return Promise<CompletionItem[]|CompletionList> * @psalm-return Promise<array<empty, empty>>|Promise<CompletionList>
*/ */
public function completion(TextDocumentIdentifier $textDocument, Position $position): Promise public function completion(TextDocumentIdentifier $textDocument, Position $position): Promise
{ {
Expand Down
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Scanner/ClassLikeDocblockComment.php
View file
Expand Up @@ -21,7 +21,7 @@ class ClassLikeDocblockComment
public $internal = false; public $internal = false;


/** /**
* @var array<int, array<int, string>> * @var array<int, array{string, ?string, ?string, bool}>
*/ */
public $templates = []; public $templates = [];


Expand Down
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Scanner/FunctionDocblockComment.php
View file
Expand Up @@ -80,7 +80,7 @@ class FunctionDocblockComment
public $return_type_line_number; public $return_type_line_number;


/** /**
* @var array<int, array<int, string>> * @var array<int, array{string, ?string, ?string, bool}>
*/ */
public $templates = []; public $templates = [];


Expand Down
28 changes: 14 additions & 14 deletions src/Psalm/Internal/Stubs/CoreGenericClasses.php
View file
Expand Up @@ -4,8 +4,8 @@
* Interface to detect if a class is traversable using &foreach;. * Interface to detect if a class is traversable using &foreach;.
* @link http://php.net/manual/en/class.traversable.php * @link http://php.net/manual/en/class.traversable.php
* *
* @template TKey * @template-covariant TKey
* @template TValue * @template-covariant TValue
*/ */
interface Traversable { interface Traversable {
} }
Expand All @@ -14,8 +14,8 @@ interface Traversable {
* Interface to create an external Iterator. * Interface to create an external Iterator.
* @link http://php.net/manual/en/class.iteratoraggregate.php * @link http://php.net/manual/en/class.iteratoraggregate.php
* *
* @template TKey * @template-covariant TKey
* @template TValue * @template-covariant TValue
* *
* @template-extends Traversable<TKey, TValue> * @template-extends Traversable<TKey, TValue>
*/ */
Expand All @@ -36,8 +36,8 @@ public function getIterator();
* themselves internally. * themselves internally.
* @link http://php.net/manual/en/class.iterator.php * @link http://php.net/manual/en/class.iterator.php
* *
* @template TKey * @template-covariant TKey
* @template TValue * @template-covariant TValue
* *
* @template-extends Traversable<TKey, TValue> * @template-extends Traversable<TKey, TValue>
*/ */
Expand Down Expand Up @@ -86,10 +86,10 @@ public function rewind();
} }


/** /**
* @template TKey * @template-covariant TKey
* @template TValue * @template-covariant TValue
* @template TSend * @template-covariant TSend
* @template TReturn * @template-covariant TReturn
* *
* @template-implements Traversable<TKey, TValue> * @template-implements Traversable<TKey, TValue>
*/ */
Expand Down Expand Up @@ -452,8 +452,8 @@ public function getIteratorClass() { }
/** /**
* The Seekable iterator. * The Seekable iterator.
* @link https://php.net/manual/en/class.seekableiterator.php * @link https://php.net/manual/en/class.seekableiterator.php
* @template TKey * @template-covariant TKey
* @template TValue * @template-covariant TValue
* @template-extends Iterator<TKey, TValue> * @template-extends Iterator<TKey, TValue>
*/ */
interface SeekableIterator extends Iterator { interface SeekableIterator extends Iterator {
Expand Down Expand Up @@ -725,7 +725,7 @@ public function getElementsByTagNameNS ($namespaceURI, $localName) {}
} }


/** /**
* @template TNode as DOMNode * @template-covariant TNode as DOMNode
* @template-implements Traversable<int, TNode> * @template-implements Traversable<int, TNode>
*/ */
class DOMNodeList implements Traversable, Countable { class DOMNodeList implements Traversable, Countable {
Expand Down Expand Up @@ -1155,7 +1155,7 @@ public function getHash($object) {}
} }


/** /**
* @template T as object * @template-covariant T as object
* *
* @property-read class-string<T> $name * @property-read class-string<T> $name
*/ */
Expand Down

0 comments on commit 751253d

Please sign in to comment.