Skip to content

Commit c68b0c0

Browse files
committed
Fix namespace renaming
1 parent fd58e07 commit c68b0c0

5 files changed

Lines changed: 37 additions & 56 deletions

File tree

docs/CHANGELOG.md

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

5353
### Fixed
5454

55+
- **Renaming a namespace no longer corrupts group `use` statements.** Renaming a namespace segment that is imported with a group `use` (e.g. `use App\Old\{Foo, Bar};`) previously rewrote the group's shared prefix and then also spliced the new prefix into each member name, producing invalid PHP like `use App\New\{App\New\Foo, Bar};`. The member names are left untouched now, since the prefix rewrite alone already updates the whole statement correctly.
5556
- **Parameter name inlay hints no longer shift to the wrong parameter when only part of a multi-line call is visible.** Editors request inlay hints only for the currently visible viewport, and when a call's arguments were split across the viewport boundary, every hint after the first excluded argument was labelled with the previous parameter's name instead of its own. This was most noticeable on multi-line constructor calls with several arguments, such as those using constructor property promotion.
5657
- **Linked editing no longer types into unrelated parts of the file.** Adding a line above a variable (for example inserting a `/** */` docblock) and then typing could insert the typed characters at two arbitrary places further down, such as in the middle of a method name and in front of a trailing comment. PHPantom is re-reading the file in the background while you type, and linked editing was handing the editor positions measured against the previous version of the file, which the editor then edited on trust. Linked editing now verifies that the positions it reports still point at the variable, and offers nothing at all if they do not, so a keystroke can never land somewhere unexpected. It also becomes available in Blade templates, where the reported positions are checked against the template itself rather than the generated PHP.
5758
- **Rename no longer rewrites unrelated code when a file has just been edited.** PHPantom re-reads a file in the background after every keystroke, and rename could hand the editor positions measured against the previous version of that file, so confirming the rename edited whatever now sat at those positions. Rename and the rename preview now check that every position they report still points at the symbol, in each file the rename touches, and offer nothing at all when one does not. Retrying a moment later, once the background re-read has caught up, renames normally.

