Fix #311 and introduce PossiblyInvalidArrayOffset

config.xsd

@@ -159,6 +159,7 @@
             <xs:element name="PossiblyFalseReference" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidArgument" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidArrayAccess" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="PossiblyInvalidArrayOffset" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidMethodCall" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidPropertyAssignment" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidPropertyFetch" type="IssueHandlerType" minOccurs="0" />

src/Psalm/Checker/Statements/Expression/FetchChecker.php

@@ -9,6 +9,7 @@ use Psalm\Checker\MethodChecker;
 use Psalm\Checker\Statements\ExpressionChecker;
 use Psalm\Checker\StatementsChecker;
 use Psalm\Checker\TraitChecker;
+use Psalm\Checker\TypeChecker;
 use Psalm\CodeLocation;
 use Psalm\Context;
 use Psalm\Issue\DeprecatedProperty;
@@ -28,6 +29,7 @@ use Psalm\Issue\NullPropertyFetch;
 use Psalm\Issue\NullReference;
 use Psalm\Issue\ParentNotFound;
 use Psalm\Issue\PossiblyInvalidArrayAccess;
+use Psalm\Issue\PossiblyInvalidArrayOffset;
 use Psalm\Issue\PossiblyInvalidPropertyFetch;
 use Psalm\Issue\PossiblyNullArrayAccess;
 use Psalm\Issue\PossiblyNullPropertyFetch;
@@ -905,10 +907,6 @@ class FetchChecker
             return false;
         }
 
