fix(project-tree): search a file's own symbols, not its package, in Safe Delete and Find Usages (#1688)

Safe Delete and Find Usages on a file searched every top-level outline item, including a Java `package` declaration. References to the package are imports of any class in it, so deleting an unused class reported its siblings' usages as its own.

Packages and namespaces are shared with other files, so their members are searched in their place (Java packages, C++/C#/PHP namespaces). Modules a file declares itself (a TypeScript namespace, a Rust `mod`) are still searched as before.

Fixes #1656

Co-authored-by: jaw52 <[email protected]>

src/editor/refactor/safeDelete.test.ts

@@ -3,7 +3,7 @@ import { EditorState, Text } from "@codemirror/state";
 import { EditorView } from "@codemirror/view";
 import { history, undo } from "@codemirror/commands";
 import type { MessageRequest } from "@/workbench/messages";
-import type { LocalSymbol, Location, Locals, OutlineItem, Range, StructuralRanges, SyntaxNode } from "@/bindings";
+import type { LocalSymbol, Location, Locals, OutlineItem, Pos, Range, StructuralRanges, SyntaxNode } from "@/bindings";
 
 // --- backend fakes ---------------------------------------------------------
 
@@ -15,11 +15,12 @@ const unavailable = () => ({ status: "error" as const, error: { kind: "unavailab
 
 let goto: Answer<Location[]> = ok([]);
 let outline: Answer<OutlineItem[]> = ok([]);
-let references: Answer<Location[]> = ok([]);
+/** Called with the position searched from. */
+let references: (pos: Pos) => ReturnType<Answer<Location[]>> = ok([]);
 const lspGoto = vi.fn(async () => goto());
 const docOutline = vi.fn(async () => outline());
-const lspReferences = vi.fn(async () => {
-  const r = references();
+const lspReferences = vi.fn(async (_request: number, _doc: number, pos: Pos) => {
+  const r = references(pos);
   return r.status === "ok" ? { status: "ok" as const, data: { locations: r.data, incomplete: null } } : r;
 });
 const lspCodeActions = vi.fn(async () => ({ status: "ok", data: [] }));
@@ -411,6 +412,78 @@ describe("fileUsages", () => {
     expect(docClose).toHaveBeenCalledTimes(2);
   });
 
+  // #1656: packages and namespaces are shared with other files; their members are what the file declares.
+  describe("searches members of packages and namespaces, not the container", () => {
+    const at = (path: string, line: number): Location => ({
+      path,
+      range: { start: { line, col: 0 }, end: { line, col: 1 } },
+    });
+    const within = (parent: OutlineItem, ...children: OutlineItem[]): OutlineItem => ({ ...parent, children });
+    /** References per searched name; `searched` records the order. */
+    function serve(text: string, refs: Record<string, Location[]>) {
+      const searched: string[] = [];
+      references = (pos) => {
+        const off = doc(text).line(pos.line + 1).from + pos.col;
+        const name = Object.keys(refs).find((n) => text.startsWith(n, off)) ?? "?";
+        searched.push(name);
+        return { status: "ok", data: refs[name] ?? [] };
+      };
+      return searched;
+    }
+
+    it("Java: the package's other classes imported elsewhere are not usages", async () => {
+      const text = "package com.example.config;\n\npublic class MessageSourceConfig {}\n";
+      outline = ok([
+        item(text, "com.example.config", "package com.example.config;", "package"),
+        item(text, "MessageSourceConfig", "public class MessageSourceConfig {}", "class"),
+      ]);
+      // References to the package: an import of its sibling class CommunicationConfig.
+      const searched = serve(text, {
+        "com.example.config": [at("/p/server/CommandSender.java", 2)],
+        MessageSourceConfig: [],
+      });
+      expect(await fileUsages(["/p/config/MessageSourceConfig.java"])).toEqual({ status: "ok", usages: [] });
+      expect(searched).toEqual(["MessageSourceConfig"]);
+    });
+
+    it("C++: a class inside a namespace is searched", async () => {
+      const text = "namespace config {\nclass Used {};\nnamespace detail {\nint helper();\n}\n}\n";
+      const ns = item(
+        text,
+        "config",
+        "namespace config {\nclass Used {};\nnamespace detail {\nint helper();\n}\n}",
+        "namespace",
+      );
+      const detail = item(text, "detail", "namespace detail {\nint helper();\n}", "namespace");
+      outline = ok([
+        within(
+          ns,
+          item(text, "Used", "class Used {};", "class"),
+          within(detail, item(text, "helper", "int helper();")),
+        ),
+      ]);
+      const searched = serve(text, { Used: [at("/p/main.cpp", 4)], helper: [at("/p/main.cpp", 5)] });
+      expect(await fileUsages(["/p/config/used.h"])).toEqual({
+        status: "ok",
+        usages: [at("/p/main.cpp", 4), at("/p/main.cpp", 5)],
+      });
+      expect(searched).toEqual(["Used", "helper"]);
+    });
+
+    it("modules a file declares (TS namespace, Rust mod) are still searched", async () => {
+      const text = "export namespace Utils {\n  export const version = 1;\n}\n";
+      outline = ok([
+        within(
+          item(text, "Utils", "export namespace Utils {\n  export const version = 1;\n}", "module"),
+          item(text, "version", "version = 1", "variable"),
+        ),
+      ]);
+      const searched = serve(text, { Utils: [at("/p/main.ts", 5)] });
+      expect(await fileUsages(["/p/lib/ns.ts"])).toEqual({ status: "ok", usages: [at("/p/main.ts", 5)] });
+      expect(searched).toEqual(["Utils"]);
+    });
+  });
+
   it("is unsupported when the language server can't search", async () => {
     outline = ok([item(text, "a", "pub fn a() {}")]);
     references = unavailable;

src/editor/refactor/safeDelete.ts

@@ -507,9 +507,20 @@ export type FileUsages =
   /** The language server can't tell (none running, no document symbols…). */
   | { status: "unsupported"; reason: string };
 
-/** At most this many top-level symbols per file are searched. */
+/** At most this many symbols per file are searched. */
 const MAX_SYMBOLS = 60;
 
+/**
+ * The symbols a file declares at its top level. Packages and namespaces are shared
+ * with other files (IDEA searches a Java file's classes, not its package), so their
+ * members stand in for them.
+ */
+function declaredSymbols(items: readonly OutlineItem[]): OutlineItem[] {
+  return items.flatMap((it) =>
+    it.kind === "package" || it.kind === "namespace" ? declaredSymbols(it.children) : [it],
+  );
+}
+
 async function withDoc<T>(path: string, fn: (doc: number) => Promise<T>): Promise<T> {
   const open = editorsFor(path)[0];
   if (open) return fn(open.ctx.doc);
@@ -522,7 +533,7 @@ async function withDoc<T>(path: string, fn: (doc: number) => Promise<T>): Promis
 }
 
 /**
- * References from outside `paths` to the top-level symbols the files declare.
+ * References from outside `paths` to the symbols the files declare ({@link declaredSymbols}).
  * Unsupported when no reference search could run for any of them.
  */
 export async function fileUsages(paths: readonly string[], signal?: AbortSignal): Promise<FileUsages> {
@@ -536,7 +547,7 @@ export async function fileUsages(paths: readonly string[], signal?: AbortSignal)
     try {
       await withDoc(path, async (doc) => {
         const outline = await call(commands.docOutline(doc));
-        for (const item of outline.slice(0, MAX_SYMBOLS)) {
+        for (const item of declaredSymbols(outline).slice(0, MAX_SYMBOLS)) {
           if (signal?.aborted) return;
           let refs: Location[];
           try {