Skip to content

Commit 410f8c4

Browse files
committed
Update roadmap
1 parent 173d252 commit 410f8c4

2 files changed

Lines changed: 239 additions & 39 deletions

File tree

docs/todo.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ within the same impact tier.
2323

2424
| # | Item | Impact | Effort |
2525
| --- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ---------- |
26+
| T26 | [Carry `PhpType` through bracket-access segment walks](todo/type-inference.md#t26-carry-phptype-through-bracket-access-segment-walks) | High | Low |
27+
| T27 | [Keep substituted return types as `PhpType` in RHS resolution](todo/type-inference.md#t27-keep-substituted-return-types-as-phptype-in-rhs-resolution) | High | Low |
28+
| T28 | [Accept `&[PhpType]` in generic-arg resolution](todo/type-inference.md#t28-accept-phptype-in-generic-arg-resolution) | Medium | Medium |
29+
| T29 | [Migrate `type_strings_joined` call sites to `types_joined`](todo/type-inference.md#t29-migrate-type_strings_joined-call-sites-to-types_joined) | Medium | Low |
30+
| T30 | [Thread `PhpType` through remaining string-based API boundaries](todo/type-inference.md#t30-thread-phptype-through-remaining-string-based-api-boundaries) | Medium | Medium |
2631
| | **Release 0.7.0** | | |
2732

2833
## Sprint 5 — Polish for office adoption
@@ -95,10 +100,10 @@ unlikely to move the needle for most users.
95100
| C10 | [Deprecation markers on class-name completions from all sources](todo/completion.md#c10-deprecation-markers-on-class-name-completions-from-all-sources) | Low | Low |
96101
| | **[Type Inference](todo/type-inference.md)** | | |
97102
| T25 | [Forward-walking scope model](todo/type-inference.md#t25-forward-walking-scope-model-for-variable-type-resolution) (eliminate backward-scanning depth limit) | High | Very High |
98-
| T19 | [Structured type representation](todo/type-inference.md#t19-structured-type-representation) (replace string-based types with `PhpType` enum) | High | Very High |
103+
| T19 | [Structured type representation](todo/type-inference.md#t19-structured-type-representation) (partially complete; remaining API boundary cleanup tracked as T26–T30) | High | Very High |
99104
| T20 | [Type narrowing reconciliation engine](todo/type-inference.md#t20-type-narrowing-reconciliation-engine) (sure/sureNot tracking, AND/OR algebra) | Medium-High | High |
100105
| T6 | `Closure::bind()` / `Closure::fromCallable()` return type preservation | Low-Medium | Low-Medium |
101-
| T12 | [Intersection types flattened to unions by `type_strings_joined`](todo/type-inference.md#t12-intersection-types-flattened-to-unions-by-type_strings_joined) | Low-Medium | Low (after M4) |
106+
| T12 | [Intersection types flattened to unions by `type_strings_joined`](todo/type-inference.md#t12-intersection-types-flattened-to-unions-by-type_strings_joined) (subsumed by T29) | Low-Medium | Low |
102107
| T13 | [Closure variables lose callable signature detail](todo/type-inference.md#t13-closure-variables-lose-callable-signature-detail) | Low-Medium | Medium |
103108
| T4 | Non-empty-\* type narrowing and propagation | Low | Low |
104109
| T5 | Fiber type resolution | Low | Low |

docs/todo/type-inference.md

Lines changed: 232 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ $one-> // should see Foo members
328328
---
329329

330330
## T12. Intersection types flattened to unions by `type_strings_joined`
331-
**Impact: Low-Medium · Effort: Low (after M4)**
331+
**Impact: Low-Medium · Effort: Low**
332332

333333
`ResolvedType::type_strings_joined` joins all resolved type entries
334334
with `|`. When a variable has an intersection type (`A&B`), the
@@ -350,10 +350,10 @@ function measure(Countable&Serializable $thing): void {
350350
// `Countable&Serializable $thing`.
351351
```
352352

353-
**Blocked by M4.** The fix requires `PhpType::Intersection` from the
354-
mago-type-syntax migration. The current `Vec<ResolvedType>` has no way
355-
to distinguish "these types form an intersection" from "these types
356-
form a union". With `PhpType`, the intersection is a single tree node.
353+
**No longer blocked.** `PhpType::Intersection` already exists.
354+
`ResolvedType::types_joined()` returns a `PhpType` that preserves
355+
intersection structure. This will be naturally fixed when T29 migrates
356+
all `type_strings_joined` call sites to `types_joined`.
357357

358358
**After fixing:** verify that extract function docblock generation
359359
preserves intersection types in both the native hint and the `@param`
@@ -415,38 +415,25 @@ emits the concrete callable signature in the `@param` tag.
415415
## T19. Structured type representation
416416
**Impact: High · Effort: Very High**
417417

418-
PHPantom represents types as strings (e.g. `"Collection<string>|null"`)
419-
and manipulates them via string splitting, regex, and concatenation.
420-
PHPStan, Psalm, and Mago all use structured type trees (enums/classes)
421-
where each type is a node with typed children. This causes:
422-
423-
- Fragile parsing on every type comparison or manipulation
424-
- No proper subtype checking (can't answer "is `Cat` a subtype of
425-
`Animal`?")
426-
- String-based template substitution that breaks on nested generics
427-
- No union simplification (`string|string` stays as-is, `true|false`
428-
doesn't merge to `bool`)
429-
- Intersection types can't be properly distributed over unions
430-
431-
A structured type representation (a Rust `enum PhpType`) would
432-
eliminate these issues. PHPantom already has `PhpType` via
433-
`mago-type-syntax` for parsing. The gap is using it as the primary
434-
representation throughout the resolution pipeline instead of raw
435-
strings.
436-
437-
**Migration path:** incremental. Start by making the hottest paths
438-
(template substitution, type narrowing) operate on `PhpType` values
439-
instead of strings, converting at the boundary. Expand outward over
440-
time.
441-
442-
**Reference:** PHPStan's `Type` interface (~120 methods), Psalm's
443-
`Union`/`Atomic` hierarchy, Mago's `TUnion`/`TAtomic` enum. All three
444-
converge on the same architecture.
445-
446-
**Note:** the `mago-type-syntax` integration tracked in `refactor.md`
447-
is a stepping stone toward this. The remaining items there
448-
(`ArrayShapeEntry.value_type` to `PhpType`, `split_type_token`
449-
replacement) should be completed first.
418+
**Status: partially complete.** The `PhpType` enum exists in
419+
`src/php_type.rs` with full parsing via `mago-type-syntax`, a rich
420+
method set (substitution, extraction, display, name resolution), and
421+
is already used as the primary representation in core data structures
422+
(`MethodInfo.return_type`, `PropertyInfo.type_hint`,
423+
`ParameterInfo.type_hint`, `ResolvedType.type_string`). Template
424+
substitution, type narrowing, and inheritance merging all operate on
425+
`PhpType` values.
426+
427+
The remaining work is eliminating `PhpType -> String -> PhpType`
428+
round-trips at internal API boundaries. Many functions still accept
429+
`&str` or return `Option<String>` for types, forcing callers to
430+
stringify and downstream consumers to re-parse. The concrete migration
431+
tasks are tracked as T26 through T30.
432+
433+
Subtype checking (`is Cat a subtype of Animal?`), union simplification
434+
(`string|string`, `true|false -> bool`), and intersection distribution
435+
over unions remain unimplemented and are out of scope for the API
436+
boundary cleanup.
450437

451438
---
452439

@@ -604,3 +591,211 @@ hover, diagnostics) once the forward walker covers enough cases.
604591
statement analyzers. Both converge on the same architecture: the
605592
scope is the single source of truth, populated eagerly as the walk
606593
progresses.
594+
595+
---
596+
597+
## T26. Carry `PhpType` through bracket-access segment walks
598+
**Impact: High · Effort: Low**
599+
600+
In `rhs_resolution.rs` (`resolve_rhs_array_access`, ~L885-915), the
601+
bracket-access segment walk carries `current_type` as a `String`. On
602+
every iteration it re-parses to `PhpType`, calls `.shape_value_type()`
603+
or `.extract_value_type()`, then stringifies the result to continue.
604+
For chained access like `$result['items'][0]['name']` this produces a
605+
`PhpType -> String -> PhpType` round-trip per bracket level.
606+
607+
A similar pattern exists in `resolver.rs`
608+
(`try_chained_array_access_with_candidates`) where candidates arrive
609+
as `String` even though callers already have `PhpType` values.
610+
611+
**What to change:**
612+
613+
1. In `resolve_rhs_array_access`, change `current_type` from `String`
614+
to `PhpType`. Call `.shape_value_type()` / `.extract_value_type()`
615+
directly and assign the result without stringifying. Only convert
616+
to string at the end for the final `type_hint_to_classes` call.
617+
618+
2. In `resolver.rs`, change `try_chained_array_access_with_candidates`
619+
to accept `PhpType` candidates instead of `String`. Update the
620+
callers (`resolve_target_classes_expr` ArrayAccess branch,
621+
`resolve_property_type_hint` call site) to pass `PhpType` directly
622+
instead of stringifying.
623+
624+
**Files:** `src/completion/variable/rhs_resolution.rs`,
625+
`src/completion/resolver.rs`.
626+
627+
**Part of:** T19 (structured type representation migration).
628+
629+
---
630+
631+
## T27. Keep substituted return types as `PhpType` in RHS resolution
632+
**Impact: High · Effort: Low**
633+
634+
In `rhs_resolution.rs`, `resolve_rhs_method_call_inner` (~L1699-1726)
635+
and `resolve_rhs_static_call` (~L1895-1916) produce a `PhpType` via
636+
`substitute() -> replace_self()`, then stringify it, only for the
637+
caller to immediately re-parse with `PhpType::parse(hint)`:
638+
639+
```
640+
let substituted = ret.substitute(&subs).replace_self(&owner.name);
641+
let ret_type_string = substituted.to_string(); // stringify
642+
// ...
643+
ResolvedType::from_classes_with_hint(classes, PhpType::parse(hint)) // re-parse
644+
```
645+
646+
This runs for every method/static call in an assignment RHS.
647+
648+
**What to change:**
649+
650+
1. Keep `ret_type_string` as `Option<PhpType>` instead of
651+
`Option<String>` in both functions.
652+
653+
2. Pass the `PhpType` directly to `from_classes_with_hint` (which
654+
already accepts `PhpType`).
655+
656+
3. Apply the same fix in `resolve_rhs_static_call`.
657+
658+
**Files:** `src/completion/variable/rhs_resolution.rs`.
659+
660+
**Part of:** T19.
661+
662+
---
663+
664+
## T28. Accept `&[PhpType]` in generic-arg resolution
665+
**Impact: Medium · Effort: Medium**
666+
667+
In `completion/types/resolution.rs`, when handling
668+
`PhpType::Generic(name, args)`, the args are converted back to strings
669+
so they can be passed to `build_generic_subs()` and
670+
`apply_generic_args()` in `inheritance.rs`, which accept `&[&str]`.
671+
Those functions then re-parse each arg via `PhpType::parse()`.
672+
673+
The same pattern appears in `resolve_named_type()` where template
674+
bound lookups stringify a `PhpType` to re-enter the string-based
675+
resolution path (~L425, L428).
676+
677+
**What to change:**
678+
679+
1. Change `build_generic_subs()` and `apply_generic_args()` in
680+
`inheritance.rs` to accept `&[PhpType]` instead of `&[&str]`.
681+
Remove the internal `PhpType::parse()` calls.
682+
683+
2. Update `type_hint_to_classes_typed_depth()` to pass the generic
684+
args as `&[PhpType]` directly instead of stringifying them.
685+
686+
3. Update `resolve_named_type()` to use `PhpType` values for template
687+
bound lookups instead of round-tripping through strings.
688+
689+
4. Remove the `apply_substitution()` string shim in `inheritance.rs`
690+
(~L972-1000) once all callers work with `PhpType` directly.
691+
692+
**Files:** `src/inheritance.rs`, `src/completion/types/resolution.rs`.
693+
694+
**Part of:** T19.
695+
696+
---
697+
698+
## T29. Migrate `type_strings_joined` call sites to `types_joined`
699+
**Impact: Medium · Effort: Low**
700+
701+
`ResolvedType::types_joined()` returns a structured `PhpType` and was
702+
created as the replacement for `type_strings_joined()`, which joins
703+
types into a `String` with `|` separators. The string variant is still
704+
called in ~12 places across the resolution pipeline:
705+
706+
- `resolver.rs` (2 call sites)
707+
- `resolution.rs` / variable resolution (3 call sites)
708+
- `rhs_resolution.rs` (1 call site)
709+
- `call_resolution.rs` (1 call site)
710+
- `foreach_resolution.rs` (1 call site)
711+
- `array_shape.rs` (1 call site)
712+
- `fix_return_type.rs` (1 call site)
713+
714+
Each produces a `String` that is later re-parsed by downstream
715+
consumers.
716+
717+
**What to change:**
718+
719+
1. At each call site, replace `type_strings_joined` with
720+
`types_joined` and thread the resulting `PhpType` through to the
721+
consumer.
722+
723+
2. Where the consumer is a function that accepts `&str` (e.g.
724+
`check_unresolvable_class_name`), change that function to accept
725+
`&PhpType`.
726+
727+
3. Once all call sites are migrated, remove `type_strings_joined`
728+
entirely (or mark it `#[deprecated]` first if external tests use
729+
it).
730+
731+
**Files:** `src/completion/resolver.rs`,
732+
`src/completion/variable/resolution.rs`,
733+
`src/completion/variable/rhs_resolution.rs`,
734+
`src/completion/call_resolution.rs`,
735+
`src/completion/variable/foreach_resolution.rs`,
736+
`src/completion/array_shape.rs`,
737+
`src/code_actions/phpstan/fix_return_type.rs`,
738+
`src/types.rs`.
739+
740+
**Part of:** T19.
741+
742+
---
743+
744+
## T30. Thread `PhpType` through remaining string-based API boundaries
745+
**Impact: Medium · Effort: Medium**
746+
747+
Several internal APIs accept `&str` or `Option<String>` for types when
748+
their callers already have a `PhpType`. This creates parse-on-entry
749+
overhead and loses structural information. The items below can be done
750+
independently in any order.
751+
752+
### `VarResolutionCtx.enclosing_return_type`
753+
754+
In `resolver.rs` (~L128), `enclosing_return_type` is `Option<String>`.
755+
It is parsed with `PhpType::parse()` every time it is consumed (e.g.
756+
for `generator_send_type()` during yield inference). Change to
757+
`Option<PhpType>` and parse once at construction.
758+
759+
### `resolve_type_alias()` return type
760+
761+
In `completion/types/resolution.rs` (~L462-544), type alias
762+
definitions are stored as `PhpType` in `TypeAliasDef::Local(PhpType)`
763+
but immediately stringified to return `Option<String>`. Change to
764+
return `Option<PhpType>`.
765+
766+
### `is_type_subclass_of()` base name extraction
767+
768+
In `call_resolution.rs` (~L1054-1143), a `PhpType` is stringified
769+
then split on `<` and stripped of `\\` to get the base class name. Use
770+
`PhpType::base_name()` instead.
771+
772+
### Shape/list builders in variable resolution
773+
774+
In `resolution.rs`, `merge_shape_key()`, `merge_push_type()`, and
775+
`merge_keyed_type()` build type strings via `format!("array{{...}}")`
776+
and `format!("list<...>")` that are immediately re-parsed. Build
777+
`PhpType::ArrayShape(entries)` and `PhpType::Generic("list", ...)`
778+
directly.
779+
780+
### Hover formatting entry points
781+
782+
In `hover/formatting.rs`, `build_var_annotation()` and
783+
`build_param_return_section()` accept `Option<&str>` (produced by
784+
`type_hint_str()` which stringifies a `PhpType`), then immediately
785+
re-parse. Change to accept `Option<&PhpType>`.
786+
787+
### `type_hint_to_classes_depth()` string preprocessing
788+
789+
In `completion/types/resolution.rs` (~L88-128), `strip_nullable()`
790+
and parenthesis stripping are done via string manipulation before
791+
parsing. `PhpType::parse()` already handles `?Foo` and `(A&B)|C`
792+
natively, making this preprocessing redundant. Parse first, then
793+
handle alias resolution on the parsed result.
794+
795+
**Files:** `src/completion/resolver.rs`,
796+
`src/completion/types/resolution.rs`,
797+
`src/completion/call_resolution.rs`,
798+
`src/completion/variable/resolution.rs`,
799+
`src/hover/formatting.rs`.
800+
801+
**Part of:** T19.

0 commit comments

Comments
 (0)