Accept &[PhpType] in generic-arg resolution

Author: AJenbo

docs/todo.md

@@ -24,7 +24,6 @@ within the same impact tier.
 | #   | Item                                                                                                                                                      | Impact | Effort     |
 | --- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ---------- |
 | 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        |
 | 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**                                                                                                                                         |        |            |

docs/todo/type-inference.md

@@ -627,40 +627,6 @@ This runs for every method/static call in an assignment RHS.
 
 ---
 
-## T28. Accept `&[PhpType]` in generic-arg resolution
-**Impact: Medium · Effort: Medium**
-
-In `completion/types/resolution.rs`, when handling
-`PhpType::Generic(name, args)`, the args are converted back to strings
-so they can be passed to `build_generic_subs()` and
-`apply_generic_args()` in `inheritance.rs`, which accept `&[&str]`.
-Those functions then re-parse each arg via `PhpType::parse()`.
-
-The same pattern appears in `resolve_named_type()` where template
-bound lookups stringify a `PhpType` to re-enter the string-based
-resolution path (~L425, L428).
-
-**What to change:**
-
-1. Change `build_generic_subs()` and `apply_generic_args()` in
-   `inheritance.rs` to accept `&[PhpType]` instead of `&[&str]`.
-   Remove the internal `PhpType::parse()` calls.
-
-2. Update `type_hint_to_classes_typed_depth()` to pass the generic
-   args as `&[PhpType]` directly instead of stringifying them.
-
-3. Update `resolve_named_type()` to use `PhpType` values for template
-   bound lookups instead of round-tripping through strings.
-
-4. Remove the `apply_substitution()` string shim in `inheritance.rs`
-   (~L972-1000) once all callers work with `PhpType` directly.
-
-**Files:** `src/inheritance.rs`, `src/completion/types/resolution.rs`.
-
-**Part of:** T19.
-
----
-
 ## T29. Migrate `type_strings_joined` call sites to `types_joined`
 **Impact: Medium · Effort: Low**
 

src/completion/types/resolution.rs

@@ -224,17 +224,14 @@ fn type_hint_to_classes_typed_depth(
         ),
 
         // ── Generic type ───────────────────────────────────────────
-        PhpType::Generic(name, args) => {
-            let arg_strings: Vec<String> = args.iter().map(|a| a.to_string()).collect();
-            resolve_named_type(
-                name,
-                &arg_strings,
-                owning_class_name,
-                all_classes,
-                class_loader,
-                depth,
-            )
-        }
+        PhpType::Generic(name, args) => resolve_named_type(
+            name,
+            args,
+            owning_class_name,
+            all_classes,
+            class_loader,
+            depth,
+        ),
 
         // ── Array slice (T[]) ──────────────────────────────────────
         // Not a class type itself; skip.