-        // this is the key type that we infer from the array/string/object/whatever
-        // later we'll check it against $used_key_type
-        $inferred_key_type = null;
-
         $project_checker = $statements_checker->getFileChecker()->project_checker;
 
         if (isset($stmt->var->inferredType)) {
@@ -938,6 +936,9 @@ class FetchChecker
             $has_array_access = false;
             $non_array_types = [];
 
+            $has_valid_offset = false;
+            $invalid_offset_types = [];
+
             foreach ($var_type->types as &$type) {
                 if ($type instanceof TNull) {
                     if (IssueBuffer::accepts(
@@ -973,13 +974,14 @@ class FetchChecker
                             if ($array_assignment && $type->type_params[0]->isEmpty()) {
                                 $type->type_params[0] = $used_key_type;
                             } elseif (!$type->type_params[0]->isEmpty()) {
-                                if ($inferred_key_type) {
+                                if (!TypeChecker::isContainedBy(
-                                    $inferred_key_type = Type::combineUnionTypes(
+                                    $project_checker,
-                                        $inferred_key_type,
+                                    $used_key_type,
-                                        $type->type_params[0]
+                                    $type->type_params[0]
-                                    );
+                                )) {
+                                    $invalid_offset_types[] = (string)$type->type_params[0];
                                 } else {
-                                    $inferred_key_type = $type->type_params[0];
+                                    $has_valid_offset = true;
                                 }
                             }
                         }
@@ -1128,46 +1130,39 @@ class FetchChecker
                             }
                         }
                     } elseif ($type instanceof Type\Atomic\ObjectLike) {
-                        $object_like_keys = array_keys($type->properties);
+                        if ($string_key_value || $int_key_value !== null) {
-                        if ($object_like_keys) {
+                            if ($string_key_value && isset($type->properties[$string_key_value])) {
-                            if (count($object_like_keys) === 1) {
+                                $has_valid_offset = true;
-                                $expected_keys_string = '\'' . $object_like_keys[0] . '\'';
+                                $stmt->inferredType = clone $type->properties[$string_key_value];
+                            } elseif ($int_key_value !== null && isset($type->properties[(string)$int_key_value])) {
+                                $has_valid_offset = true;
+                                $stmt->inferredType = clone $type->properties[(string)$int_key_value];
                             } else {
-                                $last_key = array_pop($object_like_keys);
+                                $invalid_offset_types[] = '"' . ($string_key_value ?: $int_key_value) . '"';
-                                $expected_keys_string = '\'' . implode('\', \'', $object_like_keys) .
-                                    '\' or \'' . $last_key . '\'';
                             }
-                        } else {
+                        } elseif (TypeChecker::isContainedBy(
-                            $expected_keys_string = 'string';
+                            $project_checker,
-                        }
+                            $used_key_type,
-
+                            Type::getString()
-                        if ($string_key_value && isset($type->properties[$string_key_value])) {
+                        )) {
-                            $stmt->inferredType = clone $type->properties[$string_key_value];
+                            $has_valid_offset = true;
-                        } elseif ($int_key_value !== null && isset($type->properties[(string)$int_key_value])) {
-                            $stmt->inferredType = clone $type->properties[(string)$int_key_value];
-                        } elseif ($used_key_type->hasInt()) {
-                            if (IssueBuffer::accepts(
-                                new InvalidArrayOffset(
-                                    'Cannot access value on array variable ' . $var_id . ' using int offset - ' .
-                                        'expecting ' . $expected_keys_string,
-                                    new CodeLocation($statements_checker->getSource(), $stmt)
-                                ),
-                                $statements_checker->getSuppressedIssues()
-                            )) {
-                                return false;
-                            }
-                        } elseif ($used_key_type->hasString()) {
                             $stmt->inferredType = $type->getGenericTypeParam();
+                        } else {
+                            $invalid_offset_types[] = 'string';
                         }
                     }
                     continue;
                 }
 
                 if ($type instanceof TString) {
-                    if (!$inferred_key_type) {
+                    if (!TypeChecker::isContainedBy(
-                        $inferred_key_type = Type::getInt();
+                        $project_checker,
+                        $used_key_type,
+                        Type::getInt()
+                    )) {
+                        $invalid_offset_types[] = 'int';
                     } else {
-                        $inferred_key_type = Type::combineUnionTypes($inferred_key_type, Type::getInt());
+                        $has_valid_offset = true;
                     }
 
                     $stmt->inferredType = Type::getString();
@@ -1226,6 +1221,46 @@ class FetchChecker
                     $stmt->inferredType = Type::getMixed();
                 }
             }
+
+            if ($used_key_type->isMixed()) {
+                if (IssueBuffer::accepts(
+                    new MixedArrayOffset(
+                        'Cannot access value on variable ' . $var_id . ' using mixed offset',
+                        new CodeLocation($statements_checker->getSource(), $stmt)
+                    ),
+                    $statements_checker->getSuppressedIssues()
+                )) {
+                    return false;
+                }
+            }
+
+            if ($invalid_offset_types) {
+                $invalid_offset_type = $invalid_offset_types[0];
+
+                if ($has_valid_offset) {
+                    if (IssueBuffer::accepts(
+                        new PossiblyInvalidArrayOffset(
+                            'Cannot access value on array variable ' . $var_id . ' using ' . $used_key_type
+                                . ' offset, expecting ' . $invalid_offset_type,
+                            new CodeLocation($statements_checker->getSource(), $stmt)
+                        ),
+                        $statements_checker->getSuppressedIssues()
+                    )) {
+                        return false;
+                    }
+                } else {
+                    if (IssueBuffer::accepts(
+                        new InvalidArrayOffset(
+                            'Cannot access value on array variable ' . $var_id . ' using ' . $used_key_type
+                                . ' offset, expecting ' . $invalid_offset_type,
+                            new CodeLocation($statements_checker->getSource(), $stmt)
+                        ),
+                        $statements_checker->getSuppressedIssues()
+                    )) {
+                        return false;
+                    }
+                }
+            }
         }
 
         if ($keyed_array_var_id && $context->hasVariable($keyed_array_var_id)) {
@@ -1236,39 +1271,6 @@ class FetchChecker
             $stmt->inferredType = Type::getMixed();
         }
 
-        if ($stmt->dim) {
-            if (isset($stmt->dim->inferredType) && $inferred_key_type) {
-                foreach ($used_key_type->types as $at) {
-                    if (($at instanceof TMixed || $at instanceof TEmpty)
-                        && $inferred_key_type
-                        && !$inferred_key_type->isMixed()
-                    ) {
-                        if (IssueBuffer::accepts(
-                            new MixedArrayOffset(
-                                'Cannot access value on variable ' . $var_id . ' using mixed offset - expecting ' .
-                                    $inferred_key_type,
-                                new CodeLocation($statements_checker->getSource(), $stmt)
-                            ),
-                            $statements_checker->getSuppressedIssues()
-                        )) {
-                            return false;
-                        }
-                    } elseif (!$at->isIn($project_checker, $inferred_key_type)) {
-                        if (IssueBuffer::accepts(
-                            new InvalidArrayOffset(
-                                'Cannot access value on variable ' . $var_id . ' using ' . $at . ' offset - ' .
-                                    'expecting ' . $inferred_key_type,
-                                new CodeLocation($statements_checker->getSource(), $stmt)
-                            ),
-                            $statements_checker->getSuppressedIssues()
-                        )) {
-                            return false;
-                        }
-                    }
-                }
-            }
-        }
-
         return null;
     }
 

src/Psalm/Issue/PossiblyInvalidArrayOffset.php

@@ -0,0 +1,6 @@
+<?php
+namespace Psalm\Issue;
+
+class PossiblyInvalidArrayOffset extends CodeError
+{
+}

tests/ArrayAccessTest.php

@@ -107,6 +107,18 @@ class ArrayAccessTest extends TestCase
                     $y = $x["b"];',
                 'error_message' => 'InvalidArrayOffset',
             ],
+            'possiblyInvalidArrayOffsetWithInt' => [
+                '<?php
+                    $x = rand(0, 5) > 2 ? ["a" => 5] : "hello";
+                    $y = $x[0];',
+                'error_message' => 'PossiblyInvalidArrayOffset',
+            ],
+            'possiblyInvalidArrayOffsetWithString' => [
+                '<?php
+                    $x = rand(0, 5) > 2 ? ["a" => 5] : "hello";
+                    $y = $x["a"];',
+                'error_message' => 'PossiblyInvalidArrayOffset',
+            ],
             'possiblyInvalidArrayAccess' => [
                 '<?php
                     $a = rand(0, 10) > 5 ? 5 : ["hello"];

tests/LoopScopeTest.php

@@ -338,7 +338,7 @@ class LoopScopeTest extends TestCase
             'loopWithArrayKey' => [
                 '<?php
                     /**
-                     * @param array[][] $args
+                     * @param array<array<int, array<string, string>>> $args
                      * @return array[]
                      */
                     function get_merged_dict(array $args) {