Replace type_strings_joined with types_joined and to_string

Author: AJenbo

docs/todo.md

@@ -23,7 +23,6 @@ within the same impact tier.
 
 | #   | Item                                                                                                                                                      | Impact | Effort     |
 | --- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ---------- |
-| 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**                                                                                                                                         |        |            |
 
@@ -97,10 +96,10 @@ 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 T29–T30)          | 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      |
 | 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            |
+| 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            |
 | T13  | [Closure variables lose callable signature detail](todo/type-inference.md#t13-closure-variables-lose-callable-signature-detail)                                             | Low-Medium  | Medium         |
 | T4   | Non-empty-\* type narrowing and propagation                                                                                                                                 | Low         | Low            |
 | T5   | Fiber type resolution                                                                                                                                                       | Low         | Low            |

docs/todo/type-inference.md

@@ -350,10 +350,12 @@ function measure(Countable&Serializable $thing): void {
 // `Countable&Serializable $thing`.
 ```
 
-**No longer blocked.** `PhpType::Intersection` already exists.
-`ResolvedType::types_joined()` returns a `PhpType` that preserves
-intersection structure. This will be naturally fixed when T29 migrates
-all `type_strings_joined` call sites to `types_joined`.
+**Remaining work.** `PhpType::Intersection` already exists and
+`ResolvedType::types_joined()` returns a structured `PhpType`, but
+the resolution pipeline still produces separate `ResolvedType` entries
+for each part of an intersection. The fix is to make the pipeline emit
+a single `ResolvedType` with `PhpType::Intersection` when the source
+type is an intersection.
 
 **After fixing:** verify that extract function docblock generation
 preserves intersection types in both the native hint and the `@param`
@@ -594,52 +596,6 @@ progresses.
 
 ---
 
-## T29. Migrate `type_strings_joined` call sites to `types_joined`
-**Impact: Medium · Effort: Low**
-
-`ResolvedType::types_joined()` returns a structured `PhpType` and was
-created as the replacement for `type_strings_joined()`, which joins
-types into a `String` with `|` separators. The string variant is still
-called in ~12 places across the resolution pipeline:
-
-- `resolver.rs` (2 call sites)
-- `resolution.rs` / variable resolution (3 call sites)
-- `rhs_resolution.rs` (1 call site)
-- `call_resolution.rs` (1 call site)
-- `foreach_resolution.rs` (1 call site)
-- `array_shape.rs` (1 call site)
-- `fix_return_type.rs` (1 call site)
-
-Each produces a `String` that is later re-parsed by downstream
-consumers.
-
-**What to change:**
-
-1. At each call site, replace `type_strings_joined` with
-   `types_joined` and thread the resulting `PhpType` through to the
-   consumer.
-
-2. Where the consumer is a function that accepts `&str` (e.g.
-   `check_unresolvable_class_name`), change that function to accept
-   `&PhpType`.
-
-3. Once all call sites are migrated, remove `type_strings_joined`
-   entirely (or mark it `#[deprecated]` first if external tests use
-   it).
-
-**Files:** `src/completion/resolver.rs`,
-`src/completion/variable/resolution.rs`,
-`src/completion/variable/rhs_resolution.rs`,
-`src/completion/call_resolution.rs`,
-`src/completion/variable/foreach_resolution.rs`,
-`src/completion/array_shape.rs`,
-`src/code_actions/phpstan/fix_return_type.rs`,
-`src/types.rs`.
-
-**Part of:** T19.
-
----
-
 ## T30. Thread `PhpType` through remaining string-based API boundaries
 **Impact: Medium · Effort: Medium**
 

src/code_actions/phpstan/fix_return_type.rs

@@ -465,7 +465,7 @@ pub(crate) fn infer_return_type(
                     class_loader,
                     Loaders::with_function(function_loader),
                 );
-                let type_str = ResolvedType::type_strings_joined(&results);
+                let type_str = ResolvedType::types_joined(&results).to_string();
                 if !type_str.is_empty() {
                     return_types.push(type_str);
                     continue;

src/completion/array_shape.rs

@@ -547,7 +547,7 @@ impl Backend {
         if resolved.is_empty() {
             None
         } else {
-            Some(ResolvedType::type_strings_joined(&resolved))
+            Some(ResolvedType::types_joined(&resolved).to_string())
         }
     }
 }

src/completion/call_resolution.rs

@@ -1402,7 +1402,7 @@ impl Backend {
                 Loaders::with_function(ctx.function_loader),
             );
             if !resolved.is_empty() {
-                return Some(ResolvedType::type_strings_joined(&resolved));
+                return Some(ResolvedType::types_joined(&resolved).to_string());
             }
             return None;
         }

