Skip to content

Commit 1c81ceb

Browse files
committed
Handle instanceof self/static/parent narrowing
1 parent 0c865d2 commit 1c81ceb

6 files changed

Lines changed: 378 additions & 30 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
- **`instanceof self/static/parent` narrowing.** Type narrowing with `instanceof self`, `instanceof static`, and `instanceof parent` now works correctly in all contexts (assert, if-blocks, guard clauses, compound conditions). Previously these keywords were ignored because the AST parser produces dedicated node types rather than identifiers, and `parent` was not recognized as a type hint during resolution.
1213
- **Variable type after reassignment.** When a method parameter is reassigned mid-body (e.g. `$file = $result->getFile()`), subsequent member accesses now resolve against the new type instead of the original parameter type. Previously the diagnostic subject cache reused the first resolution for the entire method scope, producing false-positive "not found" warnings on members that exist only on the reassigned type.
1314
- **Null-safe method chain resolution.** Null-safe method calls (`$obj?->method()`) now resolve the return type correctly for variable type inference, including cross-file chains. Previously `?->` calls were ignored by the RHS resolution pipeline, losing the type for any variable assigned from a null-safe chain.
1415
- **Nullable and generic types in class lookup.** Variables typed as `?ClassName` or `Collection<Item>` now resolve correctly across all code paths. Previously the `?` prefix and generic parameters were not stripped before class lookup, causing the type engine to treat them as unknown types. This fixes completion, hover, go-to-definition, and false-positive diagnostics for any variable whose type uses the nullable shorthand or carries generic arguments.

docs/todo/bugs.md

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -15,36 +15,6 @@ within the same impact tier.
1515

1616
---
1717

18-
#### B9. `assert($x instanceof self)` narrowing ignores `self`/`static`/`parent`
19-
20-
| | |
21-
|---|---|
22-
| **Impact** | Medium-High |
23-
| **Effort** | Low |
24-
25-
`try_extract_instanceof` in `completion/types/narrowing.rs` only matches
26-
`Expression::Identifier` for the RHS of an `instanceof` check. When the
27-
RHS is `self`, `static`, or `parent`, the Mago AST produces
28-
`Expression::Self_`, `Expression::Static`, or `Expression::Parent`
29-
instead, which fall through to `None`. This means
30-
`assert($feature instanceof self)` never narrows the variable.
31-
32-
The fix is to add three arms to the `match bin.rhs` block in
33-
`try_extract_instanceof` that map `Self_``"self"`, `Static`
34-
`"static"`, `Parent``"parent"`. The downstream
35-
`apply_instanceof_inclusion` already resolves `"self"` etc. to the
36-
current class via `type_hint_to_classes`.
37-
38-
**Reproduce:** any method with a `BaseCatalogFeature` parameter that
39-
calls `assert($feature instanceof self)` then accesses subclass-only
40-
methods on `$feature`. Also affects Mockery test patterns:
41-
`$mock = $this->mock(X::class); assert($mock instanceof X);`.
42-
43-
**Triage count:** ~30 diagnostics in luxplus/shared (18 BaseCatalogFeature,
44-
11 MockInterface, 1 Elasticsearch).
45-
46-
---
47-
4818
#### B10. Negative narrowing after early return not applied
4919

5020
| | |

example.php

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,34 @@ public function demo(): void
211211
}
212212

213213

214+
// ── instanceof self/static/parent Narrowing ────────────────────────────────
215+
216+
class InstanceofSelfDemo extends ScaffoldingSedan
217+
{
218+
public function sport(): void {}
219+
220+
public function demo(ScaffoldingMotor $m): void
221+
{
222+
// instanceof self — narrows to InstanceofSelfDemo
223+
assert($m instanceof self);
224+
$m->cruise(); // inherited from ScaffoldingSedan
225+
$m->sport(); // own method via self narrowing
226+
227+
// instanceof static — narrows to InstanceofSelfDemo
228+
$x = getUnknownValue();
229+
if ($x instanceof static) {
230+
$x->sport(); // narrowed to static (this class)
231+
}
232+
233+
// instanceof parent — narrows to ScaffoldingSedan
234+
$y = getUnknownValue();
235+
if ($y instanceof parent) {
236+
$y->cruise(); // narrowed to parent (ScaffoldingSedan)
237+
}
238+
}
239+
}
240+
241+
214242
// ── Custom Assert Narrowing ─────────────────────────────────────────────────
215243

