Skip to content

Commit 72f3d3b

Browse files
committed
Fix linked editing
1 parent 50245a4 commit 72f3d3b

3 files changed

Lines changed: 231 additions & 3 deletions

File tree

src/linked_editing.rs

Lines changed: 115 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ fn span_in_region(
219219
span_offset: u32,
220220
symbol_map: &SymbolMap,
221221
var_name: &str,
222+
all_regions: &[Region],
222223
) -> bool {
223224
// Case 1: the span is the definition token of this region.
224225
if span_offset == region.def_offset {
@@ -236,8 +237,16 @@ fn span_in_region(
236237
// Compound assignments (+=, -=, .=, etc.) modify in place
237238
// and belong to the current region.
238239
Some(VarDefKind::CompoundAssignment) => return true,
239-
// Any other definition kind starts its own region.
240-
Some(_) => {}
240+
// If this definition starts its own region, it doesn't
241+
// belong here. But if it was skipped by build_regions
242+
// (e.g. a deeper-nested conditional assignment), it
243+
// belongs to the enclosing region.
244+
Some(_) => {
245+
let starts_own_region = all_regions.iter().any(|r| r.def_offset == span_offset);
246+
if !starts_own_region {
247+
return true;
248+
}
249+
}
241250
}
242251
}
243252

@@ -295,7 +304,7 @@ fn collect_variable_ranges_in_region(
295304
if span_scope != scope_start {
296305
continue;
297306
}
298-
if !span_in_region(region, span.start, symbol_map, var_name) {
307+
if !span_in_region(region, span.start, symbol_map, var_name, &regions) {
299308
continue;
300309
}
301310
if !seen_offsets.insert(span.start) {
@@ -327,6 +336,109 @@ fn collect_variable_ranges_in_region(
327336
}
328337
}
329338

339+
// ── Closure use() capture bridging ──────────────────────────────────
340+
//
341+
// When a variable is captured via `use ($var)`, renaming it must
342+
// propagate across both the outer scope (where the variable is
343+
// defined) and the inner closure scope (where it is used). The
344+
// `use ($var)` token sits physically in the outer scope but has a
345+
// `ClosureCapture` VarDefSite scoped to the closure body.
346+
//
347+
// Case A (cursor in outer scope): find ClosureCapture defs whose
348+
// offset falls within this region, then include all occurrences of
349+
// the variable inside each captured closure.
350+
//
351+
// Case B (cursor in closure scope): if the region's definition is
352+
// a ClosureCapture, bridge outward to the outer scope's region and
353+
// include those occurrences too.
354+
355+
// Case A: outer scope → closure bodies
356+
for def in &symbol_map.var_defs {
357+
if def.name == var_name
358+
&& def.kind == VarDefKind::ClosureCapture
359+
&& span_in_region(region, def.offset, symbol_map, var_name, &regions)
360+
{
361+
// The use() token itself (in outer scope) is already collected
362+
// above. Now collect all occurrences inside the closure body.
363+
let closure_scope = def.scope_start;
364+
for span in &symbol_map.spans {
365+
if let SymbolKind::Variable { name } = &span.kind {
366+
if name != var_name {
367+
continue;
368+
}
369+
let ss = symbol_map.find_variable_scope(name, span.start);
370+
if ss != closure_scope {
371+
continue;
372+
}
373+
if !seen_offsets.insert(span.start) {
374+
continue;
375+
}
376+
ranges.push(variable_range_without_sigil(
377+
content,
378+
span.start as usize,
379+
span.end as usize,
380+
));
381+
}
382+
}
383+
}
384+
}
385+
386+
// Case B: closure scope → outer scope
387+
// Check if the current region's definition is a ClosureCapture.
388+
if let Some(capture_def) = symbol_map.var_defs.iter().find(|d| {
389+
d.name == var_name
390+
&& d.kind == VarDefKind::ClosureCapture
391+
&& d.offset == region.def_offset
392+
&& d.scope_start == scope_start
393+
}) {
394+
// The use() token sits in the outer scope. Find which outer
395+
// region it belongs to and include all outer occurrences.
396+
let outer_scope = symbol_map.find_enclosing_scope(capture_def.offset);
397+
let outer_regions = build_regions(symbol_map, var_name, outer_scope);
398+
if let Some(outer_region) = outer_regions.iter().find(|r| {
399+
capture_def.offset >= r.effective_from && capture_def.offset < r.reads_until
400+
|| capture_def.offset == r.def_offset
401+
}) {
402+
for span in &symbol_map.spans {
403+
if let SymbolKind::Variable { name } = &span.kind {
404+
if name != var_name {
405+
continue;
406+
}
407+
let ss = symbol_map.find_variable_scope(name, span.start);
408+
if ss != outer_scope {
409+
continue;
410+
}
411+
if !span_in_region(
412+
outer_region,
413+
span.start,
414+
symbol_map,
415+
var_name,
416+
&outer_regions,
417+
) {
418+
continue;
419+
}
420+
if !seen_offsets.insert(span.start) {
421+
continue;
422+
}
423+
ranges.push(variable_range_without_sigil(
424+
content,
425+
span.start as usize,
426+
span.end as usize,
427+
));
428+
}
429+
}
430+
// Also include the outer region's def site if not yet seen.
431+
if seen_offsets.insert(outer_region.def_offset) {
432+
let end_offset = outer_region.def_offset + 1 + var_name.len() as u32;
433+
ranges.push(variable_range_without_sigil(
434+
content,
435+
outer_region.def_offset as usize,
436+
end_offset as usize,
437+
));
438+
}
439+
}
440+
}
441+
330442
// Sort by position for a deterministic response.
331443
ranges.sort_by(|a, b| {
332444
a.start

src/symbol_map/extraction.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,9 +436,12 @@ fn extract_from_statement<'a>(
436436
}
437437
Statement::Try(try_stmt) => {
438438
emit_keyword(&try_stmt.r#try, ctx);
439+
let try_block_end = try_stmt.block.span().end.offset;
440+
push_cond_nesting(ctx, try_block_end);
439441
for s in try_stmt.block.statements.iter() {
440442
extract_from_statement(s, ctx, scope_start);
441443
}
444+
pop_cond_nesting(ctx);
442445
for catch in try_stmt.catch_clauses.iter() {
443446
emit_keyword(&catch.r#catch, ctx);
444447
// Catch type hint is a navigable class reference.

tests/integration/linked_editing.rs

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,3 +568,116 @@ function demo() {
568568
// `$abc` in `echo $abc` starts at col 9, so `abc` is col 10..13.
569569
assert_range(&ranges[1], 3, 10, 13);
570570
}
571+
572+
#[test]
573+
fn linked_editing_conditional_reassignment_belongs_to_outer_region() {
574+
let backend = create_test_backend();
575+
let php = r#"<?php
576+
function test(bool $bool): void {
577+
$a = 'a';
578+
$a .= 'a';
579+
$a = $a . 'b';
580+
if ($bool) {
581+
$a = 'b';
582+
}
583+
echo $a;
584+
}
585+
"#;
586+
587+
// Cursor on `echo $a` (line 8, col 10) — region 2 starts at `$a = $a . 'b'`
588+
// and should include the conditional `$a = 'b'` inside the if and `echo $a`.
589+
let result = linked_editing_at(&backend, "file:///test.php", php, 8, 10);
590+
let ranges = result.expect("expected linked editing ranges").ranges;
591+
// Region 2: `$a` on line 4 (LHS of `$a = $a . 'b'`), `$a` on line 6
592+
// (inside if), `$a` on line 8 (echo)
593+
assert_eq!(
594+
ranges.len(),
595+
3,
596+
"should include LHS assignment, conditional reassignment, and echo"
597+
);
598+
assert_range(&ranges[0], 4, 5, 6); // $a = $a . 'b' (LHS)
599+
assert_range(&ranges[1], 6, 9, 10); // $a = 'b' inside if
600+
assert_range(&ranges[2], 8, 10, 11); // echo $a
601+
}
602+
603+
#[test]
604+
fn linked_editing_try_catch_reassignment_same_region() {
605+
let backend = create_test_backend();
606+
let php = r#"<?php
607+
function test(): void {
608+
$conn = null;
609+
try {
610+
$conn = 'connected';
611+
} catch (\Exception $e) {
612+
$conn = 'failed';
613+
}
614+
echo $conn;
615+
}
616+
"#;
617+
618+
// Cursor on `echo $conn` (line 8) — all $conn should be one region
619+
let result = linked_editing_at(&backend, "file:///test.php", php, 8, 10);
620+
let ranges = result.expect("expected linked editing ranges").ranges;
621+
assert_eq!(
622+
ranges.len(),
623+
4,
624+
"should include initial assignment, try, catch, and echo: got {:?}",
625+
ranges
626+
);
627+
}
628+
629+
#[test]
630+
fn linked_editing_closure_use_capture_links_across_scopes() {
631+
let backend = create_test_backend();
632+
let php = r#"<?php
633+
function demo(): void {
634+
$name = 'world';
635+
$fn = function () use ($name) {
636+
echo $name;
637+
};
638+
echo $name;
639+
}
640+
"#;
641+
642+
// All 4 occurrences of $name should link: the outer assignment,
643+
// the use() capture, the inner echo, and the outer echo.
644+
// Cursor on `$name` inside the closure body (line 4)
645+
let result = linked_editing_at(&backend, "file:///test.php", php, 4, 14);
646+
let ranges = result.expect("inner $name should bridge to outer").ranges;
647+
assert_eq!(
648+
ranges.len(),
649+
4,
650+
"expected all 4 occurrences linked: got {:?}",
651+
ranges
652+
);
653+
654+
// Cursor on `$name` in the use() clause (line 3)
655+
let result = linked_editing_at(&backend, "file:///test.php", php, 3, 29);
656+
let ranges = result.expect("use($name) should link all").ranges;
657+
assert_eq!(
658+
ranges.len(),
659+
4,
660+
"expected all 4 occurrences from use() cursor: got {:?}",
661+
ranges
662+
);
663+
664+
// Cursor on outer `$name` (line 2, assignment)
665+
let result = linked_editing_at(&backend, "file:///test.php", php, 2, 5);
666+
let ranges = result.expect("outer $name should bridge to closure").ranges;
667+
assert_eq!(
668+
ranges.len(),
669+
4,
670+
"expected all 4 occurrences from outer cursor: got {:?}",
671+
ranges
672+
);
673+
674+
// Cursor on outer `echo $name` (line 6)
675+
let result = linked_editing_at(&backend, "file:///test.php", php, 6, 10);
676+
let ranges = result.expect("outer echo $name should link all").ranges;
677+
assert_eq!(
678+
ranges.len(),
679+
4,
680+
"expected all 4 occurrences from outer echo: got {:?}",
681+
ranges
682+
);
683+
}

0 commit comments

Comments
 (0)