Skip to content

Commit 004ef4f

Browse files
committed
Fix closure param inference for function-level templates and property
chains
1 parent 46172a9 commit 004ef4f

11 files changed

Lines changed: 360 additions & 45 deletions

File tree

docs/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5656

5757
### Fixed
5858

59+
- **Closure parameter inference from function-level `@template` bindings.** Functions like `array_any`, `array_all`, and `array_find` that declare `@template` parameters bound through an array parameter and used in a `callable` parameter now infer concrete types for untyped closure arguments. For example, `array_any($this->items, fn($item) => $item->name !== '')` where `$this->items` is `array<int, Product>` now resolves `$item` as `Product`. Previously `$item` was unresolved, producing false-positive `unresolved_member_access` diagnostics. This also fixes a related issue where `@param` tags in phpstorm-stubs that omit the parameter name (e.g. `@param callable(TValue, TKey): bool` without `$callback`) failed to enrich the parameter's type hint, preventing callable parameter type extraction.
60+
- **Property chain arguments in template substitution.** Expressions like `$this->items` or `$obj->prop` passed as arguments to templated functions now resolve their type for template binding. Previously only bare variables (e.g. `$items`) were resolved, so `array_any($this->items, fn($x) => ...)` could not infer the element type even when the property had a fully substituted generic type.
5961
- **Variable reassignment inside `try` blocks now tracked by the analyzer.** When a variable was reassigned to a different type inside a `try` block, subsequent accesses within the same block still resolved against the original type, producing false-positive `unknown_member` diagnostics. The same applied to `catch` and `finally` blocks. Assignments preceding the cursor within the same block are now treated as sequential.
6062
- **Types with `covariant` or `contravariant` variance annotations in generic args now parse correctly.** PHPStan/Larastan annotations like `BelongsTo<Category, covariant $this>` previously failed to parse, causing the entire type to become unresolvable. This broke Eloquent relationship property synthesis (virtual properties were not generated when the `@return` annotation used variance modifiers) and member lookup on relationship method return types (e.g. `$model->category()->associate()` reported `associate()` as not found). The variance keywords are now stripped from generic parameter positions before parsing.
6163
- **Diagnostics now work for vendor files open in the editor.** Projects using `--prefer-source` or monorepo setups where libraries are edited from the `vendor/` directory no longer have diagnostics suppressed. Previously all vendor files were unconditionally skipped, hiding syntax errors, deprecation warnings, and all other diagnostics. The `analyze` command also accepts vendor paths as explicit overrides (e.g. `phpantom_lsp analyze vendor/some-package/src/`).

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ within the same impact tier.
2323

2424
| # | Item | Impact | Effort |
2525
| --- | ------------------------------------------------------------------------------------------------------------------------------------------------ | ------ | ------ |
26-
| B5 | [`$this->items` on custom Collection subclass not typed](todo/bugs.md#b5-thisitems-on-custom-collection-subclass-not-typed) | Low | Medium |
2726
| B6 | [Scope methods not found on Builder in analyzer chains](todo/bugs.md#b6-scope-methods-not-found-on-builder-in-analyzer-chains) | High | Medium |
2827
| B7 | [PHPDoc `@param` generic array type not merged with native `array` hint](todo/bugs.md#b7-phpdoc-param-generic-array-type-not-merged-with-native-array-hint) | Low | Medium |
2928
| B8 | [Variadic parameter element type lost in `foreach`](todo/bugs.md#b8-variadic-parameter-element-type-lost-in-foreach) | Low | Low |

docs/todo/bugs.md

Lines changed: 1 addition & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,5 @@
11
# PHPantom — Bug Fixes
22

3-
## B5: `$this->items` on custom Collection subclass not typed
4-
5-
When a class extends `Collection<int, T>` via `@extends`, accessing
6-
`$this->items` should yield `array<int, T>`. Currently, `$this->items`
7-
resolves as `array` (the base `Collection`'s declared property type)
8-
without applying the generic substitution. This means iterating
9-
`$this->items` in a `foreach` or passing it to `array_any()` loses the
10-
element type.
11-
12-
Real-world example — `PurchaseFileProductCollection.php`:
13-
14-
```php
15-
/**
16-
* @extends Collection<int, PurchaseFileProduct>
17-
*/
18-
final class PurchaseFileProductCollection extends Collection
19-
{
20-
public function hasIssues(): bool
21-
{
22-
return array_any($this->items, fn($item) => $item->order_amount > 0);
23-
// ^^^^^ unresolved
24-
}
25-
}
26-
```
27-
28-
`$this->items` should be `array<int, PurchaseFileProduct>`, so `$item`
29-
in the closure should be `PurchaseFileProduct`. Instead, `$item` is
30-
unresolved because the generic substitution is not applied to inherited
31-
properties when accessed via `$this->`.
32-
33-
**Impact:** 2 diagnostics in the shared project
34-
(`PurchaseFileProductCollection:25` — two property accesses on `$item`).
35-
363
## B6: Scope methods not found on Builder in analyzer chains
374

385
PHPantom's completion engine correctly injects scope methods onto
@@ -161,4 +128,4 @@ case-insensitive matching) when looking up relationship-derived virtual
161128
properties on Eloquent models.
162129

163130
**Impact:** 1 direct diagnostic (`FlowService:477`) plus 1 cascading
164-
(`FlowService:517` — compound with `Collection::reduce()` type loss).
131+
(`FlowService:517` — compound with `Collection::reduce()` type loss).

example.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1772,6 +1772,12 @@ public function demo(): void
17721772

17731773
// Arrow function variant
17741774
$pipeline->tap(fn($p) => $p->through([]));
1775+
1776+
// Function-level @template callable inference
1777+
// array_any(@param array<TKey, TValue>, @param callable(TValue, TKey): bool)
1778+
// $item is inferred as Pen from the array's element type via template substitution
1779+
$holder = new ScaffoldingTemplateCallableHolder();
1780+
array_any($holder->tools, fn($item) => $item->write() !== '');
17751781
}
17761782
}
17771783

@@ -3497,6 +3503,12 @@ class ScaffoldingClosureParamInference
34973503
public function __construct() { $this->items = new FluentCollection([new Pen('red'), new Pen('blue')]); }
34983504
}
34993505

3506+
class ScaffoldingTemplateCallableHolder
3507+
{
3508+
/** @var array<int, Pen> */
3509+
public array $tools = [];
3510+
}
3511+
35003512
/**
35013513
* @template TValue
35023514
*/
@@ -5305,6 +5317,12 @@ function runDemoAssertions(): void
53055317
});
53065318
assert(count($closureReceived) === 2, 'each() must invoke callback for every item');
53075319

