Refactor PhpType usage to prefer structured types over strings

Author: AJenbo

docs/todo.md

@@ -26,6 +26,7 @@ within the same impact tier.
 | #    | Item                                                                                                                                                            | Impact     | Effort |
 | ---- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | ------ |
 
+
 |      | **Release 0.7.0**                                                                                                                                               |            |        |
 
 ## Sprint 5 — Polish for office adoption

docs/todo/performance.md

@@ -804,10 +804,10 @@ so this fast-path would apply to the majority of checks.
    flowing through `mago-names` / `mago-syntax` are already atoms.
    Using `Atom` or `Arc<str>` for class names in PHPantom's own
    data structures would reduce memory and make the subtype cache
-   keys cheaper to hash and compare. This becomes a natural
-   consequence of T19 (structured type representation) since each
-   type node would store an interned name rather than an owned
-   `String`.
+   keys cheaper to hash and compare. Now that `PhpType` is the
+   structured type representation throughout the codebase, interning
+   the name strings inside each `PhpType` node (replacing owned
+   `String` with `Atom` or `Arc<str>`) is a natural next step.
 
 ---
 

docs/todo/type-inference.md

@@ -411,8 +411,9 @@ NonEmptyCountable.
 `Psalm/Storage/Assertion/`. PHPStan's `TypeSpecifier` returns
 `SpecifiedTypes` with dual sure/sureNot maps.
 
-**Depends on:** T19 (structured types make reconciliation much
-simpler, but basic reconciliation can work with strings too).
+**Depends on:** The structured type representation (`PhpType`) has
+landed, which makes reconciliation much simpler than working with
+raw strings.
 
 ---
 
@@ -519,8 +520,8 @@ This eliminates the depth limit entirely and makes the resolution
 cost proportional to the number of statements before the cursor,
 not the depth of the assignment chain.
 
-**Depends on:** T19 (structured type representation) should land
-first so the scope map stores `PhpType` values instead of strings.
+**Note:** The structured type representation (`PhpType`) has landed,
+so the scope map can store `PhpType` values directly.
 
 **Migration path:** Start with a parallel implementation behind a
 feature flag. The existing backward-scanning resolver stays as a

src/code_actions/phpstan/remove_unused_return_type.rs

