From fdfdd2e35fe47d9379af1e4dd843430675e92f12 Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Tue, 18 Aug 2026 15:41:46 +0200 Subject: [PATCH] fix(zig): address eighth gitnexus-check review pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Named dependency modules: `parseZigBuildModuleRoots` scanned `addModule("", .{ … })` with a `[^}]*` regex, so a nested field before `.root_source_file` (`.imports = &.{ .{ … } }`) ended the match at the inner `}` and demoted the module to an unnamed fallback — the first exe/lib root in the file then answered `@import("")`. The named lookup now walks the balanced `addModule(…)` argument list (same scanner as `parseZigRootModules`, comment-stripped, string-aware); the unnamed fallbacks are unchanged. Regression test in `zig-import-resolver.test.ts`. - Type-position `@import`: `var x: @import("m.zig").T = undefined;` was read as an import binding of `x` on both sides — the query rules match the `type:` child like a value, and `isZigContainerOrImportBinding` scanned every named child — so `x` was never declared and became a named import of `T`. The helper now skips the `type:` field, and `emitZigScopeCaptures` drops binding-rule matches whose `@import` sits in the annotation (`isZigTypePositionImport`) without claiming the source, so `x` binds as a variable and the file edge survives as a side-effect import. Regression tests in `zig-extractors.test.ts` (variable extractor + scope captures). - Union in the class-capture skip guard: the parse-worker's inline class-like predicate lacked `Union`, so a `Union` definition bypassed `shouldSkipClassCapture` unlike every other `ClassLikeNodeLabel`. Added the label; no Zig behavior changes (Zig defines no skip hook), so no test. - File-owned method ids in `findEnclosingFunctionId`: the arity lookup used `findEnclosingClassNode` while the owner came from the file-owner aware `cachedFindEnclosingClassInfo`, so a Zig file-struct's top-level fn produced `Method::Page.get` without the `#` suffix. It now uses `findEnclosingClassNodeOrFileOwner`, the definition-phase lookup. Consistency fix only: `ParseWorkerResult.calls` / `.assignments` (the sole consumers of this id) are merged but not read since #942 — CALLS edges come from the scope pipeline, whose ids were already right — so no observable graph change and no test. Not re-fixed: - `test/helpers/literal-collectors.ts` `DIR_LANG` has no `zig` entry (raised for the seventh time): the entry exists (`zig: SupportedLanguages.Zig`, added by the second-pass commit), so `languages/zig/**` literals are already validated against the Zig grammar alone; documented in the PR body since the fifth pass. --- .../src/core/ingestion/language-config.ts | 26 +++++++--- .../core/ingestion/languages/zig/captures.ts | 41 ++++++++++++++++ .../core/ingestion/workers/parse-worker.ts | 9 +++- gitnexus/test/unit/zig-extractors.test.ts | 48 +++++++++++++++++++ .../test/unit/zig-import-resolver.test.ts | 18 +++++++ 5 files changed, 134 insertions(+), 8 deletions(-) diff --git a/gitnexus/src/core/ingestion/language-config.ts b/gitnexus/src/core/ingestion/language-config.ts index 5eadd2ec1..f4064a2fc 100644 --- a/gitnexus/src/core/ingestion/language-config.ts +++ b/gitnexus/src/core/ingestion/language-config.ts @@ -633,14 +633,28 @@ export function parseZigBuildModuleRoots(buildZig: string, preferredName: string seen.add(norm); into.push(norm); }; - const namedRe = - /addModule\(\s*"([^"\n]+)"\s*,\s*\.\{[^}]*?\.root_source_file\s*=\s*b\.path\(\s*"([^"\n]+)"\s*\)/g; + const rootRe = /\.root_source_file\s*=\s*b\.path\(\s*"([^"\n]+)"\s*\)/; + // The named module: scan the whole `addModule(…)` argument list, balanced + // on parentheses, so a nested field before `.root_source_file` (`.imports = + // &.{ .{ … } }`) does not end the match early — a `[^}]*` regex stopped at + // that inner `}` and silently demoted the module to an unnamed fallback. + const text = stripZonComments(buildZig); + const mask = zonStringMask(text); + const callRe = /\baddModule\s*\(/g; let m: RegExpExecArray | null; - while ((m = namedRe.exec(buildZig)) !== null) { - if (m[1] === preferredName) add(named, m[2]!); + while ((m = callRe.exec(text)) !== null) { + if (mask[m.index] !== 0) continue; + const argsStart = m.index + m[0].length; + const argsEnd = findZigParenEnd(text, argsStart); + if (argsEnd < 0) break; + const args = text.slice(argsStart, argsEnd); + const nameMatch = /^\s*"([^"\n]+)"\s*,/.exec(args); + if (nameMatch?.[1] !== preferredName) continue; + const rootMatch = rootRe.exec(args); + if (rootMatch) add(named, rootMatch[1]!); } - const anyRe = /\.root_source_file\s*=\s*b\.path\(\s*"([^"\n]+)"\s*\)/g; - while ((m = anyRe.exec(buildZig)) !== null) add(unnamed, m[1]!); + const anyRe = new RegExp(rootRe.source, 'g'); + while ((m = anyRe.exec(text)) !== null) add(unnamed, m[1]!); return [...named, ...unnamed]; } diff --git a/gitnexus/src/core/ingestion/languages/zig/captures.ts b/gitnexus/src/core/ingestion/languages/zig/captures.ts index 0465b2ceb..6fa48cfdd 100644 --- a/gitnexus/src/core/ingestion/languages/zig/captures.ts +++ b/gitnexus/src/core/ingestion/languages/zig/captures.ts @@ -62,15 +62,36 @@ export function isZigTypeShadowingBinding(declNode: SyntaxNode): boolean { } export function isZigContainerOrImportBinding(declNode: SyntaxNode): boolean { + const typeNode = declNode.childForFieldName('type'); for (let i = 0; i < declNode.namedChildCount; i++) { const child = declNode.namedChild(i); if (child === null) continue; if (ZIG_CONTAINER_TYPES.has(child.type)) return true; + // Only the VALUE binds an import: `var x: @import("foo.zig").T = + // undefined;` types a plain variable by an imported type, it imports + // nothing under `x` (see `isZigTypePositionImport`). + if (child.id === typeNode?.id) continue; if (zigImportRootOf(child) !== null) return true; } return false; } +/** Does the `@import(…)` a binding rule matched sit in the TYPE annotation of + * its declaration (`var x: @import("foo.zig").T = undefined;`) rather than + * in its value? `variable_declaration` is fieldless except for `type:`, so + * the query cannot tell the two positions apart; such a match must not bind + * `x` as an import — `x` is a variable, and the `@import` is only a file + * dependency (the `@import.inline` rule records it as a side-effect import + * once this match releases the source node). */ +export function isZigTypePositionImport(stmt: SyntaxNode, source: SyntaxNode): boolean { + const typeNode = stmt.childForFieldName('type'); + return ( + typeNode !== null && + typeNode.startIndex <= source.startIndex && + source.endIndex <= typeNode.endIndex + ); +} + /** Does this `variable_declaration` carry a `const` / `var` keyword child? * tree-sitter-zig 1.1.2 parses statement-position ASSIGNMENTS (`x = 5;`, * `x += 1;`, `_ = expr;`) as `variable_declaration` too — the only thing @@ -916,6 +937,16 @@ export function emitZigScopeCaptures( const importName = byName.get('import.name'); const importSource = byName.get('import.source'); const importStmt = byName.get('import.statement'); + // A binding-rule match whose `@import` is the declaration's TYPE + // annotation binds nothing: leave the source unclaimed so the + // `@import.inline` rule keeps the file edge. + if ( + importSource !== undefined && + importStmt !== undefined && + isZigTypePositionImport(importStmt, importSource) + ) { + continue; + } if ( importSource !== undefined && (importStmt !== undefined || @@ -1092,6 +1123,16 @@ export function emitZigScopeCaptures( if (value?.type === 'call_expression' && isZigTypeConstructorCall(value)) continue; } + // `var x: @import("foo.zig").T = undefined;` — the binding rules match + // the type-position `@import` too; that group binds nothing (the + // variable group for `x` and the inline file edge cover it). + if ( + nodeMap['@import.statement'] !== undefined && + nodeMap['@import.source'] !== undefined && + isZigTypePositionImport(nodeMap['@import.statement'], nodeMap['@import.source']) + ) { + continue; + } // Drop the plain-variable group for container/import bindings — their // dedicated rules already bind the name (as Struct/Enum/Union or import). // The query already requires a `const`/`var` keyword, so statement diff --git a/gitnexus/src/core/ingestion/workers/parse-worker.ts b/gitnexus/src/core/ingestion/workers/parse-worker.ts index 2be65d507..4af861f97 100644 --- a/gitnexus/src/core/ingestion/workers/parse-worker.ts +++ b/gitnexus/src/core/ingestion/workers/parse-worker.ts @@ -1032,8 +1032,12 @@ const findEnclosingFunctionId = ( ? undefined : standaloneMethodInfo.parameters.length; } else { + // Same owner lookup as the definition-phase Method id builder: a Zig + // file-struct's top-level fn is owned by the file root, and its + // id carries the `#` suffix only if that owner is found. const classNode = - findEnclosingClassNode(current) ?? findClassNodeByQualifiedName(current); + findEnclosingClassNodeOrFileOwner(current, provider, filePath) ?? + findClassNodeByQualifiedName(current); if (classNode && encLang) { const methodMap = getMethodInfo(classNode, provider, { filePath, @@ -2186,7 +2190,8 @@ const processFileGroup = ( nodeLabel === 'Struct' || nodeLabel === 'Interface' || nodeLabel === 'Enum' || - nodeLabel === 'Record'; + nodeLabel === 'Record' || + nodeLabel === 'Union'; if ( isClassLikeLabel && provider.classExtractor?.shouldSkipClassCapture?.({ diff --git a/gitnexus/test/unit/zig-extractors.test.ts b/gitnexus/test/unit/zig-extractors.test.ts index 3ccecf1dd..b9c501eff 100644 --- a/gitnexus/test/unit/zig-extractors.test.ts +++ b/gitnexus/test/unit/zig-extractors.test.ts @@ -202,6 +202,27 @@ var count = @as(u32, 0); expect(names).toEqual(['limit', 'count']); }); + it('an `@import` in TYPE position (`var x: @import("m.zig").T = undefined;`) still declares the variable', () => { + // The import-binding guard used to scan every named child, so the type + // annotation's `@import` made `x` look like an import binding and no + // Variable was emitted; a typed import binding (value position) stays out. + const root = parse(` +var x: @import("m.zig").T = undefined; +const y: type = @import("m.zig"); +const z = @import("m.zig").T; +`).rootNode; + const names: string[] = []; + const importBindings: boolean[] = []; + for (let i = 0; i < root.namedChildCount; i++) { + const decl = root.namedChild(i)!; + importBindings.push(isZigContainerOrImportBinding(decl)); + const info = extractor.extract(decl, ctx); + if (info) names.push(info.name); + } + expect(importBindings).toEqual([false, true, true]); + expect(names).toEqual(['x']); + }); + it('reads the type from the `type:` field and never from the initializer', () => { // The old positional fallback returned `target` as the type of // `const f = target;` and gave up on compound annotations (`*Foo`). @@ -221,6 +242,33 @@ extern var g: T; }); }); +describeZig('Zig scope captures — `@import` in TYPE position is not an import binding', () => { + it('`var x: @import("m.zig").T = undefined;` binds a variable `x`, keeps the file edge, imports no name', () => { + // Every import rule matches a `variable_declaration` with an `@import` + // child; the grammar's only field is `type:`, so a type annotation + // spelled through `@import` matched too and bound `x` as a NAMED import of + // `T` — a variable typed by an imported type became an alias of the type. + const matches = emitZigScopeCaptures( + 'var x: @import("m.zig").T = undefined;\nconst z = @import("n.zig").T;\n', + 'test.zig', + ); + const importNames = matches + .filter((m) => m['@import.name'] !== undefined) + .map((m) => `${m['@import.name']!.text}<-${m['@import.imported']?.text ?? '*'}`); + expect(importNames).toEqual(['z<-T']); + const variables = matches + .filter((m) => m['@declaration.variable'] !== undefined) + .map((m) => m['@declaration.name']!.text); + expect(variables).toEqual(['x']); + // The type-position `@import` still counts as a file dependency (a + // side-effect import); the value-position one is claimed by its binding. + const sideEffects = matches + .filter((m) => m['@import.side-effect'] !== undefined) + .map((m) => m['@import.source']!.text); + expect(sideEffects).toEqual(['"m.zig"']); + }); +}); + describeZig('Zig scope captures — receiver is the FIRST parameter named self', () => { function parameterBindings(src: string) { return emitZigScopeCaptures(src, 'test.zig') diff --git a/gitnexus/test/unit/zig-import-resolver.test.ts b/gitnexus/test/unit/zig-import-resolver.test.ts index dd0c20fab..3e3376ffb 100644 --- a/gitnexus/test/unit/zig-import-resolver.test.ts +++ b/gitnexus/test/unit/zig-import-resolver.test.ts @@ -410,6 +410,24 @@ _ = b.addModule("w", .{ .root_source_file = b.path("../outside.zig") }); expect(parseZigBuildModuleRoots(buildZig, 'y')).toEqual(['src/y.zig']); }); + it('still names the module when a nested field precedes `.root_source_file`', () => { + // `.imports = &.{ .{ … } }` closes an inner `}` before the root field; a + // `[^}]*` regex ended there and demoted "dep" to an unnamed fallback, + // so `@import("dep")` resolved to whichever root came first in the file. + const buildZig = ` +pub fn build(b: *std.Build) void { + const exe = b.addExecutable(.{ .name = "tool", .root_source_file = b.path("src/main.zig") }); + _ = b.addModule("dep", .{ + .imports = &.{ .{ .name = "util", .module = util } }, + .root_source_file = b.path("lib/root.zig"), + }); + // _ = b.addModule("dep", .{ .root_source_file = b.path("lib/commented_out.zig") }); + b.installArtifact(exe); +} +`; + expect(parseZigBuildModuleRoots(buildZig, 'dep')).toEqual(['lib/root.zig', 'src/main.zig']); + }); + it('returns [] for a build.zig that declares no module root', () => { expect(parseZigBuildModuleRoots('pub fn build(b: *std.Build) void { _ = b; }', 'x')).toEqual( [],