@@ -260,7 +257,7 @@ fn type_hint_to_classes_typed_depth(
 /// substitution.
 fn resolve_named_type(
     name: &str,
-    generic_arg_strings: &[String],
+    generic_args: &[PhpType],
     owning_class_name: &str,
     all_classes: &[Arc<ClassInfo>],
     class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
@@ -280,11 +277,10 @@ fn resolve_named_type(
 
     // ── self / static / $this ──────────────────────────────────────
     if matches!(name, "self" | "static" | "$this") {
-        if !generic_arg_strings.is_empty() {
+        if !generic_args.is_empty() {
             // `self<RuleError>` → rewrite to `OwningClass<RuleError>`.
-            let args_str = generic_arg_strings.join(", ");
-            let rewritten = format!("{}<{}>", owning_class_name, args_str);
-            return type_hint_to_classes_depth(
+            let rewritten = PhpType::Generic(owning_class_name.to_string(), generic_args.to_vec());
+            return type_hint_to_classes_typed_depth(
                 &rewritten,
                 owning_class_name,
                 all_classes,
@@ -321,18 +317,26 @@ fn resolve_named_type(
     }
 
     // ── Resolve static/self/$this inside generic arguments ────────
-    let resolved_generic_args: Vec<String> = generic_arg_strings
-        .iter()
-        .map(|arg| {
-            let trimmed = arg.trim();
-            if trimmed == "static" || trimmed == "self" || trimmed == "$this" {
-                owning_class_name.to_string()
-            } else {
-                trimmed.to_string()
-            }
-        })
-        .collect();
-    let generic_args: Vec<&str> = resolved_generic_args.iter().map(|s| s.as_str()).collect();
+    let resolved_generic_args: Vec<PhpType>;
+    let generic_args: &[PhpType] = if generic_args.iter().any(
+        |a| matches!(a, PhpType::Named(n) if matches!(n.as_str(), "static" | "self" | "$this")),
+    ) {
+        resolved_generic_args = generic_args
+            .iter()
+            .map(|arg| {
+                if let PhpType::Named(n) = arg
+                    && matches!(n.as_str(), "static" | "self" | "$this")
+                {
+                    PhpType::Named(owning_class_name.to_string())
+                } else {
+                    arg.clone()
+                }
+            })
+            .collect();
+        &resolved_generic_args
+    } else {
+        generic_args
+    };
 
     let short = short_name(name);
 
@@ -347,7 +351,7 @@ fn resolve_named_type(
             let cls = laravel::try_swap_custom_collection(
                 cls,
                 name,
-                &generic_args,
+                generic_args,
                 all_classes,
                 class_loader,
             );
@@ -360,7 +364,7 @@ fn resolve_named_type(
                 } else {
                     virtual_members::resolve_class_fully(&cls, class_loader)
                 };
-                let mut result = apply_generic_args(&resolved, &generic_args);
+                let mut result = apply_generic_args(&resolved, generic_args);
 
                 // ── Template-param mixin resolution ────────────────
                 // When a class declares `@mixin TParam` where `TParam`
@@ -369,7 +373,7 @@ fn resolve_named_type(
                 // is not yet known.  Now that generic args are concrete,
                 // resolve those mixins and merge their members.
                 if cls.mixins.iter().any(|m| cls.template_params.contains(m)) {
-                    let subs = build_generic_subs(&cls, &generic_args);
+                    let subs = build_generic_subs(&cls, generic_args);
                     if !subs.is_empty() {
                         let mixin_members = virtual_members::phpdoc::resolve_template_param_mixins(
                             &cls,
@@ -387,7 +391,7 @@ fn resolve_named_type(
                     &mut result,
                     &cls,
                     name,
-                    &generic_args,
+                    generic_args,
                     class_loader,
                 );
 
@@ -401,7 +405,7 @@ fn resolve_named_type(
                 laravel::try_inject_mixin_builder_scopes(
                     &mut result,
                     &cls,
-                    &generic_args,
+                    generic_args,
                     class_loader,
                 );
 
@@ -425,9 +429,8 @@ fn resolve_named_type(
                 && owner.template_params.contains(&short.to_string())
                 && let Some(bound) = owner.template_param_bounds.get(short)
             {
-                let bound_str = bound.to_string();
-                return type_hint_to_classes_depth(
-                    &bound_str,
+                return type_hint_to_classes_typed_depth(
+                    bound,
                     owning_class_name,
                     all_classes,
                     class_loader,

src/completion/variable/rhs_resolution.rs

@@ -456,17 +456,11 @@ fn resolve_rhs_instantiation(
                     let rctx = ctx.as_resolution_ctx();
                     let subs = build_constructor_template_subs(cls, ctor, &text_args, &rctx, ctx);
                     if !subs.is_empty() {
-                        let type_arg_strings: Vec<String> = cls
+                        let type_args: Vec<PhpType> = cls
                             .template_params
                             .iter()
-                            .map(|p| {
-                                subs.get(p)
-                                    .map(|s| s.to_string())
-                                    .unwrap_or_else(|| p.clone())
-                            })
+                            .map(|p| subs.get(p).cloned().unwrap_or_else(|| PhpType::parse(p)))
                             .collect();
-                        let type_args: Vec<&str> =
-                            type_arg_strings.iter().map(|s| s.as_str()).collect();
                         let resolved =
                             crate::virtual_members::resolve_class_fully(cls, ctx.class_loader);
                         let mut substituted =

src/inheritance.rs

@@ -1009,7 +1009,7 @@ pub(crate) fn apply_substitution<'a>(
 /// `template_params` or `type_args` is empty).
 pub(crate) fn build_generic_subs(
     class: &ClassInfo,
-    type_args: &[&str],
+    type_args: &[PhpType],
 ) -> HashMap<String, PhpType> {
     if class.template_params.is_empty() || type_args.is_empty() {
         return HashMap::new();
@@ -1044,7 +1044,7 @@ pub(crate) fn build_generic_subs(
             continue;
         }
         if let Some(arg) = type_args.get(i - offset) {
-            subs.insert(param_name.clone(), PhpType::parse(arg));
+            subs.insert(param_name.clone(), arg.clone());
         }
     }
 
@@ -1064,10 +1064,10 @@ pub(crate) fn build_generic_subs(
 /// # Example
 ///
 /// Given a `Collection` class with `@template TKey` and `@template TValue`,
-/// calling `apply_generic_args(&collection_class, &["int", "User"])` will
-/// substitute every occurrence of `TKey` with `int` and `TValue` with `User`
+/// calling `apply_generic_args(&collection_class, &[PhpType::parse("int"), PhpType::parse("User")])`
+/// will substitute every occurrence of `TKey` with `int` and `TValue` with `User`
 /// in the class's methods and properties.
-pub(crate) fn apply_generic_args(class: &ClassInfo, type_args: &[&str]) -> ClassInfo {
+pub(crate) fn apply_generic_args(class: &ClassInfo, type_args: &[PhpType]) -> ClassInfo {
     let subs = build_generic_subs(class, type_args);
 
     if subs.is_empty() {

src/inheritance_tests.rs

@@ -453,7 +453,7 @@ fn test_apply_generic_args_right_aligns_single_arg_for_collection() {
         ..ClassInfo::default()
     };
 
-    let result = apply_generic_args(&collection, &["SectionTranslation"]);
+    let result = apply_generic_args(&collection, &[PhpType::parse("SectionTranslation")]);
 
     let first = result
         .methods
@@ -478,7 +478,10 @@ fn test_apply_generic_args_no_right_align_when_all_args_provided() {
         ..ClassInfo::default()
     };
 
-    let result = apply_generic_args(&collection, &["int", "User"]);
+    let result = apply_generic_args(
+        &collection,
+        &[PhpType::parse("int"), PhpType::parse("User")],
+    );
 
     let first = result
         .methods
@@ -499,7 +502,7 @@ fn test_apply_generic_args_no_right_align_without_key_bound() {
         ..ClassInfo::default()
     };
 
-    let result = apply_generic_args(&cls, &["Foo"]);
+    let result = apply_generic_args(&cls, &[PhpType::parse("Foo")]);
 
     let first = result
         .methods
@@ -524,7 +527,7 @@ fn test_apply_generic_args_right_align_with_int_bound() {
         ..ClassInfo::default()
     };
 
-    let result = apply_generic_args(&cls, &["Product"]);
+    let result = apply_generic_args(&cls, &[PhpType::parse("Product")]);
 
     let get = result
         .methods

src/virtual_members/laravel/mod.rs

@@ -113,6 +113,7 @@ use casts::cast_type_to_php_type;
 pub use factory::LaravelFactoryProvider;
 pub(crate) use factory::{factory_to_model_fqn, model_to_factory_fqn};
 
+use crate::php_type::PhpType;
 use crate::types::{ClassInfo, PropertyInfo};
 
 use super::{ResolvedClassCache, VirtualMemberProvider, VirtualMembers};
@@ -151,7 +152,7 @@ pub const ELOQUENT_BUILDER_FQN: &str = "Illuminate\\Database\\Eloquent\\Builder"
 pub(crate) fn try_swap_custom_collection(
     cls: ClassInfo,
     base_fqn: &str,
-    generic_args: &[&str],
+    generic_args: &[PhpType],
     all_classes: &[Arc<ClassInfo>],
     class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
 ) -> ClassInfo {
@@ -160,10 +161,10 @@ pub(crate) fn try_swap_custom_collection(
     }
 
     // The last generic arg is typically the model type.
-    let model_arg = generic_args.last().unwrap();
-    let model_class = find_class_in(all_classes, model_arg)
+    let model_arg = generic_args.last().unwrap().to_string();
+    let model_class = find_class_in(all_classes, &model_arg)
         .cloned()
-        .or_else(|| class_loader(model_arg).map(Arc::unwrap_or_clone));
+        .or_else(|| class_loader(&model_arg).map(Arc::unwrap_or_clone));
 
     if let Some(ref mc) = model_class
         && let Some(coll_name) = mc.laravel().and_then(|l| l.custom_collection.as_ref())
@@ -200,17 +201,17 @@ pub(crate) fn try_inject_builder_scopes(
     result: &mut ClassInfo,
     raw_cls: &ClassInfo,
     base_fqn: &str,
-    generic_args: &[&str],
+    generic_args: &[PhpType],
     class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
 ) {
     if !is_eloquent_builder_fqn(base_fqn, raw_cls) || generic_args.is_empty() {
         return;
     }
 
     // The first (or only) generic arg is the model type.
-    let model_arg = generic_args.first().unwrap();
+    let model_arg = generic_args.first().unwrap().to_string();
 
-    inject_scopes_and_model_methods(result, model_arg, class_loader);
+    inject_scopes_and_model_methods(result, &model_arg, class_loader);
 }
 
 /// Inject scope methods and model virtual methods onto a class that has
@@ -232,12 +233,11 @@ pub(crate) fn try_inject_builder_scopes(
 pub(crate) fn try_inject_mixin_builder_scopes(
     result: &mut ClassInfo,
     raw_cls: &ClassInfo,
-    generic_args: &[&str],
+    generic_args: &[PhpType],
     class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
 ) {
     use std::collections::HashMap;
 
-    use crate::php_type::PhpType;
     use crate::types::MAX_INHERITANCE_DEPTH;
     use crate::util::short_name;
 
@@ -252,7 +252,7 @@ pub(crate) fn try_inject_mixin_builder_scopes(
     let mut root_subs: HashMap<String, PhpType> = HashMap::new();
     for (i, param_name) in raw_cls.template_params.iter().enumerate() {
         if let Some(arg) = generic_args.get(i) {
-            root_subs.insert(param_name.clone(), PhpType::parse(arg));
+            root_subs.insert(param_name.clone(), arg.clone());
         }
     }