Skip to content

Commit af8ae58

Browse files
committed
Propagate variable rename into closure and arrow function captures
1 parent 44ebe97 commit af8ae58

3 files changed

Lines changed: 217 additions & 13 deletions

File tree

docs/CHANGELOG.md

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

5656
### Fixed
5757

58-
- **Blade diagnostics in `analyze` command.** Blade template files now receive full diagnostics (unknown members, unknown classes, unused variables, etc.) when running `phpantom_lsp analyze`.
59-
- **Blade unknown-member diagnostic positions.** Unknown-member diagnostics in Blade files now point to the correct line in the original template instead of the virtual PHP line number.
60-
- **False-positive unused `$loop` in Blade templates.** The `$loop` variable injected by `@foreach`/`@forelse` is no longer flagged as unused when the template body doesn't reference it.
58+
- **Rename propagates into closures and arrow functions.** Renaming a variable now follows explicit `use ($var)` captures into closure bodies and implicit captures into arrow function bodies, instead of leaving those occurrences unchanged.
6159
- **Spurious function auto-imports.** Import statements like `use function is_array;` were misidentified as function declarations, polluting the completion list with phantom entries that inserted incorrect imports.
6260
- **Duplicate `use function` insertion.** Accepting a function completion no longer inserts a `use function` statement when the exact import already exists in the file.
6361
- **Function import conflict handling.** When a different function with the same short name is already imported, completing a namespaced function now inserts the fully-qualified name instead of the ambiguous short name.
@@ -71,10 +69,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
7169
- **Foreach narrowing with break in else.** The variable state from break paths is now included in the post-loop type.
7270
- **Foreach target type after non-empty literal array.** The pre-loop sentinel value no longer survives as a possible post-loop type.
7371
- **Foreach over `::class` literal arrays resolves static access.** `$className::CONST` and `$className::method()` no longer produce unresolved-member diagnostics.
74-
- **Foreach loop variable resolution in Blade.** Loop variables now resolve their element type correctly when the iterable is typed via a standalone `@var` docblock.
7572
- **Hover on reassigned variable shows post-assignment type.** Hovering on the left-hand side of a reassignment now shows the type produced by the assignment.
76-
- **Blade hover positions.** Hover ranges in `.blade.php` files now highlight the correct symbol under the cursor.
77-
- **Blade echo delimiter hover.** Hovering on `{{` or `}}` now shows hover for the `e()` function.
7873
- **Multi-namespace class resolution.** Short class names now resolve against the correct namespace for the current scope.
7974
- **Multi-namespace variable isolation.** Variable resolution now only considers the namespace block containing the cursor.
8075
- **Multi-namespace function return type resolution.** Function return types are now resolved against the function's own namespace.

src/references/mod.rs

Lines changed: 166 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use std::sync::Arc;
3030
use tower_lsp::lsp_types::{Location, Position, Range, Url};
3131

