From a79ef479e301976310b1bfda7cdde25b8140d572 Mon Sep 17 00:00:00 2001 From: Caleb White Date: Thu, 18 Jun 2026 07:56:36 -0500 Subject: [PATCH] feat(rename): link compact strings to local variables --- src/definition/resolve.rs | 14 ++-- src/definition/type_definition.rs | 2 +- src/highlight/mod.rs | 42 ++++++------ src/hover/mod.rs | 2 +- src/references/mod.rs | 46 +++++++------ src/references/tests.rs | 25 +++++++ src/rename/mod.rs | 29 +++++++-- src/rename/tests.rs | 79 +++++++++++++++++++++++ src/semantic_tokens.rs | 2 +- src/symbol_map/extraction.rs | 43 ++++++++++++ src/symbol_map/mod.rs | 11 ++++ tests/integration/definition_variables.rs | 54 ++++++++++++++++ 12 files changed, 293 insertions(+), 56 deletions(-) diff --git a/src/definition/resolve.rs b/src/definition/resolve.rs index e243aaf9..4318e8cb 100644 --- a/src/definition/resolve.rs +++ b/src/definition/resolve.rs @@ -169,7 +169,7 @@ impl Backend { cursor_offset: u32, ) -> Option> { match kind { - SymbolKind::Variable { name } => { + SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => { // Try the precomputed var_defs map first. // This avoids re-parsing the file at request time. @@ -191,10 +191,14 @@ impl Backend { let parsed_uri = Url::parse(uri).ok()?; let start = crate::util::offset_to_position(content, cursor_offset as usize); - let end = crate::util::offset_to_position( - content, - cursor_offset as usize + 1 + name.len(), - ); + let end_offset = match kind { + SymbolKind::Variable { .. } => cursor_offset as usize + 1 + name.len(), + SymbolKind::CompactVariable { .. } => { + cursor_offset as usize + name.len() + } + _ => unreachable!(), + }; + let end = crate::util::offset_to_position(content, end_offset); return Some(vec![Location { uri: parsed_uri, range: Range { start, end }, diff --git a/src/definition/type_definition.rs b/src/definition/type_definition.rs index 60b4d85c..33fdbd9b 100644 --- a/src/definition/type_definition.rs +++ b/src/definition/type_definition.rs @@ -47,7 +47,7 @@ impl Backend { let function_loader = self.function_loader(&ctx); let resolved_types: Vec = match &symbol.kind { - SymbolKind::Variable { name } => { + SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => { let in_static = self .symbol_maps .read() diff --git a/src/highlight/mod.rs b/src/highlight/mod.rs index 0fb8424d..57149e8b 100644 --- a/src/highlight/mod.rs +++ b/src/highlight/mod.rs @@ -41,7 +41,7 @@ impl Backend { let symbol_map = maps.get(uri)?; let highlights = match &span.kind { - SymbolKind::Variable { name } => { + SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => { // Check if this is actually a property declaration — if // so, highlight member accesses instead of local vars. if let Some(VarDefKind::Property) = symbol_map.var_def_kind_at(name, span.start) { @@ -111,27 +111,29 @@ impl Backend { // Collect from symbol spans. for span in &symbol_map.spans { - if let SymbolKind::Variable { name } = &span.kind { - if name != var_name { - continue; - } - let span_scope = symbol_map.find_variable_scope(name, span.start); - if span_scope != scope_start { - continue; - } - seen_offsets.insert(span.start); + let name = match &span.kind { + SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => name, + _ => continue, + }; + if name != var_name { + continue; + } + let span_scope = symbol_map.find_variable_scope(name, span.start); + if span_scope != scope_start { + continue; + } + seen_offsets.insert(span.start); - let kind = if symbol_map.var_def_kind_at(name, span.start).is_some() { - DocumentHighlightKind::WRITE - } else { - DocumentHighlightKind::READ - }; + let kind = if symbol_map.var_def_kind_at(name, span.start).is_some() { + DocumentHighlightKind::WRITE + } else { + DocumentHighlightKind::READ + }; - highlights.push(DocumentHighlight { - range: byte_range_to_lsp_range(content, span.start as usize, span.end as usize), - kind: Some(kind), - }); - } + highlights.push(DocumentHighlight { + range: byte_range_to_lsp_range(content, span.start as usize, span.end as usize), + kind: Some(kind), + }); } // Include var_def sites that may not have a matching Variable span diff --git a/src/hover/mod.rs b/src/hover/mod.rs index 4e119c0e..b25c83b2 100644 --- a/src/hover/mod.rs +++ b/src/hover/mod.rs @@ -490,7 +490,7 @@ impl Backend { let function_loader = self.function_loader(&ctx); match kind { - SymbolKind::Variable { name } => { + SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => { // Suppress hover when the cursor is on a variable at its // definition site where the type is already visible in // the signature (properties, static/global declarations). diff --git a/src/references/mod.rs b/src/references/mod.rs index 1edd0dd5..694a3711 100644 --- a/src/references/mod.rs +++ b/src/references/mod.rs @@ -120,7 +120,7 @@ impl Backend { include_declaration: bool, ) -> Vec { match kind { - SymbolKind::Variable { name } => { + SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => { // Property declarations use Variable spans (so GTD can // jump to the type hint), but Find References should // search for member accesses, not local variable uses. @@ -330,26 +330,28 @@ impl Backend { let reachable_scopes = Self::collect_capture_scopes(symbol_map, var_name, scope_start); for span in &symbol_map.spans { - if let SymbolKind::Variable { name } = &span.kind { - if name != var_name { - continue; - } - // Check that this variable is in a reachable scope. - let span_scope = symbol_map.find_variable_scope(name, span.start); - if !reachable_scopes.contains(&span_scope) { - continue; - } - // Optionally skip declaration sites. - if !include_declaration && symbol_map.var_def_kind_at(name, span.start).is_some() { - continue; - } - let start = offset_to_position(content, span.start as usize); - let end = offset_to_position(content, span.end as usize); - locations.push(Location { - uri: parsed_uri.clone(), - range: Range { start, end }, - }); + let name = match &span.kind { + SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => name, + _ => continue, + }; + if name != var_name { + continue; + } + // Check that this variable is in a reachable scope. + let span_scope = symbol_map.find_variable_scope(name, span.start); + if !reachable_scopes.contains(&span_scope) { + continue; + } + // Optionally skip declaration sites. + if !include_declaration && symbol_map.var_def_kind_at(name, span.start).is_some() { + continue; } + let start = offset_to_position(content, span.start as usize); + let end = offset_to_position(content, span.end as usize); + locations.push(Location { + uri: parsed_uri.clone(), + range: Range { start, end }, + }); } // Also include var_def sites if include_declaration is set, @@ -412,7 +414,9 @@ impl Backend { scope_ends: &HashMap, ) -> bool { symbol_map.spans.iter().any(|s| { - if let SymbolKind::Variable { name } = &s.kind { + if let SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } = + &s.kind + { name == var_name && scope_ends .get(&scope_start) diff --git a/src/references/tests.rs b/src/references/tests.rs index 163fafa4..768b026d 100644 --- a/src/references/tests.rs +++ b/src/references/tests.rs @@ -128,6 +128,31 @@ async fn test_variable_references_exclude_declaration() { ); } +#[tokio::test] +async fn test_variable_references_include_compact_string() { + let backend = Backend::new_test(); + let uri = Url::parse("file:///test.php").unwrap(); + let text = concat!( + "` or `?->` don't include it. let is_property = self.is_property_rename(&span.kind, uri, &span); - let is_variable = matches!(&span.kind, SymbolKind::Variable { .. }) && !is_property; + let is_variable = matches!( + &span.kind, + SymbolKind::Variable { .. } | SymbolKind::CompactVariable { .. } + ) && !is_property; // For class renames, delegate to the specialised handler that // understands `use` statements, aliases, and collisions. @@ -163,12 +166,22 @@ impl Backend { }; let edit_text = if is_variable { - // Variables: the reference range includes the `$`, so - // the new name should also include it. - if new_name.starts_with('$') { - new_name.to_string() - } else { - format!("${}", new_name) + let bare_name = new_name.strip_prefix('$').unwrap_or(new_name); + let loc_symbol = loc_content.as_deref().and_then(|c| { + self.lookup_symbol_at_position(&loc_uri_str, c, location.range.start) + }); + match loc_symbol { + Some(crate::symbol_map::SymbolSpan { + kind: SymbolKind::CompactVariable { .. }, + .. + }) => bare_name.to_string(), + _ => { + if new_name.starts_with('$') { + new_name.to_string() + } else { + format!("${}", new_name) + } + } } } else if is_property { // Properties: the reference may or may not include `$`. @@ -539,6 +552,7 @@ impl Backend { // Include the `$` prefix in the range — the span already does. Some((format!("${}", name), range)) } + SymbolKind::CompactVariable { name } => Some((name.clone(), range)), SymbolKind::ClassReference { name, .. } => Some((name.clone(), range)), SymbolKind::ClassDeclaration { name } => Some((name.clone(), range)), SymbolKind::MemberAccess { member_name, .. } => Some((member_name.clone(), range)), @@ -614,6 +628,7 @@ impl Backend { self.lookup_var_def_kind_at(uri, name, span.start) .is_some_and(|k| k == crate::symbol_map::VarDefKind::Property) } + SymbolKind::CompactVariable { .. } => false, _ => false, } } diff --git a/src/rename/tests.rs b/src/rename/tests.rs index f4e9d7c0..b432b270 100644 --- a/src/rename/tests.rs +++ b/src/rename/tests.rs @@ -159,6 +159,58 @@ async fn rename_variable_without_dollar_prefix() { } } +#[tokio::test] +async fn rename_variable_updates_compact_string() { + let backend = Backend::new_test(); + let uri = Url::parse("file:///test.php").unwrap(); + let text = concat!( + " { + SymbolKind::Variable { name } | SymbolKind::CompactVariable { name } => { // Check if this variable is a parameter. let (tt, mut mods) = self.classify_variable(name, span.start, symbol_map, uri, ctx); diff --git a/src/symbol_map/extraction.rs b/src/symbol_map/extraction.rs index 900364ed..8c6b6791 100644 --- a/src/symbol_map/extraction.rs +++ b/src/symbol_map/extraction.rs @@ -1889,6 +1889,13 @@ fn extract_from_expression<'a>( Expression::Identifier(ident) => { let name = bytes_to_str(ident.value()).to_string(); let name_clean = strip_fqn_prefix(&name).to_string(); + if name_clean.eq_ignore_ascii_case("compact") { + try_emit_compact_string_spans( + &func_call.argument_list, + ctx.content, + &mut ctx.spans, + ); + } ctx.spans.push(SymbolSpan { start: ident.span().start.offset, end: ident.span().end.offset, @@ -3365,6 +3372,42 @@ fn try_emit_laravel_string_span( }); } +/// If `argument_list` belongs to a `compact()` call, emit one +/// [`SymbolKind::CompactVariable`] span per direct string-literal argument. +fn try_emit_compact_string_spans( + argument_list: &ArgumentList<'_>, + content: &str, + spans: &mut Vec, +) { + for arg in argument_list.arguments.iter() { + let Expression::Literal(literal::Literal::String(s)) = arg.value() else { + continue; + }; + let inner_start = s.span.start.offset + 1; + let inner_end = s.span.end.offset - 1; + if inner_start >= inner_end || inner_end as usize > content.len() { + continue; + } + + let name = if let Some(value) = s.value { + bytes_to_str(value) + } else { + &content[inner_start as usize..inner_end as usize] + }; + if name.is_empty() { + continue; + } + + spans.push(SymbolSpan { + start: inner_start, + end: inner_end, + kind: SymbolKind::CompactVariable { + name: name.to_string(), + }, + }); + } +} + /// Returns `true` if `name` is a method on Laravel's `Repository` config contract /// that accepts a config key as its first argument. fn is_config_repository_method(name: &str) -> bool { diff --git a/src/symbol_map/mod.rs b/src/symbol_map/mod.rs index 92653c8b..f340c0fd 100644 --- a/src/symbol_map/mod.rs +++ b/src/symbol_map/mod.rs @@ -138,6 +138,17 @@ pub(crate) enum SymbolKind { name: String, }, + /// A string literal argument inside `compact('var')` that references + /// a local variable by name. + /// + /// The span covers the string content inside the quotes so rename and + /// go-to-definition can treat it as a variable reference while still + /// replacing only the bare name text. + CompactVariable { + /// Variable name without `$` prefix. + name: String, + }, + /// Standalone function call name (not a method call). /// /// When `is_definition` is `true`, the span covers the function name diff --git a/tests/integration/definition_variables.rs b/tests/integration/definition_variables.rs index c917eb13..98734b47 100644 --- a/tests/integration/definition_variables.rs +++ b/tests/integration/definition_variables.rs @@ -978,6 +978,60 @@ async fn test_goto_definition_variable_jumps_to_assignment() { } } +#[tokio::test] +async fn test_goto_definition_compact_string_jumps_to_variable_assignment() { + let backend = create_test_backend(); + + let uri = Url::parse("file:///var_goto_compact.php").unwrap(); + let text = concat!( + " { + assert_eq!(location.uri, uri); + assert_eq!(location.range.start.line, 2, "$user is assigned on line 2"); + assert_eq!( + location.range.start.character, 4, + "$user starts at column 4" + ); + } + other => panic!("Expected Scalar location, got: {:?}", other), + } +} + /// When the cursor is on a variable at its definition site (the assignment), /// GTD should return the self-location so editors can fall back to Find References. #[tokio::test]