Skip to content

Commit c137a81

Browse files
committed
Closure/arrow-function parameters passed after reordering named
arguments now infer correctly
1 parent 91b687c commit c137a81

6 files changed

Lines changed: 144 additions & 87 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4646

4747
- **Completion works after closing a multi-line closure argument on the same line as the next chain operator.** Typing `->` immediately after a call like `->map(function (...) { ... })->` now resolves the full receiver instead of returning no member suggestions until the operator is moved to a new line. Contributed by @calebdw.
4848
- **Conditional return types recognize interpolated strings as strings.** A function with a conditional return type like `($key is string ? mixed : null)` now correctly resolves the `string` branch when called with an interpolated string argument (e.g. `config("{$prefix}.host")`). Previously the interpolated string was not recognized as a string literal, causing the return type to fall through to the else branch and resolve as `null`. Contributed by @calebdw.
49+
- **Closure/arrow-function parameters passed after reordering named arguments now infer correctly.** When a call used named arguments to reorder or skip parameters ahead of a closure argument (e.g. `process(class: Product::class, cb: function ($p) {...}, flag: true)`), the closure's own parameter type stopped being inferred from the target function/method's `callable(...)`/`Closure(...)` signature, silently losing completions and hover for the closure's parameters. Argument-to-parameter binding now follows PHP's actual named-argument rules instead of assuming call position matches declared position.
4950
- **Deprecated enum cases are recognized.** Enum cases annotated with `@deprecated` or `#[Deprecated]` now carry deprecation metadata, so usages like `self::Low` can be highlighted as deprecated in contextual semantic-token mode. Contributed by @calebdw.
5051
- **Class constant accesses use constant semantic highlighting.** `self::CONSTANT`, `static::CONSTANT`, `parent::CONSTANT`, and `ClassName::CONSTANT` now emit the `enumMember` semantic token instead of being colored as properties, while `ClassName::$property` still emits `property`. Contributed by @calebdw.
5152
- **PHP attributes use decorator semantic highlighting.** Attribute class names in `#[...]` now emit the `decorator` semantic token instead of `class`, so editor themes can color attributes differently from normal class references. Contributed by @calebdw.

docs/todo/bugs.md

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -64,60 +64,3 @@ own reasons (semantic tokens at `semantic_tokens.rs:757-770`, hover at
6464
There is no existing test coverage for promoted-property rename or
6565
references (`src/rename/tests.rs` and `tests/integration/references.rs`
6666
have no `__construct`/promoted-property cases).
67-
68-
## B115. Closure-parameter inference uses call-position instead of the declared parameter index, so named arguments that reorder a call break it
69-
70-
**Severity: Low-Medium (closure/arrow-function parameters silently lose
71-
their inferred type — falling back to no completions/hover — for any
72-
call that uses named arguments to reorder or skip parameters ahead of
73-
the callable argument) · Discovered while deduplicating the receiver
74-
resolution helpers shared between `closure_resolution.rs` and
75-
`forward_walk/callable_inference.rs`; confirmed with a regression test**
76-
77-
```php
78-
/**
79-
* @template T
80-
* @param bool $flag
81-
* @param class-string<T> $class
82-
* @param callable(T): void $cb
83-
*/
84-
function process(bool $flag, string $class, callable $cb): void {}
85-
86-
process(class: Product::class, flag: true, function ($p) {
87-
$p-> // no completions: T never binds to Product
88-
});
89-
```
90-
91-
The forward walker's closure-argument inference
92-
(`walk_closures_in_call_args` and `walk_closure_in_partial_call_args` in
93-
`src/type_engine/variable/forward_walk/diagnostic_walk.rs`, and the
94-
completion/hover-path counterpart `infer_callable_params_for_call` in
95-
`src/type_engine/variable/forward_walk/callable_inference.rs`) computes
96-
`arg_idx` from `arguments.iter().enumerate()` — the argument's position
97-
in the *call*. That index is then used directly as the *declared*
98-
parameter index, both to decide whether the argument at that slot is a
99-
closure and to look up `fi.parameters[arg_idx]` /
100-
`class.get_method(..).parameters[arg_idx]` for the callable's own
101-
`callable(...)`/`Closure(...)` type hint (see
102-
`extract_callable_params_at_fw` and its callers).
103-
104-
PHP argument binding does not guarantee call-position equals declared
105-
position once named arguments are involved: a named argument can appear
106-
anywhere in the call, and named arguments before the closure can be
107-
reordered or a preceding optional parameter can be omitted entirely.
108-
Whenever that happens, `arg_idx` no longer identifies the closure's own
109-
parameter, so `extract_callable_params_at_fw` looks up the wrong
110-
parameter (typically a non-callable one), returns no callable param
111-
types, and the closure gets none of its parameters inferred — even
112-
though the same call resolves correctly when arguments are positional
113-
and in declaration order.
114-
115-
Fix by resolving each argument's *declared* parameter index (via
116-
something like the existing `bind_text_args_to_params` /
117-
`param_name_bare` machinery in `src/call_args.rs`, which already knows
118-
how to route named arguments to their declared slot) before using that
119-
index to look up the callable's own parameter type or to decide which
120-
argument is "the closure at position N". This affects both the
121-
diagnostic-path walker (`diagnostic_walk.rs`) and the completion/hover
122-
path (`callable_inference.rs`), which must stay in sync per the
123-
project's shared-pipeline convention.

