More Mago migration

Author: AJenbo

docs/CHANGELOG.md

@@ -59,6 +59,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Docblock navigation lands on the right name in types that mix a `*` wildcard with a non-ASCII name.** In a type such as `@return Map<Café, *, User>`, go-to-definition, find references, and rename measured every name after the accented one against the wrong bytes, so clicking `User` resolved nothing (or the wrong symbol). The PHPStan `*` wildcard is now read directly by the type grammar rather than rewritten to `mixed` beforehand, which removes the byte-offset bookkeeping that was corrupting the positions.
 - **Formatting a short method chain starting with `new X(...)` no longer breaks it across lines unnecessarily.** When the constructor call's own arguments were long enough to wrap, the formatter also forced a short trailing chain like `(new Foo(...))->bar()` onto separate lines even though it would have fit on one. Upstream fix from mago 1.44.0.
 - **`@method` and `@property` tags on an implemented interface are now always applied.** A class that declared no docblock of its own missed the magic methods and properties its interfaces declared, so they did not complete, hover, or resolve, and calls to them were reported as unknown members. Tags on an interface (and on the interfaces it extends) are now picked up regardless of what the implementing class documents.
 - **Find References, Rename, and Go to Implementation no longer look stalled during startup indexing.** A search started while the background index is still parsing the workspace waits for that index to finish, since acting on a partial index would silently miss results. That wait now shows in the request's own progress bar as "Waiting for workspace index" alongside the index's live file counts, instead of sitting at "Resolving…" with no indication of what it is waiting for.

docs/todo/performance.md

@@ -711,20 +711,6 @@ long as the extracted data. The first is mechanical but touches every
 extractor in `docblock/`; the second changes the ownership model of every
 caller. Sized for several sprint items.
 
-Two smaller pieces of the same cleanup can land independently:
-
-- **The `*` wildcard pre-pass is obsolete.** `PhpType::parse` and
-  `symbol_map/docblock.rs::emit_type_spans` both still rewrite PHPStan's
-  `*` generic wildcard to `mixed` in the type string before parsing, and
-  the latter maintains a byte offset map to undo the 1→5 character
-  expansion afterwards. The grammar now models `*` natively as
-  `Type::Wildcard`, so both passes (and the offset map) can go, replaced
-  by mapping `Type::Wildcard` to `mixed` in `convert`.
-- **`get_docblock_text_for_node` re-reads raw comment text.** Tags are
-  parsed from a `&str` recovered from trivia rather than from the trivia
-  the main `mago-syntax` parse already produced. Feeding the parser the
-  trivia bytes directly would drop a copy per docblock.
-
 ---
 
 ## P30. Evaluate migrating parse/resolve/docblock pipeline to `mago-hir`

src/docblock/parser.rs

@@ -19,7 +19,7 @@
 //! so the extractors in [`super::tags`] and friends see the type and
 //! description exactly as the author wrote them.
 
-use mago_allocator::{Arena, LocalArena};
+use mago_allocator::LocalArena;
 use mago_phpdoc_syntax::PHPDocParser;
 use mago_phpdoc_syntax::cst::{Document, Element};
 use mago_span::{HasSpan, Position, Span};
@@ -139,13 +139,11 @@ pub fn parse_docblock(docblock: &str, base_span: Span) -> Option<DocblockInfo> {
         return None;
     }
 
+    // `PHPDocParser::parse_with_span` ties the arena borrow and the content
+    // borrow to one lifetime, and the caller's bytes already outlive this
+    // scope, so they can be handed over directly — no copy into the arena.
     let arena = LocalArena::new();
-
-    // `PHPDocParser::parse_with_span` requires `content: &'arena [u8]`, so
-    // the bytes are allocated into the arena to make the borrow live long
-    // enough.
-    let content: &[u8] = arena.alloc_slice_copy(docblock.as_bytes());
-    let document = PHPDocParser::parse_with_span(&arena, content, base_span);
+    let document = PHPDocParser::parse_with_span(&arena, docblock.as_bytes(), base_span);
 
     Some(collect_tags(&document, docblock, base_span.start.offset))
 }

src/php_type/mod.rs

@@ -20,7 +20,7 @@ use std::hash::{Hash, Hasher};
 use std::ops::Deref;
 use std::sync::Arc;
 
-use mago_allocator::{Arena, LocalArena};
+use mago_allocator::LocalArena;
 use mago_database::file::FileId;
 use mago_phpdoc_syntax::cst::r#type as cst;
 use mago_span::{Position, Span};

src/php_type/parse.rs

@@ -17,12 +17,10 @@ impl PhpType {
             return PhpType::untyped();
         }
 
-        // Replace PHPStan `*` wildcards in generic positions with `mixed`.
-        let cleaned = replace_star_wildcards(input);
         // Replace known hyphenated pseudo-types (e.g. `model-property`)
         // with underscore placeholders so Mago can parse the surrounding
         // type structure.  The placeholders are restored in the result.
