Skip to content

Commit 60ce94c

Browse files
calebdwAJenbo
authored andcommitted
feat(rename): link compact strings to local variables
Signed-off-by: Anders Jenbo <anders@jenbo.dk>
1 parent 2c59134 commit 60ce94c

13 files changed

Lines changed: 294 additions & 56 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1919
- **Laravel custom Eloquent builder support.** Models using the `#[UseEloquentBuilder]` attribute now have their custom builder's methods forwarded as static methods on the model. `query()`, `newQuery()`, and `newModelQuery()` return the custom builder type with correct generic model substitution. Contributed by @MingJen in https://github.com/AJenbo/phpantom_lsp/pull/118.
2020
- **Eloquent relation and column string completion.** Typing inside string arguments to `with()`, `load()`, `whereHas()`, and other Eloquent methods that accept relation names now offers relationship method names as completions, with dot-notation traversal for nested relations. Similarly, `where()`, `orderBy()`, `select()`, `pluck()`, and other column-accepting methods offer model column names (from `$casts`, `$fillable`, `@property` tags, timestamps, etc.).
2121
- **`model-property<T>` pseudo-type recognition.** The Larastan `model-property<Model>` type no longer triggers "unknown class" diagnostics. It is treated as a string subtype.
22+
- **`compact()` strings are linked to local variables.** A string argument to `compact('user')` is now treated as a reference to the matching local variable. Renaming the variable updates the string (and renaming from the string updates the variable and its other uses), find-references includes the string, and go-to-definition on the string jumps to the variable's assignment. Contributed by @calebdw in https://github.com/PHPantom-dev/phpantom_lsp/pull/159.
2223

2324
### Changed
2425

