From d54eebfe024ebfa65f49ff3c1809184132a39885 Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Wed, 1 Jun 2022 18:05:47 -0500 Subject: [PATCH 1/4] Fix various array spread issues. - Correctly infer `array` and `list` instead of `non-empty-array` and `non-empty-list` (fixes #7296) - Add support for spreading string keys (fixes #7297). - Show issue when trying to unpack non-iterable - Show issue when trying to unpack iterable with non-array-key key - Re-added invalid PHP 8.0 tests removed in #6613 - Unpacked lists with known keys will be inferred as eg `array{0: int, 1: int}, int>` now but will still be treated as lists - Unpacked arrays with known keys will now be inferred as eg `array{a: string, b: string}` instead of `array` --- .../Statements/Expression/ArrayAnalyzer.php | 224 +++++++------ .../Expression/ArrayCreationInfo.php | 2 + src/Psalm/Type/Atomic.php | 40 +++ tests/ArrayAssignmentTest.php | 302 +++++++++++++++++- 4 files changed, 458 insertions(+), 110 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php index a5c755a860f..1745064e02b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php @@ -8,12 +8,15 @@ use Psalm\Context; use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; +use Psalm\Internal\Codebase\ConstantTypeResolver; use Psalm\Internal\Codebase\TaintFlowGraph; use Psalm\Internal\Codebase\VariableUseGraph; use Psalm\Internal\DataFlow\DataFlowNode; +use Psalm\Internal\Type\Comparator\UnionTypeComparator; use Psalm\Internal\Type\TypeCombiner; use Psalm\Issue\DuplicateArrayKey; use Psalm\Issue\InvalidArrayOffset; +use Psalm\Issue\InvalidOperand; use Psalm\Issue\MixedArrayOffset; use Psalm\Issue\ParseError; use Psalm\IssueBuffer; @@ -24,9 +27,7 @@ use Psalm\Type\Atomic\TBool; use Psalm\Type\Atomic\TFalse; use Psalm\Type\Atomic\TFloat; -use Psalm\Type\Atomic\TGenericObject; use Psalm\Type\Atomic\TInt; -use Psalm\Type\Atomic\TIterable; use Psalm\Type\Atomic\TKeyedArray; use Psalm\Type\Atomic\TList; use Psalm\Type\Atomic\TLiteralClassString; @@ -93,29 +94,7 @@ public static function analyze( ); } - // if this array looks like an object-like array, let's return that instead - if ($array_creation_info->can_create_objectlike - && $array_creation_info->property_types - ) { - $object_like = new TKeyedArray( - $array_creation_info->property_types, - $array_creation_info->class_strings - ); - $object_like->sealed = true; - $object_like->is_list = $array_creation_info->all_list; - - $stmt_type = new Union([$object_like]); - - if ($array_creation_info->parent_taint_nodes) { - $stmt_type->parent_nodes = $array_creation_info->parent_taint_nodes; - } - - $statements_analyzer->node_data->setType($stmt, $stmt_type); - - return true; - } - - if ($array_creation_info->item_key_atomic_types) { + if (!empty($array_creation_info->item_key_atomic_types)) { $item_key_type = TypeCombiner::combine( $array_creation_info->item_key_atomic_types, $codebase, @@ -127,7 +106,7 @@ public static function analyze( $item_key_type = null; } - if ($array_creation_info->item_value_atomic_types) { + if (!empty($array_creation_info->item_value_atomic_types)) { $item_value_type = TypeCombiner::combine( $array_creation_info->item_value_atomic_types, $codebase, @@ -139,6 +118,28 @@ public static function analyze( $item_value_type = null; } + // if this array looks like an object-like array, let's return that instead + if (!empty($array_creation_info->property_types)) { + $atomic_type = new TKeyedArray($array_creation_info->property_types, $array_creation_info->class_strings); + if ($array_creation_info->can_create_objectlike) { + $atomic_type->sealed = true; + } else { + $atomic_type->previous_key_type = $item_key_type ?? Type::getArrayKey(); + $atomic_type->previous_value_type = $item_value_type ?? Type::getMixed(); + } + $atomic_type->is_list = $array_creation_info->all_list; + + $stmt_type = new Union([$atomic_type]); + + if ($array_creation_info->parent_taint_nodes) { + $stmt_type->parent_nodes = $array_creation_info->parent_taint_nodes; + } + + $statements_analyzer->node_data->setType($stmt, $stmt_type); + + return true; + } + if ($item_key_type === null && $item_value_type === null) { $statements_analyzer->node_data->setType($stmt, Type::getEmptyArray()); @@ -146,12 +147,10 @@ public static function analyze( } if ($array_creation_info->all_list) { - if (empty($array_creation_info->item_key_atomic_types)) { + if ($array_creation_info->can_be_empty) { $array_type = new TList($item_value_type ?? Type::getMixed()); } else { $array_type = new TNonEmptyList($item_value_type ?? Type::getMixed()); - /** @psalm-suppress InvalidPropertyAssignmentValue */ - $array_type->count = count($array_creation_info->property_types); } $stmt_type = new Union([ @@ -233,13 +232,11 @@ public static function analyze( } } - $array_type = new TNonEmptyArray([ + $array_args = [ $item_key_type && !$item_key_type->hasMixed() ? $item_key_type : Type::getArrayKey(), $item_value_type ?? Type::getMixed(), - ]); - - /** @psalm-suppress InvalidPropertyAssignmentValue */ - $array_type->count = count($array_creation_info->property_types); + ]; + $array_type = $array_creation_info->can_be_empty ? new TArray($array_args) : new TNonEmptyArray($array_args); $stmt_type = new Union([ $array_type, @@ -311,6 +308,8 @@ private static function analyzeArrayItem( $item_key_type = null; $item_is_list_item = false; + $array_creation_info->can_be_empty = false; + if ($item->key) { $was_inside_general_use = $context->inside_general_use; $context->inside_general_use = true; @@ -338,11 +337,6 @@ private static function analyzeArrayItem( $key_type = Type::getInt(false, (int) $item->key->value); } - $array_creation_info->item_key_atomic_types = array_merge( - $array_creation_info->item_key_atomic_types, - array_values($key_type->getAtomicTypes()) - ); - if ($key_type->isSingleStringLiteral()) { $item_key_literal_type = $key_type->getSingleStringLiteral(); $item_key_value = $item_key_literal_type->value; @@ -360,11 +354,15 @@ private static function analyzeArrayItem( $array_creation_info->int_offset = $item_key_value + 1; } } + } else { + $key_type = Type::getArrayKey(); } } else { $item_is_list_item = true; $item_key_value = $array_creation_info->int_offset++; - $array_creation_info->item_key_atomic_types[] = new TLiteralInt($item_key_value); + $key_atomic_type = new TLiteralInt($item_key_value); + $array_creation_info->item_key_atomic_types[] = $key_atomic_type; + $key_type = new Union([$key_atomic_type]); } if (ExpressionAnalyzer::analyze($statements_analyzer, $item->value, $context) === false) { @@ -493,12 +491,15 @@ private static function analyzeArrayItem( $array_creation_info->property_types[$item_key_value] = $item_value_type; } else { $array_creation_info->can_create_objectlike = false; + $array_creation_info->item_key_atomic_types = array_merge( + $array_creation_info->item_key_atomic_types, + array_values($key_type->getAtomicTypes()) + ); + $array_creation_info->item_value_atomic_types = array_merge( + $array_creation_info->item_value_atomic_types, + array_values($item_value_type->getAtomicTypes()) + ); } - - $array_creation_info->item_value_atomic_types = array_merge( - $array_creation_info->item_value_atomic_types, - array_values($item_value_type->getAtomicTypes()) - ); } else { $array_creation_info->item_value_atomic_types[] = new TMixed(); @@ -508,6 +509,11 @@ private static function analyzeArrayItem( $array_creation_info->property_types[$item_key_value] = Type::getMixed(); } else { $array_creation_info->can_create_objectlike = false; + $array_creation_info->item_key_atomic_types = array_merge( + $array_creation_info->item_key_atomic_types, + array_values($key_type->getAtomicTypes()) + ); + $array_creation_info->item_value_atomic_types[] = new TMixed(); } } } @@ -519,6 +525,8 @@ private static function handleUnpackedArray( Union $unpacked_array_type, Codebase $codebase ): void { + $all_non_empty = true; + foreach ($unpacked_array_type->getAtomicTypes() as $unpacked_atomic_type) { if ($unpacked_atomic_type instanceof TKeyedArray) { foreach ($unpacked_atomic_type->properties as $key => $property_value) { @@ -532,79 +540,109 @@ private static function handleUnpackedArray( $statements_analyzer->getSuppressedIssues() ); - return; + continue 2; } $new_offset = $key; $array_creation_info->item_key_atomic_types[] = new TLiteralString($new_offset); + $array_creation_info->all_list = false; } else { $new_offset = $array_creation_info->int_offset++; $array_creation_info->item_key_atomic_types[] = new TLiteralInt($new_offset); } - $array_creation_info->item_value_atomic_types = array_merge( - $array_creation_info->item_value_atomic_types, - array_values($property_value->getAtomicTypes()) - ); - $array_creation_info->array_keys[$new_offset] = true; $array_creation_info->property_types[$new_offset] = $property_value; } + + if (!$unpacked_atomic_type->isNonEmpty()) { + $all_non_empty = false; + } } else { $codebase = $statements_analyzer->getCodebase(); - if ($unpacked_atomic_type instanceof TArray - || $unpacked_atomic_type instanceof TIterable - || ( - $unpacked_atomic_type instanceof TGenericObject - && $unpacked_atomic_type->hasTraversableInterface($codebase) - && count($unpacked_atomic_type->type_params) === 2 - )) { - /** @psalm-suppress PossiblyUndefinedArrayOffset provably true, but Psalm can’t see it */ - if ($unpacked_atomic_type->type_params[1]->isNever()) { - continue; - } + if (!$unpacked_atomic_type instanceof TNonEmptyList + && !$unpacked_atomic_type instanceof TNonEmptyArray + ) { + $all_non_empty = false; + } + + if (!$unpacked_atomic_type->isIterable($codebase)) { $array_creation_info->can_create_objectlike = false; + $array_creation_info->item_key_atomic_types[] = new TArrayKey(); + $array_creation_info->item_value_atomic_types[] = new TMixed(); + IssueBuffer::maybeAdd( + new InvalidOperand( + "Cannot use spread operator on non-iterable type {$unpacked_array_type->getId()}", + new CodeLocation($statements_analyzer->getSource(), $item->value), + ), + $statements_analyzer->getSuppressedIssues(), + ); + continue; + } - if ($unpacked_atomic_type->type_params[0]->hasString()) { - if ($codebase->analysis_php_version_id <= 8_00_00) { - IssueBuffer::maybeAdd( - new DuplicateArrayKey( - 'String keys are not supported in unpacked arrays', - new CodeLocation($statements_analyzer->getSource(), $item->value) - ), - $statements_analyzer->getSuppressedIssues() - ); + $iterable_type = $unpacked_atomic_type->getIterable($codebase); - return; - } + if ($iterable_type->type_params[0]->isNever()) { + continue; + } - $array_creation_info->item_key_atomic_types[] = new TString(); - } elseif ($unpacked_atomic_type->type_params[0]->hasInt()) { - $array_creation_info->item_key_atomic_types[] = new TInt(); - } + $array_creation_info->can_create_objectlike = false; - $array_creation_info->item_value_atomic_types = array_merge( - $array_creation_info->item_value_atomic_types, - array_values( - isset($unpacked_atomic_type->type_params[1]) - ? $unpacked_atomic_type->type_params[1]->getAtomicTypes() - : [new TMixed()] - ) + if (!UnionTypeComparator::canBeContainedBy( + $codebase, + $iterable_type->type_params[0], + Type::getArrayKey(), + )) { + IssueBuffer::maybeAdd( + new InvalidOperand( + "Cannot use spread operator on iterable with key type " + . $iterable_type->type_params[0]->getId(), + new CodeLocation($statements_analyzer->getSource(), $item->value), + ), + $statements_analyzer->getSuppressedIssues(), ); - } elseif ($unpacked_atomic_type instanceof TList) { - if ($unpacked_atomic_type->type_param->isNever()) { + continue; + } + + if ($iterable_type->type_params[0]->hasString()) { + if ($codebase->analysis_php_version_id <= 8_00_00) { + IssueBuffer::maybeAdd( + new DuplicateArrayKey( + 'String keys are not supported in unpacked arrays', + new CodeLocation($statements_analyzer->getSource(), $item->value) + ), + $statements_analyzer->getSuppressedIssues() + ); + continue; } - $array_creation_info->can_create_objectlike = false; - - $array_creation_info->item_key_atomic_types[] = new TInt(); + $array_creation_info->all_list = false; + } - $array_creation_info->item_value_atomic_types = array_merge( - $array_creation_info->item_value_atomic_types, - array_values($unpacked_atomic_type->type_param->getAtomicTypes()) - ); + // Unpacked array might overwrite known properties, so values are merged when the keys intersect. + foreach ($array_creation_info->property_types as $prop_key_val => $prop_val) { + $prop_key = new Union([ConstantTypeResolver::getLiteralTypeFromScalarValue($prop_key_val)]); + // Since $prop_key is a single literal type, the types intersect iff $prop_key is contained by the + // template type (ie $prop_key cannot overlap with the template type without being contained by it). + if (UnionTypeComparator::isContainedBy($codebase, $prop_key, $iterable_type->type_params[0])) { + $new_prop_val = Type::combineUnionTypes($prop_val, $iterable_type->type_params[1]); + $array_creation_info->property_types[$prop_key_val] = $new_prop_val; + } } + + $array_creation_info->item_key_atomic_types = array_merge( + $array_creation_info->item_key_atomic_types, + array_values($iterable_type->type_params[0]->getAtomicTypes()) + ); + $array_creation_info->item_value_atomic_types = array_merge( + $array_creation_info->item_value_atomic_types, + array_values($iterable_type->type_params[1]->getAtomicTypes()) + ); } } + + if ($all_non_empty) { + $array_creation_info->can_be_empty = false; + } } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayCreationInfo.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayCreationInfo.php index 98a0635cbb4..cf7447a1057 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayCreationInfo.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayCreationInfo.php @@ -55,4 +55,6 @@ class ArrayCreationInfo * @var array */ public $parent_taint_nodes = []; + + public bool $can_be_empty = true; } diff --git a/src/Psalm/Type/Atomic.php b/src/Psalm/Type/Atomic.php index 02c182c5eda..126ac673269 100644 --- a/src/Psalm/Type/Atomic.php +++ b/src/Psalm/Type/Atomic.php @@ -2,10 +2,12 @@ namespace Psalm\Type; +use InvalidArgumentException; use Psalm\Codebase; use Psalm\Exception\TypeParseTreeException; use Psalm\Internal\Analyzer\StatementsAnalyzer; use Psalm\Internal\Type\TemplateResult; +use Psalm\Internal\Type\TemplateStandinTypeReplacer; use Psalm\Internal\Type\TypeAlias; use Psalm\Internal\Type\TypeAlias\LinkableTypeAlias; use Psalm\Type; @@ -379,6 +381,41 @@ public function isIterable(Codebase $codebase): bool || $this instanceof TList; } + /** + * @throws InvalidArgumentException if $this is not an iterable type. + */ + public function getIterable(Codebase $codebase): TIterable + { + if ($this instanceof TIterable) { + return $this; + } + if ($this instanceof TArray) { + return new TIterable($this->type_params); + } + if ($this instanceof TList) { + return new TIterable([new Union([new TIntRange(0, null)]), $this->type_param]); + } + if ($this instanceof TKeyedArray) { + return new TIterable([$this->getGenericKeyType(), $this->getGenericValueType()]); + } + if ($this->hasTraversableInterface($codebase)) { + if (strtolower($this->value) === "traversable") { + if ($this instanceof TGenericObject) { + return new TIterable($this->type_params); + } + return new TIterable([Type::getMixed(), Type::getMixed()]); + } + + $implemented_traversable_templates = TemplateStandinTypeReplacer::getMappedGenericTypeParams( + $codebase, + $this, + new TGenericObject("Traversable", [Type::getMixed(), Type::getMixed()]), + ); + return new TIterable($implemented_traversable_templates); + } + throw new InvalidArgumentException("{$this->getId()} is not an iterable"); + } + public function isCountable(Codebase $codebase): bool { return $this->hasCountableInterface($codebase) @@ -387,6 +424,9 @@ public function isCountable(Codebase $codebase): bool || $this instanceof TList; } + /** + * @psalm-assert-if-true TNamedObject $this + */ public function hasTraversableInterface(Codebase $codebase): bool { return $this instanceof TNamedObject diff --git a/tests/ArrayAssignmentTest.php b/tests/ArrayAssignmentTest.php index 377886620eb..8cc2f3a30a7 100644 --- a/tests/ArrayAssignmentTest.php +++ b/tests/ArrayAssignmentTest.php @@ -32,7 +32,7 @@ public function testConditionalAssignment(): void } /** - * @return iterable,ignored_issues?:list}> + * @return iterable,ignored_issues?:list,php_version?:string}> */ public function providerValidCodeParse(): iterable { @@ -1514,14 +1514,36 @@ function unpackIterable(Traversable $data): array return [...$data]; }' ], - 'unpackCanBeEmpty' => [ + 'unpackEmptyArrayIsEmpty' => [ 'code' => ' ['$x' => 'array'] + 'assertions' => ['$x===' => 'array'], + ], + 'unpackListCanBeEmpty' => [ + 'code' => ' */ + $x = []; + /** @var list */ + $y = []; + + $x = [...$x, ...$y]; + ', + 'assertions' => ['$x===' => 'list'], + ], + 'unpackNonEmptyListIsNotEmpty' => [ + 'code' => ' */ + $x = []; + /** @var non-empty-list */ + $y = []; + + $x = [...$x, ...$y]; + ', + 'assertions' => ['$x===' => 'non-empty-list'], ], 'unpackEmptyKeepsCorrectKeys' => [ 'code' => ' ['$e' => 'array{int, int, int}'] + 'assertions' => ['$e===' => 'array{1, 2, 3}'], + ], + 'unpackArrayCanBeEmpty' => [ + 'code' => ' */ + $x = []; + /** @var array */ + $y = []; + + $x = [...$x, ...$y]; + ', + 'assertions' => ['$x===' => 'array'], + 'ignored_issues' => [], + 'php_version' => '8.1', + ], + 'unpackNonEmptyArrayIsNotEmpty' => [ + 'code' => ' */ + $x = []; + /** @var non-empty-array */ + $y = []; + + $x = [...$x, ...$y]; + ', + 'assertions' => ['$x===' => 'non-empty-array'], + 'ignored_issues' => [], + 'php_version' => '8.1', + ], + 'unpackIntKeyedArrayResultsInList' => [ + 'code' => ' */ + $x = []; + /** @var array */ + $y = []; + + $x = [...$x, ...$y]; + ', + 'assertions' => ['$x===' => 'list'], ], - 'unpackNonObjectlikePreventsObjectlikeArray' => [ + 'unpackStringKeyedArrayPhp8.1' => [ + 'code' => ' */ + $x = []; + /** @var array */ + $y = []; + + $x = [...$x, ...$y]; + ', + 'assertions' => ['$x===' => 'array'], + 'ignored_issues' => [], + 'php_version' => '8.1', + ], + 'unpackLiteralStringKeyedArrayPhp8.1' => [ + 'code' => ' */ + $x = []; + /** @var array<"baz", int> */ + $y = []; + + $x = [...$x, ...$y]; + ', + 'assertions' => ['$x===' => "array<'bar'|'baz'|'foo', int>"], + 'ignored_issues' => [], + 'php_version' => '8.1', + ], + 'unpackArrayShapesUnionsLaterUnpacks' => [ + 'code' => ' 1, "bar" => 2, 10 => 3]; + /** @var array */ + $a = []; + /** @var list<5> */ + $b = []; + /** @var array */ + $c = []; + + $x = [...$a, ...$b, ...$c, ...$shape]; // Shape is last so it overrides previous + $y = [...$shape, ...$a, ...$b, ...$c]; // Shape is first, but only possibly matching keys union their values + ', + 'assertions' => [ + '$x===' => 'array{0: 3, bar: 2, foo: 1}', + '$y===' => 'array{0: 3|4|5|6, bar: 2|6, foo: 1|6}', + ], + 'ignored_issues' => [], + 'php_version' => '8.1', + ], + 'unpackNonObjectlike' => [ 'code' => ' */ function test(): array { @@ -1543,8 +1648,113 @@ function test(): array { $x = [...test(), "a" => "b"]; ', - 'assertions' => ['$x' => 'non-empty-array'] + 'assertions' => ['$x===' => "array{a: 'b'}, mixed>"], ], + 'checkTraversableUnpackTemplatesCorrectly' => [ + 'code' => ' + */ + interface Foo extends Traversable {} + + /** + * @param Foo<"a"|"b", "c"|"d", "e"|"f", "g"|"h"> $foo + * @return array<"e"|"f", "g"|"h"> + */ + function foobar(Foo $foo): array + { + return [...$foo]; + } + ', + 'assertions' => [], + 'ignored_issues' => [], + 'php_version' => '8.1', + ], + 'unpackIncorrectlyExtendedTraversable' => [ + 'code' => ' */ + interface Foo extends Traversable {} + + /** + * @return array + */ + function foobar(Foo $foo): array + { + return [...$foo]; + } + ', + ], + 'unpackGrandchildOfTraversable' => [ + 'code' => ' + */ + interface Foo extends Traversable {} + + /** @extends Foo<"a", "b", "c", "d"> */ + interface Bar extends Foo {} + + /** + * @return array<"c", "d"> + */ + function foobar(Bar $bar): array + { + return [...$bar]; + } + ', + 'assertions' => [], + 'ignored_issues' => [], + 'php_version' => '8.1', + ], + 'unpackNonGenericGrandchildOfTraversable' => [ + 'code' => ' */ + interface Foo extends Traversable {} + + interface Bar extends Foo {} + + /** + * @return array + */ + function foobar(Bar $bar): array + { + return [...$bar]; + } + ', + 'assertions' => [], + 'ignored_issues' => [], + 'php_version' => '8.1', + ], + 'unpackTNamedObjectShouldUseTemplateConstraints' => [ + 'code' => ' + */ + interface Foo extends Traversable {} + + /** + * @return array<"a"|"b", "c"|"d"> + */ + function foobar(Foo $foo): array + { + return [...$foo]; + } + ', + 'assertions' => [], + 'ignored_issues' => [], + 'php_version' => '8.1', + ], + 'ArrayOffsetNumericSupPHPINTMAX' => [ 'code' => ' $data - * @return list + * @return array */ function unpackIterable(iterable $data): array { @@ -1674,7 +1884,7 @@ function unpackIterable(iterable $data): array /** * @param Traversable $data - * @return list + * @return array */ function unpackIterable(Traversable $data): array { @@ -1689,7 +1899,7 @@ function unpackIterable(Traversable $data): array /** * @param array $data - * @return list + * @return array */ function unpackArray(array $data): array { @@ -1714,12 +1924,9 @@ function posiviteIntegers(): array return [1]; } - $_a = [...posiviteIntegers(), int()];', - 'assertions' => [ - '$_a' => 'non-empty-list', - ], - 'ignored_issues' => [], - 'php_version' => '8.1' + $_a = [...posiviteIntegers(), int()]; + /** @psalm-check-type $_a = non-empty-list */ + ', ], 'nullableDestructuring' => [ 'code' => ' 'MixedStringOffsetAssignment', - 'error_level' => ['MixedAssignment'], + 'ignored_issues' => ['MixedAssignment'], ], 'mixedArrayArgument' => [ 'code' => ' 'MixedArgumentTypeCoercion', - 'error_level' => ['MixedAssignment'], + 'ignored_issues' => ['MixedAssignment'], ], 'arrayPropertyAssignment' => [ 'code' => ' 'InvalidArrayOffset' ], + 'unpackTypedIterableWithStringKeysIntoArray' => [ + 'code' => ' $data + * @return list + */ + function unpackIterable(iterable $data): array + { + return [...$data]; + } + ', + 'error_message' => 'DuplicateArrayKey', + 'ignored_issues' => [], + 'php_version' => '8.0', + ], + 'unpackTypedTraversableWithStringKeysIntoArray' => [ + 'code' => ' $data + * @return list + */ + function unpackIterable(Traversable $data): array + { + return [...$data]; + } + ', + 'error_message' => 'DuplicateArrayKey', + 'ignored_issues' => [], + 'php_version' => '8.0', + ], + 'unpackArrayWithArrayKeyIntoArray' => [ + 'code' => ' $data + * @return list + */ + function unpackArray(array $data): array + { + return [...$data]; + } + ', + 'error_message' => 'DuplicateArrayKey', + 'ignored_issues' => [], + 'php_version' => '8.0', + ], + 'unpackNonIterable' => [ + 'code' => ' 'InvalidOperand', + ], + 'cantUnpackWhenKeyIsntArrayKey' => [ + 'code' => ' */ + $foo = []; + $bar = [...$foo]; + ', + 'error_message' => 'InvalidOperand', + ], ]; } } From 2b389df270dc834a9cd47f77c4896acd70a08d95 Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Thu, 2 Jun 2022 12:57:22 -0500 Subject: [PATCH 2/4] Use `count` instead of `empty`. --- .../Analyzer/Statements/Expression/ArrayAnalyzer.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php index 1745064e02b..d161f40cdd1 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php @@ -63,7 +63,7 @@ public static function analyze( Context $context ): bool { // if the array is empty, this special type allows us to match any other array type against it - if (empty($stmt->items)) { + if (count($stmt->items) === 0) { $statements_analyzer->node_data->setType($stmt, Type::getEmptyArray()); return true; @@ -94,7 +94,7 @@ public static function analyze( ); } - if (!empty($array_creation_info->item_key_atomic_types)) { + if (count($array_creation_info->item_key_atomic_types) !== 0) { $item_key_type = TypeCombiner::combine( $array_creation_info->item_key_atomic_types, $codebase, @@ -106,7 +106,7 @@ public static function analyze( $item_key_type = null; } - if (!empty($array_creation_info->item_value_atomic_types)) { + if (count($array_creation_info->item_value_atomic_types) !== 0) { $item_value_type = TypeCombiner::combine( $array_creation_info->item_value_atomic_types, $codebase, @@ -119,7 +119,7 @@ public static function analyze( } // if this array looks like an object-like array, let's return that instead - if (!empty($array_creation_info->property_types)) { + if (count($array_creation_info->property_types) !== 0) { $atomic_type = new TKeyedArray($array_creation_info->property_types, $array_creation_info->class_strings); if ($array_creation_info->can_create_objectlike) { $atomic_type->sealed = true; From ee68184527105c93d5041580903c8832b472abd1 Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Thu, 2 Jun 2022 15:18:09 -0500 Subject: [PATCH 3/4] Fix key comparison when unpacking Traversables. --- .../Statements/Expression/ArrayAnalyzer.php | 2 +- tests/ArrayAssignmentTest.php | 38 ++++++++++++++++--- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php index d161f40cdd1..c9106febd15 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php @@ -588,7 +588,7 @@ private static function handleUnpackedArray( $array_creation_info->can_create_objectlike = false; - if (!UnionTypeComparator::canBeContainedBy( + if (!UnionTypeComparator::isContainedBy( $codebase, $iterable_type->type_params[0], Type::getArrayKey(), diff --git a/tests/ArrayAssignmentTest.php b/tests/ArrayAssignmentTest.php index 8cc2f3a30a7..9558b0f6590 100644 --- a/tests/ArrayAssignmentTest.php +++ b/tests/ArrayAssignmentTest.php @@ -1674,17 +1674,30 @@ function foobar(Foo $foo): array 'ignored_issues' => [], 'php_version' => '8.1', ], - 'unpackIncorrectlyExtendedTraversable' => [ + 'unpackIncorrectlyExtendedInterface' => [ 'code' => ' */ + /** + * @template TKey + * @template TValue of scalar + * @extends Traversable + */ interface Foo extends Traversable {} /** - * @return array + * @psalm-suppress MissingTemplateParam + * @template TKey + * @extends Foo */ - function foobar(Foo $foo): array + interface Bar extends Foo {} + + /** + * @param Bar $bar + * @return list + */ + function foobar(Bar $bar): array { - return [...$foo]; + $unpacked = [...$bar]; + return $unpacked; } ', ], @@ -2407,6 +2420,21 @@ class Foo {} ', 'error_message' => 'InvalidOperand', ], + 'unpackTraversableWithKeyOmitted' => [ + 'code' => ' */ + interface Foo extends Traversable {} + + /** + * @return array + */ + function foobar(Foo $foo): array + { + return [...$foo]; + } + ', + 'error_message' => 'InvalidOperand', + ], ]; } } From 094621d5bb0c91860dc499b59bab3f4f9210c569 Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Mon, 6 Jun 2022 16:33:29 -0500 Subject: [PATCH 4/4] Fix array-shape value type being mixed instead of single key being mixed. --- .../Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php index c9106febd15..f33c0d82ad4 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php @@ -501,8 +501,6 @@ private static function analyzeArrayItem( ); } } else { - $array_creation_info->item_value_atomic_types[] = new TMixed(); - if ($item_key_value !== null && count($array_creation_info->property_types) <= $config->max_shaped_array_size ) {