diff --git a/src/Phan/AST/UnionTypeVisitor.php b/src/Phan/AST/UnionTypeVisitor.php index 39cb0d320e..d5e344e0ee 100644 --- a/src/Phan/AST/UnionTypeVisitor.php +++ b/src/Phan/AST/UnionTypeVisitor.php @@ -2086,8 +2086,39 @@ private function resolveArrayShapeElementTypes(Node $node, UnionType $union_type return null; } if ($resulting_element_type === false) { + // Fix for issue #4926: Don't emit error if some array shapes in the union don't have the key + // Check if ANY type (including PHPDoc types) could have the key + $could_have_key_phpdoc = false; + $could_have_key_real = self::couldRealTypesHaveKey($union_type->getRealTypeSet(), $dim_value); + + // Also check PHPDoc types - resolveArrayShapeElementTypesForOffset returns false + // when it loops through PHPDoc types and finds none with the key, but there might + // be generic arrays in the real types + foreach ($union_type->getTypeSet() as $type) { + if ($type instanceof ArrayShapeType) { + if (isset($type->getFieldTypes()[$dim_value])) { + $could_have_key_phpdoc = true; + break; + } + } elseif ($type instanceof ListType) { + // ListType can only have integer keys >= 0 + $filtered = \is_int($dim_value) ? $dim_value : \filter_var($dim_value, \FILTER_VALIDATE_INT); + if (\is_int($filtered) && $filtered >= 0) { + $could_have_key_phpdoc = true; + break; + } + } elseif (!($type instanceof ArrayType) || $type instanceof GenericArrayType) { + // Non-array-shape types (generic arrays, etc.) could have any key + $could_have_key_phpdoc = true; + break; + } + } + + $could_have_key = $could_have_key_phpdoc || $could_have_key_real; + // XXX not sure what to do here. For now, just return null and only warn in cases where requested to. - if ($check_invalid_dim) { + if ($check_invalid_dim && !$could_have_key) { + // Only emit error if NO type in the union could possibly have this key $exception = new IssueException( Issue::fromType(Issue::TypeInvalidDimOffset)( $this->context->getFile(), @@ -2103,7 +2134,7 @@ private function resolveArrayShapeElementTypes(Node $node, UnionType $union_type } // $union_type is exclusively array shape types, but those don't contain the field $dim_value. // It's undefined (which becomes null) - if (self::couldRealTypesHaveKey($union_type->getRealTypeSet(), $dim_value)) { + if ($could_have_key) { return NullType::instance(false)->asPHPDocUnionType(); } return NullType::instance(false)->asRealUnionType(); @@ -2173,10 +2204,24 @@ public static function resolveArrayShapeElementTypesForOffset(UnionType $union_t $resulting_element_type = StringType::instance(false)->asPHPDocUnionType(); } } - } elseif ($type->isArrayLike($code_base) || $type->isObject() || $type instanceof MixedType) { - if ($type instanceof ListType && (!\is_numeric($dim_value) || $dim_value < 0)) { - continue; + } elseif ($type instanceof ListType) { + // Fix for issue #4926: Extract element type from ListType + $filtered = \is_int($dim_value) ? $dim_value : \filter_var($dim_value, \FILTER_VALIDATE_INT); + if (\is_int($filtered) && $filtered >= 0) { + // Valid int index for list - extract element type + // ListType extends GenericArrayType which has genericArrayElementType() method + $element_type = $type->genericArrayElementType(); + $element_union_type = $element_type->asPHPDocUnionType(); + if (!$element_union_type->isEmpty()) { + if ($resulting_element_type instanceof UnionType) { + $resulting_element_type = $resulting_element_type->withUnionType($element_union_type); + } else { + $resulting_element_type = $element_union_type; + } + } } + continue; + } elseif ($type->isArrayLike($code_base) || $type->isObject() || $type instanceof MixedType) { if ($is_computing_real_type_set) { // Avoid false positives for real type checking. // TODO: Improve handling for GenericArrayType, strings, etc. diff --git a/src/Phan/Analysis/AssignmentVisitor.php b/src/Phan/Analysis/AssignmentVisitor.php index 00b3aeff0c..69e48cf6f7 100644 --- a/src/Phan/Analysis/AssignmentVisitor.php +++ b/src/Phan/Analysis/AssignmentVisitor.php @@ -883,10 +883,61 @@ public function visitDim(Node $node): Context } if ($dim_type !== null && !\is_object($dim_value)) { - // TODO: This is probably why Phan has bugs with multi-dimensional assignment adding new union types instead of combining with existing ones. - $right_type = ArrayShapeType::fromFieldTypes([ - $dim_value => $this->right_type, - ], false)->asRealUnionType(); + // Check the base expression's type to decide between ArrayShapeType and generic array + // Fix for issue #4926: Don't create restrictive array shapes for mixed/unknown types + $expr_union_type = UnionTypeVisitor::unionTypeFromNode( + $this->code_base, + $this->context, + $expr_node, + false + ); + + // Check if we should use array shape or generic array + // Fix for issue #4926: For mixed/unknown base types, use mixed element type + // to avoid false positives when accessing different fields + $has_mixed_type = !$expr_union_type->isEmpty() && $expr_union_type->hasMixedOrNonEmptyMixedType(); + + // Check if the base comes from an external mixed/generic array source + // vs. being a new structure we're creating (like GLOBALS['a']) + $use_generic_for_mixed = false; + if ($has_mixed_type) { + // Find the root variable to check if it's from an external source + $root_node = $expr_node; + while ($root_node instanceof Node && $root_node->kind === \ast\AST_DIM) { + $root_node = $root_node->children['expr']; + } + if ($root_node instanceof Node && $root_node->kind === \ast\AST_VAR) { + $root_var_name = (new ContextNode($this->code_base, $this->context, $root_node))->getVariableName(); + // Hardcoded variables like $GLOBALS should use array shapes + if (!Variable::isHardcodedVariableInScopeWithName($root_var_name, $this->context->isInGlobalScope())) { + $root_type = UnionTypeVisitor::unionTypeFromNode( + $this->code_base, + $this->context, + $root_node, + false + ); + // Use generic array if root has generic/mixed array types (external source) + // Use array shape if root is undefined/empty (creating new structure) + $use_generic_for_mixed = !$root_type->isEmpty() && + ($root_type->hasGenericArray() || $root_type->hasMixedOrNonEmptyMixedType()); + } + } + } + + if (!$has_mixed_type || !$use_generic_for_mixed) { + // Base is not mixed OR we're creating a new structure → create array shape + $right_type = ArrayShapeType::fromFieldTypes([ + $dim_value => $this->right_type, + ], false)->asRealUnionType(); + } else { + // Base is mixed from external source (json_decode, etc.) → use generic array + // This prevents false positives when the same array is accessed with different keys + $key_type_enum = GenericArrayType::keyTypeFromUnionTypeValues($dim_type); + $right_type = GenericArrayType::fromElementType(MixedType::instance(false), false, $key_type_enum)->asRealUnionType(); + if (!$right_type->hasRealTypeSet()) { + $right_type = $right_type->withRealTypeSet(UnionType::typeSetFromString('non-empty-array')); + } + } } else { // Make the right type a generic (i.e. int -> int[]) if ($dim_node !== null) { diff --git a/tests/files/expected/4926_array_mixed_access.php.expected b/tests/files/expected/4926_array_mixed_access.php.expected new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/files/src/4926_array_mixed_access.php b/tests/files/src/4926_array_mixed_access.php new file mode 100644 index 0000000000..ae125cdc28 --- /dev/null +++ b/tests/files/src/4926_array_mixed_access.php @@ -0,0 +1,25 @@ + $arr'; + foreach ($arr as $row) { + $row["x"] = strlen($row["x"]); + $row["y"] = strlen($row["y"]); + } + } +}