src/definition/resolve.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ impl Backend {
169169
cursor_offset: u32,
170170
) -> Option<Vec<Location>> {
171171
match kind {
172-
SymbolKind::Variable { name } => {
172+
SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => {
173173
// Try the precomputed var_defs map first.
174174
// This avoids re-parsing the file at request time.
175175

@@ -191,10 +191,14 @@ impl Backend {
191191
let parsed_uri = Url::parse(uri).ok()?;
192192
let start =
193193
crate::util::offset_to_position(content, cursor_offset as usize);
194-
let end = crate::util::offset_to_position(
195-
content,
196-
cursor_offset as usize + 1 + name.len(),
197-
);
194+
let end_offset = match kind {
195+
SymbolKind::Variable { .. } => cursor_offset as usize + 1 + name.len(),
196+
SymbolKind::CompactVariable { .. } => {
197+
cursor_offset as usize + name.len()
198+
}
199+
_ => unreachable!(),
200+
};
201+
let end = crate::util::offset_to_position(content, end_offset);
198202
return Some(vec![Location {
199203
uri: parsed_uri,
200204
range: Range { start, end },

src/definition/type_definition.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ impl Backend {
4747
let function_loader = self.function_loader(&ctx);
4848

4949
let resolved_types: Vec<PhpType> = match &symbol.kind {
50-
SymbolKind::Variable { name } => {
50+
SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => {
5151
let in_static = self
5252
.symbol_maps
5353
.read()

src/highlight/mod.rs

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ impl Backend {
4141
let symbol_map = maps.get(uri)?;
4242

4343
let highlights = match &span.kind {
44-
SymbolKind::Variable { name } => {
44+
SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => {
4545
// Check if this is actually a property declaration — if
4646
// so, highlight member accesses instead of local vars.
4747
if let Some(VarDefKind::Property) = symbol_map.var_def_kind_at(name, span.start) {
@@ -111,27 +111,29 @@ impl Backend {
111111

112112
// Collect from symbol spans.
113113
for span in &symbol_map.spans {
114-
if let SymbolKind::Variable { name } = &span.kind {
115-
if name != var_name {
116-
continue;
117-
}
118-
let span_scope = symbol_map.find_variable_scope(name, span.start);
119-
if span_scope != scope_start {
120-
continue;
121-
}
122-
seen_offsets.insert(span.start);
114+
let name = match &span.kind {
115+
SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => name,
116+
_ => continue,
117+
};
118+
if name != var_name {
119+
continue;
120+
}
121+
let span_scope = symbol_map.find_variable_scope(name, span.start);
122+
if span_scope != scope_start {
123+
continue;
124+
}
125+
seen_offsets.insert(span.start);
123126

124-
let kind = if symbol_map.var_def_kind_at(name, span.start).is_some() {
125-
DocumentHighlightKind::WRITE
126-
} else {
127-
DocumentHighlightKind::READ
128-
};
127+
let kind = if symbol_map.var_def_kind_at(name, span.start).is_some() {
128+
DocumentHighlightKind::WRITE
129+
} else {
130+
DocumentHighlightKind::READ
131+
};
129132

130-
highlights.push(DocumentHighlight {
131-
range: byte_range_to_lsp_range(content, span.start as usize, span.end as usize),
132-
kind: Some(kind),
133-
});
134-
}
133+
highlights.push(DocumentHighlight {
134+
range: byte_range_to_lsp_range(content, span.start as usize, span.end as usize),
135+
kind: Some(kind),
136+
});
135137
}
136138

137139
// Include var_def sites that may not have a matching Variable span

src/hover/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ impl Backend {
490490
let function_loader = self.function_loader(&ctx);
491491

492492
match kind {
493-
SymbolKind::Variable { name } => {
493+
SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => {
494494
// Suppress hover when the cursor is on a variable at its
495495
// definition site where the type is already visible in
496496
// the signature (properties, static/global declarations).

src/references/mod.rs

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ impl Backend {
120120
include_declaration: bool,
121121
) -> Vec<Location> {
122122
match kind {
123-
SymbolKind::Variable { name } => {
123+
SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => {
124124
// Property declarations use Variable spans (so GTD can
125125
// jump to the type hint), but Find References should
126126
// search for member accesses, not local variable uses.
@@ -364,26 +364,28 @@ impl Backend {
364364
let reachable_scopes = Self::collect_capture_scopes(symbol_map, var_name, scope_start);
365365

366366
for span in &symbol_map.spans {
367-
if let SymbolKind::Variable { name } = &span.kind {
368-
if name != var_name {
369-
continue;
370-
}
371-
// Check that this variable is in a reachable scope.
372-
let span_scope = symbol_map.find_variable_scope(name, span.start);
373-
if !reachable_scopes.contains(&span_scope) {
374-
continue;
375-
}
376-
// Optionally skip declaration sites.
377-
if !include_declaration && symbol_map.var_def_kind_at(name, span.start).is_some() {
378-
continue;
379-
}
380-
let start = offset_to_position(content, span.start as usize);
381-
let end = offset_to_position(content, span.end as usize);
382-
locations.push(Location {
383-
uri: parsed_uri.clone(),
384-
range: Range { start, end },
385-
});
367+
let name = match &span.kind {
368+
SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => name,
369+
_ => continue,
370+
};
371+
if name != var_name {
372+
continue;
373+
}
374+
// Check that this variable is in a reachable scope.
375+
let span_scope = symbol_map.find_variable_scope(name, span.start);
376+
if !reachable_scopes.contains(&span_scope) {
377+
continue;
378+
}
379+
// Optionally skip declaration sites.
380+
if !include_declaration && symbol_map.var_def_kind_at(name, span.start).is_some() {
381+
continue;
386382
}
383+
let start = offset_to_position(content, span.start as usize);
384+
let end = offset_to_position(content, span.end as usize);
385+
locations.push(Location {
386+
uri: parsed_uri.clone(),
387+
range: Range { start, end },
388+
});
387389
}
388390

389391
// Also include var_def sites if include_declaration is set,
@@ -446,7 +448,9 @@ impl Backend {
446448
scope_ends: &HashMap<u32, u32>,
447449
) -> bool {
448450
symbol_map.spans.iter().any(|s| {
449-
if let SymbolKind::Variable { name } = &s.kind {
451+
if let SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } =
452+
&s.kind
453+
{
450454
name == var_name
451455
&& scope_ends
452456
.get(&scope_start)

src/references/tests.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,31 @@ async fn test_variable_references_exclude_declaration() {
128128
);
129129
}
130130

131+
#[tokio::test]
132+
async fn test_variable_references_include_compact_string() {
133+
let backend = Backend::new_test();
134+
let uri = Url::parse("file:///test.php").unwrap();
135+
let text = concat!(
136+
"<?php\n",
137+
"function demo(): array {\n",
138+
" $user = 'alice';\n",
139+
" return compact('user');\n",
140+
"}\n",
141+
);
142+
143+
open_file(&backend, &uri, text).await;
144+
145+
let locs = find_references(&backend, &uri, 2, 6, true).await;
146+
assert!(
147+
locs.iter().any(|loc| {
148+
loc.range.start.line == 3
149+
&& loc.range.start.character == 20
150+
&& loc.range.end.character == 24
151+
}),
152+
"Expected compact('user') string contents to be included in variable references: {locs:?}"
153+
);
154+
}
155+
131156
// ─── Class References ───────────────────────────────────────────────────────
132157

133158
#[tokio::test]

src/rename/mod.rs

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,10 @@ impl Backend {
140140
// special because the `$` prefix is part of the declaration but
141141
// usage sites via `->` or `?->` don't include it.
142142
let is_property = self.is_property_rename(&span.kind, uri, &span);
143-
let is_variable = matches!(&span.kind, SymbolKind::Variable { .. }) && !is_property;
143+
let is_variable = matches!(
144+
&span.kind,
145+
SymbolKind::Variable { .. } | SymbolKind::CompactVariable { .. }
146+
) && !is_property;
144147

145148
// For class renames, delegate to the specialised handler that
146149
// understands `use` statements, aliases, and collisions.
@@ -163,12 +166,22 @@ impl Backend {
163166
};
164167

165168
let edit_text = if is_variable {
166-
// Variables: the reference range includes the `$`, so
167-
// the new name should also include it.
168-
if new_name.starts_with('$') {
169-
new_name.to_string()
170-
} else {
171-
format!("${}", new_name)
169+
let bare_name = new_name.strip_prefix('$').unwrap_or(new_name);
170+
let loc_symbol = loc_content.as_deref().and_then(|c| {
171+
self.lookup_symbol_at_position(&loc_uri_str, c, location.range.start)
172+
});
173+
match loc_symbol {
174+
Some(crate::symbol_map::SymbolSpan {
175+
kind: SymbolKind::CompactVariable { .. },
176+
..
177+
}) => bare_name.to_string(),
178+
_ => {
179+
if new_name.starts_with('$') {
180+
new_name.to_string()
181+
} else {
182+
format!("${}", new_name)
183+
}
184+
}
172185
}
173186
} else if is_property {
174187
// Properties: the reference may or may not include `$`.
@@ -539,6 +552,7 @@ impl Backend {
539552
// Include the `$` prefix in the range — the span already does.
540553
Some((format!("${}", name), range))
541554
}
555+
SymbolKind::CompactVariable { name } => Some((name.clone(), range)),
542556
SymbolKind::ClassReference { name, .. } => Some((name.clone(), range)),
543557
SymbolKind::ClassDeclaration { name } => Some((name.clone(), range)),
544558
SymbolKind::MemberAccess { member_name, .. } => Some((member_name.clone(), range)),
@@ -614,6 +628,7 @@ impl Backend {
614628
self.lookup_var_def_kind_at(uri, name, span.start)
615629
.is_some_and(|k| k == crate::symbol_map::VarDefKind::Property)
616630
}
631+
SymbolKind::CompactVariable { .. } => false,
617632
_ => false,
618633
}
619634
}

src/rename/tests.rs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,58 @@ async fn rename_variable_without_dollar_prefix() {
159159
}
160160
}
161161

162+
#[tokio::test]
163+
async fn rename_variable_updates_compact_string() {
164+
let backend = Backend::new_test();
165+
let uri = Url::parse("file:///test.php").unwrap();
166+
let text = concat!(
167+
"<?php\n",
168+
"function demo(): array {\n",
169+
" $user = 'alice';\n",
170+
" return compact('user');\n",
171+
"}\n",
172+
);
173+
174+
open_file(&backend, &uri, text).await;
175+
176+
let edit = rename(&backend, &uri, 2, 6, "$person").await;
177+
assert!(
178+
edit.is_some(),
179+
"Expected a workspace edit for variable rename"
180+
);
181+
182+
let file_edits = edits_for_uri(&edit.unwrap(), &uri);
183+
let updated = apply_edits(text, &file_edits);
184+
assert!(updated.contains("$person = 'alice';"));
185+
assert!(updated.contains("compact('person')"));
186+
}
187+
188+
#[tokio::test]
189+
async fn rename_from_compact_string_updates_variable() {
190+
let backend = Backend::new_test();
191+
let uri = Url::parse("file:///test.php").unwrap();
192+
let text = concat!(
193+
"<?php\n",
194+
"function demo(): array {\n",
195+
" $user = 'alice';\n",
196+
" return compact('user');\n",
197+
"}\n",
198+
);
199+
200+
open_file(&backend, &uri, text).await;
201+
202+
let edit = rename(&backend, &uri, 3, 21, "person").await;
203+
assert!(
204+
edit.is_some(),
205+
"Expected a workspace edit for compact rename"
206+
);
207+
208+
let file_edits = edits_for_uri(&edit.unwrap(), &uri);
209+
let updated = apply_edits(text, &file_edits);
210+
assert!(updated.contains("$person = 'alice';"));
211+
assert!(updated.contains("compact('person')"));
212+
}
213+
162214
#[tokio::test]
163215
async fn prepare_rename_variable() {
164216
let backend = Backend::new_test();
@@ -186,6 +238,33 @@ async fn prepare_rename_variable() {
186238
}
187239
}
188240

241+
#[tokio::test]
242+
async fn prepare_rename_compact_string_uses_bare_name() {
243+
let backend = Backend::new_test();
244+
let uri = Url::parse("file:///test.php").unwrap();
245+
let text = concat!(
246+
"<?php\n",
247+
"function demo(): array {\n",
248+
" $user = 'alice';\n",
249+
" return compact('user');\n",
250+
"}\n",
251+
);
252+
253+
open_file(&backend, &uri, text).await;
254+
255+
let response = prepare_rename(&backend, &uri, 3, 21).await;
256+
assert!(
257+
response.is_some(),
258+
"Expected prepare rename on compact string"
259+
);
260+
261+
if let Some(PrepareRenameResponse::RangeWithPlaceholder { placeholder, .. }) = response {
262+
assert_eq!(placeholder, "user");
263+
} else {
264+
panic!("Expected RangeWithPlaceholder response");
265+
}
266+
}
267+
189268
// ─── Non-Renameable Symbols ─────────────────────────────────────────────────
190269

191270
#[tokio::test]

src/semantic_tokens.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ impl Backend {
306306
(tt, mods)
307307
}
308308

309-
SymbolKind::Variable { name } => {
309+
SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => {
310310
// Check if this variable is a parameter.
311311
let (tt, mut mods) =
312312
self.classify_variable(name, span.start, symbol_map, uri, ctx);

0 commit comments

Comments
 (0)