Fix a few issues

Author: AJenbo

docs/CHANGELOG.md

@@ -119,6 +119,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **A `@property` tag beats an inherited property nobody can reach.** PHP only calls `__get()` when no *accessible* property of that name exists, so a `protected` declaration up the chain is never what the read yields. PHPantom reported its type anyway: an Eloquent model documenting `@property string $connection` still resolved `$model->connection` through `Model`'s own `\UnitEnum|string|null`, and the same happened for `$table` and `$keyType`, which models shadow routinely. The tag now describes the read it was written for. A property the class declares *itself* is a different matter and keeps its own type, since it is in scope everywhere the tag is, and so does an accessible inherited one.
+- **An accumulator that starts as `[]` counts in whole numbers.** `$totals[$k] = ($totals[$k] ?? 0) + $n` is the standard way to tally by key, and the first pass read the empty array as an unknown value rather than a miss, so the sum came out `int|float` and failed every `array<string, int>` it was declared as. An offset read on `[]` now yields `null`, which is what PHP produces and what the `??` was written to catch. The empty array also stops trailing along beside the array a later write produced: a variable seeded with `[]` and appended to in a loop, or captured by reference and filled in by a closure, reports the array it ends up holding instead of that alternative plus the empty one it started from, so reading an element out of it no longer carries a `null` from the empty half.
 - **A ternary's arms see what its condition proved.** `is_string($req) ? $req : 'today'` handed both arms the raw `string|array|null`, so a value the condition had just established was still reported against every `string` the ternary fed. Each arm is now resolved under its own polarity of the condition, using the same narrowing an `if`/`else` body gets, and it happens wherever the ternary is written rather than only in some positions: assignment, argument, and return all behave the same. That covers the whole family of conditions rather than a list of recognised shapes, so a type guard, a null or falsy check, `instanceof`, a member-existence proof, and anything added to narrowing later all reach the arms. A nested ternary's else arm carries the outer conditions' inverse narrowing as well as its own, and `?:` still yields the truthy half of its subject.
 - **A negated compound guard narrows by every conjunct.** `if (!is_string($payload) || $payload === '') { return; }` is the standard way to reject everything a function cannot handle, and the code after it was left with the un-narrowed union: the guard proved nothing. Falling through an `||` means every operand was false, so each operand's own inverse now applies, whatever kind of check it is. Previously only `instanceof` and member-existence checks were read one operand at a time, and the rest were matched against the whole `||` expression, which never matched. Every exit form works (`return`, `throw`, `continue`, `abort()`), so does the `else` branch, and so do chains of more than two conjuncts. `is_resource()` joins the `is_*` family it was missing from, and `!== ''` / `!== []` now refine to `non-empty-string` / `non-empty-array` rather than only removing a literal that was never in the union.
 - **An array written under a `string` key stays keyed by `string`.** Every non-literal string key widened to `int|string`, on the grounds that a numeric string becomes an int key at runtime. Only a *literal* decimal-integer string does, so a function building `array<string, string>` reported `array<int|string, string>` and failed its own declared return type, including after an explicit `(string)` cast, a backed enum's `->value`, and `ReflectionProperty::getName()`. A key expression now keeps its own domain: `string` stays `string`, `int` stays `int`, and the int conversion applies to literal decimal keys alone. `++$i` and `$i++` resolve as well, so a counter used as a write key no longer falls back to `array-key`.

docs/todo.md

@@ -25,10 +25,8 @@ within the same impact tier.
 
 | #    | Item                                                                                                                                                      | Impact      | Effort      |
 | ---- | ----------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | ----------- |
