Skip to content

Commit 754b133

Browse files
committed
Prioritize @phpstan-param and @psalm-param over @param tags in docblock
param type extraction
1 parent 55a3894 commit 754b133

4 files changed

Lines changed: 83 additions & 60 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2828
- **Trait `self` return type resolution through inheritance.** When a trait method has return type `self`, calling it on a subclass now correctly resolves to the class that uses the trait (the declaring class), not the calling subclass. `static` and `$this` continue to resolve to the runtime class.
2929
- **`NoRewindIterator` generic resolution.** Added stub patch so `new NoRewindIterator($generator)` propagates the wrapped iterator's type parameters.
3030
- **Template param inference from type bounds.** When a class template parameter has a bound like `TIterator as Iterator<TKey, TValue>` and `TIterator` is resolved to a concrete generic type, the nested template params (`TKey`, `TValue`) are now inferred from the concrete type's generic arguments.
31+
- **`@psalm-param`/`@phpstan-param` priority over `@param`.** When a docblock contained both `@param array $x` and `@psalm-param array<TKey, T> $x`, the parameter type was resolved from whichever tag appeared first in the docblock. Now `@phpstan-param` always takes precedence over `@psalm-param`, which always takes precedence over `@param`, matching PHPStan and Psalm behaviour. This fixes generic constructor inference when stubs use paired `@param`/`@psalm-param` tags.
3132
- **`static` return type through first-class callables.** `self::method(...)()`, `static::method(...)()`, `parent::method(...)()`, and `$this->method(...)()` now preserve `static` in the return type when the underlying method declares `@return static`.
3233
- **ArrayAccess array-access assignment.** `$obj[$key] = $val` on objects implementing `ArrayAccess` no longer overwrites the variable's generic type annotation with an array type.
3334
- **Method-level `@template` with `key-of` bound.** When a method declares `@template K as key-of<TData>` and returns `TData[K]`, passing a string literal argument (e.g. `$config->get('name')`) now infers K from the literal and resolves the return type to the specific array shape value type.

docs/todo/bugs.md

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,31 @@ to second-guess upstream output.
2020
Remaining failures have multiple root causes (the original
2121
multi-namespace theory was incorrect for most of them):
2222

23-
- **Lines 16, 29, 41, 56, 68:** Generic constructor inference
24-
through iterator decorators (`CachingIterator(new ArrayIterator(...))`)
25-
does not propagate template parameters. Fails in single-namespace
26-
files too.
27-
- **Line 602:** Union generic method resolution (`C<A>|C<B>``->get()`)
28-
does not resolve per-branch template substitutions.
29-
- **Line 752:** `new ArrayCollection()` with no args infers
30-
`ArrayCollection<array, array>` instead of `ArrayCollection<never, never>`.
31-
- **Line 788:** Static method call `Collection::fromClassString(A::class)`
32-
does not propagate the method-level template to the return type.
33-
34-
**Fixed:** Line 122 — `@var` docblocks with additional tags
35-
(e.g. `@psalm-suppress`) after the type corrupted the type string.
36-
Fixed in `parse_inline_var_docblock_no_var`.
23+
- **Lines 16, 29, 41, 56, 68:** SPL iterator stubs
24+
(`CachingIterator`, `InfiniteIterator`, `LimitIterator`,
25+
`CallbackFilterIterator`, `NoRewindIterator`) lack `@template`
26+
annotations, so generic type propagation through iterator
27+
decorator constructors is impossible with current stubs.
28+
- **Lines 602, 788:** Union generic method resolution and static
29+
method template inference work correctly in single-namespace
30+
files. The failures are caused by the multi-namespace test
31+
runner not resolving short class names to FQN across namespace
32+
blocks in the same file.
33+
34+
**Fixed:**
35+
- Line 122 — `@var` docblocks with additional tags
36+
(e.g. `@psalm-suppress`) after the type corrupted the type
37+
string. Fixed in `parse_inline_var_docblock_no_var`.
38+
- Line 752 — `new ArrayCollection([])` inferred
39+
`ArrayCollection<array, array>` instead of
40+
`ArrayCollection<never, never>`. Root cause: when both `@param`
41+
and `@psalm-param` existed for the same parameter,
42+
`extract_param_raw_type_from_info` returned the first match in
43+
document order instead of respecting `@phpstan-param` >
44+
`@psalm-param` > `@param` priority.
3745

