Fix #2408 - existing offsets checked with isset should be valid

vimeo · Dec 18, 2019 · af28d65 · af28d65
1 parent 456aa1c
commit af28d65
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 4 deletions.
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php
@@ -144,6 +144,7 @@ public static function updateArrayType(
 
         $parent_var_id = null;
 
+        $offset_already_existed = false;
         $full_var_id = true;
 
         $child_stmt = null;
@@ -330,6 +331,14 @@ public static function updateArrayType(
             && !$child_stmt_var_type->hasObjectType()
         ) {
             $array_var_id = $root_var_id . implode('', $var_id_additions);
+            $parent_var_id = $root_var_id . implode('', \array_slice($var_id_additions, 0, -1));
+
+            if (isset($context->vars_in_scope[$array_var_id])
+                && !$context->vars_in_scope[$array_var_id]->possibly_undefined
+            ) {
+                $offset_already_existed = true;
+            }
+
             $context->vars_in_scope[$array_var_id] = clone $assignment_type;
         }
 
@@ -519,10 +528,21 @@ public static function updateArrayType(
                     $array_atomic_key_type = Type::getMixed();
                 }
 
-                $array_atomic_type = new TNonEmptyArray([
-                    $array_atomic_key_type,
-                    $current_type,
-                ]);
+                if ($offset_already_existed
+                    && $child_stmt
+                    && $parent_var_id
+                    && ($parent_type = $context->vars_in_scope[$parent_var_id] ?? null)
+                    && $parent_type->hasList()
+                ) {
+                    $array_atomic_type = new TNonEmptyList(
+                        $current_type,
+                    );
+                } else {
+                    $array_atomic_type = new TNonEmptyArray([
+                        $array_atomic_key_type,
+                        $current_type,
+                    ]);
+                }
             } else {
                 $array_atomic_type = new TNonEmptyList($current_type);
             }

diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php
@@ -618,6 +618,14 @@ public function hasArray()
         return isset($this->types['array']);
     }
 
+    /**
+     * @return bool
+     */
+    public function hasList()
+    {
+        return isset($this->types['array']) && $this->types['array'] instanceof Atomic\TList;
+    }
+
     /**
      * @return bool
      */

diff --git a/tests/ArrayAssignmentTest.php b/tests/ArrayAssignmentTest.php
@@ -1273,6 +1273,19 @@ function foo(array $arr) : string {
                     '$arr3' => 'array{1: int, 2: int, 3: int, 4: int}',
                 ]
             ],
+            'listPropertyAssignmentAfterIsset' => [
+                '<?php
+                    class Collection {
+                        /** @var list<string> */
+                        private $list = [];
+
+                        public function override(int $offset): void {
+                            if (isset($this->list[$offset])) {
+                                $this->list[$offset] = "a";
+                            }
+                        }
+                    }',
+            ],
         ];
     }