Skip to content

Commit b57d55a

Browse files
committed
Fix multi-line @var docblock parsing with extra tags
1 parent f675acf commit b57d55a

5 files changed

Lines changed: 106 additions & 16 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1717
- **Generic method return types from `@var` annotations.** When a variable is annotated with a generic type (e.g. `/** @var Collection<int, User> */ $items`), method calls on that variable now correctly substitute class-level template parameters into the return type. Previously, return types like `TValue` remained unsubstituted.
1818
- **Short class name resolution in type hints.** When resolving an unqualified class name from a property or return type annotation, the resolver now prefers the class in the same namespace as the owning type before falling back to first-match.
1919
- **Foreach narrowing with break in else.** When a foreach body contained an `if` with an `else { break; }` branch, the variable state from the break path was not included in the post-loop type, causing the merged type to be too narrow.
20+
- **`@var` docblocks with additional tags.** When a `@var` annotation shared a docblock with other tags (e.g. `@psalm-suppress`, `@deprecated`), the extra lines were included in the type string, corrupting type resolution. Multi-line `@var` docblocks now correctly extract only the type from the first line.
2021
- **Foreach element type from untyped arrays.** Variables bound in a `foreach` over an untyped `array` (e.g. from a function returning bare `array`) now resolve to `mixed` instead of empty, so assignments from the loop variable propagate correctly.
2122
- **Foreach target type after non-empty literal array.** When iterating a non-empty literal array such as `["a", "b", "c"]`, the pre-loop sentinel value of the target variable (e.g. `null` from `$tag = null`) no longer survives as a possible post-loop type.
2223
- **Object cast type inference.** `(object)$scalar` now resolves to `object{scalar:T}` and `(object)$array` resolves to an object shape matching the array's keys, instead of bare `stdClass`.

docs/todo/bugs.md

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,26 @@ Remaining gaps:
4949
**Discovered:** SKIP audit of
5050
`tests/psalm_assertions/template_class_template.php`.
5151

52-
Remaining failures are all caused by the multi-namespace
53-
infrastructure limitation: `FileContext.namespace` stores only the
54-
first namespace, so the class loader resolves bare names (e.g.
55-
`Foo`, `Collection`) against the wrong namespace when the same
56-
short name appears in multiple namespace blocks within one file.
52+
Remaining failures have multiple root causes (the original
53+
multi-namespace theory was incorrect for most of them):
54+
55+
- **Lines 16, 29, 41, 56, 68:** Generic constructor inference
56+
through iterator decorators (`CachingIterator(new ArrayIterator(...))`)
57+
does not propagate template parameters. Fails in single-namespace
58+
files too.
59+
- **Line 602:** Union generic method resolution (`C<A>|C<B>``->get()`)
60+
does not resolve per-branch template substitutions.
61+
- **Line 752:** `new ArrayCollection()` with no args infers
62+
`ArrayCollection<array, array>` instead of `ArrayCollection<never, never>`.
63+
- **Line 788:** Static method call `Collection::fromClassString(A::class)`
64+
does not propagate the method-level template to the return type.
65+
66+
**Fixed:** Line 122 — `@var` docblocks with additional tags
67+
(e.g. `@psalm-suppress`) after the type corrupted the type string.
68+
Fixed in `parse_inline_var_docblock_no_var`.
5769

5870
**Tests:** SKIPs in `tests/psalm_assertions/template_class_template.php`
59-
(lines 16, 29, 41, 56, 68, 122, 602, 752, 788).
71+
(lines 16, 29, 41, 56, 68, 602, 752, 788).
6072

6173

6274

