Improve handling of unsupported references (fixes vimeo#8018).

config.xsd

@@ -467,6 +467,7 @@
             <xs:element name="UnresolvableInclude" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="UnsafeGenericInstantiation" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="UnsafeInstantiation" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="UnsupportedReferenceUsage" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="UnusedClass" type="ClassIssueHandlerType" minOccurs="0" />
             <xs:element name="UnusedClosureParam" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="UnusedConstructor" type="MethodIssueHandlerType" minOccurs="0" />

docs/running_psalm/issues.md

@@ -278,6 +278,7 @@
  - [UnresolvableInclude](issues/UnresolvableInclude.md)
  - [UnsafeGenericInstantiation](issues/UnsafeGenericInstantiation.md)
  - [UnsafeInstantiation](issues/UnsafeInstantiation.md)
+ - [UnsupportedReferenceUsage](issues/UnsupportedReferenceUsage.md)
  - [UnusedClass](issues/UnusedClass.md)
  - [UnusedClosureParam](issues/UnusedClosureParam.md)
  - [UnusedConstructor](issues/UnusedConstructor.md)

docs/running_psalm/issues/UnsupportedReferenceUsage.md

@@ -0,0 +1,34 @@
+# UnsupportedReferenceUsage
+
+Emitted when Psalm encounters a reference that it is not currently able to track (for instance a reference to an array
+offset of an array offset: `$foo = &$bar[$baz[0]]`). When an unsupported reference is encountered, Psalm will issue this
+warning and treat the variable as though it wasn't actually a reference.
+
+## How to fix
+
+This can sometimes be fixed by using a temporary variable:
+
+```php
+<?php
+
+/** @var non-empty-list<int> */
+$bar = [1, 2, 3];
+/** @var non-empty-list<int> */
+$baz = [1, 2, 3];
+
+$foo = &$bar[$baz[0]];
+```
+
+can be turned into
+
+```php
+<?php
+
+/** @var non-empty-list<int> */
+$bar = [1, 2, 3];
+/** @var non-empty-list<int> */
+$baz = [1, 2, 3];
+
+$offset = $baz[0];
+$foo = &$bar[$offset];
+```

src/Psalm/Context.php

@@ -2,6 +2,7 @@
 
 namespace Psalm;
 
+use InvalidArgumentException;
 use Psalm\Internal\Analyzer\StatementsAnalyzer;
 use Psalm\Internal\Clause;
 use Psalm\Internal\ReferenceConstraint;
@@ -608,11 +609,7 @@ public function removePossibleReference(string $remove_var_id): void
             }
         }
         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) {
-                throw new RuntimeException("Incorrect referenced count found");
-            }
-            --$reference_count;
+            $this->decrementReferenceCount($remove_var_id);
         }
         unset(
             $this->vars_in_scope[$remove_var_id],
@@ -623,6 +620,24 @@ public function removePossibleReference(string $remove_var_id): void
         );
     }
 
