Fix array shape issues

Author: AJenbo

docs/CHANGELOG.md

@@ -122,6 +122,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - **A `"\x8b"` escape no longer takes the server down with it.** A hex or octal escape in a double-quoted string decodes to a raw byte, and a string built that way (`strpos($output, "\x8b")`, a gzip magic number, a binary delimiter) is text no character encoding can express. PHPantom read those literals as though they were ordinary source text, which is undefined behaviour: on the command line the analysis died part-way through, in the editor the server exited, and in a release build it silently read whatever happened to follow in memory. Every place a literal's value is read now checks it first, so a literal like this is simply one PHPantom has nothing to say about. The rest of the file is analysed as before.
+- **An array literal keeps the value written at each position.** `[$violation, $file, $line]` collapsed to a list of everything it held, so every way of reading one slot back out gave the same `RuleViolation|string|int`: destructuring the row with `[$violation, $file, $line] = $row`, indexing it with `$row[1]`, or pulling it out of the collection it was pushed into. Rows written this way now record what sits at each position, so a slot read is the one value that slot holds, and passing it on no longer reports a type mismatch against everything the row happened to contain. This already worked for a literal nested inside another; it now holds wherever the literal is written. A literal that spreads another array, or one long enough that its arity is beside the point, still describes itself as a list.
+- **A literal keyed on something PHP works out at runtime is described by its keys and values.** `[$name => 1]` was reported as `array{mixed: 1}`, a shape with a field named after the key's type rather than after anything in the code, and a read off it found neither the key that was written nor the one that was asked for. Such a literal is now an `array<K, V>` built from the key and value types it does hold, and a key that is not written as a plain string or integer keeps the type it resolves to, so `[Event::class => $handler]` satisfies a parameter declared `array<class-string, …>`. PHP coerces `null`, `true` and `false` before using them as keys, so those now land on the `''`, `1` and `0` entries they index at runtime.
+- **`(object) []` is a `stdClass`.** Casting an empty array to an object produced an `object{}`, a shape with no properties, which nothing else in the engine produces and which was rejected by every parameter declared `stdClass`. It is now the `stdClass` PHP builds. A cast of a non-empty array still keeps the properties it names.
+- **`+` between two arrays keeps their keys.** The compound `+=` already merged both operands' keys, but writing the same union as `$merged = $defaults + $overrides` collapsed it to a bare `array`, so nothing about the merged array completed or hovered. Both spellings now go through the same merge.
 - **`$matches` outside the guard says the match may not have happened.** `preg_match()` fills its out-parameter with the keys the pattern describes, and that shape was applied wherever the call appeared, the code after a guard that may not have been taken included. So a group read on a line PHP reaches with the empty array a failed match leaves behind was typed as a plain `string`, and nothing said the key might not be there. The call now writes what it leaves either way, and the condition that tests its result decides which of the two a branch is looking at: inside the guard the keys are there and a group read is a `string`, the branch that runs on a failed match gets the empty array, and where the two rejoin the keys are marked as ones that may be missing. Comparing the result guards exactly as the bare call does, in an `if`, an `elseif`, a guard clause that returns on a failed match, and a `while`. `preg_match_all()` is unaffected, since a failed match there still writes one empty list per capture group.
 - **Reading a key a shape marks optional carries the `null` a missing offset yields.** A `@var array{file: string, type?: string}` says the `type` entry may not be there, but reading it produced the same `string` a required key does, so a value that is absent at runtime was passed on as though it could not be. The read now says it may be missing, which is what makes the `??` and the `isset()` around it mean something. A key the shape requires is unchanged.
 - **`!empty($row['name'])` proves the key is there.** `isset($row['name'])` narrowed the entry it named, but the `!empty()` spelling of the same check only ever narrowed a plain variable, so a proof about an array entry or a property path was dropped. Both spellings now record what they prove, in an expression position (a ternary, a `match (true)` arm) as much as in an `if` body, so the guarded read no longer reports the falsy half the guard ruled out.

docs/todo.md

