fix(lsp): callHierarchy/incomingCalls returns top-level/script callers (#3093)

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");
+        }
+    }
 }

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")]

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(())
 }