+    /**
+     * Decrement the reference count of the variable that $ref_id is referring to. This needs to
+     * be done before $ref_id is changed to no longer reference its currently referenced variable,
+     * for example by unsetting, reassigning to another reference, or being shadowed by a global.
+     */
+    public function decrementReferenceCount(string $ref_id): void
+    {
+        if (!isset($this->referenced_counts[$this->references_in_scope[$ref_id]])) {
+            throw new InvalidArgumentException("$ref_id is not a reference");
+        }
+        $reference_count = $this->referenced_counts[$this->references_in_scope[$ref_id]];
+        if ($reference_count < 1) {
+            throw new RuntimeException("Incorrect referenced count found");
+        }
+        --$reference_count;
+        $this->referenced_counts[$this->references_in_scope[$ref_id]] = $reference_count;
+    }
+
     /**
      * @param Clause[]             $clauses
      * @param array<string, bool>  $changed_var_ids

src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php

@@ -5,7 +5,6 @@
 use PhpParser;
 use PhpParser\Node\Expr\ArrayDimFetch;
 use PhpParser\Node\Expr\PropertyFetch;
-use PhpParser\Node\Expr\Variable;
 use Psalm\CodeLocation;
 use Psalm\CodeLocation\DocblockTypeLocation;
 use Psalm\Codebase;
@@ -82,7 +81,6 @@
 use Psalm\Type\Atomic\TNonEmptyList;
 use Psalm\Type\Atomic\TNull;
 use Psalm\Type\Union;
-use RuntimeException;
 use UnexpectedValueException;
 
 use function array_filter;
@@ -947,11 +945,7 @@ public static function analyzeAssignmentRef(
 
         if (isset($context->references_in_scope[$lhs_var_id])) {
             // Decrement old referenced variable's reference count
-            $reference_count = &$context->referenced_counts[$context->references_in_scope[$lhs_var_id]];
-            if ($reference_count < 1) {
-                throw new RuntimeException("Incorrect referenced count found");
-            }
-            --$reference_count;
+            $context->decrementReferenceCount($lhs_var_id);
 
             // Remove old reference parent node so previously referenced variable usage doesn't count as reference usage
             $old_type = $context->vars_in_scope[$lhs_var_id];
@@ -981,44 +975,20 @@ public static function analyzeAssignmentRef(
         }
 
         $lhs_location = new CodeLocation($statements_analyzer->getSource(), $stmt->var);
-        $statements_analyzer->registerVariableAssignment(
-            $lhs_var_id,
-            $lhs_location
-        );
+        if (!$stmt->var instanceof ArrayDimFetch && !$stmt->var instanceof PropertyFetch) {
+            // If left-hand-side is an array offset or object property, usage is too difficult to track,
+            // so it's not registered as an unused variable (this mirrors behavior for non-references).
+            $statements_analyzer->registerVariableAssignment(
+                $lhs_var_id,
+                $lhs_location
+            );
+        }
 
         $lhs_node = DataFlowNode::getForAssignment($lhs_var_id, $lhs_location);
 
         $context->vars_in_scope[$lhs_var_id]->parent_nodes[$lhs_node->id] = $lhs_node;
 
-        if ($statements_analyzer->data_flow_graph !== null
-            && ($stmt->var instanceof ArrayDimFetch || $stmt->var instanceof PropertyFetch)
-            && $stmt->var->var instanceof Variable
-            && is_string($stmt->var->var->name)
-        ) {
-            // If left-hand-side is an array offset or object property, add an edge to the root
-            // variable, so if the root variable is used, the offset/property is also marked as used.
-            $root_var_id = "$" . $stmt->var->var->name;
-            foreach ($context->vars_in_scope[$root_var_id]->parent_nodes as $root_parent) {
-                $statements_analyzer->data_flow_graph->addPath(
-                    $lhs_node,
-                    $root_parent,
-                    $stmt->var instanceof ArrayDimFetch
-                        ? "offset-assignment-as-reference"
-                        : "property-assignment-as-reference"
-                );
-            }
-
-            if ($root_var_id === '$this') {
-                // Variables on `$this` are always used
-                $statements_analyzer->data_flow_graph->addPath(
-                    $lhs_node,
-                    new DataFlowNode('variable-use', 'variable use', null),
-                    'variable-use',
-                );
-            }
-        }
-
-        if ($stmt->var instanceof ArrayDimFetch) {
+        if ($stmt->var instanceof ArrayDimFetch && $stmt->var->dim !== null) {
             // Analyze offset so that variables in the offset get marked as used
             $was_inside_general_use = $context->inside_general_use;
             $context->inside_general_use = true;

src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php

@@ -50,6 +50,7 @@
 use Psalm\Internal\Type\TemplateResult;
 use Psalm\Issue\ForbiddenCode;
 use Psalm\Issue\UnrecognizedExpression;
+use Psalm\Issue\UnsupportedReferenceUsage;
 use Psalm\IssueBuffer;
 use Psalm\Plugin\EventHandler\Event\AfterExpressionAnalysisEvent;
 use Psalm\Storage\FunctionLikeParameter;
@@ -174,26 +175,7 @@ private static function handleExpression(
         }
 
         if ($stmt instanceof PhpParser\Node\Expr\Assign) {
-            $assignment_type = AssignmentAnalyzer::analyze(
-                $statements_analyzer,
-                $stmt->var,
-                $stmt->expr,
-                null,
-                $context,
-                $stmt->getDocComment(),
-                [],
-                !$from_stmt ? $stmt : null
-            );
-
-            if ($assignment_type === false) {
-                return false;
-            }
-
-            if (!$from_stmt) {
-                $statements_analyzer->node_data->setType($stmt, $assignment_type);
-            }
-
-            return true;
+            return self::analyzeAssignment($statements_analyzer, $stmt, $context, $from_stmt);
         }
 
         if ($stmt instanceof PhpParser\Node\Expr\AssignOp) {
@@ -372,7 +354,20 @@ private static function handleExpression(
         }
 
         if ($stmt instanceof PhpParser\Node\Expr\AssignRef) {
-            return AssignmentAnalyzer::analyzeAssignmentRef($statements_analyzer, $stmt, $context);
+            if (!AssignmentAnalyzer::analyzeAssignmentRef($statements_analyzer, $stmt, $context)) {
+                IssueBuffer::maybeAdd(
+                    new UnsupportedReferenceUsage(
+                        "This reference cannot be analyzed by Psalm",
+                        new CodeLocation($statements_analyzer->getSource(), $stmt)
+                    ),
+                    $statements_analyzer->getSuppressedIssues(),
+                );
+
+                // Analyze as if it were a normal assignent and just pretend the reference doesn't exist
+                return self::analyzeAssignment($statements_analyzer, $stmt, $context, $from_stmt);
+            }
+
+            return true;
         }
 
         if ($stmt instanceof PhpParser\Node\Expr\ErrorSuppress) {
@@ -530,4 +525,35 @@ public static function isMock(string $fq_class_name): bool
     {
         return in_array(strtolower($fq_class_name), Config::getInstance()->getMockClasses(), true);
     }
+
+    /**
+     * @param PhpParser\Node\Expr\Assign|PhpParser\Node\Expr\AssignRef $stmt
+     */
+    private static function analyzeAssignment(
+        StatementsAnalyzer $statements_analyzer,
+        PhpParser\Node\Expr $stmt,
+        Context $context,
+        bool $from_stmt,
+    ): bool {
+        $assignment_type = AssignmentAnalyzer::analyze(
+            $statements_analyzer,
+            $stmt->var,
+            $stmt->expr,
+            null,
+            $context,
+            $stmt->getDocComment(),
+            [],
+            !$from_stmt ? $stmt : null
+        );
+
+        if ($assignment_type === false) {
+            return false;
+        }
+
+        if (!$from_stmt) {
+            $statements_analyzer->node_data->setType($stmt, $assignment_type);
+        }
+
+        return true;
+    }
 }

