diff --git a/Cargo.lock b/Cargo.lock index 8a4ff7ad..47aa8f22 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1404,6 +1404,7 @@ dependencies = [ "mago-php-version", "mago-span", "mago-syntax", + "mago-text-edit", "mago-type-syntax", "memchr", "parking_lot", diff --git a/Cargo.toml b/Cargo.toml index 727146f6..e9911ce8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ mago-type-syntax = "=1.29.0" mago-formatter = "=1.29.0" mago-php-version = "=1.29.0" mago-composer = "=1.29.0" +mago-text-edit = "=1.29.0" bumpalo = "3" ignore = "0.4" memchr = "2" diff --git a/src/diagnostics/unused_variables.rs b/src/diagnostics/unused_variables.rs index f73594dc..d3c73770 100644 --- a/src/diagnostics/unused_variables.rs +++ b/src/diagnostics/unused_variables.rs @@ -622,6 +622,22 @@ function foo() { assert!(diags.is_empty()); } + #[test] + fn no_diagnostic_when_variable_is_used_in_dynamic_property_access() { + let diags = collect( + r#"{$attribute}; +} +"#, + ); + assert!( + diags.is_empty(), + "dynamic property selector should count as a read" + ); + } + #[test] fn skips_unused_parameter() { // Parameters are intentionally not flagged until suppression diff --git a/src/rename/tests.rs b/src/rename/tests.rs index 87103f7d..b47658c7 100644 --- a/src/rename/tests.rs +++ b/src/rename/tests.rs @@ -185,6 +185,36 @@ async fn rename_variable_updates_compact_string() { assert!(updated.contains("compact('person')")); } +#[tokio::test] +async fn rename_variable_updates_dynamic_property_selector() { + let backend = Backend::new_test(); + let uri = Url::parse("file:///test.php").unwrap(); + let text = concat!( + "{$attribute})) {\n", + " return;\n", + " }\n", + " echo $attribute;\n", + "}\n", + ); + + open_file(&backend, &uri, text).await; + + let edit = rename(&backend, &uri, 2, 6, "$field").await; + assert!( + edit.is_some(), + "Expected a workspace edit for variable rename" + ); + + let file_edits = edits_for_uri(&edit.unwrap(), &uri); + let updated = apply_edits(text, &file_edits); + assert!(updated.contains("$field = strtolower($type);")); + assert!(updated.contains("$message->{$field}")); + assert!(updated.contains("echo $field;")); +} + #[tokio::test] async fn rename_from_compact_string_updates_variable() { let backend = Backend::new_test(); diff --git a/src/scope_collector/mod.rs b/src/scope_collector/mod.rs index 8428234a..b70882fc 100644 --- a/src/scope_collector/mod.rs +++ b/src/scope_collector/mod.rs @@ -1046,10 +1046,25 @@ fn walk_expression(expr: &Expression<'_>, collector: &mut Collector<'_>) { Expression::Access(access) => match access { Access::Property(pa) => { walk_expression(pa.object, collector); - // property selector is not a variable read + match &pa.property { + ClassLikeMemberSelector::Identifier(_) + | ClassLikeMemberSelector::Missing(_) => {} + ClassLikeMemberSelector::Variable(var) => walk_variable_read(var, collector), + ClassLikeMemberSelector::Expression(selector) => { + walk_expression(selector.expression, collector) + } + } } Access::NullSafeProperty(pa) => { walk_expression(pa.object, collector); + match &pa.property { + ClassLikeMemberSelector::Identifier(_) + | ClassLikeMemberSelector::Missing(_) => {} + ClassLikeMemberSelector::Variable(var) => walk_variable_read(var, collector), + ClassLikeMemberSelector::Expression(selector) => { + walk_expression(selector.expression, collector) + } + } } Access::StaticProperty(spa) => { walk_expression(spa.class, collector); @@ -1354,9 +1369,23 @@ fn walk_expression_as_write(expr: &Expression<'_>, collector: &mut Collector<'_> Expression::Access(Access::Property(pa)) => { // `$obj->prop = …` — $obj is read, prop is written. walk_expression(pa.object, collector); + match &pa.property { + ClassLikeMemberSelector::Identifier(_) | ClassLikeMemberSelector::Missing(_) => {} + ClassLikeMemberSelector::Variable(var) => walk_variable_read(var, collector), + ClassLikeMemberSelector::Expression(selector) => { + walk_expression(selector.expression, collector) + } + } } Expression::Access(Access::NullSafeProperty(pa)) => { walk_expression(pa.object, collector); + match &pa.property { + ClassLikeMemberSelector::Identifier(_) | ClassLikeMemberSelector::Missing(_) => {} + ClassLikeMemberSelector::Variable(var) => walk_variable_read(var, collector), + ClassLikeMemberSelector::Expression(selector) => { + walk_expression(selector.expression, collector) + } + } } Expression::Access(Access::StaticProperty(spa)) => { walk_expression(spa.class, collector); diff --git a/src/scope_collector/tests.rs b/src/scope_collector/tests.rs index 1d16ceb0..7fba8e96 100644 --- a/src/scope_collector/tests.rs +++ b/src/scope_collector/tests.rs @@ -532,6 +532,20 @@ function test() { assert_eq!(count_kind(&scope_map, "$config", AccessKind::Read), 1); } +#[test] +fn dynamic_property_selector_reads_variable() { + let php = r#"{$attribute}; +} +"#; + let scope_map = collect_from_function(php); + + assert_eq!(count_kind(&scope_map, "$attribute", AccessKind::Write), 1); + assert_eq!(count_kind(&scope_map, "$attribute", AccessKind::Read), 1); +} + // ─── Unset ────────────────────────────────────────────────────────────────── #[test] diff --git a/src/symbol_map/extraction.rs b/src/symbol_map/extraction.rs index 51f44dda..5f1c0c3c 100644 --- a/src/symbol_map/extraction.rs +++ b/src/symbol_map/extraction.rs @@ -1767,6 +1767,34 @@ fn extract_from_hint_ctx(hint: &Hint<'_>, spans: &mut Vec, ref_ctx: // ─── Expression extractor ─────────────────────────────────────────────────── +fn extract_variable_symbol_spans<'a>( + var: &'a Variable<'a>, + ctx: &mut ExtractionCtx<'a>, + scope_start: u32, +) { + match var { + Variable::Direct(dv) => { + let raw = bytes_to_str(dv.name); + if raw == "$this" { + ctx.spans.push(SymbolSpan { + start: dv.span.start.offset, + end: dv.span.end.offset, + kind: SymbolKind::SelfStaticParent(SelfStaticParentKind::This), + }); + } else { + let name = raw.strip_prefix('$').unwrap_or(raw).to_string(); + ctx.spans.push(SymbolSpan { + start: dv.span.start.offset, + end: dv.span.end.offset, + kind: SymbolKind::Variable { name }, + }); + } + } + Variable::Indirect(iv) => extract_from_expression(iv.expression, ctx, scope_start), + Variable::Nested(nv) => extract_variable_symbol_spans(nv.variable, ctx, scope_start), + } +} + fn extract_from_expression<'a>( expr: &'a Expression<'a>, ctx: &mut ExtractionCtx<'a>, @@ -2105,38 +2133,56 @@ fn extract_from_expression<'a>( let subject_text = expr_to_subject_text(pa.object); extract_from_expression(pa.object, ctx, scope_start); - if let ClassLikeMemberSelector::Identifier(ident) = &pa.property { - let member_name = bytes_to_str(ident.value).to_string(); - ctx.spans.push(SymbolSpan { - start: ident.span.start.offset, - end: ident.span.end.offset, - kind: SymbolKind::MemberAccess { - subject_text, - member_name, - is_static: false, - is_method_call: false, - is_docblock_reference: false, - }, - }); + match &pa.property { + ClassLikeMemberSelector::Identifier(ident) => { + let member_name = bytes_to_str(ident.value).to_string(); + ctx.spans.push(SymbolSpan { + start: ident.span.start.offset, + end: ident.span.end.offset, + kind: SymbolKind::MemberAccess { + subject_text, + member_name, + is_static: false, + is_method_call: false, + is_docblock_reference: false, + }, + }); + } + ClassLikeMemberSelector::Variable(var) => { + extract_variable_symbol_spans(var, ctx, scope_start) + } + ClassLikeMemberSelector::Expression(selector) => { + extract_from_expression(selector.expression, ctx, scope_start) + } + ClassLikeMemberSelector::Missing(_) => {} } } Access::NullSafeProperty(pa) => { let subject_text = expr_to_subject_text(pa.object); extract_from_expression(pa.object, ctx, scope_start); - if let ClassLikeMemberSelector::Identifier(ident) = &pa.property { - let member_name = bytes_to_str(ident.value).to_string(); - ctx.spans.push(SymbolSpan { - start: ident.span.start.offset, - end: ident.span.end.offset, - kind: SymbolKind::MemberAccess { - subject_text, - member_name, - is_static: false, - is_method_call: false, - is_docblock_reference: false, - }, - }); + match &pa.property { + ClassLikeMemberSelector::Identifier(ident) => { + let member_name = bytes_to_str(ident.value).to_string(); + ctx.spans.push(SymbolSpan { + start: ident.span.start.offset, + end: ident.span.end.offset, + kind: SymbolKind::MemberAccess { + subject_text, + member_name, + is_static: false, + is_method_call: false, + is_docblock_reference: false, + }, + }); + } + ClassLikeMemberSelector::Variable(var) => { + extract_variable_symbol_spans(var, ctx, scope_start) + } + ClassLikeMemberSelector::Expression(selector) => { + extract_from_expression(selector.expression, ctx, scope_start) + } + ClassLikeMemberSelector::Missing(_) => {} } } Access::StaticProperty(spa) => { diff --git a/tests/integration/references.rs b/tests/integration/references.rs index cfe31c3c..7593f31b 100644 --- a/tests/integration/references.rs +++ b/tests/integration/references.rs @@ -387,6 +387,37 @@ function test(): void { ); } +#[test] +fn variable_references_include_dynamic_property_selector() { + let backend = create_test_backend(); + let uri = "file:///tmp/test_refs_dynamic_selector.php"; + let content = r#"{$attribute})) { + return; + } + echo $attribute; +} +"#; + + open_file(&backend, uri, content); + + let results = backend + .find_references(uri, content, Position::new(3, 5), true) + .expect("should find references"); + + assert_no_duplicates(&results, "variable_references_dynamic_selector"); + assert_eq!( + results.len(), + 3, + "Expected 3 references (definition + dynamic selector + echo), got {}: {:#?}", + results.len(), + results + ); +} + // ─── Static member references ─────────────────────────────────────────────── #[test]