5320+
// Function-level @template callable inference (array_any pattern)
5321+
$tplHolder = new ScaffoldingTemplateCallableHolder();
5322+
$tplHolder->tools = [new Pen('red'), new Pen('blue')];
5323+
$tplResult = array_any($tplHolder->tools, fn($t) => $t->color() === 'red');
5324+
assert($tplResult === true, 'array_any with template callable must work');
5325+
53085326
// ── Type alias resolution ───────────────────────────────────────────
53095327
$aliasDemo = new TypeAliasDemo();
53105328
$userData = $aliasDemo->getUserData();

src/completion/variable/closure_resolution.rs

Lines changed: 66 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -994,7 +994,14 @@ fn try_resolve_in_closure_call<'b>(
994994
&fc.argument_list.arguments,
995995
ctx,
996996
results,
997-
|arg_idx| infer_callable_params_from_function(&func_name, arg_idx, ctx),
997+
|arg_idx| {
998+
infer_callable_params_from_function(
999+
&func_name,
1000+
arg_idx,
1001+
&fc.argument_list.arguments,
1002+
ctx,
1003+
)
1004+
},
9981005
)
9991006
{
10001007
return true;
@@ -1429,9 +1436,17 @@ fn extract_function_name_from_call(fc: &FunctionCall<'_>) -> Option<String> {
14291436

14301437
/// Infer callable parameter types for a closure passed at position
14311438
/// `arg_idx` to a standalone function call.
1439+
///
1440+
/// When the function has `@template` parameters (e.g.
1441+
/// `array_any(array<TKey, TValue> $array, callable(TValue, TKey): bool $cb)`),
1442+
/// the template substitution map is built from the other call-site
1443+
/// arguments and applied to the callable's parameter types. This turns
1444+
/// raw template names like `TValue` into concrete types like
1445+
/// `PurchaseFileProduct`.
14321446
fn infer_callable_params_from_function(
14331447
func_name: &str,
14341448
arg_idx: usize,
1449+
arguments: &TokenSeparatedSequence<'_, argument::Argument<'_>>,
14351450
ctx: &VarResolutionCtx<'_>,
14361451
) -> Vec<String> {
14371452
let rctx = ctx.as_resolution_ctx();
@@ -1441,12 +1456,61 @@ fn infer_callable_params_from_function(
14411456
None
14421457
};
14431458
if let Some(fi) = func_info {
1444-
extract_callable_params_at(&fi.parameters, arg_idx, ctx)
1459+
let mut params = extract_callable_params_at(&fi.parameters, arg_idx, ctx);
1460+
1461+
// When the function has template parameters, build a
1462+
// substitution map from the concrete call-site arguments and
1463+
// apply it to the extracted callable param types. Without
1464+
// this, functions like `array_any($this->items, fn($item) => …)`
1465+
// would pass raw template names (`TValue`) instead of the
1466+
// concrete element type (`PurchaseFileProduct`).
1467+
if !params.is_empty() && !fi.template_params.is_empty() && !fi.template_bindings.is_empty()
1468+
{
1469+
let text_args = extract_argument_texts(arguments, ctx.content);
1470+
let text_args_joined = text_args.join(", ");
1471+
let subs =
1472+
super::rhs_resolution::build_function_template_subs(&fi, &text_args_joined, &rctx);
1473+
if !subs.is_empty() {
1474+
params = params
1475+
.into_iter()
1476+
.map(|p| {
1477+
let substituted = crate::inheritance::apply_substitution(&p, &subs);
1478+
substituted.into_owned()
1479+
})
1480+
.collect();
1481+
}
1482+
}
1483+
1484+
params
14451485
} else {
14461486
vec![]
14471487
}
14481488
}
14491489

1490+
/// Extract the source text of each positional argument in a call's
1491+
/// argument list.
1492+
fn extract_argument_texts(
1493+
arguments: &TokenSeparatedSequence<'_, argument::Argument<'_>>,
1494+
content: &str,
1495+
) -> Vec<String> {
1496+
arguments
1497+
.iter()
1498+
.map(|arg| {
1499+
let span = match arg {
1500+
argument::Argument::Positional(pos) => pos.value.span(),
1501+
argument::Argument::Named(named) => named.value.span(),
1502+
};
1503+
let start = span.start.offset as usize;
1504+
let end = span.end.offset as usize;
1505+
if end <= content.len() {
1506+
content[start..end].to_string()
1507+
} else {
1508+
String::new()
1509+
}
1510+
})
1511+
.collect()
1512+
}
1513+
14501514
/// Infer callable parameter types for a closure passed at position
14511515
/// `arg_idx` to an instance method call whose receiver expression
14521516
/// spans `[obj_start, obj_end)` in the source text.

src/completion/variable/rhs_resolution.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,6 +1035,32 @@ fn resolve_arg_variable_raw_type(
10351035
return None;
10361036
}
10371037

1038+
// ── Property chain: `$this->items`, `$obj->prop` ────────────
1039+
// When the argument is a property access chain, resolve the base
1040+
// object's type and look up the property's type hint. This is
1041+
// needed for template substitution in calls like
1042+
// `array_any($this->items, fn($item) => …)` where `$this->items`
1043+
// is `array<int, PurchaseFileProduct>` after generic substitution.
1044+
if let Some(arrow_pos) = var_name.find("->") {
1045+
let base = &var_name[..arrow_pos];
1046+
let prop = &var_name[arrow_pos + 2..];
1047+
// Only handle simple single-level property access for now.
1048+
if !prop.is_empty() && !prop.contains("->") && !prop.contains('(') {
1049+
let base_classes = crate::completion::resolver::resolve_target_classes(
1050+
base,
1051+
crate::types::AccessKind::Arrow,
1052+
rctx,
1053+
);
1054+
for cls in &base_classes {
1055+
if let Some(hint) =
1056+
crate::inheritance::resolve_property_type_hint(cls, prop, rctx.class_loader)
1057+
{
1058+
return Some(hint.to_string());
1059+
}
1060+
}
1061+
}
1062+
}
1063+
10381064
// 1. Try docblock annotation (@var).
10391065
if let Some(raw) = crate::docblock::find_iterable_raw_type_in_source(
10401066
rctx.content,

src/diagnostics/unknown_members.rs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5035,4 +5035,67 @@ class Handler {
50355035
"expected no diagnostics for reassigned variable inside catch block, got: {diags:?}"
50365036
);
50375037
}
5038+
5039+
#[test]
5040+
fn no_diagnostic_for_this_items_on_generic_collection_subclass() {
5041+
// B5: When a class extends `Collection<int, T>` via `@extends`,
5042+
// accessing `$this->items` should yield `array<int, T>` with the
5043+
// generic substitution applied. Iterating `$this->items` in a
5044+
// `foreach` or passing it to `array_any()` should resolve the
5045+
// element type so that property access on `$item` works.
5046+
let php = r#"<?php
5047+
/**
5048+
* @template TKey
5049+
* @template TValue
5050+
*/
5051+
class Collection {
5052+
/** @var array<TKey, TValue> */
5053+
public array $items = [];
5054+
5055+
/** @return TValue|null */
5056+
public function first(): mixed { return null; }
5057+
}
5058+
5059+
class PurchaseFileProduct {
5060+
public int $order_amount = 0;
5061+
public string $name = '';
5062+
}
5063+
5064+
/**
5065+
* @template TKey
5066+
* @template TValue
5067+
* @param array<TKey, TValue> $array
5068+
* @param callable(TValue, TKey): bool $callback
5069+
* @return bool
5070+
*/
5071+
function array_any(array $array, callable $callback): bool { return false; }
5072+
5073+
/**
5074+
* @extends Collection<int, PurchaseFileProduct>
5075+
*/
5076+
final class PurchaseFileProductCollection extends Collection {
5077+
public function hasIssues(): bool {
5078+
return array_any($this->items, fn($item) => $item->order_amount > 0);
5079+
}
5080+
5081+
public function hasName(): bool {
5082+
return array_any($this->items, fn($item) => $item->name !== '');
5083+
}
5084+
5085+
public function foreachWorks(): void {
5086+
foreach ($this->items as $item) {
5087+
$item->order_amount;
5088+
$item->name;
5089+
}
5090+
}
5091+
}
5092+
"#;
5093+
let backend = Backend::new_test();
5094+
backend.config.lock().diagnostics.unresolved_member_access = Some(true);
5095+
let diags = collect(&backend, "file:///test.php", php);
5096+
assert!(
5097+
diags.is_empty(),
5098+
"expected no diagnostics for $this->items on generic Collection subclass, got: {diags:?}"
5099+
);
5100+
}
50385101
}

src/docblock/mod.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,15 @@ pub use tags::{
5151
extract_mixin_tags, extract_mixin_tags_from_info, extract_param_closure_this,
5252
extract_param_closure_this_from_info, extract_param_description,
5353
extract_param_description_from_info, extract_param_raw_type, extract_param_raw_type_from_info,
54-
extract_removed_version, extract_return_description, extract_return_description_from_info,
55-
extract_return_type, extract_return_type_from_info, extract_see_references,
56-
extract_see_references_from_info, extract_throws_tags, extract_throws_tags_from_info,
57-
extract_type_assertions, extract_type_assertions_from_info, extract_var_type,
58-
extract_var_type_from_info, extract_var_type_with_name, extract_var_type_with_name_from_info,
59-
find_enclosing_return_type, find_inline_var_docblock, find_iterable_raw_type_in_source,
60-
find_var_raw_type_in_source, get_docblock_info_for_node, get_docblock_text_for_node,
61-
has_deprecated_tag, has_deprecated_tag_from_info, resolve_effective_type, should_override_type,
54+
extract_param_types_positional_from_info, extract_removed_version, extract_return_description,
55+
extract_return_description_from_info, extract_return_type, extract_return_type_from_info,
56+
extract_see_references, extract_see_references_from_info, extract_throws_tags,
57+
extract_throws_tags_from_info, extract_type_assertions, extract_type_assertions_from_info,
58+
extract_var_type, extract_var_type_from_info, extract_var_type_with_name,
59+
extract_var_type_with_name_from_info, find_enclosing_return_type, find_inline_var_docblock,
60+
find_iterable_raw_type_in_source, find_var_raw_type_in_source, get_docblock_info_for_node,
61+
get_docblock_text_for_node, has_deprecated_tag, has_deprecated_tag_from_info,
62+
resolve_effective_type, should_override_type,
6263
};
6364

6465
// Template / generics / type alias tags

src/docblock/tags.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,46 @@ pub fn extract_all_param_tags_from_info(info: &DocblockInfo) -> Vec<(String, Str
645645
results
646646
}
647647

648+
/// Extract `@param` type strings in order of appearance, including
649+
/// tags that omit the parameter name.
650+
///
651+
/// Returns a list of `(Option<param_name>, type_string)` pairs in
652+
/// docblock order. When a `@param` tag has no `$name` token (common
653+
/// in phpstorm-stubs, e.g. `@param callable(TValue, TKey): bool`),
654+
/// the first element is `None`. Callers can match these entries to
655+
/// native parameters by position.
656+
///
657+
/// This is used as a positional fallback when name-based matching
658+
/// via [`extract_param_raw_type_from_info`] fails to find a docblock
659+
/// type for a parameter.
660+
pub fn extract_param_types_positional_from_info(
661+
info: &DocblockInfo,
662+
) -> Vec<(Option<String>, String)> {
663+
let mut results = Vec::new();
664+
665+
for tag in info.tags_by_kinds(&[TagKind::PhpstanParam, TagKind::Param]) {
666+
let desc = tag.description.trim();
667+
if desc.is_empty() {
668+
continue;
669+
}
670+
671+
let (type_token, remainder) = split_type_token(desc);
672+
673+
let param_name = remainder.split_whitespace().next().and_then(|name| {
674+
let name = name.strip_prefix("...").unwrap_or(name);
675+
if name.starts_with('$') {
676+
Some(name.to_string())
677+
} else {
678+
None
679+
}
680+
});
681+
682+
results.push((param_name, type_token.to_string()));
683+
}
684+
685+
results
686+
}
687+
648688
/// Extract all `@param-closure-this` declarations from a docblock.
649689
///
650690
/// The tag format is `@param-closure-this TypeName $paramName`, declaring

0 commit comments

Comments
 (0)