3846
**Tests:** SKIPs in `tests/psalm_assertions/template_class_template.php`
39-
(lines 16, 29, 41, 56, 68, 602, 752, 788).
47+
(lines 16, 29, 41, 56, 68, 602, 788).
4048

4149

4250

@@ -64,11 +72,10 @@ tests that may now pass. Run
6472
`cargo nextest run --test assert_type_runner --no-fail-fast` with
6573
the SKIP removed to verify.
6674

67-
Remaining SKIPs (12) are:
68-
- `template_class_template.php` (8) — B14 multi-namespace and
69-
genuine type engine gaps (union generic method resolution,
70-
generic constructor inference with `never`, static method
71-
generic inference)
75+
Remaining SKIPs (11) are:
76+
- `template_class_template.php` (7) — B14: SPL stubs lack
77+
@template annotations (5), multi-namespace test runner
78+
limitation (2)
7279
- `magic_method_annotation.php` (3) — B14 cross-namespace
7380
resolution in single-file test runner
7481
- `mixin_annotation.php` (1) — `IteratorIterator` not in fixture

src/docblock/tags.rs

Lines changed: 46 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -598,21 +598,30 @@ pub fn extract_param_raw_type(docblock: &str, var_name: &str) -> Option<PhpType>
598598

599599
/// Like [`extract_param_raw_type`], but operates on a pre-parsed [`DocblockInfo`].
600600
pub fn extract_param_raw_type_from_info(info: &DocblockInfo, var_name: &str) -> Option<PhpType> {
601-
for tag in info.tags_by_kinds(&[TagKind::PhpstanParam, TagKind::PsalmParam, TagKind::Param]) {
602-
let desc = tag.description.trim();
603-
if desc.is_empty() {
604-
continue;
605-
}
606-
607-
// Extract the full type token (respects `<…>` nesting).
608-
let (type_token, remainder) = split_type_token(desc);
601+
// Check tags in priority order: @phpstan-param > @psalm-param > @param.
602+
// When both `@param` and `@psalm-param` exist for the same parameter,
603+
// the more specific variant must win. Because `tags_by_kinds` yields
604+
// tags in document order, iterating all kinds at once would return
605+
// whichever appears first in the docblock. Instead, check each
606+
// priority level separately so that `@phpstan-param` always beats
607+
// `@psalm-param` which always beats `@param`.
608+
for kind in &[TagKind::PhpstanParam, TagKind::PsalmParam, TagKind::Param] {
609+
for tag in info.tags_by_kind(*kind) {
610+
let desc = tag.description.trim();
611+
if desc.is_empty() {
612+
continue;
613+
}
609614

610-
// The next token should be the parameter name.
611-
// Handle `...$name` (variadic) by stripping the leading `...`.
612-
if let Some(name) = remainder.split_whitespace().next() {
613-
let name = name.strip_prefix("...").unwrap_or(name);
614-
if name == var_name {
615-
return sanitise_and_parse_docblock_type(type_token);
615+
// Extract the full type token (respects `<…>` nesting).
616+
let (type_token, remainder) = split_type_token(desc);
617+
618+
// The next token should be the parameter name.
619+
// Handle `...$name` (variadic) by stripping the leading `...`.
620+
if let Some(name) = remainder.split_whitespace().next() {
621+
let name = name.strip_prefix("...").unwrap_or(name);
622+
if name == var_name {
623+
return sanitise_and_parse_docblock_type(type_token);
624+
}
616625
}
617626
}
618627
}
@@ -640,26 +649,31 @@ pub fn extract_all_param_tags(docblock: &str) -> Vec<(String, PhpType)> {
640649
/// Like [`extract_all_param_tags`], but operates on a pre-parsed [`DocblockInfo`].
641650
pub fn extract_all_param_tags_from_info(info: &DocblockInfo) -> Vec<(String, PhpType)> {
642651
let mut results = Vec::new();
652+
let mut seen_params = std::collections::HashSet::new();
643653

644-
// Only match `@param` and `@phpstan-param`, not compound tags like
645-
// `@param-closure-this` (those have their own TagKind).
646-
for tag in info.tags_by_kinds(&[TagKind::PhpstanParam, TagKind::PsalmParam, TagKind::Param]) {
647-
let desc = tag.description.trim();
648-
if desc.is_empty() {
649-
continue;
650-
}
651-
652-
// Extract the full type token (respects `<…>` nesting).
653-
let (type_token, remainder) = split_type_token(desc);
654+
// Iterate in priority order: @phpstan-param > @psalm-param > @param.
655+
// When both `@param` and `@psalm-param` exist for the same parameter,
656+
// the more specific variant wins.
657+
for kind in &[TagKind::PhpstanParam, TagKind::PsalmParam, TagKind::Param] {
658+
for tag in info.tags_by_kind(*kind) {
659+
let desc = tag.description.trim();
660+
if desc.is_empty() {
661+
continue;
662+
}
654663

655-
// The next token should be the parameter name.
656-
// Handle `...$name` (variadic) by stripping the leading `...`.
657-
if let Some(name) = remainder.split_whitespace().next() {
658-
let name = name.strip_prefix("...").unwrap_or(name);
659-
if name.starts_with('$')
660-
&& let Some(parsed) = sanitise_and_parse_docblock_type(type_token)
661-
{
662-
results.push((name.to_string(), parsed));
664+
// Extract the full type token (respects `<…>` nesting).
665+
let (type_token, remainder) = split_type_token(desc);
666+
667+
// The next token should be the parameter name.
668+
// Handle `...$name` (variadic) by stripping the leading `...`.
669+
if let Some(name) = remainder.split_whitespace().next() {
670+
let name = name.strip_prefix("...").unwrap_or(name);
671+
if name.starts_with('$')
672+
&& seen_params.insert(name.to_string())
673+
&& let Some(parsed) = sanitise_and_parse_docblock_type(type_token)
674+
{
675+
results.push((name.to_string(), parsed));
676+
}
663677
}
664678
}
665679
}

tests/psalm_assertions/template_class_template.php

Lines changed: 9 additions & 8 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 generic iterator constructor inference
16+
assertType('null|string', $value); // SKIP SPL stubs lack @template annotations
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 generic iterator constructor inference
29+
assertType('null|string', $value); // SKIP SPL stubs lack @template annotations
3030
}
3131

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

41-
assertType('null|string', $value); // SKIP generic iterator constructor inference
41+
assertType('null|string', $value); // SKIP SPL stubs lack @template annotations
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 generic iterator constructor inference
56+
assertType('null|string', $value); // SKIP SPL stubs lack @template annotations
5757
}
5858

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

68-
assertType('null|string', $value); // SKIP generic iterator constructor inference
68+
assertType('null|string', $value); // SKIP SPL stubs lack @template annotations
6969
}
7070

71+
7172
// Test: classTemplate
7273
namespace PsalmTest_template_class_template_6 {
7374
class A {}
@@ -599,7 +600,7 @@ public function get() {
599600
$a_or_b = $random_collection->get();
600601

601602
assertType('C<A>|C<B>', $random_collection);
602-
assertType('A|B', $a_or_b); // SKIP union generic method resolution
603+
assertType('A|B', $a_or_b); // SKIP multi-namespace test runner limitation (works in real code)
603604
}
604605

605606
// Test: templatedGet
@@ -749,7 +750,7 @@ public function add($key, $t) : void {
749750
}
750751
}
751752

752-
assertType('ArrayCollection<never, never>', $a); // SKIP generic constructor inference (never)
753+
assertType('ArrayCollection<never, never>', $a);
753754
}
754755

755756
// Test: unionClassStringInferenceAndDefaultEmptyArray
@@ -785,7 +786,7 @@ public static function fromClassString(string $classString, array $elements = []
785786
}
786787
}
787788

788-
assertType('Collection<A>', $packages); // SKIP static method call generic inference
789+
assertType('Collection<A>', $packages); // SKIP multi-namespace test runner limitation (works in real code)
789790
}
790791

791792
// Test: newWithoutInferredTemplate

0 commit comments

Comments
 (0)