-        let cleaned = replace_hyphenated_keywords(&cleaned);
+        let cleaned = replace_hyphenated_keywords(input);
         let effective: &str = &cleaned;
 
         let span = Span::new(
@@ -32,8 +30,7 @@ impl PhpType {
         );
 
         let arena = LocalArena::new();
-        let effective = arena.alloc_slice_copy(effective.as_bytes());
-        match mago_phpdoc_syntax::parse_type(&arena, effective, span) {
+        match mago_phpdoc_syntax::parse_type(&arena, effective.as_bytes(), span) {
             Ok(ty) => restore_hyphenated_keywords(convert(&ty)),
             Err(_) => try_parse_hyphenated_generic(input).unwrap_or_else(|| PhpType::raw(input)),
         }
@@ -107,22 +104,6 @@ pub(crate) fn parse_php_float_literal(raw: &str) -> Option<f64> {
     clean.parse::<f64>().ok()
 }
 
-/// Replace PHPStan `*` wildcards in generic type argument positions with
-/// `mixed`.
-///
-/// PHPStan's phpdoc-parser supports `*` as a bivariant wildcard inside
-/// generic angle brackets, e.g. `Relation<TRelatedModel, *, *>`.  The
-/// `*` simply means "any type" and is equivalent to `mixed`.
-/// `mago-phpdoc-syntax` does not model it as a type, so we pre-process it.
-///
-/// Only replaces `*` tokens that appear inside angle brackets at generic
-/// argument boundaries: preceded (ignoring whitespace) by `<` or `,` and
-/// followed (ignoring whitespace) by `,` or `>`.  This avoids mangling:
-/// - `Foo::*` — member references (preceded by `::`)
-/// - `int-mask-of<self::FOO_*>` — constant wildcard patterns (preceded
-///   by `_` or identifier chars)
-///
-/// Returns the input unchanged (no allocation) when no wildcards are found.
 /// Hyphenated PHPDoc pseudo-type names that `mago_phpdoc_syntax` cannot
 /// parse because hyphens are not valid PHP identifier characters.
 /// Each pair is `(hyphenated, placeholder)`.
@@ -180,77 +161,6 @@ fn try_parse_hyphenated_generic(input: &str) -> Option<PhpType> {
     Some(PhpType::generic(base, args))
 }
 
-pub(crate) fn replace_star_wildcards(s: &str) -> std::borrow::Cow<'_, str> {
-    if !s.contains('*') {
-        return std::borrow::Cow::Borrowed(s);
-    }
-
-    let bytes = s.as_bytes();
-
-    // First pass: check if any `*` is actually a generic wildcard.
-    let has_generic_wildcard =
-        (0..bytes.len()).any(|i| bytes[i] == b'*' && is_generic_wildcard(bytes, i));
-
-    if !has_generic_wildcard {
-        return std::borrow::Cow::Borrowed(s);
-    }
-
-    let mut result = String::with_capacity(s.len() + 16);
-    let mut i = 0usize;
-
-    while i < bytes.len() {
-        if bytes[i] == b'*' && is_generic_wildcard(bytes, i) {
-            result.push_str("mixed");
-            i += 1;
-        } else {
-            // Copy the whole UTF-8 character, not a single byte, so
-            // multibyte characters in the type string are not mangled.
-            let ch = s[i..].chars().next().unwrap();
-            result.push(ch);
-            i += ch.len_utf8();
-        }
-    }
-
-    std::borrow::Cow::Owned(result)
-}
-
-/// Check whether the `*` at position `pos` in `bytes` is a PHPStan
-/// generic wildcard (as opposed to a member reference like `Foo::*`
-/// or a constant pattern like `self::FOO_*`).
-///
-/// A generic wildcard `*` is preceded (ignoring whitespace) by `<` or
-/// `,` and followed (ignoring whitespace) by `,` or `>`.
-pub(crate) fn is_generic_wildcard(bytes: &[u8], pos: usize) -> bool {
-    // Check preceding non-whitespace character.
-    let prev_ok = {
-        let mut j = pos;
-        loop {
-            if j == 0 {
-                break false;
-            }
-            j -= 1;
-            if !bytes[j].is_ascii_whitespace() {
-                break bytes[j] == b'<' || bytes[j] == b',';
-            }
-        }
-    };
-
-    if !prev_ok {
-        return false;
-    }
-
-    // Check following non-whitespace character.
-    let mut k = pos + 1;
-    while k < bytes.len() {
-        if !bytes[k].is_ascii_whitespace() {
-            return bytes[k] == b',' || bytes[k] == b'>';
-        }
-        k += 1;
-    }
-
-    false
-}
-
 /// The written name of a class-like reference.
 ///
 /// `mago-phpdoc-syntax` distinguishes the relative keywords (`self`,
@@ -432,6 +342,11 @@ pub(crate) fn convert(ty: &cst::Type<'_>) -> PhpType {
         cst::Type::LiteralFloat(l) => PhpType::literal_float(bytes_to_str(l.raw)),
         cst::Type::LiteralString(l) => PhpType::literal_string_raw(bytes_to_str(l.raw)),
 
+        // -- Wildcard: PHPStan's bivariant `*` / `_` --------------------------
+        // `Relation<TRelatedModel, *, *>` means "any type" in that generic
+        // position, which is exactly `mixed` for our purposes.
+        cst::Type::Wildcard(_) => PhpType::mixed(),
+
         // -- Negated / Posited literals (e.g. -42, +42) -----------------------
         cst::Type::Negated(n) => literal_number_type(format!("-{}", n.operand)),
         cst::Type::Posited(p) => literal_number_type(format!("+{}", p.operand)),

src/php_type_tests.rs

@@ -14,8 +14,7 @@ fn assert_round_trip(input: &str) {
         Position::new(input.len() as u32),
     );
     let arena = LocalArena::new();
-    let input_arena = arena.alloc_slice_copy(input.as_bytes());
-    let mago_canonical = match mago_phpdoc_syntax::parse_type(&arena, input_arena, span) {
+    let mago_canonical = match mago_phpdoc_syntax::parse_type(&arena, input.as_bytes(), span) {
         Ok(ty) => ty.to_string(),
         Err(_) => {
             // If mago can't parse it, PhpType should produce Raw.
@@ -400,15 +399,10 @@ fn parse_generic_star_does_not_mangle_member_reference() {
 }
 
 #[test]
-fn replace_star_wildcards_does_not_mangle_constant_pattern() {
+fn parse_generic_star_does_not_mangle_constant_pattern() {
     // `int-mask-of<self::FOO_*>` — the `*` is part of a constant
-    // pattern, not a generic wildcard (preceded by `_`, not `<`/`,`).
-    // Our pre-processing must leave it untouched.  (mago itself may
-    // or may not parse the result, but that's a separate issue.)
-    use super::replace_star_wildcards;
-    let result = replace_star_wildcards("int-mask-of<self::FOO_*>");
-    assert_eq!(result.as_ref(), "int-mask-of<self::FOO_*>");
-    assert!(matches!(result, std::borrow::Cow::Borrowed(_)));
+    // pattern, not a bivariant wildcard, so it must survive parsing.
+    assert_round_trip("int-mask-of<self::FOO_*>");
 }
 
 #[test]
@@ -428,12 +422,10 @@ fn parse_generic_star_with_spaces() {
 }
 
 #[test]
-fn replace_star_wildcards_preserves_multibyte() {
-    // A multibyte character alongside a generic wildcard must survive
-    // the rewrite intact (not be mangled byte-by-byte).
-    use super::replace_star_wildcards;
-    let result = replace_star_wildcards("Map<Café, *>");
-    assert_eq!(result.as_ref(), "Map<Café, mixed>");
+fn parse_generic_star_preserves_multibyte() {
+    // A multibyte character alongside a bivariant wildcard must survive
+    // intact (not be mangled byte-by-byte).
+    assert_round_trip_expected("Map<Café, *>", "Map<Café, mixed>");
 }
 
 #[test]
@@ -454,45 +446,17 @@ fn variance_annotations_are_parsed_and_discarded() {
 }
 
 #[test]
-fn replace_star_wildcards_no_star() {
-    use super::replace_star_wildcards;
-    let result = replace_star_wildcards("Collection<int, User>");
-    assert_eq!(result.as_ref(), "Collection<int, User>");
-    // Should borrow, not allocate
-    assert!(matches!(result, std::borrow::Cow::Borrowed(_)));
-}
-
-#[test]
-fn replace_star_wildcards_member_ref() {
-    use super::replace_star_wildcards;
-    let result = replace_star_wildcards("Foo::*");
-    assert_eq!(result.as_ref(), "Foo::*");
-    // Should borrow, not allocate
-    assert!(matches!(result, std::borrow::Cow::Borrowed(_)));
-}
-
-#[test]
-fn replace_star_wildcards_constant_pattern() {
-    use super::replace_star_wildcards;
-    let result = replace_star_wildcards("int-mask-of<self::FOO_*>");
-    assert_eq!(result.as_ref(), "int-mask-of<self::FOO_*>");
-    assert!(matches!(result, std::borrow::Cow::Borrowed(_)));
-}
-
-#[test]
-fn replace_star_wildcards_generic() {
-    use super::replace_star_wildcards;
-    let result = replace_star_wildcards("Relation<TRelatedModel, *, *>");
-    assert_eq!(result.as_ref(), "Relation<TRelatedModel, mixed, mixed>");
+fn parse_generic_wildcard_becomes_mixed() {
+    assert_round_trip_expected(
+        "Relation<TRelatedModel, *, *>",
+        "Relation<TRelatedModel, mixed, mixed>",
+    );
 }
 
 #[test]
-fn replace_star_wildcards_single_star() {
-    use super::replace_star_wildcards;
-    let result = replace_star_wildcards("Voter<self::*>");
-    // `self::*` — the `*` is preceded by `::`, not `<` or `,`
-    assert_eq!(result.as_ref(), "Voter<self::*>");
-    assert!(matches!(result, std::borrow::Cow::Borrowed(_)));
+fn parse_generic_star_keeps_member_wildcard() {
+    // `self::*` is a member reference wildcard, not a bivariant `*`.
+    assert_round_trip("Voter<self::*>");
 }
 
 #[test]