diff --git a/crates/perl-lsp-rs/src/call_hierarchy_provider/mod.rs b/crates/perl-lsp-rs/src/call_hierarchy_provider/mod.rs index e82362274b..0759de7b17 100644 --- a/crates/perl-lsp-rs/src/call_hierarchy_provider/mod.rs +++ b/crates/perl-lsp-rs/src/call_hierarchy_provider/mod.rs @@ -46,6 +46,36 @@ pub struct CallHierarchyProvider { position_mapper: PositionMapper, } +/// Extract the filename from a URI (e.g. `"file:///path/to/foo.pl"` → `"foo.pl"`). +/// +/// Used to name synthetic file-level callers for top-level call sites that are +/// not enclosed in any named subroutine. +fn uri_basename(uri: &str) -> String { + uri.rsplit('/').find(|s| !s.is_empty()).unwrap_or(uri).to_string() +} + +/// Synthesize a `CallHierarchyItem` representing a file-level (top-level) caller. +/// +/// Used when a call site is not enclosed in any named subroutine — the script +/// file itself becomes the logical "caller" so it appears in `incomingCalls` +/// instead of being silently dropped. +/// +/// Both the open-document traversal path (`mod.rs`) and the workspace-index +/// path (`hierarchy.rs`) use this helper, keeping synthesis logic in one +/// `--lib`-testable location. +pub(crate) fn synthetic_file_level_caller(uri: &str, range: Range) -> CallHierarchyItem { + CallHierarchyItem { + name: uri_basename(uri), + kind: "file".to_string(), + uri: uri.to_string(), + range, + selection_range: range, + detail: None, + package_name: None, + qualified_name: None, + } +} + impl CallHierarchyProvider { /// Create a new call hierarchy provider for a source file /// @@ -222,47 +252,49 @@ impl CallHierarchyProvider { qualified_name: None, }; - // Search within this function + // Search within this named function; return early so the + // bottom visitor does not re-visit children with the outer + // (possibly None) context, which would create spurious + // file-level callers for calls inside this sub. self.visit_children(node, |child| { self.find_incoming_calls(child, target_name, calls, Some(&item)); None::<()> }); + return; } + // Anonymous sub — fall through to the bottom visitor. } NodeKind::FunctionCall { name, .. } => { // Match exact name or package-qualified name (e.g. "Utils::format_string") let matches = name == target_name || name.ends_with(&format!("::{}", target_name)); if matches { - if let Some(from) = current_function { - let ranges = vec![self.node_to_range(node)]; - - // Check if we already have a call from this function - if let Some(existing) = calls.iter_mut().find(|c| c.from.name == from.name) - { - existing.from_ranges.extend(ranges); - } else { - calls.push(CallHierarchyIncomingCall { - from: from.clone(), - from_ranges: ranges, - }); - } + let call_range = self.node_to_range(node); + let from = current_function.cloned().unwrap_or_else(|| { + // Top-level call site — synthesize a file-level caller so the + // script appears in incomingCalls instead of being silently dropped. + synthetic_file_level_caller(uri, call_range) + }); + let ranges = vec![call_range]; + if let Some(existing) = calls.iter_mut().find(|c| c.from.name == from.name) { + existing.from_ranges.extend(ranges); + } else { + calls.push(CallHierarchyIncomingCall { from, from_ranges: ranges }); } } } NodeKind::MethodCall { method, .. } => { if method == target_name { - if let Some(from) = current_function { - let ranges = vec![self.node_to_range(node)]; - - if let Some(existing) = calls.iter_mut().find(|c| c.from.name == from.name) - { - existing.from_ranges.extend(ranges); - } else { - calls.push(CallHierarchyIncomingCall { - from: from.clone(), - from_ranges: ranges, - }); - } + let call_range = self.node_to_range(node); + let from = current_function.cloned().unwrap_or_else(|| { + // Top-level call site — synthesize a file-level caller so the + // script appears in incomingCalls instead of being silently dropped. + synthetic_file_level_caller(uri, call_range) + }); + let ranges = vec![call_range]; + if let Some(existing) = calls.iter_mut().find(|c| c.from.name == from.name) { + existing.from_ranges.extend(ranges); + } else { + calls.push(CallHierarchyIncomingCall { from, from_ranges: ranges }); } } } @@ -656,6 +688,7 @@ impl CallHierarchyItem { "kind": match self.kind.as_str() { "function" => 12, // SymbolKind.Function "method" => 6, // SymbolKind.Method + "file" => 1, // SymbolKind.File _ => 12, }, "uri": self.uri, @@ -916,4 +949,91 @@ sub helper { assert!(called_names.contains(&&"method_call".to_string())); } } + + /// `synthetic_file_level_caller` must return a `CallHierarchyItem` with kind + /// `"file"`, the basename of the URI as the name, and both `range` / + /// `selection_range` set to the supplied range. + #[test] + fn test_synthetic_file_level_caller_returns_file_item() { + let range = Range { + start: Position { line: 5, character: 0 }, + end: Position { line: 5, character: 20 }, + }; + let item = synthetic_file_level_caller("file:///path/to/script.pl", range); + assert_eq!(item.name, "script.pl"); + assert_eq!(item.kind, "file"); + assert_eq!(item.uri, "file:///path/to/script.pl"); + assert_eq!(item.range.start.line, 5); + assert_eq!(item.range.end.character, 20); + assert_eq!(item.selection_range.start.line, 5); + assert!(item.detail.is_none()); + assert!(item.package_name.is_none()); + assert!(item.qualified_name.is_none()); + } + + /// A top-level `MethodCall` (not inside any sub) must produce a file-level + /// caller rather than being silently dropped. + #[test] + fn test_incoming_calls_top_level_method_call_synthesizes_file_caller() { + let code = "App->run();\n"; + let mut parser = Parser::new(code); + if let Ok(ast) = parser.parse() { + let provider = + CallHierarchyProvider::new(code.to_string(), "file:///script.pl".to_string()); + let target_item = CallHierarchyItem { + name: "run".to_string(), + kind: "method".to_string(), + uri: "file:///App.pm".to_string(), + range: Range { + start: Position { line: 0, character: 0 }, + end: Position { line: 2, character: 1 }, + }, + selection_range: Range { + start: Position { line: 1, character: 4 }, + end: Position { line: 1, character: 7 }, + }, + detail: None, + package_name: None, + qualified_name: None, + }; + let incoming = provider.incoming_calls(&ast, &target_item); + assert_eq!(incoming.len(), 1, "expected exactly one file-level caller"); + assert_eq!(incoming[0].from.name, "script.pl"); + assert_eq!(incoming[0].from.kind, "file"); + assert_eq!(incoming[0].from.uri, "file:///script.pl"); + } + } + + /// A top-level `FunctionCall` (not inside any sub) must produce a file-level + /// caller rather than being silently dropped. + #[test] + fn test_incoming_calls_top_level_function_call_synthesizes_file_caller() { + let code = "target_func();\n"; + let mut parser = Parser::new(code); + if let Ok(ast) = parser.parse() { + let provider = + CallHierarchyProvider::new(code.to_string(), "file:///script.pl".to_string()); + let target_item = CallHierarchyItem { + name: "target_func".to_string(), + kind: "function".to_string(), + uri: "file:///lib.pm".to_string(), + range: Range { + start: Position { line: 0, character: 0 }, + end: Position { line: 2, character: 1 }, + }, + selection_range: Range { + start: Position { line: 0, character: 4 }, + end: Position { line: 0, character: 15 }, + }, + detail: None, + package_name: None, + qualified_name: None, + }; + let incoming = provider.incoming_calls(&ast, &target_item); + assert_eq!(incoming.len(), 1, "expected exactly one file-level caller"); + assert_eq!(incoming[0].from.name, "script.pl"); + assert_eq!(incoming[0].from.kind, "file"); + assert_eq!(incoming[0].from.uri, "file:///script.pl"); + } + } } diff --git a/crates/perl-lsp-rs/src/runtime/language/hierarchy.rs b/crates/perl-lsp-rs/src/runtime/language/hierarchy.rs index 5a3a517da5..e10b3b4728 100644 --- a/crates/perl-lsp-rs/src/runtime/language/hierarchy.rs +++ b/crates/perl-lsp-rs/src/runtime/language/hierarchy.rs @@ -630,22 +630,29 @@ impl LspServer { let refs = index.find_refs(&symbol_key); for location in refs { - if let Some(from) = - self.find_workspace_enclosing_callable(&callable_symbols, &location) - { - let key = (from.name.clone(), from.uri.clone()); - let from_range = index_location_to_wire_range(&location); - if let Some(&idx) = seen.get(&key) { - all_calls[idx].from_ranges.push(from_range); - } else { - seen.insert(key, all_calls.len()); - all_calls.push( - crate::call_hierarchy_provider::CallHierarchyIncomingCall { - from, - from_ranges: vec![from_range], - }, - ); - } + let from_range = index_location_to_wire_range(&location); + let from = self + .find_workspace_enclosing_callable(&callable_symbols, &location) + .unwrap_or_else(|| { + // Top-level call site — no enclosing callable in the + // workspace index. Synthesize a file-level caller so the + // script appears in incomingCalls instead of being dropped. + crate::call_hierarchy_provider::synthetic_file_level_caller( + &location.uri, + from_range, + ) + }); + let key = (from.name.clone(), from.uri.clone()); + if let Some(&idx) = seen.get(&key) { + all_calls[idx].from_ranges.push(from_range); + } else { + seen.insert(key, all_calls.len()); + all_calls.push( + crate::call_hierarchy_provider::CallHierarchyIncomingCall { + from, + from_ranges: vec![from_range], + }, + ); } } } @@ -912,6 +919,75 @@ mod tests { assert!(result.is_ok(), "handle_incoming_calls must not error: {result:?}"); } + /// Verifies that the workspace-index path in `handle_incoming_calls` synthesizes + /// a file-level `CallHierarchyItem` (kind=1/File) when a reference location in + /// the index has no enclosing callable symbol — i.e., it is a top-level call. + /// + /// Covers lines 633-645 (the `unwrap_or_else` closure + seen-map insert) for the + /// Codecov/Patch-95 gate (#3093). + #[cfg(feature = "workspace")] + #[test] + fn test_incoming_calls_workspace_path_synthesizes_file_level_caller() { + let server = LspServer::new(); + server.test_enable_call_hierarchy(); + + // script.pl: static method call at the TOP LEVEL (no enclosing sub). + // App->run() is a static call so workspace_index stores it as "App::run". + let script_uri = "file:///script.pl"; + let script_text = "App->run();\n"; + + // Index the file (transitions coordinator to Building internally). + server + .test_index_file_in_building_state(script_uri, script_text) + .expect("indexing script.pl"); + // Transition coordinator to Ready so workspace path is taken. + server.test_simulate_indexing_complete(); + + // Also open as a document so open-doc fallback doesn't add duplicates. + open_doc(&server, script_uri, script_text); + + // incomingCalls for "App::run" — data.packageName drives workspace_symbol_key. + let result = server.handle_incoming_calls(Some(json!({ + "item": { + "name": "run", + "kind": 6, + "uri": "file:///App.pm", + "range": { + "start": { "line": 0, "character": 0 }, + "end": { "line": 2, "character": 1 } + }, + "selectionRange": { + "start": { "line": 1, "character": 4 }, + "end": { "line": 1, "character": 7 } + }, + "data": { + "packageName": "App", + "qualifiedName": "App::run" + } + } + }))); + + assert!(result.is_ok(), "handle_incoming_calls must not error: {result:?}"); + let value = result.expect("already checked"); + let value = value.expect("handler must return Some value"); + // handle_incoming_calls returns the calls array directly (not wrapped in {"result":...}) + let calls = value.as_array().expect("result should be an array"); + + // The reference in script.pl has no enclosing callable, so the workspace + // path must synthesize a file-level caller with kind=1 (SymbolKind.File). + let file_caller = calls + .iter() + .find(|c| c["from"]["uri"].as_str().map_or(false, |u| u.contains("script.pl"))); + assert!(file_caller.is_some(), "expected file-level caller from script.pl, got: {calls:?}"); + let from = &file_caller.expect("already checked")["from"]; + assert_eq!( + from["kind"].as_u64(), + Some(1), + "file-level caller must have SymbolKind.File=1, got: {from:?}" + ); + assert_eq!(from["name"].as_str(), Some("script.pl")); + } + /// Verifies that `handle_outgoing_calls` executes the workspace /// index-readiness wait when indexing is in progress (#3095). #[cfg(feature = "workspace")] diff --git a/crates/perl-lsp-ux-tests/tests/ux_scenario_22_crossfile_extended_providers.rs b/crates/perl-lsp-ux-tests/tests/ux_scenario_22_crossfile_extended_providers.rs index b1e20c5763..90d56df69c 100644 --- a/crates/perl-lsp-ux-tests/tests/ux_scenario_22_crossfile_extended_providers.rs +++ b/crates/perl-lsp-ux-tests/tests/ux_scenario_22_crossfile_extended_providers.rs @@ -538,12 +538,10 @@ fn scenario_22_call_hierarchy_outgoing_from_run_hard_assert() -> anyhow::Result< /// Hard assert — callHierarchy/incomingCalls for `run` must return at least one caller. /// -/// BROKEN on current main: incomingCalls returns empty [] even though -/// script/real-baseline.pl calls `$app->run`. OO arrow-method caller lookup -/// is not resolving back to the CallHierarchyItem. -/// Tracking: #3093 +/// Fixed in #3093: top-level callers (not inside any `sub`) are now returned as +/// file-level CallHierarchyItems instead of being silently dropped. +/// script/real-baseline.pl calls `$app->run` at the top level — must appear. #[test] -#[ignore = "real gap — incomingCalls returns empty for OO method callers; tracking #3093"] fn scenario_22_call_hierarchy_incoming_to_run_hard_assert() -> anyhow::Result<()> { if !binary_available() { eprintln!("SKIP scenario_22: perl-lsp binary not found"); @@ -597,6 +595,17 @@ fn scenario_22_call_hierarchy_incoming_to_run_hard_assert() -> anyhow::Result<() script/real-baseline.pl calls `$app->run`. Got: []" ); + // The caller must be the script file — not just any non-empty result. + // This guards against vacuous passes where an unrelated item happens to appear. + let script_caller = calls.iter().find(|c| { + c["from"]["uri"].as_str().map(|u| u.contains("real-baseline.pl")).unwrap_or(false) + }); + assert!( + script_caller.is_some(), + "incomingCalls for `App::run` must include `real-baseline.pl` as a caller \ + (top-level `$app->run` call). Got callers: {calls:?}" + ); + harness.assert_no_crash(); Ok(()) }