From 3e0c4cfb75f647e8e7821976b47d8f65d184a6b7 Mon Sep 17 00:00:00 2001
From: Brown <github@muglug.com>
Date: Sat, 2 May 2020 20:36:41 -0400
Subject: [PATCH] Fix #3210 - prevent possibly-null array access from
 destructure

---
 .../Expression/AssignmentAnalyzer.php         | 61 ++++++++++++++++++-
 tests/ArrayAccessTest.php                     | 13 ++++
 2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
index 8006041c1..76049c433 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
@@ -17,13 +17,17 @@ use Psalm\Internal\FileManipulation\FileManipulationBuffer;
 use Psalm\Issue\AssignmentToVoid;
 use Psalm\Issue\ImpureByReferenceAssignment;
 use Psalm\Issue\ImpurePropertyAssignment;
+use Psalm\Issue\InvalidArrayAccess;
 use Psalm\Issue\InvalidArrayOffset;
 use Psalm\Issue\InvalidDocblock;
 use Psalm\Issue\InvalidScope;
 use Psalm\Issue\LoopInvalidation;
 use Psalm\Issue\MissingDocblockType;
 use Psalm\Issue\MixedAssignment;
+use Psalm\Issue\MixedArrayAccess;
 use Psalm\Issue\NoValue;
+use Psalm\Issue\PossiblyInvalidArrayAccess;
+use Psalm\Issue\PossiblyNullArrayAccess;
 use Psalm\Issue\PossiblyUndefinedArrayOffset;
 use Psalm\Issue\ReferenceConstraintViolation;
 use Psalm\Issue\UnnecessaryVarAnnotation;
@@ -537,7 +541,60 @@ class AssignmentAnalyzer
                             $doc_comment
                         );
 
-                        continue 2;
+                        continue;
+                    }
+
+                    if ($assign_value_atomic_type instanceof Type\Atomic\TMixed) {
+                        if (IssueBuffer::accepts(
+                            new MixedArrayAccess(
+                                'Cannot access array value on mixed variable ' . $array_var_id,
+                                new CodeLocation($statements_analyzer->getSource(), $var)
+                            ),
+                            $statements_analyzer->getSuppressedIssues()
+                        )) {
+                            // fall through
+                        }
+                    } elseif ($assign_value_atomic_type instanceof Type\Atomic\TNull) {
+                        if (IssueBuffer::accepts(
+                            new PossiblyNullArrayAccess(
+                                'Cannot access array value on null variable ' . $array_var_id,
+                                new CodeLocation($statements_analyzer->getSource(), $var)
+                            ),
+                            $statements_analyzer->getSuppressedIssues()
+                        )
+                        ) {
+                            // do nothing
+                        }
+                    } elseif (!$assign_value_atomic_type instanceof Type\Atomic\TArray
+                        && !$assign_value_atomic_type instanceof Type\Atomic\ObjectLike
+                        && !$assign_value_atomic_type instanceof Type\Atomic\TList
+                    ) {
+                        if ($assign_value_type->hasArray()) {
+                            if (IssueBuffer::accepts(
+                                new PossiblyInvalidArrayAccess(
+                                    'Cannot access array value on non-array variable '
+                                        . $array_var_id . ' of type ' . $assign_value_atomic_type->getId(),
+                                    new CodeLocation($statements_analyzer->getSource(), $var)
+                                ),
+                                $statements_analyzer->getSuppressedIssues()
+                            )
+                            ) {
+                                // do nothing
+                            }
+                        } else {
+                            if (IssueBuffer::accepts(
+                                new InvalidArrayAccess(
+                                    'Cannot access array value on non-array variable '
+                                        . $array_var_id . ' of type ' . $assign_value_atomic_type->getId(),
+                                    new CodeLocation($statements_analyzer->getSource(), $var)
+                                ),
+                                $statements_analyzer->getSuppressedIssues()
+                            )
+                            ) {
+                                // do nothing
+                            }
+                        }
+
                     }
 
                     if ($var instanceof PhpParser\Node\Expr\List_
@@ -564,6 +621,8 @@ class AssignmentAnalyzer
                             $context,
                             $doc_comment
                         );
+
+                        continue;
                     }
 
                     if ($list_var_id) {
diff --git a/tests/ArrayAccessTest.php b/tests/ArrayAccessTest.php
index 1511bea80..943ce4ed3 100644
--- a/tests/ArrayAccessTest.php
+++ b/tests/ArrayAccessTest.php
@@ -896,6 +896,7 @@ class ArrayAccessTest extends TestCase
 
                             $popped = array_pop($this->a);
 
+                            /** @psalm-suppress MixedArrayAccess */
                             [$this->b, $this->c] = $popped;
                         }
                     }'
@@ -1224,6 +1225,18 @@ class ArrayAccessTest extends TestCase
                     }',
                 'error_message' => 'InvalidArrayOffset'
             ],
+            'destructureNullable' => [
+                '<?php
+                    /**
+                     * @return null|array
+                     */
+                    function maybeReturnArray(): ?array {
+                        return rand(0, 1) ? null : ["key" => 1];
+                    }
+
+                    ["key" => $a] = maybeReturnArray();',
+                'error_message' => 'PossiblyNullArrayAccess',
+            ],
         ];
     }
 }