@@ -72,11 +72,10 @@ fn extract_unused_type(message: &str) -> Option<&str> {
 /// Also handles `?Type` (nullable shorthand): `?string` with unused
 /// `null` becomes `string`, and `?string` with unused `string` becomes
 /// `null`.
-fn remove_type_from_union(full_type: &str, unused_type: &str) -> Option<PhpType> {
-    let parsed = PhpType::parse(full_type);
+fn remove_type_from_union(full_type: &PhpType, unused_type: &str) -> Option<PhpType> {
     let unused_parsed = PhpType::parse(unused_type);
 
-    match &parsed {
+    match full_type {
         PhpType::Union(members) => {
             let remaining: Vec<&PhpType> = members
                 .iter()
@@ -411,7 +410,7 @@ impl Backend {
             let native_type = extract_native_return_type(&lines, paren_line, paren_col, brace_line);
             let has_native_match = native_type
                 .as_ref()
-                .is_some_and(|t| remove_type_from_union(&t.to_string(), unused_type).is_some());
+                .is_some_and(|t| remove_type_from_union(t, unused_type).is_some());
 
             // Check the docblock @return tag.
             let func_line = find_func_keyword_line(&lines, paren_line).unwrap_or(diag_line);
@@ -427,7 +426,7 @@ impl Backend {
             };
             let has_doc_match = doc_return_type
                 .as_ref()
-                .is_some_and(|t| remove_type_from_union(&t.to_string(), unused_type).is_some());
+                .is_some_and(|t| remove_type_from_union(t, unused_type).is_some());
 
             // Only offer the action if we can actually remove the type
             // from at least one location.
@@ -484,7 +483,7 @@ impl Backend {
         // ── Update native return type ───────────────────────────────
         if let Some(native_type) =
             extract_native_return_type(&lines, paren_line, paren_col, brace_line)
-            && let Some(new_type) = remove_type_from_union(&native_type.to_string(), unused_type)
+            && let Some(new_type) = remove_type_from_union(&native_type, unused_type)
         {
             // Convert the new type to a valid native hint.
             let native_hint = new_type
@@ -513,7 +512,7 @@ impl Backend {
                 docblock_info.doc_start_line,
                 docblock_info.doc_end_line,
             )
-            && let Some(new_type) = remove_type_from_union(&doc_type.to_string(), unused_type)
+            && let Some(new_type) = remove_type_from_union(&doc_type, unused_type)
             && let Some(edit) = find_and_replace_return_tag_type(
                 &lines,
                 docblock_info.doc_start_line,
@@ -578,7 +577,7 @@ pub(crate) fn is_remove_unused_return_type_stale(
 
     // Check native return type.
     if let Some(native_type) = extract_native_return_type(&lines, paren_line, paren_col, brace_line)
-        && remove_type_from_union(&native_type.to_string(), unused_type).is_some()
+        && remove_type_from_union(&native_type, unused_type).is_some()
     {
         // The unused type is still present → not stale.
         return false;
@@ -594,7 +593,7 @@ pub(crate) fn is_remove_unused_return_type_stale(
             docblock_info.doc_start_line,
             docblock_info.doc_end_line,
         )
-        && remove_type_from_union(&doc_type.to_string(), unused_type).is_some()
+        && remove_type_from_union(&doc_type, unused_type).is_some()
     {
         return false;
     }
@@ -640,67 +639,73 @@ mod tests {
     #[test]
     fn removes_null_from_string_null() {
         assert_eq!(
-            remove_type_from_union("string|null", "null"),
+            remove_type_from_union(&PhpType::parse("string|null"), "null"),
             Some(PhpType::parse("string"))
         );
     }
 
     #[test]
     fn removes_string_from_string_null() {
         assert_eq!(
-            remove_type_from_union("string|null", "string"),
+            remove_type_from_union(&PhpType::parse("string|null"), "string"),
             Some(PhpType::parse("null"))
         );
     }
 
     #[test]
     fn removes_from_three_member_union() {
         assert_eq!(
-            remove_type_from_union("string|int|null", "null"),
+            remove_type_from_union(&PhpType::parse("string|int|null"), "null"),
             Some(PhpType::parse("string|int"))
         );
     }
 
     #[test]
     fn removes_middle_member() {
         assert_eq!(
-            remove_type_from_union("string|int|bool", "int"),
+            remove_type_from_union(&PhpType::parse("string|int|bool"), "int"),
             Some(PhpType::parse("string|bool"))
         );
     }
 
     #[test]
     fn removes_from_intersection() {
         assert_eq!(
-            remove_type_from_union("Foo&Bar", "Bar"),
+            remove_type_from_union(&PhpType::parse("Foo&Bar"), "Bar"),
             Some(PhpType::parse("Foo"))
         );
     }
 
     #[test]
     fn removes_null_from_nullable() {
         assert_eq!(
-            remove_type_from_union("?string", "null"),
+            remove_type_from_union(&PhpType::parse("?string"), "null"),
             Some(PhpType::parse("string"))
         );
     }
 
     #[test]
     fn removes_inner_from_nullable() {
         assert_eq!(
-            remove_type_from_union("?string", "string"),
+            remove_type_from_union(&PhpType::parse("?string"), "string"),
             Some(PhpType::parse("null"))
         );
     }
 
     #[test]
     fn returns_none_when_not_found() {
-        assert_eq!(remove_type_from_union("string|int", "bool"), None);
+        assert_eq!(
+            remove_type_from_union(&PhpType::parse("string|int"), "bool"),
+            None
+        );
     }
 
     #[test]
     fn returns_none_for_single_type() {
-        assert_eq!(remove_type_from_union("string", "string"), None);
+        assert_eq!(
+            remove_type_from_union(&PhpType::parse("string"), "string"),
+            None
+        );
     }
 
     // ── stale detection ────────────────────────────────────────────

src/completion/phpdoc/generation.rs

@@ -51,7 +51,7 @@ use crate::completion::resolver::FunctionLoaderFn;
 use crate::completion::source::comment_position::position_to_byte_offset;
 use crate::completion::source::throws_analysis::{self, ThrowsContext};
 use crate::completion::use_edit::{analyze_use_block, build_use_edit};
-use crate::php_type::{PhpType, is_keyword_type};
+use crate::php_type::PhpType;
 use crate::types::{ClassInfo, FunctionLoader};
 use crate::util::{byte_offset_to_utf16_col, utf16_col_to_byte_offset};
 
@@ -1739,28 +1739,24 @@ fn property_var_type_snippet(
         }
         Some(th) => {
             let shortened = th.shorten();
-            let clean = shortened.to_string();
             // Callable types get a signature placeholder.
             if th.is_callable() {
-                let s = format!("(${{{}:{}()}})", *tab_stop, &clean);
+                let s = format!("(${{{}:{}()}})", *tab_stop, shortened);
                 *tab_stop += 1;
                 return s;
             }
-            if !matches!(
-                th,
-                PhpType::Union(_) | PhpType::Intersection(_) | PhpType::Nullable(_)
-            ) && !is_keyword_type(&clean)
-                && let Some(cls) = class_loader(&clean)
+            if let Some(name) = shortened.base_name()
+                && let Some(cls) = class_loader(name)
                 && !cls.template_params.is_empty()
             {
                 let mut parts = Vec::new();
                 for tp in &cls.template_params {
                     parts.push(format!("${{{}:{}}}", *tab_stop, tp));
                     *tab_stop += 1;
                 }
-                return format!("{}<{}>", &clean, parts.join(", "));
+                return format!("{}<{}>", name, parts.join(", "));
             }
-            clean
+            shortened.to_string()
         }
     }
 }
@@ -1775,21 +1771,17 @@ fn property_var_type_plain(
         Some(th) if th.is_bare_array() => "array".to_string(),
         Some(th) => {
             let shortened = th.shorten();
-            let clean = shortened.to_string();
             if th.is_callable() {
-                return format!("({}())", &clean);
+                return format!("({}())", shortened);
             }
-            if !matches!(
-                th,
-                PhpType::Union(_) | PhpType::Intersection(_) | PhpType::Nullable(_)
-            ) && !is_keyword_type(&clean)
-                && let Some(cls) = class_loader(&clean)
+            if let Some(name) = shortened.base_name()
+                && let Some(cls) = class_loader(name)
                 && !cls.template_params.is_empty()
             {
                 let parts: Vec<&str> = cls.template_params.iter().map(|s| s.as_str()).collect();
-                return format!("{}<{}>", &clean, parts.join(", "));
+                return format!("{}<{}>", name, parts.join(", "));
             }
-            clean
+            shortened.to_string()
         }
     }
 }