diff --git a/packages/shared/code-nav.test.ts b/packages/shared/code-nav.test.ts index 49edcc88b..1c5a8b2d1 100644 --- a/packages/shared/code-nav.test.ts +++ b/packages/shared/code-nav.test.ts @@ -1,6 +1,10 @@ -import { describe, expect, test } from "bun:test"; +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { buildRgArgs, + resolveCodeNav, buildSignature, classifyMatch, classifyMatchDetailed, @@ -964,3 +968,139 @@ describe("resolveCodeNavHover", () => { resetRgCache(); }); }); + +// --------------------------------------------------------------------------- +// Directory exclusions (#1558) +// --------------------------------------------------------------------------- + +describe("buildRgArgs directory exclusions (#1558)", () => { + test("always-ignored names are excluded at any depth (unanchored glob)", () => { + const args = buildRgArgs("x"); + expect(args).toContain("!node_modules"); + expect(args).not.toContain("!/node_modules"); + }); + + test("ambiguous names are excluded only at the search root (anchored glob)", () => { + const args = buildRgArgs("x"); + for (const dir of ["vendor", "target", "build", "dist", "coverage"]) { + expect(args).toContain(`!/${dir}`); + expect(args).not.toContain(`!${dir}`); + } + }); + + test("a segment the origin file lives under is not excluded for that request", () => { + const args = buildRgArgs( + "fetchContent", + "java", + "src/main/java/com/example/vendor/app/ExampleService.java", + ); + expect(args).not.toContain("!/vendor"); + // Unrelated exclusions are untouched. + expect(args).toContain("!node_modules"); + expect(args).toContain("!/target"); + }); + + test("the origin-file rule also lifts an always-ignored segment", () => { + const args = buildRgArgs("x", undefined, "packages/node_modules/dep/index.js"); + expect(args).not.toContain("!node_modules"); + }); +}); + +// --------------------------------------------------------------------------- +// Real-ripgrep exclusion behavior (#1558) +// --------------------------------------------------------------------------- + +const RG_PATH = Bun.which("rg"); +const describeRg = RG_PATH ? describe : describe.skip; + +describeRg("resolveCodeNav against real ripgrep (#1558)", () => { + const realRuntime: CodeNavRuntime = { + async runCommand(command, args, options) { + const proc = Bun.spawn([command, ...args], { + cwd: options?.cwd, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + ]); + return { stdout, stderr, exitCode: await proc.exited }; + }, + }; + + let root = ""; + + const javaSource = (className: string) => + [ + `public class ${className} {`, + ` public String fetchContent() { return "example"; }`, + "}", + "", + ].join("\n"); + + beforeAll(async () => { + root = await mkdtemp(join(tmpdir(), "plannotator-code-nav-")); + // First-party Java package whose path contains a `vendor` segment. + await Bun.write( + `${root}/src/main/java/com/example/vendor/app/ExampleService.java`, + javaSource("ExampleService"), + ); + // Genuine third-party tree at the repo root. + await Bun.write( + `${root}/vendor/third_party/Vendored.java`, + javaSource("Vendored"), + ); + // Dependency output nested below the root. + await Bun.write( + `${root}/src/app/node_modules/dep/Dep.java`, + javaSource("Dep"), + ); + }); + + afterAll(async () => { + if (root) await rm(root, { recursive: true, force: true }); + }); + + const resolve = (filePath: string) => { + resetRgCache(); + return resolveCodeNav( + realRuntime, + { + symbol: "fetchContent", + filePath, + line: 1, + charStart: 0, + side: "new", + language: "java", + }, + root, + [], + ); + }; + + const paths = (result: Awaited>) => + [...result.definitions, ...result.references].map((l) => l.filePath); + + test("finds a symbol in a Java package containing a `vendor` segment", async () => { + const found = paths(await resolve("src/main/java/com/example/Caller.java")); + expect( + found.some((p) => p.includes("com/example/vendor/app/ExampleService.java")), + ).toBe(true); + }); + + test("a root-level vendor/ directory is still excluded", async () => { + const found = paths(await resolve("src/main/java/com/example/Caller.java")); + expect(found.some((p) => p.includes("vendor/third_party/"))).toBe(false); + }); + + test("node_modules is excluded at any depth", async () => { + const found = paths(await resolve("src/main/java/com/example/Caller.java")); + expect(found.some((p) => p.includes("node_modules"))).toBe(false); + }); + + test("a request from inside an excluded segment can find its own siblings", async () => { + const found = paths(await resolve("src/app/node_modules/dep/Caller.java")); + expect(found.some((p) => p.includes("node_modules/dep/Dep.java"))).toBe(true); + }); +}); diff --git a/packages/shared/code-nav.ts b/packages/shared/code-nav.ts index b0a8a65ae..53ce5c180 100644 --- a/packages/shared/code-nav.ts +++ b/packages/shared/code-nav.ts @@ -130,22 +130,42 @@ export interface CodeNavHoverResponse { // Constants // --------------------------------------------------------------------------- -const CODE_NAV_IGNORED_GLOBS = [ +/** + * Directory exclusions are split in two (#1558). + * + * A ripgrep glob with no slash matches a path SEGMENT at any depth, so + * `--glob !vendor` prunes every directory named `vendor` anywhere in the tree + * — including a first-party Java package such as + * `src/main/java/com/example/vendor/app/`, whose symbols then resolve to + * "No results". The same trap applies to `target` (Maven's build dir, but also + * an ordinary package/module name) and to `build` / `dist` / `coverage`, which + * are common nested directories in monorepos and in source trees alike. + * + * ALWAYS_IGNORED names can only ever be tool output, so they stay excluded at + * any depth. ROOT_ONLY names are ambiguous and are excluded only at the search + * root (`--glob !/vendor`, anchored like a gitignore rule); nested copies that + * really are build output are already skipped by ripgrep's default .gitignore + * handling. + */ +const CODE_NAV_ALWAYS_IGNORED_DIRS = [ "node_modules", ".git", - "dist", - "build", ".next", "__pycache__", ".turbo", ".cache", - "target", - "vendor", - "coverage", ".venv", ".pytest_cache", ]; +const CODE_NAV_ROOT_ONLY_IGNORED_DIRS = [ + "vendor", + "target", + "build", + "dist", + "coverage", +]; + const RG_TYPE_MAP: Record = { typescript: "ts", javascript: "js", @@ -261,7 +281,22 @@ function isTestFile(filePath: string): boolean { // rg argument construction // --------------------------------------------------------------------------- -export function buildRgArgs(symbol: string, language?: string): string[] { +/** Path segments of a repo-relative file path, `/` and `\` alike. */ +function pathSegments(filePath: string): Set { + return new Set(filePath.split(/[/\\]/).filter(Boolean)); +} + +export function buildRgArgs( + symbol: string, + language?: string, + /** + * Repo-relative path the request originated from. Belt and braces for + * #1558: a segment the origin file itself lives under is never excluded for + * that request, so a symbol in a changed file can always find its own + * siblings even if the directory name looks like tool output. + */ + originFilePath?: string, +): string[] { const args: string[] = [ "--json", "--line-number", @@ -273,10 +308,22 @@ export function buildRgArgs(symbol: string, language?: string): string[] { "--no-messages", ]; - for (const dir of CODE_NAV_IGNORED_GLOBS) { + const originSegments = originFilePath + ? pathSegments(originFilePath) + : new Set(); + + for (const dir of CODE_NAV_ALWAYS_IGNORED_DIRS) { + if (originSegments.has(dir)) continue; args.push("--glob", `!${dir}`); } + for (const dir of CODE_NAV_ROOT_ONLY_IGNORED_DIRS) { + if (originSegments.has(dir)) continue; + // Leading slash anchors the glob to the search root, so only a top-level + // `vendor/` (etc.) is pruned — not a same-named package deeper in the tree. + args.push("--glob", `!/${dir}`); + } + if (language) { const rgType = RG_TYPE_MAP[language]; if (rgType) args.push("--type", rgType); @@ -509,7 +556,7 @@ export async function resolveCodeNav( }; } - const args = buildRgArgs(request.symbol, request.language); + const args = buildRgArgs(request.symbol, request.language, request.filePath); const result = await runtime.runCommand("rg", args, { cwd,