Refactor type alias expansion to return PhpType instead of String

Author: AJenbo

docs/todo.md

@@ -23,7 +23,7 @@ within the same impact tier.
 
 | #   | Item                                                                                                                                                      | Impact | Effort     |
 | --- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ---------- |
-| T30 | [Thread `PhpType` through remaining string-based API boundaries](todo/type-inference.md#t30-thread-phptype-through-remaining-string-based-api-boundaries) | Medium | Medium     |
+
 |     | **Release 0.7.0**                                                                                                                                         |        |            |
 
 ## Sprint 5 — Polish for office adoption
@@ -96,7 +96,7 @@ unlikely to move the needle for most users.
 | 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            |
 |      | **[Type Inference](todo/type-inference.md)**                                                                                                                                |             |                |
 | 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      |
-| T19  | [Structured type representation](todo/type-inference.md#t19-structured-type-representation) (partially complete; remaining API boundary cleanup tracked as T30)               | High        | Very High      |
+| T19  | [Structured type representation](todo/type-inference.md#t19-structured-type-representation) (partially complete; subtype checking and union simplification remain)             | High        | Very High      |
 | T20  | [Type narrowing reconciliation engine](todo/type-inference.md#t20-type-narrowing-reconciliation-engine) (sure/sureNot tracking, AND/OR algebra)                             | Medium-High | High           |
 | T6   | `Closure::bind()` / `Closure::fromCallable()` return type preservation                                                                                                      | Low-Medium  | Low-Medium     |
 | T12  | [Intersection types flattened to unions during resolution](todo/type-inference.md#t12-intersection-types-flattened-to-unions-by-type_strings_joined)                          | Low-Medium  | Low            |

docs/todo/type-inference.md

@@ -426,16 +426,12 @@ is already used as the primary representation in core data structures
 substitution, type narrowing, and inheritance merging all operate on
 `PhpType` values.
 
-The remaining work is eliminating `PhpType -> String -> PhpType`
-round-trips at internal API boundaries. Many functions still accept
-`&str` or return `Option<String>` for types, forcing callers to
-stringify and downstream consumers to re-parse. The concrete migration
-tasks are tracked as T29 and T30.
+The API boundary cleanup (T29, T30) is complete. All core internal
+APIs now accept and return `PhpType` values directly.
 
-Subtype checking (`is Cat a subtype of Animal?`), union simplification
-(`string|string`, `true|false -> bool`), and intersection distribution
-over unions remain unimplemented and are out of scope for the API
-boundary cleanup.
+The remaining work is subtype checking (`is Cat a subtype of Animal?`),
+union simplification (`string|string`, `true|false -> bool`), and
+intersection distribution over unions.
 
 ---
 
@@ -594,63 +590,4 @@ statement analyzers. Both converge on the same architecture: the
 scope is the single source of truth, populated eagerly as the walk
 progresses.
 
----
-
-## T30. Thread `PhpType` through remaining string-based API boundaries
-**Impact: Medium · Effort: Medium**
-
-Several internal APIs accept `&str` or `Option<String>` for types when
-their callers already have a `PhpType`. This creates parse-on-entry
-overhead and loses structural information. The items below can be done
-independently in any order.
-
-### `VarResolutionCtx.enclosing_return_type`
-
-In `resolver.rs` (~L128), `enclosing_return_type` is `Option<String>`.
-It is parsed with `PhpType::parse()` every time it is consumed (e.g.
-for `generator_send_type()` during yield inference). Change to
-`Option<PhpType>` and parse once at construction.
-
-### `resolve_type_alias()` return type
-
-In `completion/types/resolution.rs` (~L462-544), type alias
-definitions are stored as `PhpType` in `TypeAliasDef::Local(PhpType)`
-but immediately stringified to return `Option<String>`. Change to
-return `Option<PhpType>`.
-
-### `is_type_subclass_of()` base name extraction
-
-In `call_resolution.rs` (~L1054-1143), a `PhpType` is stringified
-then split on `<` and stripped of `\\` to get the base class name. Use
-`PhpType::base_name()` instead.
-
-### Shape/list builders in variable resolution
-
-In `resolution.rs`, `merge_shape_key()`, `merge_push_type()`, and
-`merge_keyed_type()` build type strings via `format!("array{{...}}")`
-and `format!("list<...>")` that are immediately re-parsed. Build
-`PhpType::ArrayShape(entries)` and `PhpType::Generic("list", ...)`
-directly.
-
-### Hover formatting entry points
-
-In `hover/formatting.rs`, `build_var_annotation()` and
-`build_param_return_section()` accept `Option<&str>` (produced by
-`type_hint_str()` which stringifies a `PhpType`), then immediately
-re-parse. Change to accept `Option<&PhpType>`.
-
-### `type_hint_to_classes_depth()` string preprocessing
-
-In `completion/types/resolution.rs` (~L88-128), `strip_nullable()`
-and parenthesis stripping are done via string manipulation before
-parsing. `PhpType::parse()` already handles `?Foo` and `(A&B)|C`
-natively, making this preprocessing redundant. Parse first, then
-handle alias resolution on the parsed result.
-
-**Files:** `src/completion/resolver.rs`,
-`src/completion/types/resolution.rs`,
-`src/completion/call_resolution.rs`,
-`src/completion/variable/resolution.rs`,
-`src/hover/formatting.rs`.
 
-**Part of:** T19.

src/completion/array_shape.rs

@@ -339,6 +339,7 @@ impl Backend {
             effective_classes,
             &class_loader,
         )
+        .map(|t| t.to_string())
         .unwrap_or(effective_type);
 
         // Parse the array shape entries via PhpType.

src/completion/call_resolution.rs

@@ -1057,15 +1057,11 @@ fn is_type_subclass_of(
     all_classes: &[Arc<ClassInfo>],
     class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
 ) -> bool {
-    let candidate_str = candidate_type.to_string();
-
-    // Strip leading backslash and generic parameters for name comparison.
-    let candidate_base = candidate_str
-        .strip_prefix('\\')
-        .unwrap_or(&candidate_str)
-        .split('<')
-        .next()
-        .unwrap_or(&candidate_str);
+    // Extract the base class name directly from the type without stringifying.
+    let candidate_base = match candidate_type.base_name() {
+        Some(name) => name.strip_prefix('\\').unwrap_or(name),
+        None => return false, // Not a class type
+    };
     let ancestor_base = ancestor_name
         .strip_prefix('\\')
         .unwrap_or(ancestor_name)

src/completion/resolver.rs

@@ -125,7 +125,7 @@ pub(super) struct VarResolutionCtx<'a> {
     /// The `@return` type annotation of the enclosing function/method,
     /// if known.  Used inside generator bodies to reverse-infer variable
     /// types from `Generator<TKey, TValue, TSend, TReturn>`.
-    pub enclosing_return_type: Option<String>,
+    pub enclosing_return_type: Option<PhpType>,
     /// When `true`, if/else/elseif walking only considers the branch
     /// that contains the cursor instead of unioning all branches.
     /// This produces the single type visible at the cursor position,
@@ -168,7 +168,7 @@ impl<'a> VarResolutionCtx<'a> {
     /// annotation differs from the outer scope.
     pub(super) fn with_enclosing_return_type(
         &self,
-        enclosing_return_type: Option<String>,
+        enclosing_return_type: Option<PhpType>,
     ) -> VarResolutionCtx<'a> {
         VarResolutionCtx {
             var_name: self.var_name,

src/completion/source/helpers.rs

@@ -539,7 +539,7 @@ fn walk_array_segments_and_resolve(
             all_classes,
             class_loader,
         ) {
-            PhpType::parse(&expanded)
+            expanded
         } else {
             base_type.clone()
         }
@@ -599,7 +599,7 @@ fn walk_array_segments_and_resolve(
             all_classes,
             class_loader,
         ) {
-            current = PhpType::parse(&expanded);
+            current = expanded;
         }
     }
 

src/completion/types/resolution.rs

@@ -96,34 +96,11 @@ fn type_hint_to_classes_depth(
         return vec![];
     }
 
-    let hint = crate::util::strip_nullable(type_hint);
-
-    // Strip surrounding parentheses that appear in DNF types like `(A&B)|C`.
-    let hint = hint
-        .strip_prefix('(')
-        .and_then(|h| h.strip_suffix(')'))
-        .unwrap_or(hint);
-
-    // ── Type alias resolution ──────────────────────────────────────
-    // Check if `hint` is a type alias defined on the owning class
-    // (via `@phpstan-type` / `@psalm-type` / `@phpstan-import-type`).
-    // If so, expand the alias and resolve the underlying definition.
-    //
-    // This runs before union/intersection splitting because the alias
-    // itself may expand to a union or intersection type.
-    if let Some(alias_def) = resolve_type_alias(hint, owning_class_name, all_classes, class_loader)
-    {
-        return type_hint_to_classes_depth(
-            &alias_def,
-            owning_class_name,
-            all_classes,
-            class_loader,
-            depth + 1,
-        );
-    }
-
-    // Parse and delegate to the typed implementation.
-    let parsed = PhpType::parse(hint);
+    // Parse first — PhpType::parse handles `?Foo`, `(A&B)|C`, and all
+    // other syntax natively, making manual string preprocessing redundant.
+    // Alias resolution is handled by `resolve_named_type` when the parsed
+    // type turns out to be a `Named` variant.
+    let parsed = PhpType::parse(type_hint);
     type_hint_to_classes_typed_depth(&parsed, owning_class_name, all_classes, class_loader, depth)
 }
 
@@ -264,10 +241,10 @@ fn resolve_named_type(
     depth: u8,
 ) -> Vec<ClassInfo> {
     // ── Type alias resolution ──────────────────────────────────────
-    if let Some(alias_def) = resolve_type_alias(name, owning_class_name, all_classes, class_loader)
+    if let Some(alias_type) = resolve_type_alias(name, owning_class_name, all_classes, class_loader)
     {
-        return type_hint_to_classes_depth(
-            &alias_def,
+        return type_hint_to_classes_typed_depth(
+            &alias_type,
             owning_class_name,
             all_classes,
             class_loader,
@@ -467,7 +444,7 @@ pub(crate) fn resolve_type_alias(
     owning_class_name: &str,
     all_classes: &[Arc<ClassInfo>],
     class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
-) -> Option<String> {
+) -> Option<PhpType> {
     let mut current = hint.to_string();
     let mut resolved_any = false;
 
@@ -490,7 +467,11 @@ pub(crate) fn resolve_type_alias(
         }
     }
 
-    if resolved_any { Some(current) } else { None }
+    if resolved_any {
+        Some(PhpType::parse(&current))
+    } else {
+        None
+    }
 }
 
 /// Single-level alias lookup (no chaining).

src/completion/variable/foreach_resolution.rs

@@ -243,6 +243,7 @@ pub(in crate::completion) fn try_resolve_foreach_value_type<'b>(
             ctx.all_classes,
             ctx.class_loader,
         )
+        .map(|t| t.to_string())
         .unwrap_or(rt)
     });
 
@@ -411,6 +412,7 @@ pub(in crate::completion) fn try_resolve_foreach_key_type<'b>(
             ctx.all_classes,
             ctx.class_loader,
         )
+        .map(|t| t.to_string())
         .unwrap_or(rt)
     });
 
@@ -826,6 +828,7 @@ pub(in crate::completion) fn try_resolve_destructured_type<'b>(
             all_classes,
             class_loader,
         )
+        .map(|t| t.to_string())
         .unwrap_or(rt)
     });
 

src/completion/variable/raw_type_inference.rs

@@ -13,6 +13,7 @@ use super::{ARRAY_ELEMENT_FUNCS, ARRAY_PRESERVING_FUNCS};
 
 use crate::docblock;
 use crate::parser::extract_hint_string;
+use crate::php_type::PhpType;
 use crate::types::ClassInfo;
 
 use crate::completion::resolver::VarResolutionCtx;
@@ -328,11 +329,11 @@ fn extract_array_map_element_type(
 /// using a variable that is yielded but was not explicitly typed via
 /// an assignment or parameter.
 pub(in crate::completion) fn try_infer_from_generator_yield(
-    return_type: &str,
+    return_type: &PhpType,
     ctx: &VarResolutionCtx<'_>,
 ) -> Vec<ClassInfo> {
     // Only applies to Generator return types.
-    let value_type = match crate::php_type::PhpType::parse(return_type).extract_value_type(false) {
+    let value_type = match return_type.extract_value_type(false) {
         Some(vt) => vt.to_string(),
         None => return vec![],
     };