3232
use crate::Backend;
33-
use crate::symbol_map::{SelfStaticParentKind, SymbolKind, SymbolMap};
33+
use crate::symbol_map::{SelfStaticParentKind, SymbolKind, SymbolMap, VarDefKind};
3434
use crate::types::{ClassInfo, MAX_INHERITANCE_DEPTH};
3535
use crate::util::{
3636
build_fqn, collect_php_files_gitignore, find_class_at_offset, offset_to_position,
@@ -253,17 +253,18 @@ impl Backend {
253253
Err(_) => return locations,
254254
};
255255

256+
// Build the set of reachable scopes: the primary scope plus any
257+
// closure/arrow-function scopes that capture this variable.
258+
let reachable_scopes = Self::collect_capture_scopes(symbol_map, var_name, scope_start);
259+
256260
for span in &symbol_map.spans {
257261
if let SymbolKind::Variable { name } = &span.kind {
258262
if name != var_name {
259263
continue;
260264
}
261-
// Check that this variable is in the same scope.
262-
// `find_variable_scope` correctly handles parameter
263-
// spans and docblock `@param` mentions that sit before
264-
// the body `{`.
265+
// Check that this variable is in a reachable scope.
265266
let span_scope = symbol_map.find_variable_scope(name, span.start);
266-
if span_scope != scope_start {
267+
if !reachable_scopes.contains(&span_scope) {
267268
continue;
268269
}
269270
// Optionally skip declaration sites.
@@ -291,7 +292,7 @@ impl Backend {
291292

292293
for def in &symbol_map.var_defs {
293294
if def.name == var_name
294-
&& def.scope_start == scope_start
295+
&& reachable_scopes.contains(&def.scope_start)
295296
&& seen_offsets.insert(def.offset)
296297
{
297298
let start = offset_to_position(content, def.offset as usize);
@@ -318,6 +319,164 @@ impl Backend {
318319
locations
319320
}
320321

322+
/// Collect all scopes reachable from `root_scope` for `var_name`
323+
/// through closure `use` captures and implicit arrow-function captures.
324+
///
325+
/// Returns a set containing `root_scope` plus every nested
326+
/// closure/arrow scope that captures the variable without shadowing
327+
/// it with a new parameter of the same name.
328+
fn collect_capture_scopes(
329+
symbol_map: &SymbolMap,
330+
var_name: &str,
331+
root_scope: u32,
332+
) -> HashSet<u32> {
333+
let mut reachable = HashSet::new();
334+
reachable.insert(root_scope);
335+
336+
// Explicit closure captures: `function () use ($var) { … }`
337+
// These have VarDefKind::ClosureCapture with scope_start
338+
// pointing to the closure body.
339+
for def in &symbol_map.var_defs {
340+
if def.name != var_name || def.kind != VarDefKind::ClosureCapture {
341+
continue;
342+
}
343+
// The `use ($var)` token sits physically in the outer scope.
344+
// Check if the outer scope is already reachable.
345+
let outer_scope = symbol_map.find_enclosing_scope(def.offset);
346+
if reachable.contains(&outer_scope) {
347+
reachable.insert(def.scope_start);
348+
}
349+
}
350+
351+
// Implicit arrow-function captures: `fn () => $var`
352+
// Arrow functions have a scope entry but no ClosureCapture def.
353+
// A variable is implicitly captured if:
354+
// 1. The arrow scope is directly nested in a reachable scope.
355+
// 2. There is no parameter with the same name in the arrow scope.
356+
for &(scope_start, _scope_end) in &symbol_map.scopes {
357+
if reachable.contains(&scope_start) {
358+
continue; // Already reachable, skip.
359+
}
360+
// Find the parent scope of this scope.
361+
let parent = symbol_map.find_enclosing_scope(scope_start.saturating_sub(1));
362+
if !reachable.contains(&parent) {
363+
continue;
364+
}
365+
// Check if this is an arrow function scope (no ClosureCapture
366+
// or Parameter def that would indicate a closure with `use`).
367+
// Arrow scopes don't have braces; their scope_start is the
368+
// arrow function expression's start offset.
369+
//
370+
// Skip if there's a parameter with the same name (shadowed).
371+
let has_shadowing_param = symbol_map.var_defs.iter().any(|d| {
372+
d.name == var_name
373+
&& d.scope_start == scope_start
374+
&& d.kind == VarDefKind::Parameter
375+
});
376+
if has_shadowing_param {
377+
continue;
378+
}
379+
// Check if this scope actually uses the variable (has a
380+
// Variable span in it). Only include it if the variable
381+
// appears there to avoid false positives with unrelated
382+
// nested functions.
383+
//
384+
// But we also need to check: is this scope a closure body
385+
// (not an arrow function)? Closures create new variable
386+
// scopes and require explicit `use` — if there's no
387+
// ClosureCapture def for this scope, the variable is NOT
388+
// available inside a regular closure. We only auto-include
389+
// arrow function scopes.
390+
//
391+
// Heuristic: if there's any ClosureCapture or Parameter def
392+
// for *any* variable scoped to this scope_start, and there's
393+
// no ClosureCapture for *our* variable, this is likely a
394+
// closure that didn't capture our variable — skip it.
395+
let is_closure_scope = symbol_map
396+
.var_defs
397+
.iter()
398+
.any(|d| d.scope_start == scope_start && d.kind == VarDefKind::ClosureCapture);
399+
if is_closure_scope {
400+
// It's a closure scope. Our variable is not in the `use`
401+
// list (we already handled ClosureCapture above), so the
402+
// variable is not available here.
403+
continue;
404+
}
405+
// This is an arrow function scope or similar transparent
406+
// scope. The variable is implicitly captured.
407+
let has_usage = symbol_map.spans.iter().any(|s| {
408+
if let SymbolKind::Variable { name } = &s.kind {
409+
name == var_name && symbol_map.find_variable_scope(name, s.start) == scope_start
410+
} else {
411+
false
412+
}
413+
});
414+
if has_usage {
415+
reachable.insert(scope_start);
416+
}
417+
}
418+
419+
// Recurse: newly added scopes may themselves contain nested
420+
// closures/arrows that capture the same variable.
421+
// Fixed-point iteration until no new scopes are added.
422+
let mut prev_len = 0;
423+
while reachable.len() != prev_len {
424+
prev_len = reachable.len();
425+
let current: Vec<u32> = reachable.iter().copied().collect();
426+
427+
for def in &symbol_map.var_defs {
428+
if def.name != var_name || def.kind != VarDefKind::ClosureCapture {
429+
continue;
430+
}
431+
if reachable.contains(&def.scope_start) {
432+
continue;
433+
}
434+
let outer_scope = symbol_map.find_enclosing_scope(def.offset);
435+
if reachable.contains(&outer_scope) {
436+
reachable.insert(def.scope_start);
437+
}
438+
}
439+
440+
for &(scope_start, _scope_end) in &symbol_map.scopes {
441+
if reachable.contains(&scope_start) {
442+
continue;
443+
}
444+
let parent = symbol_map.find_enclosing_scope(scope_start.saturating_sub(1));
445+
if !current.contains(&parent) {
446+
continue;
447+
}
448+
let has_shadowing_param = symbol_map.var_defs.iter().any(|d| {
449+
d.name == var_name
450+
&& d.scope_start == scope_start
451+
&& d.kind == VarDefKind::Parameter
452+
});
453+
if has_shadowing_param {
454+
continue;
455+
}
456+
let is_closure_scope = symbol_map
457+
.var_defs
458+
.iter()
459+
.any(|d| d.scope_start == scope_start && d.kind == VarDefKind::ClosureCapture);
460+
if is_closure_scope {
461+
continue;
462+
}
463+
let has_usage = symbol_map.spans.iter().any(|s| {
464+
if let SymbolKind::Variable { name } = &s.kind {
465+
name == var_name
466+
&& symbol_map.find_variable_scope(name, s.start) == scope_start
467+
} else {
468+
false
469+
}
470+
});
471+
if has_usage {
472+
reachable.insert(scope_start);
473+
}
474+
}
475+
}
476+
477+
reachable
478+
}
479+
321480
/// Find all references to `$this` within the enclosing class body.
322481
///
323482
/// `$this` is scoped to the enclosing class — it must not match

src/rename/tests.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2364,6 +2364,56 @@ async fn rename_closure_parameter_does_not_leak_to_outer_scope() {
23642364
);
23652365
}
23662366

2367+
#[tokio::test]
2368+
async fn rename_function_param_propagates_into_closure_use_and_arrow() {
2369+
let backend = Backend::new_test();
2370+
let uri = Url::parse("file:///test.php").unwrap();
2371+
let text = concat!(
2372+
"<?php\n",
2373+
"function test(string $param): void\n",
2374+
"{\n",
2375+
" function () use ($param): void {\n",
2376+
" echo $param;\n",
2377+
" };\n",
2378+
"\n",
2379+
" fn () => $param;\n",
2380+
"}\n",
2381+
);
2382+
2383+
open_file(&backend, &uri, text).await;
2384+
2385+
// Cursor on `$param` in the function parameter list (line 1, col 22).
2386+
let edit = rename(&backend, &uri, 1, 22, "$renamed").await;
2387+
assert!(edit.is_some(), "Expected a workspace edit");
2388+
let file_edits = edits_for_uri(&edit.unwrap(), &uri);
2389+
let result = apply_edits(text, &file_edits);
2390+
2391+
// Function parameter should be renamed.
2392+
assert!(
2393+
result.contains("function test(string $renamed)"),
2394+
"Function parameter not renamed: {}",
2395+
result
2396+
);
2397+
// Closure use should be renamed.
2398+
assert!(
2399+
result.contains("use ($renamed)"),
2400+
"Closure use not renamed: {}",
2401+
result
2402+
);
2403+
// Variable inside closure body should be renamed.
2404+
assert!(
2405+
result.contains("echo $renamed;"),
2406+
"Closure body variable not renamed: {}",
2407+
result
2408+
);
2409+
// Arrow function body should be renamed.
2410+
assert!(
2411+
result.contains("fn () => $renamed;"),
2412+
"Arrow function body not renamed: {}",
2413+
result
2414+
);
2415+
}
2416+
23672417
// ─── Namespace rename tests ─────────────────────────────────────────────────
23682418

23692419
#[tokio::test]

0 commit comments

Comments
 (0)