src/completion/variable/forward_walk.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3966,6 +3966,13 @@ fn parse_inline_var_docblock_no_var(doc_text: &str, _ctx: &ForwardWalkCtx<'_>) -
39663966
.or_else(|| inner.strip_prefix("* @var"))?;
39673967
let inner = inner.trim();
39683968

3969+
// For multi-line docblocks, only take the type from the first line.
3970+
// Additional lines may contain other tags like @psalm-suppress that
3971+
// would corrupt the type string.
3972+
let inner = inner.lines().next().unwrap_or(inner).trim();
3973+
// Strip trailing `*` that may remain from `* @var Type *` formatting.
3974+
let inner = inner.trim_end_matches('*').trim();
3975+
39693976
// If there's a `$` it has a variable name — not the no-var form.
39703977
if inner.contains('$') {
39713978
return None;

tests/integration/hover.rs

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
33
use crate::common::{
44
create_psr4_workspace, create_test_backend, create_test_backend_with_closure_stub,
5-
create_test_backend_with_function_stubs, create_test_backend_with_stdclass_stub,
5+
create_test_backend_with_full_stubs, create_test_backend_with_function_stubs,
6+
create_test_backend_with_stdclass_stub,
67
};
78
use phpantom_lsp::Backend;
89
use tower_lsp::lsp_types::*;
@@ -11127,3 +11128,72 @@ $result = $a;
1112711128
text
1112811129
);
1112911130
}
11131+
11132+
#[test]
11133+
fn hover_multi_namespace_template_foo_collision() {
11134+
// Regression: when `@var Foo<A>` is followed by extra tags
11135+
// (e.g. `@psalm-suppress`) in the docblock, the type parser
11136+
// would include the extra lines in the type string, breaking
11137+
// resolution. This test verifies the fix works with multiple
11138+
// namespace blocks and Foo collisions across them.
11139+
let backend = create_test_backend_with_full_stubs();
11140+
let uri = "file:///test.php";
11141+
let content = r#"<?php
11142+
namespace NS6 {
11143+
class A {}
11144+
11145+
/**
11146+
* @template T as object
11147+
*/
11148+
class Foo {
11149+
/** @var class-string<T> */
11150+
public $T;
11151+
/**
11152+
* @param class-string<T> $T
11153+
*/
11154+
public function __construct(string $T) {
11155+
$this->T = $T;
11156+
}
11157+
/**
11158+
* @return T
11159+
*/
11160+
public function bar() {
11161+
$t = $this->T;
11162+
return new $t();
11163+
}
11164+
}
11165+
11166+
/**
11167+
* @var Foo<A>
11168+
* @psalm-suppress ArgumentTypeCoercion
11169+
*/
11170+
$afoo = new Foo('A');
11171+
$afoo_bar = $afoo->bar();
11172+
}
11173+
namespace NS7 {
11174+
/**
11175+
* @template T as object
11176+
*/
11177+
class Foo {
11178+
/** @var T */
11179+
public $item;
11180+
}
11181+
}
11182+
"#;
11183+
let target_line = content
11184+
.lines()
11185+
.enumerate()
11186+
.find(|(_, l)| l.contains("$afoo_bar = $afoo->bar()"))
11187+
.map(|(i, _)| i as u32)
11188+
.unwrap();
11189+
let line_text = content.lines().nth(target_line as usize).unwrap();
11190+
let col = line_text.find("$afoo_bar").unwrap() as u32;
11191+
let hover = hover_at(&backend, uri, content, target_line, col + 1)
11192+
.expect("expected hover on $afoo_bar");
11193+
let text = hover_text(&hover);
11194+
assert!(
11195+
text.contains("A"),
11196+
"$afoo_bar should resolve to A via Foo<A>::bar(), got: {}",
11197+
text
11198+
);
11199+
}

tests/psalm_assertions/template_class_template.php

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
$key = $decoratorIterator->key();
1414
$value = $decoratorIterator->current();
1515

16-
assertType('null|string', $value); // SKIP multi-namespace class resolution
16+
assertType('null|string', $value); // SKIP generic iterator constructor inference
1717
assertType('bool', $next);
1818
}
1919

@@ -26,7 +26,7 @@
2626
$key = $decoratorIterator->key();
2727
$value = $decoratorIterator->current();
2828

29-
assertType('null|string', $value); // SKIP multi-namespace class resolution
29+
assertType('null|string', $value); // SKIP generic iterator constructor inference
3030
}
3131

3232
// Test: limitIterator
@@ -38,7 +38,7 @@
3838
$key = $decoratorIterator->key();
3939
$value = $decoratorIterator->current();
4040

41-
assertType('null|string', $value); // SKIP multi-namespace class resolution
41+
assertType('null|string', $value); // SKIP generic iterator constructor inference
4242
}
4343

4444
// Test: callbackFilterIterator
@@ -53,7 +53,7 @@ static function (string $value): bool {return "a" === $value;}
5353
$key = $decoratorIterator->key();
5454
$value = $decoratorIterator->current();
5555

56-
assertType('null|string', $value); // SKIP multi-namespace class resolution
56+
assertType('null|string', $value); // SKIP generic iterator constructor inference
5757
}
5858

5959
// Test: noRewindIterator
@@ -65,7 +65,7 @@ static function (string $value): bool {return "a" === $value;}
6565
$key = $decoratorIterator->key();
6666
$value = $decoratorIterator->current();
6767

68-
assertType('null|string', $value); // SKIP multi-namespace class resolution
68+
assertType('null|string', $value); // SKIP generic iterator constructor inference
6969
}
7070

7171
// Test: classTemplate
@@ -119,7 +119,7 @@ public function bar() {
119119
$cfoo_bar = $cfoo->bar();
120120

121121
assertType('Foo<A>', $afoo);
122-
assertType('A', $afoo_bar); // SKIP multi-namespace class resolution (Foo collision with ns6)
122+
assertType('A', $afoo_bar);
123123
assertType('Foo<B>', $bfoo);
124124
assertType('B', $bfoo_bar);
125125
assertType('Foo<C>', $cfoo);
@@ -599,7 +599,7 @@ public function get() {
599599
$a_or_b = $random_collection->get();
600600

601601
assertType('C<A>|C<B>', $random_collection);
602-
assertType('A|B', $a_or_b); // SKIP multi-namespace class resolution (C collision)
602+
assertType('A|B', $a_or_b); // SKIP union generic method resolution
603603
}
604604

605605
// Test: templatedGet
@@ -749,7 +749,7 @@ public function add($key, $t) : void {
749749
}
750750
}
751751

752-
assertType('ArrayCollection<never, never>', $a); // SKIP multi-namespace class resolution (ArrayCollection collision)
752+
assertType('ArrayCollection<never, never>', $a); // SKIP generic constructor inference (never)
753753
}
754754

755755
// Test: unionClassStringInferenceAndDefaultEmptyArray
@@ -785,7 +785,7 @@ public static function fromClassString(string $classString, array $elements = []
785785
}
786786
}
787787

788-
assertType('Collection<A>', $packages); // SKIP multi-namespace class resolution (Collection collision)
788+
assertType('Collection<A>', $packages); // SKIP static method call generic inference
789789
}
790790

791791
// Test: newWithoutInferredTemplate

0 commit comments

Comments
 (0)