@@ -25,7 +25,6 @@ within the same impact tier.
 
 | #    | Item                                                                                                                                                      | Impact      | Effort      |
 | ---- | ----------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | ----------- |
-| B147 | [Array literals are not tuples: slot reads return the union of all elements](todo/bugs.md#b147-array-literals-are-not-tuples-slot-reads-return-the-union-of-all-elements) | Medium-High | Medium |
 |      | **Release 0.10.0**                                                                                                                                        |             |             |
 
 ## Sprint 7 — 1.0 release & IDE extensions
@@ -100,6 +99,7 @@ unlikely to move the needle for most users.
 | D17 | [`docblock_native_mismatch` only judges nullability](todo/diagnostics.md#d17-docblock_native_mismatch-only-judges-nullability)                                              | Low         | Medium      |
 |     | **[Bug Fixes](todo/bugs.md)**                                                                                                                                               |             |             |
 | B151 | [`?T` and `T\|null` are judged by different rules](todo/bugs.md#b151-t-and-tnull-are-judged-by-different-rules)                                                            | High        | Low-Medium  |
+| B152 | [`array_filter` with `ARRAY_FILTER_USE_KEY` does not narrow the key type](todo/bugs.md#b152-array_filter-with-array_filter_use_key-does-not-narrow-the-key-type)           | Low-Medium  | Medium      |
 |     | **[Code Actions](todo/actions.md)**                                                                                                                                         |             |             |
 | A40 | [Generate method from call](todo/actions.md#a40-generate-method-from-call)                                                                                                  | Medium-High | Medium      |
 | A41 | [Create class from non-existing name](todo/actions.md#a41-create-class-from-non-existing-name)                                                                              | Medium      | Medium      |

docs/todo/bugs.md

@@ -76,46 +76,22 @@ have been guarded out) rather than removing it outright.
 
 ## Array types
 
-### B147. Array literals are not tuples: slot reads return the union of all elements
+### B152. `array_filter` with `ARRAY_FILTER_USE_KEY` does not narrow the key type
 
-**Impact: Medium-High · Effort: Medium**
+**Impact: Low-Medium · Effort: Medium**
 
 ```php
-$rows[] = [$violation, $location, $name];        // RuleViolation, string, string
-foreach ($rows as $row) {
-    [$violation, $location, $name] = $row;       // each: RuleViolation|string
-    $writer->write($location);                   // reported: RuleViolation|string
-}
+/** @return array<string> $data */
+$data = array_filter($data, fn (string|int $k): bool => is_string($k), ARRAY_FILTER_USE_KEY);
+$data = $this->viewData($view) + $data;    // reported: array<string|int, string>
 ```
 
-A list literal collapses to `array<union-of-values>`, so list
-destructuring and constant-offset reads cannot select a slot (6 sites
-in PHPMD/PDepend). Two adjacent literal defects: a literal with a
-*non-constant* key renders as the bogus shape `array{mixed: int}`
-(stringifying the key's type as a field name) instead of falling back
-to `array<K, V>`, and `(object) []` is not recognised as `stdClass`.
-
-**Fix:** keep constant-array shapes for literals (ordered slots +
-known keys), select slots on destructure/offset reads, fall back to a
-generic array only for non-constant keys.
-
-### B148. Element writes do not refine tracked array state
-
-**Impact: Medium · Effort: Medium-High**
-
-Several forms of the same weakness (~7 sites):
-
-- `$a[$k][] = $v` never updates the inner element type: a value
-  initialised as `[]` stays `array{}` in the outgoing type even
-  though every loop iteration appends strings.
-- A key written on every path through a loop body leaves
-  `array<int, string>` where PHPStan reports
-  `non-empty-array<int, string>`.
-- `$a += ['slot' => $obj]` degrades to unconstrained `array`.
-- A constant shape `array{item: string, qty: int}` fails the subtype
-  check against `array<string, mixed>`, so shaped rows are rejected
-  by a declared `array<int, array<string, mixed>>`.
-
-**Fix:** refine the per-key state on nested writes (including
-auto-vivification), merge `+=` like an array-shape union, and make
-constant shapes satisfy their generic supertypes.
+`array_filter` preserves its input type verbatim, so the callback's
+proof about the keys is dropped. That is invisible on its own, but the
+key type surfaces the moment the result is merged with `+` or passed to
+a parameter declared `array<string, …>` (2 sites in Bladestan). The
+`ARRAY_FILTER_USE_BOTH` mode has the same gap for the key half.
+
+**Fix:** read the callback's assertions about its key parameter (the
+same reconciliation an `if (is_string($k))` body already gets) and
+rebuild the result's key type from what survives.

examples/php/completion.php

@@ -2841,6 +2841,38 @@ public function inferredTuples(): void
             $tool = new $toolClass();
             $tool->label();               // Scaffolding\Pen|Scaffolding\Pencil created from the class-string
         }
+
+        // A literal written straight into a variable keeps its arity too,
+        // so a row pushed into a collection destructures back into the
+        // values it was written with rather than the union of all of them.
+        $entries = [];
+        $entries[] = [new Scaffolding\Pen(), 'sketchbook'];
+        foreach ($entries as $entry) {
+            [$writer, $surface] = $entry;
+            $writer->write();             // Scaffolding\Pen (slot 0, not Pen|string)
+            strlen($surface);             // string (slot 1, not Pen|string)
+        }
+    }
+
+    public function runtimeArrayKeys(string $slot): void
+    {
+        // A key PHP only works out at runtime names no shape field, so the
+        // literal is described by the key and value types it does have.
+        $bySlot = [$slot => new Scaffolding\Pen()];   // array<string, Scaffolding\Pen>
+        foreach ($bySlot as $held) {
+            $held->write();               // Scaffolding\Pen
+        }
+
+        // `+` keeps the left side's keys and adds the right side's, the
+        // same union `+=` performs.
+        $merged = ['pen' => new Scaffolding\Pen()] + ['pencil' => new Scaffolding\Pencil()];
+        $merged['pen']->write();          // Scaffolding\Pen from the left operand
+        $merged['pencil']->sketch();      // Scaffolding\Pencil from the right operand
+
+        // Casting an empty array gives the property-less stdClass PHP
+        // builds, not an object shape.
+        $bare = (object) [];              // stdClass
+        echo get_class($bare);
     }
 }
 

examples/php/scaffolding/assertions.php

@@ -298,6 +298,32 @@ function runDemoAssertions(): void
         assert($tool instanceof Scaffolding\Pen || $tool instanceof Scaffolding\Pencil, 'class-string must instantiate Scaffolding\Pen|Scaffolding\Pencil');
     }
 
+    // A row pushed as a literal destructures back into the values it was
+    // written with, one per position.
+    $entries = [];
+    $entries[] = [new Scaffolding\Pen(), 'sketchbook'];
+    foreach ($entries as $entry) {
+        [$writer, $surface] = $entry;
+        assert($writer instanceof Scaffolding\Pen, 'pushed tuple slot 0 must be Scaffolding\Pen');
+        assert(is_string($surface), 'pushed tuple slot 1 must be a string');
+    }
+
+    // A key worked out at runtime still holds the value that was written
+    // against it.
+    $slot = 'ink';
+    $bySlot = [$slot => new Scaffolding\Pen()];
+    assert($bySlot['ink'] instanceof Scaffolding\Pen, 'a runtime key must hold the value written against it');
+
+    // `+` keeps the left operand's keys and adds the right operand's.
+    $merged = ['pen' => new Scaffolding\Pen()] + ['pencil' => new Scaffolding\Pencil()];
+    assert($merged['pen'] instanceof Scaffolding\Pen, 'array union must keep the left key');
+    assert($merged['pencil'] instanceof Scaffolding\Pencil, 'array union must add the right key');
+
+    // Casting an empty array produces a property-less stdClass.
+    $bare = (object) [];
+    assert($bare instanceof \stdClass, '(object) [] must be a stdClass');
+    assert(get_object_vars($bare) === [], '(object) [] must have no properties');
+
     // ── Indexing an ArrayAccess Object ───────────────────────────────────
     $penAccess = new Scaffolding\ScaffoldingPenArrayAccess();
     assert($penAccess[0] instanceof Scaffolding\Pen, 'ArrayAccess[0] must resolve via offsetGet(): Scaffolding\Pen');

src/php_type/mod.rs

@@ -1913,6 +1913,36 @@ impl PhpType {
         }
     }
 