src/Psalm/Internal/Analyzer/Statements/GlobalAnalyzer.php

@@ -12,7 +12,6 @@
 use Psalm\Internal\ReferenceConstraint;
 use Psalm\Issue\InvalidGlobal;
 use Psalm\IssueBuffer;
-use RuntimeException;
 
 use function is_string;
 
@@ -74,11 +73,7 @@ public static function analyze(
 
                     if (isset($context->references_in_scope[$var_id])) {
                         // Global shadows existing reference
-                        $reference_count = &$context->referenced_counts[$context->references_in_scope[$var_id]];
-                        if ($reference_count < 1) {
-                            throw new RuntimeException("Incorrect referenced count found");
-                        }
-                        --$reference_count;
+                        $context->decrementReferenceCount($var_id);
                         unset($context->references_in_scope[$var_id]);
                     }
                     $statements_analyzer->registerVariable(

src/Psalm/Issue/UnsupportedReferenceUsage.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace Psalm\Issue;
+
+final class UnsupportedReferenceUsage extends CodeIssue
+{
+    public const ERROR_LEVEL = 2;
+    public const SHORTCODE = 312;
+}

tests/ReferenceTest.php

@@ -407,6 +407,33 @@ public function providerInvalidCodeParse(): iterable
                 ',
                 'error_message' => 'ReferenceReusedFromConfusingScope',
             ],
+            'unsupportedReferenceUsageWithReferenceToArrayOffsetOfArrayOffset' => [
+                'code' => '<?php
+                    /** @var array<string, string> */
+                    $arr = [];
+
+                    /** @var non-empty-list<string> */
+                    $foo = ["foo"];
+
+                    $bar = &$arr[$foo[0]];
+                ',
+                'error_message' => 'UnsupportedReferenceUsage',
+            ],
+            'unsupportedReferenceUsageContinuesAnalysis' => [
+                'code' => '<?php
+                    /** @var array<string, string> */
+                    $arr = [];
+
+                    /** @var non-empty-list<string> */
+                    $foo = ["foo"];
+
+                    /** @psalm-suppress UnsupportedReferenceUsage */
+                    $bar = &$arr[$foo[0]];
+
+                    /** @psalm-trace $bar */;
+                ',
+                'error_message' => ' - $bar: string',
+            ],
         ];
     }
 }