src/call_args.rs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,50 @@ pub(crate) fn bind_args_to_params<'b>(
6767
bound
6868
}
6969

70+
/// For each argument in `argument_list` (by call position), resolve the
71+
/// declared parameter index in `params` that it binds to under PHP's
72+
/// binding rules.
73+
///
74+
/// This is the call-position-to-declared-index counterpart of
75+
/// [`bind_args_to_params`] (which goes the other way: declared index to
76+
/// bound expression). Consumers that key off "the Nth argument in the
77+
/// call" — e.g. closure-argument inference matching a `callable(...)`
78+
/// type hint to the closure's own declared parameter — must translate
79+
/// through this first, since a named argument can appear anywhere in the
80+
/// call and reorder or skip parameters ahead of it.
81+
///
82+
/// Returns a vector parallel to `argument_list.arguments`: entry `i` holds
83+
/// the declared parameter index bound to call-position `i`, or `None` when
84+
/// the argument matches no declared parameter (an unknown named argument,
85+
/// or a positional overflow with no variadic parameter).
86+
pub(crate) fn resolve_declared_arg_indices(
87+
params: &[ParameterInfo],
88+
argument_list: &ArgumentList<'_>,
89+
) -> Vec<Option<usize>> {
90+
let variadic_idx = params.iter().position(|p| p.is_variadic);
91+
let mut next_positional = 0usize;
92+
93+
argument_list
94+
.arguments
95+
.iter()
96+
.map(|arg| match arg {
97+
Argument::Positional(_) => {
98+
let target = if next_positional < params.len() {
99+
Some(next_positional)
100+
} else {
101+
variadic_idx
102+
};
103+
next_positional += 1;
104+
target
105+
}
106+
Argument::Named(named) => {
107+
let name = bytes_to_str(named.name.value);
108+
params.iter().position(|p| param_name_bare(p) == name)
109+
}
110+
})
111+
.collect()
112+
}
113+
70114
/// Split a textual argument into an optional parameter name and its value.
71115
///
72116
/// `"signature: Foo::class"` → `(Some("signature"), "Foo::class")`.

