Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 141 additions & 1 deletion packages/shared/code-nav.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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<ReturnType<typeof resolve>>) =>
[...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);
});
});
65 changes: 56 additions & 9 deletions packages/shared/code-nav.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> = {
typescript: "ts",
javascript: "js",
Expand Down Expand Up @@ -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<string> {
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",
Expand All @@ -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<string>();

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);
Expand Down Expand Up @@ -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,
Expand Down