Fix object-like array keys when combining string and automatic keys (f…

…ixes #5427). (#5428) * Fix object-like array keys (fixes #5427). * Fix incorrect return types for tests. * Fix false positive list with literal int key.
vimeo · Mar 20, 2021 · d459071 · d459071
1 parent 42d3bce
commit d459071
Show file tree

Hide file tree

Showing 76 changed files with 122 additions and 97 deletions.
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php
@@ -39,7 +39,7 @@ public static function analyze(
 
         $array_creation_info = new ArrayCreationInfo();
 
-        foreach ($stmt->items as $int_offset => $item) {
+        foreach ($stmt->items as $item) {
             if ($item === null) {
                 \Psalm\IssueBuffer::add(
                     new \Psalm\Issue\ParseError(
@@ -55,7 +55,6 @@ public static function analyze(
                 $statements_analyzer,
                 $context,
                 $array_creation_info,
-                $int_offset,
                 $item
             );
         }
@@ -219,7 +218,6 @@ private static function analyzeArrayItem(
         StatementsAnalyzer $statements_analyzer,
         Context $context,
         ArrayCreationInfo $array_creation_info,
-        int $int_offset,
         PhpParser\Node\Expr\ArrayItem $item
     ) : void {
         if (ExpressionAnalyzer::analyze($statements_analyzer, $item->value, $context) === false) {
@@ -236,7 +234,6 @@ private static function analyzeArrayItem(
             self::handleUnpackedArray(
                 $statements_analyzer,
                 $array_creation_info,
-                $int_offset,
                 $item,
                 $unpacked_array_type
             );
@@ -269,10 +266,9 @@ private static function analyzeArrayItem(
         }
 
         $item_key_value = null;
+        $item_is_list_item = false;
 
         if ($item->key) {
-            $array_creation_info->all_list = false;
-
             $was_inside_use = $context->inside_use;
             $context->inside_use = true;
             if (ExpressionAnalyzer::analyze($statements_analyzer, $item->key, $context) === false) {
@@ -312,16 +308,22 @@ private static function analyzeArrayItem(
                 } elseif ($key_type->isSingleIntLiteral()) {
                     $item_key_value = $key_type->getSingleIntLiteral()->value;
 
-                    if ($item_key_value > $int_offset + $array_creation_info->int_offset_diff) {
-                        $array_creation_info->int_offset_diff = $item_key_value - $int_offset;
+                    if ($item_key_value >= $array_creation_info->int_offset) {
+                        if ($item_key_value === $array_creation_info->int_offset) {
+                            $item_is_list_item = true;
+                        }
+                        $array_creation_info->int_offset = $item_key_value + 1;
                     }
                 }
             }
         } else {
-            $item_key_value = $int_offset + $array_creation_info->int_offset_diff;
+            $item_is_list_item = true;
+            $item_key_value = $array_creation_info->int_offset++;
             $array_creation_info->item_key_atomic_types[] = new Type\Atomic\TInt();
         }
 
+        $array_creation_info->all_list = $array_creation_info->all_list && $item_is_list_item;
+
         if ($item_key_value !== null) {
             if (isset($array_creation_info->array_keys[$item_key_value])) {
                 if (IssueBuffer::accepts(
@@ -420,13 +422,11 @@ private static function analyzeArrayItem(
     private static function handleUnpackedArray(
         StatementsAnalyzer $statements_analyzer,
         ArrayCreationInfo $array_creation_info,
-        int $int_offset,
         PhpParser\Node\Expr\ArrayItem $item,
         Type\Union $unpacked_array_type
     ) : void {
         foreach ($unpacked_array_type->getAtomicTypes() as $unpacked_atomic_type) {
             if ($unpacked_atomic_type instanceof Type\Atomic\TKeyedArray) {
-                $unpacked_array_offset = 0;
                 foreach ($unpacked_atomic_type->properties as $key => $property_value) {
                     if (\is_string($key)) {
                         if (IssueBuffer::accepts(
@@ -448,15 +448,11 @@ private static function handleUnpackedArray(
                         array_values($property_value->getAtomicTypes())
                     );
 
-                    $new_int_offset = $int_offset + $array_creation_info->int_offset_diff + $unpacked_array_offset;
+                    $new_int_offset = $array_creation_info->int_offset++;
 
                     $array_creation_info->array_keys[$new_int_offset] = true;
                     $array_creation_info->property_types[$new_int_offset] = $property_value;
-
-                    $unpacked_array_offset++;
                 }
-
-                $array_creation_info->int_offset_diff += $unpacked_array_offset - 1;
             } else {
                 $array_creation_info->can_create_objectlike = false;
 

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayCreationInfo.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayCreationInfo.php
@@ -38,7 +38,7 @@ class ArrayCreationInfo
     /**
      * @var int
      */
-    public $int_offset_diff = 0;
+    public $int_offset = 0;
 
     /**
      * @var bool

diff --git a/tests/AnnotationTest.php b/tests/AnnotationTest.php
@@ -1181,7 +1181,7 @@ function takesFlags(int $flags) : void {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/ArgTest.php b/tests/ArgTest.php
@@ -284,7 +284,7 @@ function bar(string $p1, int $p3 = 10) : void {}'
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/ArrayAccessTest.php b/tests/ArrayAccessTest.php
@@ -987,7 +987,7 @@ function foo(?array $arr, string $s) : void {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/ArrayAssignmentTest.php b/tests/ArrayAssignmentTest.php
@@ -862,6 +862,13 @@ public function offsetSet($offset, $value): void
                     '$f' => 'array{0: string}',
                 ],
             ],
+            'dontIncrementIntOffsetForKeyedItems' => [
+                '<?php
+                    $a = [1, "a" => 2, 3];',
+                'assertions' => [
+                    '$a' => 'array{0: int, 1: int, a: int}',
+                ],
+            ],
             'assignArrayOrSetNull' => [
                 '<?php
                     $a = [];
@@ -1520,7 +1527,7 @@ function with($key): void
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/ArrayFunctionCallTest.php b/tests/ArrayFunctionCallTest.php
@@ -1922,7 +1922,7 @@ function bar(Container $container, array $data): array {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {
@@ -2104,9 +2104,9 @@ function ints(array $ints) : void {}
                     $list = [3, 2, 5, 9];
                     usort($list, fn(int $a, string $b): int => (int) ($a > $b));',
                 'error_message' => 'InvalidScalarArgument',
-                2 => [],
-                3 => false,
-                4 => '7.4',
+                [],
+                false,
+                '7.4',
             ],
             'usortInvalidComparison' => [
                 '<?php

diff --git a/tests/AssertAnnotationTest.php b/tests/AssertAnnotationTest.php
@@ -1513,7 +1513,7 @@ function allIsInstanceOf($value, $class): void {}'
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/AssignmentTest.php b/tests/AssignmentTest.php
@@ -93,7 +93,7 @@ function foo2(?string &$u, ?string &$v): void {}
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/AttributeTest.php b/tests/AttributeTest.php
@@ -130,7 +130,7 @@ class A {}
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/BinaryOperationTest.php b/tests/BinaryOperationTest.php
@@ -400,7 +400,7 @@ function foo($a, $b): void {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/CallableTest.php b/tests/CallableTest.php
@@ -868,7 +868,7 @@ abstract class TestClass {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/ClassLoadOrderTest.php b/tests/ClassLoadOrderTest.php
@@ -119,7 +119,7 @@ class C extends B {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/ClassScopeTest.php b/tests/ClassScopeTest.php
@@ -136,7 +136,7 @@ private function boop(): void {}
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/ClassStringTest.php b/tests/ClassStringTest.php
@@ -771,7 +771,7 @@ class Beep {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/ClassTest.php b/tests/ClassTest.php
@@ -650,7 +650,7 @@ abstract class C implements I {}
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/CloneTest.php b/tests/CloneTest.php
@@ -46,7 +46,7 @@ public function foo(): self {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/ClosureTest.php b/tests/ClosureTest.php
@@ -557,7 +557,7 @@ function maker(string $className) {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/ConstantTest.php b/tests/ConstantTest.php
@@ -952,7 +952,7 @@ class C {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/DeprecatedAnnotationTest.php b/tests/DeprecatedAnnotationTest.php
@@ -92,7 +92,7 @@ class A {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/DocblockInheritanceTest.php b/tests/DocblockInheritanceTest.php
@@ -150,7 +150,7 @@ function takesF(F $f) : B {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/EnumTest.php b/tests/EnumTest.php
@@ -105,7 +105,7 @@ class C {}
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/ExtendsFinalClassTest.php b/tests/ExtendsFinalClassTest.php
@@ -40,7 +40,7 @@ class B extends A {}'
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/ForbiddenCodeTest.php b/tests/ForbiddenCodeTest.php
@@ -7,7 +7,7 @@ class ForbiddenCodeTest extends TestCase
     use Traits\ValidCodeAnalysisTestTrait;
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/FunctionCallTest.php b/tests/FunctionCallTest.php
@@ -1557,7 +1557,7 @@ function foo(DateTimeImmutable $fooDate): string
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/GeneratorTest.php b/tests/GeneratorTest.php
@@ -274,7 +274,7 @@ function f() : Generator {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/ImmutableAnnotationTest.php b/tests/ImmutableAnnotationTest.php
@@ -526,7 +526,7 @@ public function get(): int {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/InterfaceTest.php b/tests/InterfaceTest.php
@@ -719,7 +719,7 @@ function takesAorB(SomeClass|SomeInterface $some): void {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/InternalAnnotationTest.php b/tests/InternalAnnotationTest.php
@@ -524,7 +524,7 @@ public function batBat() : void {
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {

diff --git a/tests/IssueSuppressionTest.php b/tests/IssueSuppressionTest.php
@@ -281,7 +281,7 @@ function foo($a)
     }
 
     /**
-     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
      */
     public function providerInvalidCodeParse(): iterable
     {
-Original file line number
+Diff line change
@@ Expand Up / @@ -1181,7 +1181,7 @@ function takesFlags(int $flags) : void { @@
         }
         /**
-         * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+         * @return iterable<string,array{string,error_message:string,1?:string[],2?:bool,3?:string}>
          */
         public function providerInvalidCodeParse(): iterable
         {
@@ Expand Down @@