Fix crash when a reference is reassigned in a loop.

vimeo · Jan 11, 2022 · 6883665 · 6883665
1 parent ecde97f
commit 6883665
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 2 deletions.
diff --git a/src/Psalm/Context.php b/src/Psalm/Context.php
@@ -18,6 +18,8 @@
 
 use function array_keys;
 use function array_search;
+use function array_shift;
+use function assert;
 use function count;
 use function in_array;
 use function is_int;
@@ -581,11 +583,32 @@ public function remove(string $remove_var_id, bool $removeDescendents = true): v
     }
 
     /**
-     * Remove a variable from the context which might be a reference to another variable.
-     * Leaves the variable as possibly-in-scope, unlike remove().
+     * Remove a variable from the context which might be a reference to another variable, or
+     * referenced by another variable. Leaves the variable as possibly-in-scope, unlike remove().
      */
     public function removePossibleReference(string $remove_var_id): void
     {
+        if (isset($this->referenced_counts[$remove_var_id]) && $this->referenced_counts[$remove_var_id] > 0) {
+            // If a referenced variable goes out of scope, we need to update the references.
+            // All of the references to this variable are still references to the same value,
+            // so we pick the first one and make the rest of the references point to it.
+            $references = [];
+            foreach ($this->references_in_scope as $reference => $referenced) {
+                if ($referenced === $remove_var_id) {
+                    $references[] = $reference;
+                    unset($this->references_in_scope[$reference]);
+                }
+            }
+            assert(!empty($references));
+            $first_reference = array_shift($references);
+            if (!empty($references)) {
+                /** @psalm-suppress PropertyTypeCoercion #7375 */
+                $this->referenced_counts[$first_reference] = count($references);
+                foreach ($references as $reference) {
+                    $this->references_in_scope[$reference] = $first_reference;
+                }
+            }
+        }
         if (isset($this->references_in_scope[$remove_var_id])) {
             $reference_count = &$this->referenced_counts[$this->references_in_scope[$remove_var_id]];
             if ($reference_count < 1) {
@@ -596,6 +619,7 @@ public function removePossibleReference(string $remove_var_id): void
         unset(
             $this->vars_in_scope[$remove_var_id],
             $this->referenced_var_ids[$remove_var_id],
+            $this->referenced_counts[$remove_var_id],
             $this->references_in_scope[$remove_var_id],
             $this->references_to_external_scope[$remove_var_id],
         );

diff --git a/tests/ReferenceTest.php b/tests/ReferenceTest.php
@@ -202,6 +202,22 @@ class Foo
                     '$bar===' => '"bar"',
                 ],
             ],
+            'referenceReassignedInLoop' => [
+                '<?php
+                    /** @psalm-param list<string> $keys */
+                    function &ensure_array(array &$what, array $keys): array
+                    {
+                        $arr = & $what;
+                        while ($key = array_shift($keys)) {
+                            if (!isset($arr[$key]) || !is_array($arr[$key])) {
+                                $arr[$key] = array();
+                            }
+                            $arr = & $arr[$key];
+                        }
+                        return $arr;
+                    }
+                ',
+            ],
         ];
     }