+    /// The generic array type a constant shape describes, dropping the
+    /// per-key detail: `array{a: int, b: string}` → `array<string,
+    /// int|string>`, `array{User, Order}` → `list<User|Order>`.
+    ///
+    /// Anything that is not an array shape is returned unchanged, so a
+    /// caller that only needs a container it can read a key and value type
+    /// off can pass any type through. An empty shape has no key or value
+    /// type to name and becomes a bare `array`.
+    pub fn generalized_array(&self) -> PhpType {
+        let TypeKind::ArrayShape(entries) = self.kind() else {
+            return self.clone();
+        };
+        if entries.is_empty() {
+            return PhpType::array();
+        }
+        let Some(value) = self.iterable_element_type() else {
+            return PhpType::array();
+        };
+        // Only an all-positional shape promises the `0, 1, 2, …` keys a
+        // `list` does; the moment one entry is named the result has to
+        // spell its key type out.
+        if entries.iter().all(|e| e.key.is_none()) {
+            return PhpType::list(value);
+        }
+        match self.iterable_key_type() {
+            Some(key) => PhpType::generic_array(key, value),
+            None => PhpType::generic_array_val(value),
+        }
+    }
+
     /// Extract the element (value) type from an iterable, including
     /// scalar element types.
     ///

src/type_engine/variable/array_func_rules.rs