docs/todo.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,7 @@ unlikely to move the needle for most users.
112112
| D3 | [Deprecated rendering — chain subject resolution](todo/diagnostics.md#d3-deprecated-rendering-chain-subject-resolution) | Low-Medium | Medium |
113113
| D5 | [External tool diagnostic suppression actions](todo/diagnostics.md#d5-external-tool-diagnostic-suppression-actions) | Low | Low |
114114
| D15 | [Unused parameter diagnostic](todo/diagnostics.md#d15-unused-parameter-diagnostic) | Low | Low |
115-
| | **[Bug Fixes](todo/bugs.md)** | | |
116-
| B2 | [Namespace rename writes an FQN into every name of a group `use`](todo/bugs.md#b2-namespace-rename-writes-an-fqn-into-every-name-of-a-group-use) | Medium | Low |
115+
| | **[Bug Fixes](todo/bugs.md)** (no outstanding items) | | |
117116
| | **[Code Actions](todo/actions.md)** | | |
118117
| A40 | [Generate method from call](todo/actions.md#a40-generate-method-from-call) | Medium-High | Medium |
119118
| A41 | [Create class from non-existing name](todo/actions.md#a41-create-class-from-non-existing-name) | Medium | Medium |

docs/todo/bugs.md

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -7,41 +7,4 @@ pipeline so it produces correct data. Downstream consumers
77
(diagnostics, hover, completion, definition) should never need
88
to second-guess upstream output.
99

10-
## B2 Namespace rename writes an FQN into every name of a group `use`
11-
12-
Renaming namespace `App\Old` to `App\New` turns
13-
14-
```php
15-
use App\Old\{Foo, Bar};
16-
```
17-
18-
into
19-
20-
```php
21-
use App\New\{App\New\Foo, App\New\Bar};
22-
```
23-
24-
which is a parse error. This is covered by
25-
`rename_namespace_updates_group_use` in `src/rename/tests.rs`, but the
26-
assertion only checks that `App\New` appears somewhere in the result, so
27-
it passes on the broken output.
28-
29-
Two paths in `rename/namespace.rs` edit the same line:
30-
31-
- `collect_use_statement_edits` scans the source and rewrites the group
32-
prefix (`App\Old``App\New`), which is the correct and complete edit.
33-
- `collect_fqn_reference_edits` walks the symbol map, where each name
34-
inside the braces is a `ClassReference` span whose recorded `name` is
35-
the *composed* FQN (`App\Old\Foo`) while the span itself covers only
36-
the trailing `Foo`. It therefore emits a second edit replacing `Foo`
37-
with the full `App\New\Foo`.
38-
39-
The two edits do not overlap, so the `dedup_by(ranges_overlap)` pass that
40-
resolves the ordinary `use App\Old\Foo;` case does not catch them.
41-
42-
The fix belongs in `collect_fqn_reference_edits`: a span whose source
43-
text is only a segment-aligned tail of its recorded name is a group-use
44-
member, and the group prefix edit already covers it, so it should be
45-
skipped. `source_spells_reference` in the same file already
46-
distinguishes the two shapes. Extend the test to assert the full
47-
expected line rather than a substring.
10+
No outstanding items.

src/rename/namespace.rs

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -434,10 +434,17 @@ impl Backend {
434434
return false;
435435
}
436436

437-
// Skip use-statement references (they don't have `\` in span
438-
// unless they are inline FQN like `\App\Foo` in code).
439-
// Actually, use-statement spans DO contain the full FQN.
440-
// We rely on deduplication to handle overlaps.
437+
// A group-use member (`use App\Old\{Foo, Bar};`) records the
438+
// composed FQN (`App\Old\Foo`) on a span that covers only the
439+
// trailing segment (`Foo`). `collect_use_statement_edits`
440+
// already rewrites the group's shared prefix, so emitting a
441+
// second edit here would duplicate that rewrite onto the
442+
// trailing segment, turning `App\New\{Foo, Bar}` into
443+
// `App\New\{App\New\Foo, Bar}`. Skip it: only a span that
444+
// spells the reference in full needs its own edit.
445+
if !source_spells_reference_fully(source, name) {
446+
continue;
447+
}
441448

442449
let new_name = if name_normalized.len() == old_prefix.len() {
443450
if name.starts_with('\\') {
@@ -550,16 +557,23 @@ impl Backend {
550557
/// counts too. Anything else means the offsets no longer describe the
551558
/// buffer.
552559
fn source_spells_reference(source: &str, name: &str) -> bool {
560+
source_spells_reference_fully(source, name) || source_spells_reference_as_tail(source, name)
561+
}
562+
563+
/// Whether `source` spells the entirety of `name` (case-insensitively).
564+
fn source_spells_reference_fully(source: &str, name: &str) -> bool {
565+
strip_fqn_prefix(source).eq_ignore_ascii_case(strip_fqn_prefix(name))
566+
}
567+
568+
/// Whether `source` spells only a segment-aligned tail of `name`, as a
569+
/// group-use member's span does (see [`source_spells_reference`]).
570+
fn source_spells_reference_as_tail(source: &str, name: &str) -> bool {
553571
let source = strip_fqn_prefix(source);
554572
let name = strip_fqn_prefix(name);
555-
source.eq_ignore_ascii_case(name)
556-
|| name
557-
.len()
558-
.checked_sub(source.len())
559-
.and_then(|split| name.split_at_checked(split))
560-
.is_some_and(|(prefix, tail)| {
561-
prefix.ends_with('\\') && tail.eq_ignore_ascii_case(source)
562-
})
573+
name.len()
574+
.checked_sub(source.len())
575+
.and_then(|split| name.split_at_checked(split))
576+
.is_some_and(|(prefix, tail)| prefix.ends_with('\\') && tail.eq_ignore_ascii_case(source))
563577
}
564578

565579
// ─── Namespace segment helpers ──────────────────────────────────────────────

src/rename/tests.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3535,10 +3535,14 @@ async fn rename_namespace_updates_group_use() {
35353535
let edits_b = edits_for_uri(&edit, &uri_b);
35363536
let result_b = apply_edits(text_b, &edits_b);
35373537

3538-
assert!(
3539-
result_b.contains("App\\New"),
3540-
"Group use prefix should be updated: {}",
3541-
result_b
3538+
assert_eq!(
3539+
result_b,
3540+
concat!(
3541+
"<?php\n",
3542+
"use App\\New\\{Foo, Bar};\n",
3543+
"function demo(Foo $f, Bar $b): void {}\n",
3544+
),
3545+
"Group use prefix should be updated without touching member names"
35423546
);
35433547
}
35443548

0 commit comments

Comments
 (0)