From df222ae35af0c380bd1f18bec2fc9c4b49c455d1 Mon Sep 17 00:00:00 2001 From: Ercan Er Date: Fri, 11 Sep 2026 04:35:29 +0300 Subject: [PATCH] fix(graph): a grammar that fails to load costs its language, not the CLI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit extract.ts imported all nine depth-tier grammars at the top of the module. They are native (node-gyp) modules, so any one of them failing to load took the whole CLI down at import time, before argv was read — `--version` and `--help` died exactly like a build, with a node-gyp-build stack trace that never says "graft". On the machine in #323 that grammar is tree-sitter-kotlin, which ships no prebuilds at all and so cannot load without a C toolchain. Load them through createRequire instead and keep what loads. The extension table is filtered by that, which is all the rest of the pipeline needs: an unavailable language stops claiming its files, and they take the paths a language graft has no grammar for takes today — the breadth tier where a generic row claims the extension, unindexed where none does. Warned once per language, the first time a file it would have claimed comes past, so a repo with no Kotlin in it stays quiet and a Kotlin repo never indexes short in silence. That alone keeps the CLI alive but leaves a language with no breadth-tier row indexing nothing — and Kotlin had none, so the machine in #323 would start and index no Kotlin at all. tree-sitter-wasm ships a kotlin grammar and queries/kotlin.scm has been in the tree unused since Kotlin was promoted to the depth tier, so the fallback is one registry row. A row whose extension the depth tier also claims is unreachable while the native grammar loads — `.java` has been exactly that all along, which is why the "must not collide" note needed rewriting rather than an exception. Both warmGenericGrammars calls now apply the tier precedence their own parse loops apply, so a fallback row cannot warm a WASM grammar the run will never call. That was already true of `.java`; a second row would have doubled it. Measured on a three-file repo (.ts/.kt/.rs) with tree-sitter-kotlin broken: the CLI does not start before this change; with the isolation alone, 6 nodes [rust, typescript]; with the fallback row too, 11 nodes [kotlin, rust, typescript]. With the grammar healthy the graph is identical either way. The test breaks a grammar both ways a module can be reached, `import` and `require`, so it still fails if the static imports ever come back. Refs #323 Co-Authored-By: Claude Opus 5 --- src/graph/build.ts | 10 +++- src/graph/check.ts | 8 ++- src/graph/extract.ts | 100 ++++++++++++++++++++++++------- src/graph/generic.ts | 12 +++- test/break-grammar-preload.cjs | 49 +++++++++++++++ test/grammar-unavailable.test.ts | 89 +++++++++++++++++++++++++++ 6 files changed, 243 insertions(+), 25 deletions(-) create mode 100644 test/break-grammar-preload.cjs create mode 100644 test/grammar-unavailable.test.ts diff --git a/src/graph/build.ts b/src/graph/build.ts index b3094051d..dea804be0 100644 --- a/src/graph/build.ts +++ b/src/graph/build.ts @@ -194,7 +194,15 @@ export async function buildGraph( // needs ONCE here (buildGraph is async) before the synchronous parse loop below // can call extractGeneric. Depth-tier (native) grammars need no warmup. await warmGenericGrammars( - new Set(files.map((f) => genericLangOf(f.abs)?.name).filter((n): n is string => !!n)), + // Tier precedence, same as the parse loop below: the breadth tier is reached + // only for files no depth grammar claims, so a fallback row (`.java`, `.kt`) + // must not warm a WASM grammar this build will never call. + new Set( + files + .filter((f) => !languageOf(f.abs)) + .map((f) => genericLangOf(f.abs)?.name) + .filter((n): n is string => !!n), + ), ); // Container tier (.vue and friends) loads its wrapper grammars the same way, // for the same reason: extractContainer runs inside the sync loop below. diff --git a/src/graph/check.ts b/src/graph/check.ts index d36f2bd27..44042bb82 100644 --- a/src/graph/check.ts +++ b/src/graph/check.ts @@ -88,7 +88,13 @@ export async function checkGraph( const onlyDirs = fpOnlyDirs && fpOnlyDirs.length > 0 ? new Set(fpOnlyDirs) : undefined; const sourceFiles = listSourceFiles(root, outDir, undefined, onlyDirs); await warmGenericGrammars( - new Set(sourceFiles.map((f) => genericLangOf(f)?.name).filter((n): n is string => !!n)), + // Tier precedence, as in buildGraph and in the loop below. + new Set( + sourceFiles + .filter((f) => !languageOf(f)) + .map((f) => genericLangOf(f)?.name) + .filter((n): n is string => !!n), + ), ); // Container-tier grammars need the same warmup as the generic ones, for the same // reason: extraction below is synchronous. Missing this is what made `graft diff --git a/src/graph/extract.ts b/src/graph/extract.ts index d0f9dff19..cecf8edbf 100644 --- a/src/graph/extract.ts +++ b/src/graph/extract.ts @@ -7,14 +7,7 @@ * resolved against the whole-repo node index later, in build.ts. */ import Parser from "tree-sitter"; -import TypeScript from "tree-sitter-typescript"; -import Python from "tree-sitter-python"; -import Go from "tree-sitter-go"; -import R from "tree-sitter-r"; -import Java from "tree-sitter-java"; -import Kotlin from "tree-sitter-kotlin"; -import Swift from "tree-sitter-swift"; -import PHP from "tree-sitter-php"; +import { createRequire } from "node:module"; import { basename } from "node:path"; import { contentHash } from "../util/id.js"; import { collectBindings, goReceiverVarOf, resolveRecvType, type FileBindings } from "./bindings.js"; @@ -22,6 +15,73 @@ import type { Kind, NodeV1, Relation } from "./types.js"; export type Language = "typescript" | "tsx" | "python" | "go" | "java" | "kotlin" | "swift" | "php" | "r"; +const require = createRequire(import.meta.url); + +/** Where each depth-tier grammar comes from. `pick` is for the modules that + * export more than one. */ +const GRAMMAR_MODULES: Record) => unknown }> = { + typescript: { pkg: "tree-sitter-typescript", pick: (m) => m.typescript }, + tsx: { pkg: "tree-sitter-typescript", pick: (m) => m.tsx }, + python: { pkg: "tree-sitter-python" }, + go: { pkg: "tree-sitter-go" }, + java: { pkg: "tree-sitter-java" }, + kotlin: { pkg: "tree-sitter-kotlin" }, + swift: { pkg: "tree-sitter-swift" }, + php: { pkg: "tree-sitter-php", pick: (m) => m.php }, + r: { pkg: "tree-sitter-r" }, +}; + +/** + * These are native (node-gyp) modules, and a native module can fail to load for + * reasons that have nothing to do with the repository being indexed: no prebuild + * for the platform and no compiler to build one (#323), an install that skipped + * build scripts, a binding that names its artifact wrong under another runtime. + * Imported at the top of this module — as they were — any single one of those + * took the whole CLI down at load time, before argv was read: `--version`, + * `--help`, and `ask` on a repo containing no Kotlin, all dying with a + * `node-gyp-build` stack trace that never says "graft". + * + * So load them the way the two WASM tiers already load theirs: a grammar that + * will not load costs its own language, not the tool. The rest follows from + * {@link entryFor} no longer claiming that language's extensions — those files + * take the paths a language graft has no grammar for takes today (the breadth + * tier where a generic row claims the extension, otherwise unindexed), and no + * other language is affected. + */ +const GRAMMARS = {} as Record; +const UNAVAILABLE = new Map(); // language → why its grammar did not load +for (const lang of Object.keys(GRAMMAR_MODULES) as Language[]) { + const { pkg, pick } = GRAMMAR_MODULES[lang]; + try { + const mod = require(pkg) as Record; + const grammar = pick ? pick(mod) : mod; + // A module that loads but exports no grammar would otherwise fail later, + // inside `parser.setLanguage` — the same fault, one file at a time. + if (!grammar) throw new Error(`${pkg} exports no grammar`); + GRAMMARS[lang] = grammar; + } catch (err) { + const why = err instanceof Error ? err.message : String(err); + UNAVAILABLE.set(lang, why.split("\n")[0]); // node-gyp-build's message is a paragraph + } +} + +const warnedUnavailable = new Set(); +/** + * Said once per language, the first time a file that language would have claimed + * comes past: silent in a repo that has none of those files — so one broken + * grammar no longer makes `graft --version` noisy on a TypeScript repo — and + * unmissable in a repo full of them, because indexing short must never be quiet. + */ +function warnUnavailable(lang: Language): void { + if (warnedUnavailable.has(lang)) return; + warnedUnavailable.add(lang); + const exts = EXTENSIONS.filter((e) => e.grammar === lang).map((e) => e.ext).join("/"); + console.warn( + `graft: ${exts} files are not parsed with their own grammar — ${GRAMMAR_MODULES[lang].pkg} failed to load ` + + `(${UNAVAILABLE.get(lang)}). Reinstalling graft rebuilds it; every other language indexes as usual.`, + ); +} + /** * Extension → the tree-sitter grammar that parses it, and the label a human expects * to see for it. @@ -61,12 +121,19 @@ const EXTENSIONS: ReadonlyArray<{ ext: string; grammar: Language; label: string function entryFor(path: string): (typeof EXTENSIONS)[number] | undefined { const p = path.toLowerCase(); - return EXTENSIONS.find((e) => p.endsWith(e.ext)); + const hit = EXTENSIONS.find((e) => p.endsWith(e.ext)); + if (hit && UNAVAILABLE.has(hit.grammar)) { + warnUnavailable(hit.grammar); + return undefined; // no grammar to parse it with, so this tier does not claim it + } + return hit; } -/** Every file extension a depth-tier (hand-written) extractor claims. */ +/** Every file extension a depth-tier (hand-written) extractor claims — minus any + * whose grammar did not load, so `-e` validation and `supportedExtensions()` + * answer for the install in front of the user rather than for the table. */ export function depthExtensions(): string[] { - return EXTENSIONS.map((e) => e.ext); + return EXTENSIONS.filter((e) => !UNAVAILABLE.has(e.grammar)).map((e) => e.ext); } /** Map a file path to a supported language, or null if unsupported. */ @@ -311,17 +378,6 @@ const FUNCTION_VALUE_TYPES = new Set([ const EMPTY_SET: ReadonlySet = new Set(); const parser = new Parser(); -const GRAMMARS: Record = { - typescript: TypeScript.typescript, - tsx: TypeScript.tsx, - python: Python, - go: Go, - r: R, - java: Java, - kotlin: Kotlin, - swift: Swift, - php: PHP.php, -}; export interface WalkCtx { rel: string; diff --git a/src/graph/generic.ts b/src/graph/generic.ts index b599deb77..52b1092e2 100644 --- a/src/graph/generic.ts +++ b/src/graph/generic.ts @@ -39,7 +39,13 @@ export interface GenericLang { } /** The breadth registry. Add a row + a queries/.scm to support a language. - * Extensions here must NOT collide with the depth tier's EXTENSIONS (extract.ts). */ + * + * An extension the depth tier also claims (extract.ts) is a FALLBACK row, not a + * collision: the depth tier is asked first everywhere, so such a row is reached + * only when that language's native grammar did not load. `.java` has always been + * one. `.kt`/`.kts` are one as of #323, where tree-sitter-kotlin ships no + * prebuilds and cannot compile without a C toolchain — with the row, that machine + * indexes Kotlin signatures instead of no Kotlin at all. */ export const GENERIC_LANGS: readonly GenericLang[] = [ { name: "rust", exts: [".rs"], wasm: "rust" }, { name: "java", exts: [".java"], wasm: "java" }, @@ -58,6 +64,10 @@ export const GENERIC_LANGS: readonly GenericLang[] = [ { name: "clojure", exts: [".clj", ".cljs", ".cljc", ".bb"], wasm: "clojure" }, { name: "nix", exts: [".nix"], wasm: "nix" }, { name: "lua", exts: [".lua"], wasm: "lua" }, + // Fallback for the depth tier (see above), unreachable while the native Kotlin + // grammar loads. queries/kotlin.scm is the one this language used before it was + // promoted, and has been sitting unused since. + { name: "kotlin", exts: [".kt", ".kts"], wasm: "kotlin" }, ]; const byExt = new Map(); diff --git a/test/break-grammar-preload.cjs b/test/break-grammar-preload.cjs new file mode 100644 index 000000000..2d277d8d1 --- /dev/null +++ b/test/break-grammar-preload.cjs @@ -0,0 +1,49 @@ +/** + * Makes one native grammar module unloadable, so a test can reproduce #323 + * without a machine that lacks a C toolchain. The package to break is named by + * `GRAFT_TEST_BREAK_GRAMMAR`; with the variable unset this file does nothing. + * + * Preloaded with `--require`, which runs before the ESM graph is linked, so + * `extract.ts` meets the failure at its own load time — where the reported + * machine meets it. BOTH ways of reaching a grammar are broken, deliberately: + * the ESM `import` that #323 was filed against, and the `createRequire` this + * module now uses. Break only the second and the test cannot fail if the static + * imports ever come back, which is the regression worth holding. + * + * The message is node-gyp-build's, near enough verbatim, because graft puts it + * in front of the user and the test asserts on what the user sees. + */ +const { register } = require("node:module"); +const Module = require("node:module"); + +const target = process.env.GRAFT_TEST_BREAK_GRAMMAR; + +function message(pkg) { + return ( + `No native build was found for platform=${process.platform} arch=${process.arch} ` + + `runtime=node abi=137 uv=1 node=${process.versions.node}\n loaded from: ${pkg}` + ); +} + +if (target) { + // `import ""` from an ES module: fail it at resolution, which is as + // fatal to the importing module as the real throw from its binding. + register( + "data:text/javascript," + + encodeURIComponent( + `const target = ${JSON.stringify(target)};\n` + + `const msg = ${JSON.stringify(message(target))};\n` + + `export function resolve(spec, ctx, next) {\n` + + ` if (spec === target) throw new Error(msg);\n` + + ` return next(spec, ctx);\n` + + `}\n`, + ), + ); + + // `require("")`, including the one `createRequire` hands out. + const load = Module._load; + Module._load = function (request, ...rest) { + if (request === target) throw new Error(message(target)); + return load.call(this, request, ...rest); + }; +} diff --git a/test/grammar-unavailable.test.ts b/test/grammar-unavailable.test.ts new file mode 100644 index 000000000..4bdd316ca --- /dev/null +++ b/test/grammar-unavailable.test.ts @@ -0,0 +1,89 @@ +/** + * #323 at the process boundary: a depth-tier grammar that will not load must cost + * its own language, not the whole CLI. + * + * The reported machine is a Windows box with no C toolchain, where + * `tree-sitter-kotlin` — which ships no prebuilds at all — cannot be built at + * install time and throws from `require()`. Because extract.ts imported all nine + * grammars at the top of the module, that killed every command, `--version` and + * `--help` included, with a `node-gyp-build` stack trace that never says "graft". + * + * `break-grammar-preload.cjs` stands in for the missing build so this runs + * anywhere — including on a runner that HAS a compiler, which is exactly why CI + * never caught the original. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdirSync, writeFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { tmpRepo } from "./helpers.js"; +import { readGraph, wiringPath } from "../src/graph/write.js"; +import { contextDirFor } from "../src/context/node-file.js"; + +const PRELOAD = fileURLToPath(new URL("./break-grammar-preload.cjs", import.meta.url)); + +/** graft, run with `pkg` made unloadable. Cwd is the repo root, as it is under `npm test`. */ +function graft(args: string[], pkg: string): { status: number | null; stdout: string; stderr: string } { + const r = spawnSync(process.execPath, ["--require", PRELOAD, "--import", "tsx", "src/cli.ts", ...args], { + encoding: "utf8", + env: { ...process.env, GRAFT_TEST_BREAK_GRAMMAR: pkg, DO_NOT_TRACK: "1" }, + }); + return { status: r.status, stdout: r.stdout ?? "", stderr: r.stderr ?? "" }; +} + +const KOTLIN = `package demo + +class Repo(private val name: String) { + fun describe(): String = "repo $name" +} +`; + +test("#323: a grammar that will not load no longer stops the CLI from starting", () => { + const r = graft(["--version"], "tree-sitter-kotlin"); + assert.equal(r.status, 0, `--version should survive an unloadable grammar\n${r.stderr}`); + assert.match(r.stdout, /\d+\.\d+\.\d+/); + // A command that parses nothing has no reason to mention the grammar at all. + assert.doesNotMatch(r.stderr, /tree-sitter-kotlin/); +}); + +test("#323: the other languages still index, and the affected one says so once", () => { + const dir = tmpRepo("grammar-unavailable"); + mkdirSync(join(dir, "src"), { recursive: true }); + writeFileSync(join(dir, "src", "app.ts"), "export function greet(): string {\n return \"hi\";\n}\n"); + // Two, so "once per language" is a claim the test can actually fail on. + writeFileSync(join(dir, "src", "Repo.kt"), KOTLIN); + writeFileSync(join(dir, "src", "Other.kt"), KOTLIN.replace("Repo", "Other")); + + const r = graft(["build", dir], "tree-sitter-kotlin"); + assert.equal(r.status, 0, `build should not die with one grammar down\n${r.stderr}`); + assert.equal( + r.stderr.match(/tree-sitter-kotlin failed to load/g)?.length, + 1, + `warned exactly once, whatever the file count\n${r.stderr}`, + ); + + const g = readGraph(wiringPath(contextDirFor(dir))); + assert.ok(g, "graph built"); + assert.ok( + g!.nodes.some((n) => n.name === "greet"), + "TypeScript is indexed as usual — one dead grammar is not nine", + ); +}); + +test("#323: Kotlin falls back to the breadth tier instead of going unindexed", () => { + const dir = tmpRepo("grammar-fallback"); + mkdirSync(join(dir, "src"), { recursive: true }); + writeFileSync(join(dir, "src", "Repo.kt"), KOTLIN); + + const r = graft(["build", dir], "tree-sitter-kotlin"); + assert.equal(r.status, 0, `build should not die with one grammar down\n${r.stderr}`); + + const g = readGraph(wiringPath(contextDirFor(dir))); + assert.ok(g, "graph built"); + const kt = g!.nodes.filter((n) => n.path.endsWith(".kt") && n.kind !== "file"); + assert.ok(kt.length > 0, "Kotlin is still indexed, at signature depth"); + assert.ok(kt.every((n) => n.origin === "generic"), `…through the breadth tier (${kt.map((n) => n.origin).join(", ")})`); + assert.ok(kt.some((n) => n.name === "Repo"), `the class is there (got ${kt.map((n) => n.name).join(", ")})`); +});