216244
class AssertNarrowingDemo
@@ -2659,6 +2687,16 @@ public function map(string $signature, mixed $source): Pen|Marker
26592687

26602688
// ── Demo-Specific Scaffolding ───────────────────────────────────────────────
26612689

2690+
class ScaffoldingMotor
2691+
{
2692+
public function start(): void {}
2693+
}
2694+
2695+
class ScaffoldingSedan extends ScaffoldingMotor
2696+
{
2697+
public function cruise(): void {}
2698+
}
2699+
26622700
abstract class ScaffoldingAbstractShape
26632701
{
26642702
abstract public function area(): float;
@@ -4351,6 +4389,17 @@ function runDemoAssertions(): void
43514389
assert(StaticAssert::isRock(new Rock()) === true, 'StaticAssert::isRock(Rock) must return true');
43524390
assert(StaticAssert::isNotRock(new Banana()) === true, 'StaticAssert::isNotRock(Banana) must return true');
43534391

4392+
// ── instanceof self/static/parent ───────────────────────────────────
4393+
$sedan = new ScaffoldingSedan();
4394+
assert($sedan instanceof ScaffoldingMotor, 'ScaffoldingSedan must extend ScaffoldingMotor');
4395+
assert(method_exists($sedan, 'cruise'), 'ScaffoldingSedan must have cruise()');
4396+
assert(method_exists($sedan, 'start'), 'ScaffoldingSedan must inherit start()');
4397+
4398+
$demo = new InstanceofSelfDemo();
4399+
assert($demo instanceof ScaffoldingSedan, 'InstanceofSelfDemo must extend ScaffoldingSedan');
4400+
assert(method_exists($demo, 'sport'), 'InstanceofSelfDemo must have sport()');
4401+
assert(method_exists($demo, 'cruise'), 'InstanceofSelfDemo must inherit cruise()');
4402+
43544403
// ── Method-level @template (runtime resolution) ─────────────────────
43554404
$locator = new ServiceLocator();
43564405
$locatedPen = $locator->get(Pen::class);

src/completion/types/narrowing.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,9 @@ pub(in crate::completion) fn try_extract_instanceof<'b>(
378378
// RHS is the class name
379379
match bin.rhs {
380380
Expression::Identifier(ident) => Some(ident.value().to_string()),
381+
Expression::Self_(_) => Some("self".to_string()),
382+
Expression::Static(_) => Some("static".to_string()),
383+
Expression::Parent(_) => Some("parent".to_string()),
381384
_ => None,
382385
}
383386
}

src/completion/types/resolution.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,25 @@ fn type_hint_to_classes_depth(
211211
.collect();
212212
}
213213

214+
// `parent` refers to the parent class of the owning class.
215+
if hint == "parent" {
216+
let parent_name = all_classes
217+
.iter()
218+
.find(|c| c.name == owning_class_name)
219+
.and_then(|c| c.parent_class.clone())
220+
.or_else(|| class_loader(owning_class_name).and_then(|c| c.parent_class.clone()));
221+
if let Some(parent) = parent_name {
222+
return all_classes
223+
.iter()
224+
.find(|c| c.name == parent)
225+
.map(|c| ClassInfo::clone(c))
226+
.or_else(|| class_loader(&parent).map(Arc::unwrap_or_clone))
227+
.into_iter()
228+
.collect();
229+
}
230+
return vec![];
231+
}
232+
214233
// ── Parse generic arguments (if any) ──
215234
// `Collection<int, User>` → base_hint = `Collection`, generic_args = ["int", "User"]
216235
// `Foo` → base_hint = `Foo`, generic_args = []

0 commit comments

Comments
 (0)