Migrate bracket-access segment walks to use PhpType throughout

Author: AJenbo

docs/todo.md

@@ -23,7 +23,6 @@ within the same impact tier.
 
 | #   | Item                                                                                                                                                      | Impact | Effort     |
 | --- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ---------- |
-| T26 | [Carry `PhpType` through bracket-access segment walks](todo/type-inference.md#t26-carry-phptype-through-bracket-access-segment-walks)                     | High   | Low        |
 | 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        |
 | T28 | [Accept `&[PhpType]` in generic-arg resolution](todo/type-inference.md#t28-accept-phptype-in-generic-arg-resolution)                                     | Medium | Medium     |
 | 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        |
@@ -100,7 +99,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 T26–T30)          | High        | Very High      |
+| T19  | [Structured type representation](todo/type-inference.md#t19-structured-type-representation) (partially complete; remaining API boundary cleanup tracked as T27–T30)          | 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 by `type_strings_joined`](todo/type-inference.md#t12-intersection-types-flattened-to-unions-by-type_strings_joined) (subsumed by T29) | Low-Medium  | Low            |

docs/todo/type-inference.md

@@ -428,7 +428,7 @@ 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 T26 through T30.
+tasks are tracked as T27 through T30.
 
 Subtype checking (`is Cat a subtype of Animal?`), union simplification
 (`string|string`, `true|false -> bool`), and intersection distribution
@@ -594,40 +594,6 @@ progresses.
 
 ---
 
-## T26. Carry `PhpType` through bracket-access segment walks
-**Impact: High · Effort: Low**
-
-In `rhs_resolution.rs` (`resolve_rhs_array_access`, ~L885-915), the
-bracket-access segment walk carries `current_type` as a `String`. On
-every iteration it re-parses to `PhpType`, calls `.shape_value_type()`
-or `.extract_value_type()`, then stringifies the result to continue.
-For chained access like `$result['items'][0]['name']` this produces a
-`PhpType -> String -> PhpType` round-trip per bracket level.
-
-A similar pattern exists in `resolver.rs`
-(`try_chained_array_access_with_candidates`) where candidates arrive
-as `String` even though callers already have `PhpType` values.
-
-**What to change:**
-
-1. In `resolve_rhs_array_access`, change `current_type` from `String`
-   to `PhpType`. Call `.shape_value_type()` / `.extract_value_type()`
-   directly and assign the result without stringifying. Only convert
-   to string at the end for the final `type_hint_to_classes` call.
-
-2. In `resolver.rs`, change `try_chained_array_access_with_candidates`
-   to accept `PhpType` candidates instead of `String`. Update the
-   callers (`resolve_target_classes_expr` ArrayAccess branch,
-   `resolve_property_type_hint` call site) to pass `PhpType` directly
-   instead of stringifying.
-
-**Files:** `src/completion/variable/rhs_resolution.rs`,
-`src/completion/resolver.rs`.
-
-**Part of:** T19 (structured type representation migration).
-
----
-
 ## T27. Keep substituted return types as `PhpType` in RHS resolution
 **Impact: High · Effort: Low**
 

src/completion/resolver.rs

@@ -801,7 +801,7 @@ pub(crate) fn resolve_target_classes_expr(
             if let SubjectExpr::CallExpr { callee, args_text } = base.as_ref() {
                 let call_raw = resolve_call_raw_return_type(callee, args_text, ctx);
                 if let Some(raw) = call_raw {
-                    let candidates = std::iter::once(raw);
+                    let candidates = std::iter::once(PhpType::parse(&raw));
                     if let Some(resolved) =
                         super::source::helpers::try_chained_array_access_with_candidates(
                             candidates,
@@ -834,7 +834,7 @@ pub(crate) fn resolve_target_classes_expr(
             // parameters like `array<string, IntCollection>` or
             // `Collection<int, Translation>` that would be lost if
             // we resolved through `type_hint_to_classes` first.
-            let property_raw_type = if let SubjectExpr::PropertyChain {
+            let property_raw_type: Option<PhpType> = if let SubjectExpr::PropertyChain {
                 base: prop_base,
                 property,
             } = base.as_ref()
@@ -843,18 +843,18 @@ pub(crate) fn resolve_target_classes_expr(
                     resolved_to_arcs(resolve_target_classes_expr(prop_base, access_kind, ctx));
                 owner_arcs.iter().find_map(|cls| {
                     crate::inheritance::resolve_property_type_hint(cls, property, class_loader)
-                        .map(|ty| ty.to_string())
                 })
             } else {
                 None
             };
 
-            let docblock_type = docblock::find_iterable_raw_type_in_source(
+            let docblock_type: Option<PhpType> = docblock::find_iterable_raw_type_in_source(
                 ctx.content,
                 ctx.cursor_offset as usize,
                 &base_var,
-            );
-            let ast_type = {
+            )
+            .map(|s| PhpType::parse(&s));
+            let ast_type: Option<PhpType> = {
                 let dummy_class;
                 let effective_class = match current_class {
                     Some(cc) => cc,
@@ -875,7 +875,7 @@ pub(crate) fn resolve_target_classes_expr(
                 if resolved.is_empty() {
                     None
                 } else {
-                    Some(ResolvedType::type_strings_joined(&resolved))
+                    Some(ResolvedType::types_joined(&resolved))
                 }
             };
 

src/completion/source/helpers.rs

@@ -17,7 +17,7 @@
 ///   `$fn = strlen(...)` or `$fn = $obj->method(...)`.
 /// - **`try_chained_array_access_with_candidates`** /
 ///   **`walk_array_segments_and_resolve`** — walk bracket segments on
-///   candidate raw type strings to resolve array access chains.
+///   candidate `PhpType` values to resolve array access chains.
 ///
 /// All functions in this module are free functions (not methods on
 /// `Backend`).  Cross-module dependencies that previously used `Self::`
@@ -485,23 +485,23 @@ pub(in crate::completion) fn extract_first_class_callable_return_type(
 /// Resolve a chained array access, trying each candidate raw type
 /// in order until one succeeds through the full segment walk.
 ///
-/// Each candidate raw type string is fed through
+/// Each candidate `PhpType` is fed through
 /// `walk_array_segments_and_resolve`.  The first that resolves
 /// through the segment walk and, if it produces a non-empty
 /// `ClassInfo` set, returned immediately.  Returns `None` when no
 /// candidate succeeds.
 pub(in crate::completion) fn try_chained_array_access_with_candidates<'a>(
-    candidates: impl Iterator<Item = String> + 'a,
+    candidates: impl Iterator<Item = PhpType> + 'a,
     segments: &[BracketSegment],
     current_class: Option<&ClassInfo>,
     all_classes: &[Arc<ClassInfo>],
     class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
 ) -> Option<Vec<ClassInfo>> {
     let current_class_name = current_class.map(|c| c.name.as_str()).unwrap_or("");
 
-    for raw_type in candidates {
+    for candidate in candidates {
         if let Some(result) = walk_array_segments_and_resolve(
-            &raw_type,
+            &candidate,
             segments,
             current_class_name,
             all_classes,
@@ -514,35 +514,38 @@ pub(in crate::completion) fn try_chained_array_access_with_candidates<'a>(
     None
 }
 
-/// Walk bracket segments on a raw type string, then resolve the
-/// resulting type to `ClassInfo`.
+/// Walk bracket segments on a `PhpType`, then resolve the resulting
+/// type to `ClassInfo`.
 ///
 /// Returns `Some(classes)` when the full segment chain resolves
 /// successfully, or `None` when a segment cannot be applied (e.g.
 /// the array shape does not contain the requested key).
 fn walk_array_segments_and_resolve(
-    raw_type: &str,
+    base_type: &PhpType,
     segments: &[BracketSegment],
     current_class_name: &str,
     all_classes: &[Arc<ClassInfo>],
     class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
 ) -> Option<Vec<ClassInfo>> {
-    let mut current_str = raw_type.to_string();
-
     // Expand type aliases before walking segments.  The raw type may
     // be an alias name like `UserData` that resolves to
     // `array{name: string, pen: Pen}`.  Without expansion the
     // segment walk would fail to extract shape values.
-    if let Some(expanded) = crate::completion::type_resolution::resolve_type_alias(
-        &current_str,
-        current_class_name,
-        all_classes,
-        class_loader,
-    ) {
-        current_str = expanded;
-    }
-
-    let mut current = PhpType::parse(&current_str);
+    let mut current = if let PhpType::Named(_) = base_type {
+        let name_str = base_type.to_string();
+        if let Some(expanded) = crate::completion::type_resolution::resolve_type_alias(
+            &name_str,
+            current_class_name,
+            all_classes,
+            class_loader,
+        ) {
+            PhpType::parse(&expanded)
+        } else {
+            base_type.clone()
+        }
+    } else {
+        base_type.clone()
+    };
 
     for seg in segments {
         // Try pure-type extraction first (array shapes, generics).

src/completion/variable/rhs_resolution.rs

@@ -840,108 +840,118 @@ fn resolve_rhs_array_access<'b>(
     // Resolve the base expression's raw type string.
     // For bare variables (`$var['key']`), use docblock or assignment scanning.
     // For property chains (`$obj->prop['key']`), resolve the property type.
-    let raw_type = if let Expression::Variable(Variable::Direct(base_dv)) = current_expr {
-        let base_var = base_dv.name.to_string();
-        docblock::find_iterable_raw_type_in_source(ctx.content, access_offset, &base_var).or_else(
-            || {
-                let resolved = super::resolution::resolve_variable_types(
-                    &base_var,
-                    ctx.current_class,
-                    ctx.all_classes,
-                    ctx.content,
-                    access_offset as u32,
-                    ctx.class_loader,
-                    Loaders::with_function(ctx.function_loader()),
-                );
-                if resolved.is_empty() {
-                    None
-                } else {
-                    Some(ResolvedType::type_strings_joined(&resolved))
-                }
-            },
-        )
-    } else {
-        // Non-variable base (e.g. property access `$obj->prop['key']`,
-        // method call `$obj->getItems()['key']`, etc.).
-        // Resolve the base expression to get its type string.
-        let base_resolved = resolve_rhs_expression(current_expr, ctx);
-        if base_resolved.is_empty() {
-            None
+    let raw_type: Option<PhpType> =
+        if let Expression::Variable(Variable::Direct(base_dv)) = current_expr {
+            let base_var = base_dv.name.to_string();
+            docblock::find_iterable_raw_type_in_source(ctx.content, access_offset, &base_var)
+                .map(|s| PhpType::parse(&s))
+                .or_else(|| {
+                    let resolved = super::resolution::resolve_variable_types(
+                        &base_var,
+                        ctx.current_class,
+                        ctx.all_classes,
+                        ctx.content,
+                        access_offset as u32,
+                        ctx.class_loader,
+                        Loaders::with_function(ctx.function_loader()),
+                    );
+                    if resolved.is_empty() {
+                        None
+                    } else {
+                        Some(ResolvedType::types_joined(&resolved))
+                    }
+                })
         } else {
-            Some(ResolvedType::type_strings_joined(&base_resolved))
-        }
-    };
+            // Non-variable base (e.g. property access `$obj->prop['key']`,
+            // method call `$obj->getItems()['key']`, etc.).
+            // Resolve the base expression to get its type.
+            let base_resolved = resolve_rhs_expression(current_expr, ctx);
+            if base_resolved.is_empty() {
+                None
+            } else {
+                Some(ResolvedType::types_joined(&base_resolved))
+            }
+        };
 
-    let Some(mut current_type) = raw_type else {
+    let Some(mut current) = raw_type else {
         return vec![];
     };
 
     // Expand type aliases so that shape/generic extraction can see the
     // underlying type (e.g. a `@phpstan-type` alias).
+    let current_str = current.to_string();
     if let Some(expanded) = crate::completion::type_resolution::resolve_type_alias(
-        &current_type,
+        &current_str,
         &ctx.current_class.name,
         ctx.all_classes,
         ctx.class_loader,
     ) {
-        current_type = expanded;
+        current = PhpType::parse(&expanded);
     }
 
     // Walk each bracket segment, narrowing the type at each step.
     for seg in &segments {
-        let parsed = crate::php_type::PhpType::parse(&current_type);
-
         // Try pure-type extraction first (array shapes, generics).
         let extracted = match seg {
-            ArrayBracketSegment::StringKey(key) => parsed
+            ArrayBracketSegment::StringKey(key) => current
                 .shape_value_type(key)
-                .map(|t| t.to_string())
-                .or_else(|| parsed.extract_value_type(true).map(|t| t.to_string())),
-            ArrayBracketSegment::ElementAccess => {
-                parsed.extract_value_type(true).map(|t| t.to_string())
-            }
+                .cloned()
+                .or_else(|| current.extract_value_type(true).cloned()),
+            ArrayBracketSegment::ElementAccess => current.extract_value_type(true).cloned(),
         };
 
         if let Some(element) = extracted {
-            current_type = element;
-            continue;
+            current = element;
+        } else {
+            // Fallback: when the current 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 `$obj->prop['key']` where `prop` is a collection
+            // class like `OpeningHours extends DataCollection<string, Day>`.
+            let type_str = current.to_string();
+            let class_element = crate::completion::type_resolution::type_hint_to_classes(
+                &type_str,
+                &ctx.current_class.name,
+                ctx.all_classes,
+                ctx.class_loader,
+            )
+            .into_iter()
+            .find_map(|cls| {
+                let merged = crate::virtual_members::resolve_class_fully(&cls, ctx.class_loader);
+                super::foreach_resolution::extract_iterable_element_type_from_class(
+                    &merged,
+                    ctx.class_loader,
+                )
+            });
+
+            if let Some(element) = class_element {
+                current = PhpType::parse(&element);
+            } else {
+                return vec![];
+            }
         }
 
-        // Fallback: when the current 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 `$obj->prop['key']` where `prop` is a collection
-        // class like `OpeningHours extends DataCollection<string, Day>`.
-        let class_element = crate::completion::type_resolution::type_hint_to_classes(
-            &current_type,
+        // After each segment, the resulting type might itself be an
+        // alias (e.g. a shape value defined as another alias).
+        let seg_str = current.to_string();
+        if let Some(expanded) = crate::completion::type_resolution::resolve_type_alias(
+            &seg_str,
             &ctx.current_class.name,
             ctx.all_classes,
             ctx.class_loader,
-        )
-        .into_iter()
-        .find_map(|cls| {
-            let merged = crate::virtual_members::resolve_class_fully(&cls, ctx.class_loader);
-            super::foreach_resolution::extract_iterable_element_type_from_class(
-                &merged,
-                ctx.class_loader,
-            )
-        });
-
-        if let Some(element) = class_element {
-            current_type = element;
-        } else {
-            return vec![];
+        ) {
+            current = PhpType::parse(&expanded);
         }
     }
 
     ResolvedType::from_classes_with_hint(
-        crate::completion::type_resolution::type_hint_to_classes(
-            &current_type,
+        crate::completion::type_resolution::type_hint_to_classes_typed(
+            &current,
             &ctx.current_class.name,
             ctx.all_classes,
             ctx.class_loader,
         ),
-        PhpType::parse(&current_type),
+        current,
     )
 }