@@ -63,7 +63,10 @@ pub(in crate::type_engine) fn array_func_raw_type(
         .iter()
         .any(|f| f.eq_ignore_ascii_case(func_name))
     {
-        let raw = args.arg_raw_type(0)?;
+        // Every one of these rearranges the array (reorders, renumbers,
+        // drops or chunks entries), so a constant shape does not survive
+        // the call and is generalized to the container it describes.
+        let raw = args.arg_raw_type(0)?.generalized_array();
         // Only a parameterised iterable carries an element type worth
         // preserving; a bare `array`/`iterable` is a `Named` kind with no
         // value argument to extract, so the rule declines and the stub's
@@ -144,7 +147,7 @@ pub(in crate::type_engine) fn array_func_element_type(
         // A scalar element is the honest answer for `array_pop(list<string>)`
         // just as `User` is for `list<User>`, so the element type is read
         // without `skip_scalar`.
-        return args.arg_raw_type(0)?.extract_value_type(false).cloned();
+        return args.arg_raw_type(0)?.iterable_element_type();
     }
 
     // `array_sum`/`array_product` are declared `int|float` because the
@@ -154,7 +157,7 @@ pub(in crate::type_engine) fn array_func_element_type(
     // `@return TValue` would answer `string` for `array_sum(list<string>)`
     // rather than the `int|float` PHP actually produces.
     if matches!(func_name, "array_sum" | "array_product") {
-        let element = args.arg_raw_type(0)?.extract_value_type(false)?.clone();
+        let element = args.arg_raw_type(0)?.iterable_element_type()?;
         let members: Vec<&PhpType> = match element.kind() {
             TypeKind::Union(m) => m.iter().collect(),
             _ => vec![&element],
@@ -222,7 +225,7 @@ fn array_map_element_type(args: &dyn ArrayFuncArgs) -> Option<PhpType> {
         return Some(declared);
     }
 
-    let input_element = args.arg_raw_type(1)?.extract_element_type()?.clone();
+    let input_element = args.arg_raw_type(1)?.iterable_element_type()?;
 
     if let Some(inferred) = args.callback_inferred_return_type(0, &input_element) {
         return Some(inferred);