src/completion/resolver.rs

@@ -1137,8 +1137,7 @@ pub(crate) fn resolve_subject_outcome(
         }
 
         // Non-scalar, non-class type — check for unresolvable class.
-        let raw_type = ResolvedType::type_strings_joined(&resolved);
-        if let Some(unresolved) = check_unresolvable_class_name(&raw_type, ctx.class_loader) {
+        if let Some(unresolved) = check_unresolvable_class_name(&joined, ctx.class_loader) {
             return SubjectOutcome::UnresolvableClass(unresolved);
         }
         return SubjectOutcome::Untyped;
@@ -1161,7 +1160,8 @@ pub(crate) fn resolve_subject_outcome(
             && let Some(fl) = ctx.function_loader
             && let Some(func_info) = fl(fn_name.as_str())
             && let Some(raw_type) = func_info.return_type_str()
-            && let Some(unresolved) = check_unresolvable_class_name(&raw_type, ctx.class_loader)
+            && let Some(unresolved) =
+                check_unresolvable_class_name(&PhpType::parse(&raw_type), ctx.class_loader)
         {
             return SubjectOutcome::UnresolvableClass(unresolved);
         }
@@ -1197,7 +1197,8 @@ pub(crate) fn resolve_subject_outcome(
             ctx.class_loader,
             Loaders::with_function(ctx.function_loader),
         )
-        && let Some(unresolved) = check_unresolvable_class_name(&raw_type, ctx.class_loader)
+        && let Some(unresolved) =
+            check_unresolvable_class_name(&PhpType::parse(&raw_type), ctx.class_loader)
     {
         return SubjectOutcome::UnresolvableClass(unresolved);
     }
@@ -1289,15 +1290,14 @@ fn resolve_call_scalar_return(
 /// find it.  Returns `None` for scalars, unions, shapes, and types
 /// that resolve successfully.
 fn check_unresolvable_class_name(
-    raw_type: &str,
+    raw_type: &PhpType,
     class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
 ) -> Option<String> {
-    let parsed = PhpType::parse(raw_type);
-    if parsed.all_members_scalar() {
+    if raw_type.all_members_scalar() {
         return None;
     }
 
-    let effective = parsed.non_null_type().unwrap_or_else(|| parsed.clone());
+    let effective = raw_type.non_null_type().unwrap_or_else(|| raw_type.clone());
     let base = effective.base_name()?;
 
     if class_loader(base).is_none() {

src/completion/variable/foreach_resolution.rs

@@ -24,7 +24,7 @@ use crate::completion::resolver::{Loaders, VarResolutionCtx};
 
 /// Resolve an expression's type string via the unified pipeline.
 ///
-/// Wraps `resolve_rhs_expression` + `type_strings_joined` into a single
+/// Wraps `resolve_rhs_expression` + `types_joined` into a single
 /// `Option<String>` suitable for callers that previously used
 /// `extract_rhs_iterable_raw_type`.  Returns `None` when the unified
 /// pipeline produces no results or an empty type string.
@@ -36,7 +36,7 @@ pub(in crate::completion) fn resolve_expression_type_string<'b>(
     if resolved.is_empty() {
         return None;
     }
-    let ts = ResolvedType::type_strings_joined(&resolved);
+    let ts = ResolvedType::types_joined(&resolved).to_string();
     if ts.is_empty() { None } else { Some(ts) }
 }
 

src/completion/variable/resolution.rs

@@ -3106,7 +3106,7 @@ pub(in crate::completion) fn check_expression_for_assignment<'b>(
                 let resolved =
                     super::rhs_resolution::resolve_rhs_expression(assignment.rhs, &rhs_ctx);
                 let value_type = if !resolved.is_empty() {
-                    ResolvedType::type_strings_joined(&resolved)
+                    ResolvedType::types_joined(&resolved).to_string()
                 } else {
                     "mixed".to_string()
                 };
@@ -3135,7 +3135,7 @@ pub(in crate::completion) fn check_expression_for_assignment<'b>(
                 let resolved =
                     super::rhs_resolution::resolve_rhs_expression(assignment.rhs, &rhs_ctx);
                 let value_type = if !resolved.is_empty() {
-                    ResolvedType::type_strings_joined(&resolved)
+                    ResolvedType::types_joined(&resolved).to_string()
                 } else {
                     "mixed".to_string()
                 };
@@ -3175,7 +3175,7 @@ pub(in crate::completion) fn check_expression_for_assignment<'b>(
             let rhs_ctx = ctx.with_cursor_offset(assignment.span().start.offset);
             let resolved = super::rhs_resolution::resolve_rhs_expression(assignment.rhs, &rhs_ctx);
             let value_type = if !resolved.is_empty() {
-                ResolvedType::type_strings_joined(&resolved)
+                ResolvedType::types_joined(&resolved).to_string()
             } else {
                 "mixed".to_string()
             };
@@ -3428,7 +3428,7 @@ fn infer_array_key_type(index: &Expression<'_>, ctx: &VarResolutionCtx<'_>) -> S
     // Resolve the expression type through the standard pipeline.
     let resolved = super::rhs_resolution::resolve_rhs_expression(index, ctx);
     if !resolved.is_empty() {
-        let joined = ResolvedType::type_strings_joined(&resolved);
+        let joined = ResolvedType::types_joined(&resolved);
         let key_str = joined.to_string();
         // Normalise the resolved type to a valid array key type.
         // PHP array keys are always int or string; bool and null are

src/completion/variable/resolution_tests.rs

@@ -408,7 +408,7 @@ function test() {
     );
 
     assert!(!results.is_empty(), "Should resolve $data to a type");
-    let ts = ResolvedType::type_strings_joined(&results);
+    let ts = ResolvedType::types_joined(&results).to_string();
     assert!(
         ts.contains("name: string"),
         "Shape should contain 'name: string', got: {ts}"
@@ -443,7 +443,7 @@ function test() {
     );
 
     assert!(!results.is_empty(), "Should resolve $config to a type");
-    let ts = ResolvedType::type_strings_joined(&results);
+    let ts = ResolvedType::types_joined(&results).to_string();
     // The base array{host: string} should be merged with the new key.
     assert!(
         ts.contains("port: int"),
@@ -475,7 +475,7 @@ function test() {
     );
 
     assert!(!results.is_empty(), "Should resolve $data to a type");
-    let ts = ResolvedType::type_strings_joined(&results);
+    let ts = ResolvedType::types_joined(&results).to_string();
     assert!(
         ts.contains("value: int"),
         "Shape key 'value' should be overridden to int, got: {ts}"
@@ -523,7 +523,7 @@ function test() {
     );
 
     assert!(!results.is_empty(), "Should resolve $items to a type");
-    let ts = ResolvedType::type_strings_joined(&results);
+    let ts = ResolvedType::types_joined(&results).to_string();
     assert!(
         ts.contains("User"),
         "List element type should contain User, got: {ts}"
@@ -558,7 +558,7 @@ function test() {
     );
 
     assert!(!results.is_empty(), "Should resolve $items to a type");
-    let ts = ResolvedType::type_strings_joined(&results);
+    let ts = ResolvedType::types_joined(&results).to_string();
     assert!(
         ts.contains("string") && ts.contains("int"),
         "List should contain string|int union, got: {ts}"
@@ -589,7 +589,7 @@ function test() {
     );
 
     assert!(!results.is_empty(), "Should resolve $items to a type");
-    let ts = ResolvedType::type_strings_joined(&results);
+    let ts = ResolvedType::types_joined(&results).to_string();
     assert_eq!(
         ts, "list<string>",
         "Duplicate pushes of same type should not duplicate, got: {ts}"
@@ -622,7 +622,7 @@ function test() {
     );
 
     assert!(!results.is_empty(), "Should resolve $x to a type");
-    let ts = ResolvedType::type_strings_joined(&results);
+    let ts = ResolvedType::types_joined(&results).to_string();
     assert_eq!(
         ts, "list<string>",
         "Reassignment should reset; only 'string' push should remain, got: {ts}"
@@ -658,7 +658,7 @@ function test() {
     let ts = if results.is_empty() {
         "array".to_string()
     } else {
-        ResolvedType::type_strings_joined(&results)
+        ResolvedType::types_joined(&results).to_string()
     };
     assert!(
         !ts.contains('{'),

src/completion/variable/rhs_resolution.rs

@@ -1153,7 +1153,7 @@ fn resolve_arg_variable_raw_type(
     if resolved.is_empty() {
         None
     } else {
-        Some(ResolvedType::type_strings_joined(&resolved))
+        Some(ResolvedType::types_joined(&resolved).to_string())
     }
 }