Skip to content

Commit b3a1c6a

Browse files
committed
Handle @mixin generic substitution
1 parent 8c65540 commit b3a1c6a

17 files changed

Lines changed: 625 additions & 226 deletions

File tree

docs/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Added
1111

1212
- **Null-coalesce (`??`) type refinement.** When the left-hand side of `??` is provably non-nullable (e.g. `new Foo()`, `clone $x`, a literal), the right-hand side is recognized as dead code and the result resolves to the LHS type only. When the LHS is nullable (e.g. a `?Foo` return type), `null` is stripped from the LHS and the result is the union of the non-null LHS with the RHS.
13+
- **`@mixin` generic substitution.** When a class declares `@mixin Builder<TRelatedModel>`, the generic arguments are now preserved and substituted into the mixin's members. Previously, `@mixin Builder<TRelatedModel>` was stored as just `Builder` (the generic argument was stripped), so methods like `firstOrFail(): TModel` returned the raw template parameter name instead of the concrete type. This resolves through inheritance chains: `BelongsTo @extends Relation<Product>` with `Relation @mixin Builder<TRelatedModel>` now correctly substitutes `TModel` to `Product` on all Builder methods.
1314

1415
### Fixed
1516

17+
- **Hover and completion type parity.** The hover type-string engine now handles pipe expressions (`|>`), closure/arrow-function literals, and generator yield-assignment (`$var = yield`) expressions, matching the completion engine. Previously these expression kinds were only handled by the completion pipeline, so hover could show a different (or missing) type for variables assigned from them.
18+
1619
- **Guard clause narrowing across instanceof branches.** After `if ($x instanceof Y) { return; }`, subsequent `instanceof` checks on the same variable no longer incorrectly resolve to `Y`. Previously the diagnostic cache reused the narrowed type from inside the first branch for all later accesses to the same variable, producing false-positive "not found" warnings (e.g. "Property 'value' not found on class 'Stringable'" inside a `BackedEnum` branch).
1720
- **`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.
1821
- **Interface constants through multi-extends chains.** Constants defined on parent interfaces are now found when an interface extends multiple other interfaces. Previously only the first parent interface was walked for member merging, so constants like `Carbon::JANUARY` (defined on the `UnitValue` interface, sixth in `CarbonInterface`'s extends list) were reported as unknown.

docs/todo.md

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,6 @@ within the same impact tier.
1919

2020
# Scheduled Sprints
2121

22-
## Sprint 3 — Generic template resolution
23-
24-
| # | Item | Impact | Effort |
25-
| --- | ------------------------------------------------------------------------------------------------------------------------------------------ | ------ | ----------- |
26-
| B1 | [Dual type-resolution engines cause hover / completion divergence](todo/bugs.md#b1-dual-type-resolution-engines-cause-hover--completion-divergence) | Medium | Medium-High |
27-
| T8 | [`@mixin` generic substitution and chain-level template resolution](todo/type-inference.md#t8-mixin-generic-substitution-and-chain-level-template-resolution) | High | Medium-High |
28-
| | **Release 0.6.0** | | |
29-
30-
> **Why this is next:** T8 accounts for ~186 of the 325 remaining
31-
> diagnostics in the `shared` triage project (57%). Two sub-problems:
32-
> (1) `@mixin` generic arguments are stripped during extraction, so
33-
> `@mixin Builder<TRelatedModel>` loses the type argument; (2)
34-
> chain-level calls like `Collection<int, Product>::first()` don't
35-
> substitute `TValue``Product`. Fixing both would drop the
36-
> diagnostic count below 140.
37-
3822
## Sprint 4 — Refactoring toolkit
3923

4024
| # | Item | Impact | Effort |

docs/todo/bugs.md

Lines changed: 1 addition & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -15,63 +15,5 @@ within the same impact tier.
1515

1616
---
1717

18-
#### B1. Dual type-resolution engines cause hover / completion divergence
19-
20-
| | |
21-
|---|---|
22-
| **Impact** | Medium |
23-
| **Effort** | Medium-High |
24-
25-
Variable type resolution has two parallel RHS expression resolvers that
26-
must be kept in sync manually:
27-
28-
1. **`resolve_rhs_expression`** in `completion/variable/rhs_resolution.rs`
29-
— returns `Vec<ClassInfo>`, used by the completion pipeline.
30-
2. **`resolve_rhs_raw_type`** in `completion/variable/raw_type_inference.rs`
31-
— returns `Option<String>`, used by hover's type-string path
32-
(`resolve_variable_type_string` → step 5).
33-
34-
Hover tries the type-string path first. When it returns `Some(…)` the
35-
ClassInfo fallback never fires, so hover shows whatever the raw-type
36-
engine inferred — even if it is wrong. The completion pipeline uses the
37-
ClassInfo engine directly and gets the correct answer.
38-
39-
The two resolvers handle **different sets of expression types**:
40-
41-
| Expression kind | ClassInfo engine | Raw-type engine |
42-
| ------------------------ | :--------------: | :-------------: |
43-
| `clone` |||
44-
| `\|>` (pipe) |||
45-
| `Closure` / arrow fn | ✓ (→ `\Closure`) ||
46-
| `yield` (send type) |||
47-
| `Call` (full resolution) || partial¹ |
48-
| `Access` (property) || partial¹ |
49-
| Scalar literals |||
50-
| Array literal inference |||
51-
52-
¹ The raw-type engine's `_ =>` catch-all delegates to
53-
`extract_rhs_iterable_raw_type`, which covers some calls and accesses
54-
but not all.
55-
56-
The `??` null-coalesce handling also diverges: the ClassInfo engine
57-
checks whether the LHS *AST node* is syntactically non-nullable (pattern
58-
match on `Clone`, `Literal`, etc.), while the raw-type engine checks
59-
whether the resolved *type string* is non-nullable. When the raw-type
60-
engine cannot resolve the LHS at all (e.g. `clone $x``None`), it
61-
falls through to returning only the RHS type, which is how
62-
`clone $pen ?? new Marker()` shows `Marker` on hover but `Pen` on
63-
completion.
64-
65-
**Possible approaches:**
66-
67-
- **Unify into one engine** — make `resolve_rhs_expression` the single
68-
source of truth and derive the type string from its result. Hover
69-
would call the ClassInfo path and format the names into a union
70-
string, falling back to `resolve_variable_type_string` only for
71-
scalar / generic types that ClassInfo cannot represent. This
72-
eliminates the synchronisation burden entirely.
73-
- **Exhaustiveness enforcement** — if keeping both engines, add a shared
74-
enum of "RHS expression kinds" and a compile-time check (or test)
75-
that both match arms cover the same set, so new expression types
76-
cannot be added to one without the other.
18+
No outstanding bugs at this time.
7719

docs/todo/type-inference.md

Lines changed: 0 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -97,105 +97,6 @@ only has a native `array` type hint.
9797

9898
---
9999

100-
## T8. `@mixin` generic substitution and chain-level template resolution
101-
**Impact: High · Effort: Medium-High**
102-
103-
PHPantom's generics system substitutes template parameters through
104-
`@extends` and `@implements` chains but fails in two related
105-
scenarios that together account for ~180 of the ~325 remaining
106-
diagnostics in the `shared` triage project (iteration 4).
107-
108-
### Problem 1: `@mixin` generic arguments are discarded
109-
110-
`extract_mixin_tags` (docblock/tags.rs) calls `base_class_name`,
111-
which calls `strip_generics`. So `@mixin Builder<TRelatedModel>`
112-
is stored as just `"Builder"`, and the generic argument is lost.
113-
114-
When `collect_mixin_members` (virtual_members/phpdoc.rs) later
115-
loads and merges Builder's methods, `TModel` in return types like
116-
`firstOrFail(): TModel` is never mapped to the concrete related
117-
model. The diagnostic reports the raw template parameter name.
118-
119-
**Concrete example:**
120-
121-
```php
122-
/**
123-
* @template TRelatedModel of Model
124-
* @template TDeclaringModel of Model
125-
* @mixin Builder<TRelatedModel> ← generic arg stripped
126-
*/
127-
abstract class Relation { ... }
128-
129-
// User code:
130-
$product = $orderline->product()->firstOrFail();
131-
// BelongsTo<Product, OrderLine> → Relation → @mixin Builder<TRelatedModel>
132-
// Builder::firstOrFail() returns TModel
133-
// TModel is never substituted → diagnostic: "subject type 'TModel' could not be resolved"
134-
```
135-
136-
**Impact:** 38 `TModel` diagnostics + cascading anonymous chains.
137-
138-
**Fix:** Preserve generic arguments in `@mixin` tags (store them
139-
alongside the mixin class name, similar to `extends_generics` /
140-
`implements_generics`). In `collect_mixin_members`, build a
141-
substitution map from the mixin class's `template_params` to the
142-
provided generic arguments and apply it to all merged members.
143-
144-
### Problem 2: chain-level generic substitution not performed
145-
146-
When a method returns a generic type like `Collection<int, Product>`,
147-
calling a method on that result (e.g. `->first()`) does not
148-
substitute the collection's template parameters into the called
149-
method's return type.
150-
151-
```php
152-
// Collection<TKey, TValue> has first(): TValue|TFirstDefault
153-
$users->map(fn($u) => $u->name)->first();
154-
// map() returns Collection<int, string>
155-
// first() should return string|TFirstDefault → resolves to string
156-
// Actually returns: TValue|TFirstDefault (unsubstituted)
157-
```
158-
159-
The completion/type resolution pipeline resolves the return type of
160-
a chain call by:
161-
1. Resolving the subject to `ClassInfo` (e.g. `Collection`).
162-
2. Looking up the method on that class.
163-
3. Returning the method's `return_type` string.
164-
165-
Step 1 produces an unparameterised `Collection` — the generic
166-
arguments `<int, Product>` from the previous call's return type
167-
are not carried forward. The method's return type `TValue` is
168-
never mapped to `Product`.
169-
170-
**Impact:** 16 `TValue|TFirstDefault` + 3 `TModel|TFirstDefault`
171-
+ 6 other template params + ~118 cascading anonymous chain
172-
diagnostics.
173-
174-
**Fix:** When the previous link in a chain returns a parameterised
175-
type like `Collection<int, Product>`, resolve the class (`Collection`),
176-
zip its `template_params` (`[TKey, TValue]`) with the provided
177-
arguments (`[int, Product]`), build a substitution map, and apply
178-
it to the called method's return type before using it as the next
179-
link's subject type. This is the call-site equivalent of what
180-
`resolve_class_with_inheritance` already does for `@extends` chains.
181-
182-
### Combined impact
183-
184-
| Symptom | Count | Root cause |
185-
|---------|-------|------------|
186-
| `TModel` unresolvable | 38 | Problem 1 (@mixin generics stripped) |
187-
| `TValue\|TFirstDefault` unresolvable | 16 | Problem 2 (chain generics not substituted) |
188-
| Other template params (`T`, `TValue`, `TModel\|TFirstDefault`, `TReduceReturnType`) | 14 | Mix of both |
189-
| Cascading anonymous chains | ~118 | Downstream of unresolved template params |
190-
| **Total** | **~186** | |
191-
192-
Fixing both problems would reduce the `shared` project from 325 to
193-
~140 diagnostics (57% reduction), with most remaining diagnostics
194-
being correct reports or `unresolved_member_access` hints on
195-
genuinely untyped code.
196-
197-
---
198-
199100
## T2. File system watching for vendor and project changes
200101
**Impact: Medium-High · Effort: Medium**
201102

example.php

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2705,6 +2705,25 @@ public function demo(): void
27052705
}
27062706

27072707

2708+
// ── Mixin Generic Substitution ──────────────────────────────────────────────
2709+
2710+
class MixinGenericDemo
2711+
{
2712+
public function demo(): void
2713+
{
2714+
$line = new ScaffoldingOrderLine();
2715+
2716+
// @mixin Builder<TRelatedModel> on Relation resolves TModel → Product
2717+
// through: BelongsTo @extends Relation<Product> → @mixin Builder<TRelatedModel>
2718+
// → TRelatedModel=Product → Builder<Product> → firstOrFail(): TModel=Product
2719+
$line->product()->firstOrFail()->getPrice();
2720+
2721+
// Same resolution through find()
2722+
$line->product()->find()->getSku();
2723+
}
2724+
}
2725+
2726+
27082727
// ═══════════════════════════════════════════════════════════════════════════
27092728
// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
27102729
// ┃ SCAFFOLDING — Supporting definitions below this line. ┃
@@ -3871,6 +3890,40 @@ class Product
38713890
use HasFactory;
38723891

38733892
public function getPrice(): float { return 0.0; }
3893+
public function getSku(): string { return ''; }
3894+
}
3895+
3896+
// ─── Mixin Generic Scaffolding ─────────────────────────────────────────────
3897+
3898+
/**
3899+
* @template TModel
3900+
*/
3901+
class ScaffoldingMixinBuilder
3902+
{
3903+
/** @return TModel */
3904+
public function firstOrFail(): mixed { return null; }
3905+
/** @return TModel */
3906+
public function find(): mixed { return null; }
3907+
}
3908+
3909+
/**
3910+
* @template TRelatedModel
3911+
* @mixin ScaffoldingMixinBuilder<TRelatedModel>
3912+
*/
3913+
class ScaffoldingMixinRelation
3914+
{
3915+
}
3916+
3917+
/**
3918+
* @extends ScaffoldingMixinRelation<Product>
3919+
*/
3920+
class ScaffoldingMixinBelongsTo extends ScaffoldingMixinRelation
3921+
{
3922+
}
3923+
3924+
class ScaffoldingOrderLine
3925+
{
3926+
public function product(): ScaffoldingMixinBelongsTo { return new ScaffoldingMixinBelongsTo(); }
38743927
}
38753928

38763929
/** @use Indexable<int, Pen> */
@@ -5049,6 +5102,17 @@ function runDemoAssertions(): void
50495102
$ctExt = $ctRouter->extend('redis', function () {});
50505103
assert($ctExt instanceof ScaffoldingClosureThisRouter, 'Router::extend() must return self');
50515104

5105+
// ── @mixin generic substitution scaffolding ─────────────────────────
5106+
$mixinBuilder = new ScaffoldingMixinBuilder();
5107+
assert($mixinBuilder->firstOrFail() === null, 'ScaffoldingMixinBuilder::firstOrFail() must return mixed');
5108+
$mixinRelation = new ScaffoldingMixinRelation();
5109+
assert($mixinRelation instanceof ScaffoldingMixinRelation, 'ScaffoldingMixinRelation instantiates');
5110+
$mixinBelongsTo = new ScaffoldingMixinBelongsTo();
5111+
assert($mixinBelongsTo instanceof ScaffoldingMixinRelation, 'ScaffoldingMixinBelongsTo extends ScaffoldingMixinRelation');
5112+
$orderLine = new ScaffoldingOrderLine();
5113+
$productRel = $orderLine->product();
5114+
assert($productRel instanceof ScaffoldingMixinBelongsTo, 'OrderLine::product() must return ScaffoldingMixinBelongsTo');
5115+
50525116
echo "All assertions passed.\n";
50535117
}
50545118

src/code_actions/update_docblock.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -812,13 +812,14 @@ fn normalize_type_for_comparison(t: &str) -> String {
812812
|rest| format!("{}|null", rest.to_lowercase()),
813813
);
814814
// Sort union components.
815-
let mut parts: Vec<&str> = split_union_depth0(&t).into_iter().map(|s| s.trim()).collect();
815+
let mut parts: Vec<&str> = split_union_depth0(&t)
816+
.into_iter()
817+
.map(|s| s.trim())
818+
.collect();
816819
parts.sort();
817820
parts.join("|")
818821
}
819822

820-
821-
822823
/// Build the updated docblock text.
823824
fn build_updated_docblock(
824825
info: &FunctionWithDocblock,

src/completion/source/throws_analysis_tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,7 @@ fn make_class_with_throws(name: &str, methods: Vec<(&str, Vec<&str>)>) -> Arc<Cl
780780
interfaces: Vec::new(),
781781
used_traits: Vec::new(),
782782
mixins: Vec::new(),
783+
mixin_generics: Vec::new(),
783784
is_final: false,
784785
is_abstract: false,
785786
deprecation_message: None,

src/completion/variable/raw_type_inference.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -915,6 +915,35 @@ fn resolve_rhs_raw_type<'b>(rhs: &'b Expression<'b>, ctx: &VarResolutionCtx<'_>)
915915
// Last resort: docblock scan (e.g. `@var` inline annotation).
916916
super::foreach_resolution::extract_rhs_iterable_raw_type(rhs, ctx)
917917
}
918+
// ── Pipe operator (PHP 8.5): `$expr |> callable(...)` ──
919+
// The result type is the return type of the callable.
920+
Expression::Pipe(pipe) => {
921+
if let Expression::PartialApplication(PartialApplication::Function(fpa)) = pipe.callable
922+
&& let Expression::Identifier(ident) = fpa.function
923+
&& let Some(fl) = ctx.function_loader
924+
&& let Some(func_info) = fl(ident.value())
925+
&& let Some(ref ret) = func_info.return_type
926+
{
927+
return Some(ret.clone());
928+
}
929+
None
930+
}
931+
// ── Closure / arrow function / first-class callable ─────────
932+
// All produce a `Closure` instance at runtime.
933+
Expression::PartialApplication(_)
934+
| Expression::Closure(_)
935+
| Expression::ArrowFunction(_) => Some("\\Closure".to_string()),
936+
// ── Generator yield-assignment: `$var = yield $expr` ────────
937+
// The value of a yield expression is the TSend type from the
938+
// enclosing function's `@return Generator<K, V, TSend, R>`.
939+
Expression::Yield(_) => {
940+
if let Some(ref ret_type) = ctx.enclosing_return_type
941+
&& let Some(send_type) = crate::docblock::extract_generator_send_type(ret_type)
942+
{
943+
return Some(send_type);
944+
}
945+
None
946+
}
918947
// ── Call / property access — delegate to iterable extractor,
919948
// with a source-scan fallback for standalone function calls
920949
// when no `function_loader` is available. ──

0 commit comments

Comments
 (0)