src/type_engine/variable/forward_walk/callable_inference.rs

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ pub(crate) fn infer_callable_params_from_function_fw(
4646
None
4747
};
4848
if let Some(fi) = func_info {
49-
let mut params = extract_callable_params_at_fw(&fi.parameters, arg_idx);
49+
let mut params = extract_callable_params_at_fw(&fi.parameters, argument_list, arg_idx);
5050

5151
if !params.is_empty() && !fi.template_params.is_empty() && !fi.template_bindings.is_empty()
5252
{
@@ -70,14 +70,15 @@ pub(crate) fn infer_callable_params_from_function_fw(
7070
/// Infer callable parameter types for a closure passed at position
7171
/// `arg_idx` to an instance method call.
7272
pub(crate) fn infer_callable_params_from_receiver_fw(
73-
obj_start: u32,
74-
obj_end: u32,
73+
obj_span: (u32, u32),
7574
method_name: &str,
7675
arg_idx: usize,
76+
argument_list: &ArgumentList<'_>,
7777
first_arg_text: Option<&str>,
7878
scope: &ScopeState,
7979
ctx: &ForwardWalkCtx<'_>,
8080
) -> Vec<PhpType> {
81+
let (obj_start, obj_end) = obj_span;
8182
let scope_locals = &scope.locals;
8283
let scope_resolver = |var_name: &str| -> Vec<ResolvedType> {
8384
scope_locals
@@ -105,7 +106,13 @@ pub(crate) fn infer_callable_params_from_receiver_fw(
105106
return override_params;
106107
}
107108

108-
let params = find_callable_params_on_classes_fw(&receiver_classes, method_name, arg_idx, ctx);
109+
let params = find_callable_params_on_classes_fw(
110+
&receiver_classes,
111+
method_name,
112+
arg_idx,
113+
argument_list,
114+
ctx,
115+
);
109116

110117
// Build a template substitution map from the receiver's generic
111118
// args. When the receiver resolves to e.g. `Builder<Product>`,
@@ -299,6 +306,7 @@ pub(crate) fn infer_callable_params_from_static_receiver_fw(
299306
class_expr: &Expression<'_>,
300307
method_name: &str,
301308
arg_idx: usize,
309+
argument_list: &ArgumentList<'_>,
302310
first_arg_text: Option<&str>,
303311
scope: &ScopeState,
304312
ctx: &ForwardWalkCtx<'_>,
@@ -334,7 +342,8 @@ pub(crate) fn infer_callable_params_from_static_receiver_fw(
334342
ctx.class_loader,
335343
ctx.resolved_class_cache,
336344
);
337-
let params = find_callable_params_on_method_fw(&resolved, method_name, arg_idx);
345+
let params =
346+
find_callable_params_on_method_fw(&resolved, method_name, arg_idx, argument_list);
338347

339348
// Build a template substitution map from the owner class.
340349
// When the owner is a generic class (e.g. `Builder<Customer>`
@@ -388,6 +397,7 @@ pub(crate) fn find_callable_params_on_classes_fw(
388397
classes: &[Arc<ClassInfo>],
389398
method_name: &str,
390399
arg_idx: usize,
400+
argument_list: &ArgumentList<'_>,
391401
ctx: &ForwardWalkCtx<'_>,
392402
) -> Vec<PhpType> {
393403
for cls in classes {
@@ -397,7 +407,7 @@ pub(crate) fn find_callable_params_on_classes_fw(
397407
// `resolve_class_fully_maybe_cached` would load the base
398408
// class definition (keyed by FQN with empty generic args),
399409
// discarding those substitutions.
400-
let result = find_callable_params_on_method_fw(cls, method_name, arg_idx);
410+
let result = find_callable_params_on_method_fw(cls, method_name, arg_idx, argument_list);
401411
if !result.is_empty() {
402412
return result;
403413
}
@@ -412,7 +422,8 @@ pub(crate) fn find_callable_params_on_classes_fw(
412422
ctx.class_loader,
413423
ctx.resolved_class_cache,
414424
);
415-
let result = find_callable_params_on_method_fw(&resolved, method_name, arg_idx);
425+
let result =
426+
find_callable_params_on_method_fw(&resolved, method_name, arg_idx, argument_list);
416427
if !result.is_empty() {
417428
return result;
418429
}
@@ -421,27 +432,39 @@ pub(crate) fn find_callable_params_on_classes_fw(
421432
}
422433

423434
/// Look up method `method_name` on `class` and extract callable
424-
/// parameter types from the parameter at position `arg_idx`.
435+
/// parameter types from the parameter bound to call-position `arg_idx`.
425436
pub(crate) fn find_callable_params_on_method_fw(
426437
class: &ClassInfo,
427438
method_name: &str,
428439
arg_idx: usize,
440+
argument_list: &ArgumentList<'_>,
429441
) -> Vec<PhpType> {
430442
let method = class.get_method(method_name);
431443
if let Some(m) = method {
432-
extract_callable_params_at_fw(&m.parameters, arg_idx)
444+
extract_callable_params_at_fw(&m.parameters, argument_list, arg_idx)
433445
} else {
434446
vec![]
435447
}
436448
}
437449

438-
/// Given a list of parameters, look at `arg_idx` and extract callable
439-
/// parameter types if the type hint is `callable(...)` or `Closure(...)`.
450+
/// Given a list of parameters and the call's argument list, resolve the
451+
/// call-position `arg_idx` to its *declared* parameter index (accounting
452+
/// for named arguments, which can reorder or skip parameters) and extract
453+
/// callable parameter types if that parameter's type hint is
454+
/// `callable(...)` or `Closure(...)`.
440455
pub(crate) fn extract_callable_params_at_fw(
441456
params: &[crate::types::ParameterInfo],
457+
argument_list: &ArgumentList<'_>,
442458
arg_idx: usize,
443459
) -> Vec<PhpType> {
444-
let param = params.get(arg_idx);
460+
let Some(declared_idx) = crate::call_args::resolve_declared_arg_indices(params, argument_list)
461+
.get(arg_idx)
462+
.copied()
463+
.flatten()
464+
else {
465+
return vec![];
466+
};
467+
let param = params.get(declared_idx);
445468
if let Some(p) = param
446469
&& let Some(ref hint) = p.type_hint
447470
&& let Some(callable_params) = hint.callable_param_types()
@@ -533,10 +556,10 @@ pub(crate) fn infer_callable_params_for_call(
533556
let first_arg =
534557
extract_first_arg_string_fw(&mc.argument_list.arguments, ctx.content);
535558
infer_callable_params_from_receiver_fw(
536-
obj_span.start.offset,
537-
obj_span.end.offset,
559+
(obj_span.start.offset, obj_span.end.offset),
538560
name,
539561
arg_idx,
562+
&mc.argument_list,
540563
first_arg.as_deref(),
541564
scope,
542565
ctx,
@@ -556,10 +579,10 @@ pub(crate) fn infer_callable_params_for_call(
556579
let first_arg =
557580
extract_first_arg_string_fw(&mc.argument_list.arguments, ctx.content);
558581
infer_callable_params_from_receiver_fw(
559-
obj_span.start.offset,
560-
obj_span.end.offset,
582+
(obj_span.start.offset, obj_span.end.offset),
561583
name,
562584
arg_idx,
585+
&mc.argument_list,
563586
first_arg.as_deref(),
564587
scope,
565588
ctx,
@@ -581,6 +604,7 @@ pub(crate) fn infer_callable_params_for_call(
581604
sc.class,
582605
name,
583606
arg_idx,
607+
&sc.argument_list,
584608
first_arg.as_deref(),
585609
scope,
586610
ctx,

src/type_engine/variable/forward_walk/diagnostic_walk.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -434,10 +434,10 @@ pub(crate) fn walk_closures_in_call<'b>(
434434
walk_closures_in_call_args(&mc.argument_list.arguments, outer_scope, ctx, |arg_idx| {
435435
if let Some(ref name) = method_name {
436436
infer_callable_params_from_receiver_fw(
437-
obj_span.start.offset,
438-
obj_span.end.offset,
437+
(obj_span.start.offset, obj_span.end.offset),
439438
name,
440439
arg_idx,
440+
&mc.argument_list,
441441
first_arg.as_deref(),
442442
outer_scope,
443443
ctx,
@@ -460,10 +460,10 @@ pub(crate) fn walk_closures_in_call<'b>(
460460
walk_closures_in_call_args(&mc.argument_list.arguments, outer_scope, ctx, |arg_idx| {
461461
if let Some(ref name) = method_name {
462462
infer_callable_params_from_receiver_fw(
463-
obj_span.start.offset,
464-
obj_span.end.offset,
463+
(obj_span.start.offset, obj_span.end.offset),
465464
name,
466465
arg_idx,
466+
&mc.argument_list,
467467
first_arg.as_deref(),
468468
outer_scope,
469469
ctx,
@@ -488,6 +488,7 @@ pub(crate) fn walk_closures_in_call<'b>(
488488
sc.class,
489489
name,
490490
arg_idx,
491+
&sc.argument_list,
491492
first_arg.as_deref(),
492493
outer_scope,
493494
ctx,

0 commit comments

Comments
 (0)