Docblock navigation works in @method and @property tags written

across several lines

Author: AJenbo

docs/CHANGELOG.md

@@ -60,6 +60,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Docblock navigation works in `@method` and `@property` tags written across several lines.** A tag whose type wrapped onto a continuation line, such as a `@method Collection<int, Item> fetchAll(Filter $filter)` broken after the `<`, only ever had its first line read. Everything after it was invisible: go-to-definition, find references, and rename did nothing on the method or property name, on the type arguments, or on the parameter types, and the truncated first line was reported as a class named `Collection<` that resolved to nothing. Docblock positions now come from the PHPDoc grammar itself, so every name in such a tag is navigable wherever it sits.
 - **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.

docs/todo.md

@@ -196,8 +196,8 @@ unlikely to move the needle for most users.
 | E6  | Stub install prompt for non-Composer projects                                                                                                                               | Low         | Low         |
 | E7  | [Stub-based framework patches](todo/external-stubs.md#e7-stub-based-framework-patches)                                                                                      | Medium      | Medium      |
 |     | **[Performance](todo/performance.md)**                                                                                       |             |             |
-| P45 | [Emit docblock symbol spans from the PHPDoc CST](todo/performance.md#p45-emit-docblock-symbol-spans-from-the-phpdoc-cst)                                                     | Low-Medium  | Medium-High |
 | P46 | [`mago-phpdoc-syntax` cannot parse `@method static (…) name()`](todo/performance.md#p46-mago-phpdoc-syntax-cannot-parse-method-static--name)                                | Low         | Low         |
+| P47 | [Inline `{@see}` references are still found by scanning raw text](todo/performance.md#p47-inline-see-references-are-still-found-by-scanning-raw-text)                        | Low         | Low-Medium  |
 | P30 | [Evaluate migrating parse/resolve/docblock pipeline to `mago-hir`](todo/performance.md#p30-evaluate-migrating-parseresolvedocblock-pipeline-to-mago-hir) (parked — re-evaluated at mago 1.45.0, still no `mago-hir` consumers upstream) | Medium-High | High        |
 | P43 | [`init_single_project` is the longest single-threaded stretch of a run](todo/performance.md#p43-init_single_project-is-the-longest-single-threaded-stretch-of-a-run)        | Medium-High | Medium      |
 | P16 | [Pre-parsed stub format (eliminate raw PHP embedding)](todo/performance.md#p16-pre-parsed-stub-format-eliminate-raw-php-embedding)                                          | High        | Medium-High |

docs/todo/performance.md

@@ -678,36 +678,25 @@ body-return inference, since fixed by memoization).
 
 ---
 
-## P45. Emit docblock symbol spans from the PHPDoc CST
-
-**Impact: Low-Medium · Effort: Medium-High**
-
-`src/symbol_map/docblock.rs` is the last consumer that re-derives tag
-structure from raw text. The extractors in `docblock/` now read the
-parsed grammar (see the `TagValueInfo` snapshot on `TagInfo`), but the
-symbol map needs a *byte offset in the file* for every identifier inside
-a type, not just the type's text, so it still finds the type itself by
-hand: `emit_type_first_tag` strips `@assert` modifiers and calls
-`split_type_token`, `extract_method_tag_symbols` re-implements the
-`[static] Return name(params)` split with its own paren-depth scan and a
-heuristic for telling a return type from a method name,
-`extract_template_tag_symbols` re-finds the `of` bound, and
-`extract_property_tag_symbols` re-splits type from variable.
-
-The CST has a span on every type, identifier and variable, which would
-replace all of that guessing. The obstacle is multi-line types:
-`join_multiline_type` builds an offset map so that a position inside the
-joined type string can be mapped back to the file, and the folded text
-kept in `TagValueInfo` cannot be mapped that way. Recording each type's
-span alongside its text is enough to anchor the *start* of a type
-precisely; the per-identifier offsets inside a folded multi-line type
-still need the offset map, or a walk over the CST type tree in place of
-`emit_type_spans`.
-
-Walking the CST directly is the endgame: it deletes `join_multiline_type`,
-`emit_type_spans_from_ast` and the offset-map machinery outright, since
-every identifier already carries its own span. That is the larger of the
-two options and is what makes this Medium-High rather than Medium.
+## P47. Inline `{@see}` references are still found by scanning raw text
+
+**Impact: Low · Effort: Low-Medium**
+
+`extract_inline_see_symbols` in `src/symbol_map/docblock.rs` is the last
+place the symbol map reads a docblock as a string: it searches for the
+literal `{@see ` and takes everything up to the next `}`. The rest of
+the module now walks the PHPDoc CST, where an inline tag is a
+`TextSegment::InlineTag` carrying a full `Tag` with its own spans.
+
+The reason the scan survived is reach rather than capability. Inline
+tags turn up in any prose, so a CST-based version has to visit the free
+text before the first tag *and* the description of every tag shape that
+has one, which means a `Text` visitor threaded through the tag match in
+`emit_tag_symbols`. The string scan reaches all of them in one pass.
+
+Doing it properly would pick up `{@link}` and the other inline tags for
+free, and stop the scan tripping over a `}` that belongs to a nested
+type rather than the inline tag.
 
 ---
 
@@ -729,11 +718,15 @@ whole tag is dropped:
  */
 ```
 
-Both forms appear in Psalm's own magic-method test suite.
-`recover_static_method_tag` in `src/docblock/virtual_members.rs` works
-around it by re-parsing the value without the modifier and reapplying
-`static` afterwards. Report it upstream; remove the workaround once the
-grammar disambiguates on the whitespace before the `(`.
+Both forms appear in Psalm's own magic-method test suite. Two places
+work around it: `recover_static_method_tag` in
+`src/docblock/virtual_members.rs` re-parses the value without the
+modifier and reapplies `static` afterwards, and
+`recover_static_method_tags` in `src/symbol_map/docblock.rs` blanks the
+keyword out with spaces and re-parses the whole docblock, because it
+needs the tag's original byte offsets as well as its signature. Report
+it upstream; remove both workarounds once the grammar disambiguates on
+the whitespace before the `(`.
 
 ---
 

examples/demo.php

@@ -4212,6 +4212,25 @@ public function demo(): void
     }
 }
 
+
+// ── Multi-Line @method / @property Tags ─────────────────────────────────────
+
+class MultiLineDocblockTagDemo
+{
+    public function demo(): void
+    {
+        // A @method or @property tag whose type wraps onto continuation lines
+        // resolves like its single-line form, and every name written inside the
+        // tag stays navigable: try go-to-definition on `FluentCollection`,
+        // `Pen`, `fetchAll` or `$penHolder` in the docblock of
+        // ScaffoldingMultiLineTags.
+        $shed = new ScaffoldingMultiLineTags();
+
+        $shed->fetchAll('black')->first()->write();  // TValue resolves to Pen
+        $shed->penHolder->first()->write();          // same through @property
+    }
+}
+
 /**
  * Convert to arrow function — place cursor on a single-expression closure
  * and trigger code actions to see "Convert to arrow function".
@@ -4369,6 +4388,31 @@ class ScaffoldingMethodTagTemplate
     public function __call(string $name, array $args): mixed { return $args[0] ?? null; }
 }
 
+// ── Multi-line @method / @property scaffolding ─────────────────────────────────
+
+/**
+ * @method FluentCollection<
+ *     int,
+ *     Pen
+ * > fetchAll(string $ink)
+ * @property FluentCollection<
+ *     int,
+ *     Pen
+ * > $penHolder
+ */
+class ScaffoldingMultiLineTags
+{
+    public function __call(string $name, array $args): mixed
+    {
+        return collect([new Pen(is_string($args[0] ?? null) ? $args[0] : 'black')]);
+    }
+
+    public function __get(string $name): mixed
+    {
+        return collect([new Pen()]);
+    }
+}
+
 // ── Template-param @mixin scaffolding ─────────────────────────────────────────
 interface ScaffoldingAstNodeInterface {
     public function getStartColumn(): int;
@@ -7273,6 +7317,17 @@ function runDemoAssertions(): void
     $firstPen = $collected->first();
     assert($firstPen instanceof Pen, 'collect(Pen[])->first() must return Pen');
 
+    // ── Multi-line @method / @property tags ──────────────────────────────
+    $multiLine = new ScaffoldingMultiLineTags();
+    assert(
+        $multiLine->fetchAll('black')->first() instanceof Pen,
+        'multi-line @method tag must yield FluentCollection<int, Pen>',
+    );
+    assert(
+        $multiLine->penHolder->first() instanceof Pen,
+        'multi-line @property tag must yield FluentCollection<int, Pen>',
+    );
+
     // ── Generic @phpstan-assert narrowing ────────────────────────────────
     $assertObj = new Pen();
     ScaffoldingAssert::assertInstanceOf(Pen::class, $assertObj);

src/docblock/mod.rs

@@ -38,6 +38,7 @@ pub(crate) mod type_strings;
 
 // Parsed docblock representation
 pub use parser::{DocblockInfo, TagInfo, parse_docblock_for_tags};
+pub(crate) use tag_kind::tag_kind;
 pub use tag_kind::{TagKind, TagVendor};
 
 // Core tags

src/docblock/parser.rs

@@ -368,6 +368,27 @@ fn vendor_rank(tag: &TagInfo) -> u8 {
 ///   When the caller does not have span information (e.g. unit tests that
 ///   work with standalone strings), pass a zero-offset span.
 pub fn parse_docblock(docblock: &str, base_span: Span) -> Option<DocblockInfo> {
+    with_docblock_cst(docblock, base_span, |document| {
+        collect_tags(document, docblock, base_span.start.offset)
+    })
+}
+
+/// Parse a docblock and hand the CST to `f`.
+///
+/// Every span in the CST is a *file* offset, because the parser is anchored
+/// at `base_span`: the lexer treats the `*` continuation prefixes as trivia,
+/// so an identifier nested inside a type written across several lines still
+/// reports where it really sits in the file.  That makes this the entry point
+/// for anything that needs source positions rather than text, and it is why
+/// the symbol map walks the CST instead of re-scanning tag values.
+///
+/// The CST borrows from an arena that dies with this call, so it cannot
+/// escape the closure.  Returns `None` if the string is not a docblock.
+pub fn with_docblock_cst<R>(
+    docblock: &str,
+    base_span: Span,
+    f: impl FnOnce(&Document<'_>) -> R,
+) -> Option<R> {
     if !docblock.trim_start().starts_with("/**") {
         return None;
     }
@@ -378,7 +399,7 @@ pub fn parse_docblock(docblock: &str, base_span: Span) -> Option<DocblockInfo> {
     let arena = LocalArena::new();
     let document = PHPDocParser::parse_with_span(&arena, docblock.as_bytes(), base_span);
 
-    Some(collect_tags(&document, docblock, base_span.start.offset))
+    Some(f(&document))
 }
 
 /// Walk a parsed `Document` and collect all tags into owned [`TagInfo`]
@@ -678,7 +699,7 @@ fn variable_text(value: &[u8]) -> String {
 /// A type written across several lines carries the `*` continuation prefix
 /// and the source indentation; both have to go before the string can be
 /// handed to the type parser.
-fn type_text(docblock: &str, base_offset: u32, span: Span) -> String {
+pub(crate) fn type_text(docblock: &str, base_offset: u32, span: Span) -> String {
     let raw = source_text(docblock, base_offset, span);
     if raw.contains('\n') {
         collapse_newlines(&raw)
@@ -696,7 +717,11 @@ fn type_text(docblock: &str, base_offset: u32, span: Span) -> String {
 /// declined to model at all would drop out of the text.  Taking the region
 /// from the tag name instead yields the value exactly as written, which is
 /// what the extractors re-parse.
-fn value_span(tag: &mago_phpdoc_syntax::cst::Tag<'_>, docblock: &str, base_offset: u32) -> Span {
+pub(crate) fn value_span(
+    tag: &mago_phpdoc_syntax::cst::Tag<'_>,
+    docblock: &str,
+    base_offset: u32,
+) -> Span {
     let name_end = tag.name.span.end;
     let end = tag.value.span().end;
     let start = if end.offset > name_end.offset {