-| B140 | [Interface phpDoc is not inherited by an implementation without its own docblock](todo/bugs.md#b140-interface-phpdoc-is-not-inherited-by-an-implementation-without-its-own-docblock) | High | Low-Medium |
 | B167 | [Factory `create()`/`make()` keep the collection half on single-model chains](todo/bugs.md#b167-factory-createmake-keep-the-collection-half-on-single-model-chains) | Medium-High | Low-Medium |
-| B171 | [A subclass `@property` tag loses to an inherited real property](todo/bugs.md#b171-a-subclass-property-tag-loses-to-an-inherited-real-property)            | Medium      | Low-Medium  |
-| B163 | [Residual `int` arithmetic and assignment widenings](todo/bugs.md#b163-residual-int-arithmetic-and-assignment-widenings)                                   | Low-Medium  | Low-Medium  |
+| B163 | [An `int` assigned to a `float` property is reported](todo/bugs.md#b163-an-int-assigned-to-a-float-property-is-reported)                                  | Low         | Low-Medium  |
 | B139 | [Conditional return types are not evaluated against argument types](todo/bugs.md#b139-conditional-return-types-are-not-evaluated-against-argument-types)   | High        | Medium      |
 | B142 | [Builtins with argument-dependent return types, round two](todo/bugs.md#b142-builtins-with-argument-dependent-return-types-round-two)                      | High        | Medium      |
 | B144 | [`preg_match` `$matches` is nullable and shapeless](todo/bugs.md#b144-preg_match-matches-is-nullable-and-shapeless)                                        | High        | Medium      |

docs/todo/bugs.md

@@ -37,7 +37,7 @@ $order = Order::find(7);   // reported Collection<int, Order>|Order|null
 Undecided type conditions from the sweep: `$id is array<mixed>|Arrayable`
 (Eloquent `find`/`findOrFail`, ~24 sites), `$items is EloquentCollection`
 / `$into is class-string<…>` (spatie/laravel-data `Data::collect()`,
-~22 sites once B140 lands), `$callback is null` (`tap()`, which also
+~22 sites), `$callback is null` (`tap()`, which also
 leaks the raw template name `TValue` into the union), and Symfony's
 `ContainerInterface::get()` (`B is 0|1` against the omitted argument's
 default `1`). Two cosmetic side effects to clear with it: a model's
@@ -51,24 +51,6 @@ resolved argument type (falling back to the declared default's type
 when the argument is omitted), recursing into nested conditionals, and
 only union the branches when the condition is genuinely undecidable.
 
-### B140. Interface phpDoc is not inherited by an implementation without its own docblock
-
-**Impact: High · Effort: Low-Medium**
-
-`Spatie\LaravelData\Concerns\BaseData::collect()` (a trait method) has
-no docblock; the conditional `@return` lives on the interface
-`Spatie\LaravelData\Contracts\BaseData` that the `Data` base class
-implements. PHPStan inherits phpDoc from implemented interfaces when
-the implementation (including one supplied by a trait) has none;
-PHPantom reads only the native signature, so every `X::collect(...)`
-returns the raw eleven-member union. ~22 sites across three Laravel
-projects, always as `array`/`Collection` inputs whose result feeds a
-declared `array<X>`/`Collection<int, X>`.
-
-**Fix:** when a method has no own docblock, look it up on the
-interfaces the declaring class (transitively) implements, the same way
-parent-class docblocks are already inherited.
-
 ### B141. A `never` conditional branch does not assert the condition
 
 **Impact: Medium-High · Effort: Medium**
@@ -233,10 +215,9 @@ 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.
-- The intermediate empty-array state from
-  `if (!isset($a[$k])) { $a[$k] = []; } $a[$k][$id] = $x;` survives
-  the loop fix-point, leaving `array{}|array<int, string>` where
-  PHPStan reports `non-empty-array<int, string>`.
+- 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
@@ -393,14 +374,19 @@ is also overridden by the annotation. The `@var` should seed the
 assignment it documents and then submit to normal flow narrowing
 (3 sites).
 
-### B163. Residual `int` arithmetic and assignment widenings
+### B163. An `int` assigned to a `float` property is reported
 
-**Impact: Low-Medium · Effort: Low-Medium**
+**Impact: Low · Effort: Low-Medium**
 
-Two leftovers from the shipped arithmetic-precision work:
-`($a[$k] ?? 0) + $int` widens to `int|float`, and an `int` value
+A leftover from the shipped arithmetic-precision work: an `int` value
 assigned to a `float`-typed property is reported instead of accepting
-the standard numeric widening.
+the standard numeric widening PHP performs even under
+`declare(strict_types=1)`.
+
+No site for this reproduces any more — not in the ten-project sweep,
+and not in the promoted-constructor, static-property, `+=`,
+`@var`-only, nullable, and array-element forms. Confirm it still
+happens before working on it.
 
 ### B174. A `break` that leaves a loop early is missing from the post-loop join
 
@@ -501,18 +487,6 @@ virtual PHP already contains the real `if`, so the walker has the
 narrowing; it is the include-contract check that reads the wrong
 scope.
 
-### B171. A subclass `@property` tag loses to an inherited real property
-
-**Impact: Medium · Effort: Low-Medium**
-
-A model declaring `@property string $connection` still resolves
-`$model->connection` through `Illuminate\Database\Eloquent\Model`'s
-inherited `protected $connection` (`string|null`) and the generic
-attribute fallback (`UnitEnum|string|null`). A magic read must prefer
-the class's own `@property` tag over a non-public inherited property.
-1 site, but the shadowing pattern (`$connection`, `$table`, `$keyType`)
-is common on Eloquent models.
-
 ## Miscellaneous
 
 ### B173. Classes shipped inside a dependency's phar are invisible

src/inheritance/mod.rs

@@ -488,6 +488,42 @@ pub(crate) fn resolve_class_with_inheritance(
         }
     }
 
+    // Retype an inherited non-public property that the class documents
+    // with a `@property` tag of its own.
+    //
+    // A tag documents a magic read, and PHP only reaches `__get()` when no
+    // *accessible* property of that name exists.  An ancestor's
+    // `protected` / `private` declaration is invisible from outside, so it
+    // is not what the read yields and its type must not describe it — an
+    // Eloquent model declaring `@property string $connection` means
+    // `string`, not `Model::$connection`'s `\UnitEnum|string|null`. The
+    // same shadowing is routine for `$table` and `$keyType`.
+    //
+    // A property the class declares *itself* is a different matter: it is
+    // in scope everywhere the tag is, so it keeps its own type (the tag is
+    // then a contradiction, and the real declaration is the truth).
+    if let Some(doc) = class.doc_members.as_deref() {
+        for (name, type_hint) in &doc.properties {
+            let Some(hint) = type_hint else {
+                continue;
+            };
+            if class.properties.iter().any(|p| p.name == *name) {
+                continue;
+            }
+            // Look the index up immutably so a class with nothing to
+            // retype keeps sharing its property vector.
+            let Some(idx) = merged
+                .properties
+                .iter()
+                .position(|p| p.name == *name && p.visibility != Visibility::Public)
+            else {
+                continue;
+            };
+            let prop = &mut merged.properties.make_mut()[idx];
+            Arc::make_mut(prop).type_hint = Some(hint.clone());
+        }
+    }
+
     // Refine the `value` property on backed enums.  The `BackedEnum`
     // interface declares `public readonly int|string $value`, but each
     // concrete backed enum knows its specific backing type.  Replace

src/php_type/mod.rs

@@ -1445,6 +1445,36 @@ impl PhpType {
         }
     }
 
+    /// Returns `true` for the shape with no entries — `array{}`, the type
+    /// of an `[]` literal.
+    ///
+    /// It is the one array type whose value set is a single value, which
+    /// makes it a member of every array type that does not demand an
+    /// entry, and makes every offset read on it a guaranteed miss.
+    pub fn is_empty_array_shape(&self) -> bool {
+        matches!(self.kind(), TypeKind::ArrayShape(entries) if entries.is_empty())
+    }
+
+    /// Returns `true` when this array type has the empty array among its
+    /// values, so an `array{}` alternative beside it is redundant.
+    ///
+    /// `array`, `array<K, V>`, `list<V>`, `T[]` and `iterable` all do. The
+    /// `non-empty-*` family does not, and neither does a shape: `array{}`
+    /// is the only shape the empty array satisfies, and every other one
+    /// names an entry it lacks.
+    pub fn accepts_empty_array(&self) -> bool {
+        let name = match self.kind() {
+            TypeKind::Array(_) => return true,
+            TypeKind::Named(name) => name,
+            TypeKind::Generic(generic) => &generic.name,
+            _ => return false,
+        };
+        matches!(
+            crate::php_type::keywords::keyword_lowercase(name).as_str(),
+            "array" | "list" | "iterable"
+        )
+    }
+
     /// Returns `true` when this type is exactly the bare, unparameterised
     /// `array` keyword — i.e. `PhpType::named("array")`.
     ///

src/php_type/normalize.rs

@@ -386,6 +386,17 @@ impl PhpType {
 ///
 /// Only scalar value domains take part; see [`is_runtime_scalar_value_domain`].
 pub(crate) fn is_runtime_value_subtype(subtype: &PhpType, supertype: &PhpType) -> bool {
+    // `array{}` is the empty array, and every array type that does not
+    // demand an entry has it as a member value.  That is real value
+    // containment rather than the variance/coercion kind
+    // [`is_runtime_scalar_value_domain`] rules out, so a branch that only
+    // produced `[]` adds nothing beside a branch that produced an array of
+    // the same family — which is what keeps the `[]` a loop or a by-ref
+    // closure capture starts from out of the joined result.
+    if subtype.is_empty_array_shape() && supertype.accepts_empty_array() {
+        return true;
+    }
+
     if !is_runtime_scalar_value_domain(subtype) || !is_runtime_scalar_value_domain(supertype) {
         return false;
     }

src/type_engine/variable/rhs_resolution/array_access.rs

@@ -219,6 +219,15 @@ fn index_segment(
         return Some(element);
     }
 
+    // An empty shape has no entry any key could address, so the read is a
+    // guaranteed miss and yields `null`, exactly like an offset read on a
+    // non-array.  Widening to `mixed` instead loses the answer to
+    // `$a[$k] ?? 0` on the `[]` a loop is about to fill, which then makes
+    // every accumulated `+` an `int|float`.
+    if base.is_empty_array_shape() {
+        return Some(PhpType::null());
+    }
+
     // Fallback: when the base type is a plain class name (e.g.
     // `OpeningHours`), resolve the class and check its iterable generics
     // (`@extends`, `@implements`) for the element type. This handles

src/types/resolved_type.rs

@@ -525,6 +525,18 @@ impl ResolvedType {
                         _ => members.push(rt.type_string.clone()),
                     }
                 }
+                // The `[]` a variable was initialised to says nothing
+                // beside the array a later write produced: `array{}` is the
+                // empty array, and an array alternative that demands no
+                // entry already contains it. Dropping it is what keeps a
+                // loop accumulator and a by-ref closure capture from
+                // reporting `array{}|list<string>`, whose offset reads then
+                // carry a spurious `null` from the empty half.
+                if members.iter().any(PhpType::is_empty_array_shape)
+                    && members.iter().any(PhpType::accepts_empty_array)
+                {
+                    members.retain(|m| !m.is_empty_array_shape());
+                }
                 PhpType::union(members)
             }
         }

tests/integration/docblock_types.rs

@@ -1515,6 +1515,59 @@ async fn test_real_property_overrides_property_tag() {
     );
 }
 
+/// Test: a `@property` tag beats a *non-public inherited* property of the
+/// same name.  PHP only reaches `__get()` when no accessible property
+/// exists, so an ancestor's `protected` declaration is not what the read
+/// yields and its type must not describe it.  This is the Eloquent
+/// `@property string $connection` / `$table` pattern.
+#[tokio::test]
+async fn test_property_tag_overrides_inherited_non_public_property() {
+    let backend = create_test_backend();
+    let base = backend.parse_php(concat!(
+        "<?php\n",
+        "class Base {\n",
+        "    /** @var \\UnitEnum|string|null */\n",
+        "    protected $connection;\n",
+        "    /** @var string|null */\n",
+        "    public $label;\n",
+        "}\n",
+    ));
+    let child = backend.parse_php(concat!(
+        "<?php\n",
+        "/**\n",
+        " * @property string $connection\n",
+        " * @property string $label\n",
+        " */\n",
+        "class Child extends Base {}\n",
+    ));
+
+    let base = std::sync::Arc::new(base.into_iter().next().unwrap());
+    let loader = move |name: &str| -> Option<std::sync::Arc<phpantom_lsp::ClassInfo>> {
+        (name == "Base").then(|| std::sync::Arc::clone(&base))
+    };
+    let merged = phpantom_lsp::resolve_class_fully(&child[0], &loader);
+
+    let type_of = |name: &str| {
+        merged
+            .properties
+            .iter()
+            .find(|p| p.name == name)
+            .and_then(|p| p.type_hint_str())
+    };
+    assert_eq!(
+        type_of("connection").as_deref(),
+        Some("string"),
+        "@property should retype the inherited protected property"
+    );
+    // A public inherited property is reachable without `__get()`, so the
+    // tag documents nothing new and the declaration keeps its own type.
+    assert_eq!(
+        type_of("label").as_deref(),
+        Some("string|null"),
+        "@property must not override an accessible inherited property"
+    );
+}
+
 /// Test: `@property-read` tags are provided lazily via `resolve_class_fully`.
 #[tokio::test]
 async fn test_parse_php_property_read_tag() {

tests/phpstan_nsrt/array-shapes.php

@@ -87,4 +87,47 @@ public function optionalKey(array $opt)
 		// assertType('string', $opt['nullable']);
 	}
 
-}
+
+	/**
+	 * The empty shape has no entry any key could address, so an offset
+	 * read on it is a guaranteed miss.
+	 *
+	 * @param array<string, int> $ints
+	 */
+	public function emptyShapeOffsets(array $ints, string $k, int $n)
+	{
+		$empty = [];
+		assertType('null', $empty['missing']);
+		assertType('0', $empty[$k] ?? 0);
+		assertType('int', ($empty[$k] ?? 0) + $n);
+
+		$totals = [];
+		foreach ($ints as $key => $value) {
+			$totals[$key] = ($totals[$key] ?? 0) + $value;
+		}
+		assertType('array<string, int>', $totals);
+	}
+
+	/**
+	 * `array{}` is the empty array, which every array alternative that
+	 * demands no entry already contains, so it drops out of the join.
+	 *
+	 * @param list<string> $names
+	 */
+	public function emptyShapeAbsorbedByArraySibling(array $names, bool $c)
+	{
+		$collected = [];
+		$append = function (string $name) use (&$collected): void {
+			$collected[] = $name;
+		};
+		assertType('list<string>', $collected);
+		assertType('string', $collected[0]);
+
+		$maybe = [];
+		if ($c) {
+			$maybe = $names;
+		}
+		assertType('list<string>', $maybe);
+	}
+
+}