From b654b662613a5f95fbd3588bf616945b8ebe0f8b Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Tue, 18 Aug 2026 04:27:17 +0200 Subject: [PATCH] fix(zig): address third gitnexus-check review pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - language-config `parseZigBuildZon`: the `.dependencies = .{` header and the per-entry `. = .{` headers are matched only OUTSIDE string literals (per-offset string mask + `matchZonHeader`). A `.name` or `.description` value spelling `.dependencies = .{ .fake = .{ .path = … } }` used to be taken as the block and returned the fake dep instead of the real top-level one. - import-resolvers/zig: an absolute import (`@import("/foo.zig")`) returns null. The path walker skipped every empty component, so the leading `/` vanished and `/foo.zig` resolved as importer-relative `src/foo.zig` — an in-repo edge for an import Zig rejects as outside the module path. - tree-sitter-languages.test.ts: the Zig parsing case gates on the PACKAGE being installed (`createRequire().resolve`, minus a deliberate `GITNEXUS_SKIP_OPTIONAL_GRAMMARS` opt-out), not on `isLanguageAvailable`, which is false for absent AND for installed-but-broken bindings — so a load failure (ABI mismatch, bad export) now fails the test instead of skipping it, as the comment already claimed. - walkers.ts `isShapeLike` doc: `Union` sits in `isClassLike` because that is the label set the ownership walkers consult, not because unions inherit — Zig has no inheritance and no heritage hooks. The previous wording ("inheritance-capable owner") said otherwise. - Not re-fixed (already addressed in the second pass, findings carried over): "receiver = any parameter named self" — `interpretZigTypeBinding` only sources a first-position parameter as `self`; `zigReceiverBinding`'s doc now states that invariant. "`DIR_LANG` has no zig entry" — it does. Extended one level out: `BASENAME_LANGS` (`zig.ts`) and `PREFIX_LANGS` (`ZIG_`) map to the Zig grammar too, so extractor configs and the export-detection set are validated against Zig alone. That immediately caught a dead `childForFieldName('parameters')` in method-extractors/configs/zig `zigParameterList` (there is no such field; the named-child lookup was already the one doing the work) — removed. Tests: zig-import-resolver (+2: absolute path, header inside a string); both fail on the previous code. --- .../core/ingestion/import-resolvers/zig.ts | 6 ++ .../src/core/ingestion/language-config.ts | 70 ++++++++++++++++--- .../ingestion/languages/zig/simple-hooks.ts | 8 ++- .../method-extractors/configs/zig.ts | 11 ++- .../scope-resolution/scope/walkers.ts | 9 ++- gitnexus/test/helpers/literal-collectors.ts | 2 + .../integration/tree-sitter-languages.test.ts | 47 ++++++++----- .../test/unit/zig-import-resolver.test.ts | 34 +++++++++ 8 files changed, 148 insertions(+), 39 deletions(-) diff --git a/gitnexus/src/core/ingestion/import-resolvers/zig.ts b/gitnexus/src/core/ingestion/import-resolvers/zig.ts index f2de3e516..3b419bec1 100644 --- a/gitnexus/src/core/ingestion/import-resolvers/zig.ts +++ b/gitnexus/src/core/ingestion/import-resolvers/zig.ts @@ -42,6 +42,12 @@ export function resolveZigImportInternal( // and only the fallback appends `.zig` for extension-less spellings. const trimmed = importPath.replace(/\\/g, '/'); + // Absolute paths point outside the repository (Zig itself rejects + // `@import("/abs.zig")` as an import outside the module path). Splitting + // would drop the empty leading component and read `/foo.zig` as an + // importer-relative `foo.zig`, fabricating an in-repo edge. + if (trimmed.startsWith('/')) return null; + // Path-bearing import: resolve relative to the current file's directory. // Zig allows both "./foo.zig" and "foo.zig" — both are filesystem-relative. if (trimmed.endsWith('.zig') || trimmed.includes('/')) { diff --git a/gitnexus/src/core/ingestion/language-config.ts b/gitnexus/src/core/ingestion/language-config.ts index bcea2afc3..9836dc0af 100644 --- a/gitnexus/src/core/ingestion/language-config.ts +++ b/gitnexus/src/core/ingestion/language-config.ts @@ -574,30 +574,78 @@ function findZonBlockEnd(text: string, start: number): number { return -1; } +/** + * Per-offset "is inside a `"…"` literal" mask for comment-stripped ZON text, + * so header regexes can reject a match that merely LOOKS like a field + * (`.name = ".dependencies = .{ … }"` is a string, not the dependencies + * block). Escaped quotes (`\"`) do not end the literal. + */ +function zonStringMask(text: string): Uint8Array { + const mask = new Uint8Array(text.length); + let inString = false; + for (let i = 0; i < text.length; i++) { + const ch = text[i]; + if (inString) { + mask[i] = 1; + if (ch === '\\' && i + 1 < text.length) mask[++i] = 1; + else if (ch === '"') inString = false; + continue; + } + if (ch === '"') { + inString = true; + mask[i] = 1; + } + } + return mask; +} + +/** + * First match of a sticky-free global `re` in `text[from, to)` whose start + * lies outside a string literal (per `mask`). Null when none. + */ +function matchZonHeader( + text: string, + re: RegExp, + mask: Uint8Array, + from: number, + to: number, +): RegExpExecArray | null { + re.lastIndex = from; + let m: RegExpExecArray | null; + while ((m = re.exec(text)) !== null && m.index < to) { + if (mask[m.index] === 0) return m; + } + return null; +} + /** Pure parser split out for testability. Returns null when no path-deps found. */ export function parseZigBuildZon(raw: string): ZigBuildZonConfig | null { const text = stripZonComments(raw); + const mask = zonStringMask(text); // Locate the `.dependencies = .{ ... }` block. Use brace counting because - // dep entries are nested anonymous structs and a naive `}` match would stop early. - const depsHeader = text.match(/\.dependencies\s*=\s*\.\{/); + // dep entries are nested anonymous structs and a naive `}` match would stop + // early — and only accept a header outside string literals, so a `.name` + // or `.description` value spelling `.dependencies = .{` cannot hijack it. + const depsHeader = matchZonHeader(text, /\.dependencies\s*=\s*\.\{/g, mask, 0, text.length); if (!depsHeader) return null; - const start = depsHeader.index! + depsHeader[0].length; + const start = depsHeader.index + depsHeader[0].length; const end = findZonBlockEnd(text, start); if (end < 0) return null; - const block = text.slice(start, end); const pathDeps = new Map(); - // Walk each `. = .{ ... }` entry; the body ends at the matching brace - // (string-aware), not at the first `}` in the text. + // Walk each `. = .{ ... }` entry inside [start, end); the body ends + // at the matching brace (string-aware), not at the first `}` in the text, + // and an entry header inside a string (`.url = "…/.x = .{"`) is not an entry. const entryHeaderRe = /\.([A-Za-z_][A-Za-z0-9_]*)\s*=\s*\.\{/g; + let cursor = start; let m: RegExpExecArray | null; - while ((m = entryHeaderRe.exec(block)) !== null) { + while ((m = matchZonHeader(text, entryHeaderRe, mask, cursor, end)) !== null) { const depName = m[1]; const bodyStart = m.index + m[0].length; - const bodyEnd = findZonBlockEnd(block, bodyStart); - if (bodyEnd < 0) break; - const body = block.slice(bodyStart, bodyEnd); - entryHeaderRe.lastIndex = bodyEnd + 1; + const bodyEnd = findZonBlockEnd(text, bodyStart); + if (bodyEnd < 0 || bodyEnd > end) break; + const body = text.slice(bodyStart, bodyEnd); + cursor = bodyEnd + 1; const pathMatch = body.match(/\.path\s*=\s*"([^"\n]+)"/); if (pathMatch) { pathDeps.set(depName, pathMatch[1]); diff --git a/gitnexus/src/core/ingestion/languages/zig/simple-hooks.ts b/gitnexus/src/core/ingestion/languages/zig/simple-hooks.ts index 25b08f7a1..199bc9c11 100644 --- a/gitnexus/src/core/ingestion/languages/zig/simple-hooks.ts +++ b/gitnexus/src/core/ingestion/languages/zig/simple-hooks.ts @@ -23,8 +23,12 @@ export function zigBindingScopeFor( return null; // default auto-hoist for other bindings } -/** Zig's receiver convention is a first parameter named `self`; the - * `self`-sourced typeBinding on the function scope carries its type. */ +/** Zig's receiver convention is a FIRST parameter named `self`; the + * `self`-sourced typeBinding on the function scope carries its type. + * Position is enforced upstream, not here: `interpretZigTypeBinding` only + * sources a binding as `self` when `emitZigScopeCaptures` tagged it + * `@type-binding.first-parameter`, so a later parameter named `self` + * arrives as `parameter-annotation` and is never returned by this hook. */ export function zigReceiverBinding(functionScope: Scope): TypeRef | null { if (functionScope.kind !== 'Function') return null; for (const binding of functionScope.typeBindings.values()) { diff --git a/gitnexus/src/core/ingestion/method-extractors/configs/zig.ts b/gitnexus/src/core/ingestion/method-extractors/configs/zig.ts index 25b43dcb1..5044c68cd 100644 --- a/gitnexus/src/core/ingestion/method-extractors/configs/zig.ts +++ b/gitnexus/src/core/ingestion/method-extractors/configs/zig.ts @@ -31,14 +31,13 @@ const extractZigName = (node: SyntaxNode): string | undefined => { /** * The `parameters` node of a function_declaration. tree-sitter-zig 1.1.2 * attaches it as a plain named child — NOT under a `parameters:` field (only - * `name`, `type` and `body` are fields) — so `childForFieldName('parameters')` - * is always null. Reading it that way silently produced empty parameter lists, - * no receiver, and `isStatic: true` for every method. + * `name`, `type` and `body` are fields), so a field lookup is always null + * (and the grammar-literal gate flags it as a dead field). Reading it that + * way silently produced empty parameter lists, no receiver, and + * `isStatic: true` for every method. */ const zigParameterList = (node: SyntaxNode): SyntaxNode | null => - node.childForFieldName('parameters') ?? - node.namedChildren.find((child): child is SyntaxNode => child?.type === 'parameters') ?? - null; + node.namedChildren.find((child): child is SyntaxNode => child?.type === 'parameters') ?? null; const extractZigReturnType = (node: SyntaxNode): string | undefined => { // tree-sitter-zig labels the return type as the `type` field on diff --git a/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts b/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts index 57a78955e..c4fc9b193 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts @@ -212,9 +212,12 @@ export function isClassLike(t: string): boolean { * * `Union` IS included, via `isClassLike`: Zig wires `union(enum)` as a member * container (methods dispatched on a union receiver — see the `main → isEnergy` - * case in `test/integration/resolvers/zig.test.ts`), so it is a shape as well - * as an inheritance-capable owner. C/C++ unions still do not emit `Union` - * defs on the scope side, so nothing changes for them. + * case in `test/integration/resolvers/zig.test.ts`), so it is a shape. It + * lives in `isClassLike` because that is the label set the ownership walkers + * consult, NOT because unions inherit: Zig has no inheritance and its scope + * resolver supplies no heritage hooks, so a `Union` never has supertypes and + * its MRO is just itself. C/C++ unions still do not emit `Union` defs on the + * scope side, so nothing changes for them. * * NOT YET INCLUDED, deliberately: `Typedef`. It belongs here conceptually but * is not wired as a member container today, so adding it would widen a diff --git a/gitnexus/test/helpers/literal-collectors.ts b/gitnexus/test/helpers/literal-collectors.ts index 7f25b6a18..a6bb41b29 100644 --- a/gitnexus/test/helpers/literal-collectors.ts +++ b/gitnexus/test/helpers/literal-collectors.ts @@ -112,6 +112,7 @@ const BASENAME_LANGS: Record = { nextjs: [SupportedLanguages.TypeScript, SupportedLanguages.JavaScript], expo: [SupportedLanguages.TypeScript, SupportedLanguages.JavaScript], 'fastapi-router-bindings': [SupportedLanguages.Python], + zig: [SupportedLanguages.Zig], }; /** const-name prefix → language (for export-detection.ts style named sets). */ @@ -129,6 +130,7 @@ const PREFIX_LANGS: Record = { CPP: [SupportedLanguages.CPlusPlus], TS: [SupportedLanguages.TypeScript], JS: [SupportedLanguages.JavaScript], + ZIG: [SupportedLanguages.Zig], }; /** Candidate grammar languages a CODE literal in `relPath` should be checked against. */ diff --git a/gitnexus/test/integration/tree-sitter-languages.test.ts b/gitnexus/test/integration/tree-sitter-languages.test.ts index 240788304..3fcf9045c 100644 --- a/gitnexus/test/integration/tree-sitter-languages.test.ts +++ b/gitnexus/test/integration/tree-sitter-languages.test.ts @@ -5,10 +5,12 @@ import { loadParser, loadLanguage, isLanguageAvailable, + isGrammarRuntimeSkipped, } from '../../src/core/tree-sitter/parser-loader.js'; import { SupportedLanguages, getLanguageFromFilename } from 'gitnexus-shared'; import { getProvider } from '../../src/core/ingestion/languages/index.js'; import Parser from 'tree-sitter'; +import { createRequire } from 'module'; const fixturesDir = path.resolve(__dirname, '..', 'fixtures', 'sample-code'); @@ -681,25 +683,36 @@ describe('Tree-sitter multi-language parsing', () => { }); describe('Zig', () => { - // Gate on the loader's own availability probe, not on a catch-all: when - // the optional grammar IS installed, a load failure (ABI mismatch, bad - // export) must fail this test, not silently skip its assertions. - it.skipIf(!isLanguageAvailable(SupportedLanguages.Zig))( - 'parses functions, structs, enums, and imports', - async () => { - await loadLanguage(SupportedLanguages.Zig); + // Gate on whether the optional PACKAGE is installed, not on the loader's + // `isLanguageAvailable` (which is false for absent AND for + // installed-but-broken bindings — the loader swallows every load error + // for optional grammars). With the package present, a load failure (ABI + // mismatch, bad export) must fail this test, not silently skip it. A + // deliberate `GITNEXUS_SKIP_OPTIONAL_GRAMMARS` opt-out in the environment + // is the one non-failure reason an installed grammar reports unavailable. + const zigPackageInstalled = (() => { + if (isGrammarRuntimeSkipped(SupportedLanguages.Zig)) return false; + try { + createRequire(import.meta.url).resolve('@tree-sitter-grammars/tree-sitter-zig'); + return true; + } catch { + return false; + } + })(); + it.skipIf(!zigPackageInstalled)('parses functions, structs, enums, and imports', async () => { + expect(isLanguageAvailable(SupportedLanguages.Zig)).toBe(true); + await loadLanguage(SupportedLanguages.Zig); - const content = readFixture('simple.zig'); - const provider = getProvider(SupportedLanguages.Zig); - const { matches } = parseAndQuery(parser, content, provider.treeSitterQueries); - const defs = extractDefinitions(matches); + const content = readFixture('simple.zig'); + const provider = getProvider(SupportedLanguages.Zig); + const { matches } = parseAndQuery(parser, content, provider.treeSitterQueries); + const defs = extractDefinitions(matches); - const defTypes = defs.map((d) => d.type); - expect(defTypes).toContain('definition.function'); - expect(defTypes).toContain('definition.struct'); - expect(defTypes).toContain('definition.enum'); - }, - ); + const defTypes = defs.map((d) => d.type); + expect(defTypes).toContain('definition.function'); + expect(defTypes).toContain('definition.struct'); + expect(defTypes).toContain('definition.enum'); + }); it('reports Zig unavailable and throws "Unsupported language" when the grammar is absent', async () => { // Force the absent-binding path instead of hoping the package is missing: diff --git a/gitnexus/test/unit/zig-import-resolver.test.ts b/gitnexus/test/unit/zig-import-resolver.test.ts index ba4e59f64..4b90a2552 100644 --- a/gitnexus/test/unit/zig-import-resolver.test.ts +++ b/gitnexus/test/unit/zig-import-resolver.test.ts @@ -41,6 +41,20 @@ describe('resolveZigImportInternal', () => { expect(resolveZigImportInternal('src/a.zig', '../bar.zig', files)).toBe('bar.zig'); }); + it('rejects absolute import paths instead of reading them as importer-relative', () => { + // The path walker skipped every empty component, so the leading `/` of + // `/foo.zig` vanished and it resolved to `src/foo.zig` — an in-repo edge + // for an import Zig itself rejects as outside the module path. + const files = new Set(['src/main.zig', 'src/foo.zig', 'foo.zig', 'main.zig']); + expect(resolveZigImportInternal('src/main.zig', '/foo.zig', files)).toBeNull(); + expect(resolveZigImportInternal('main.zig', '/foo.zig', files)).toBeNull(); + expect(resolveZigImportInternal('src/main.zig', '/src/foo.zig', files)).toBeNull(); + // Backslash-spelled absolute paths normalize to the same rejection. + expect(resolveZigImportInternal('src/main.zig', '\\foo.zig', files)).toBeNull(); + // The relative spelling next to it still resolves. + expect(resolveZigImportInternal('src/main.zig', 'foo.zig', files)).toBe('src/foo.zig'); + }); + it('returns null for a bare name when no build.zig.zon is supplied', () => { const files = new Set(['src/main.zig', 'vendor/ziggit/src/ziggit.zig']); expect(resolveZigImportInternal('src/main.zig', 'ziggit', files)).toBeNull(); @@ -175,6 +189,26 @@ describe('parseZigBuildZon', () => { ]); }); + it('does not take a `.dependencies = .{` spelled inside a string literal as the block header', () => { + // The header search was a raw regex over the whole text: a `.name` (or + // `.description`) value that spells `.dependencies = .{ … }` matched + // first, the parser started at that embedded brace, and returned the + // fake `.path` dep instead of the real top-level block. + const raw = ` +.{ + .name = ".dependencies = .{ .fake = .{ .path = \\"vendor/fake\\" } }", + .version = "0.0.0", + .dependencies = .{ + .real = .{ .path = "vendor/real" }, + }, + .paths = .{ "" }, +} +`; + const cfg = parseZigBuildZon(raw); + expect(cfg).not.toBeNull(); + expect([...cfg!.pathDeps.entries()]).toEqual([['real', 'vendor/real']]); + }); + it('returns null when no `.dependencies` block is present', () => { const raw = `.{ .name = "x", .version = "0.0.0", .paths = .{""} }`; expect(parseZigBuildZon(raw)).toBeNull();