From f8b631630fbc8bc7c2fc30a8ed35db56432b07b8 Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Tue, 18 Aug 2026 12:39:26 +0200 Subject: [PATCH] fix(zig): address sixth gitnexus-check review pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Windows absolute `.path` deps (`C:\x`, `C:/x`) return null from `normalizeZigDepPath` like POSIX ones; a `/`-only check let them through as repo-relative. - `parseZigBuildZon` accepts the `.dependencies = .{` header only at brace depth 1 (a direct field of the file's `.{`), so a same-named field nested in an earlier struct cannot hijack the block. - `importsExecuteWhereWritten: false` on the provider: `@import` is compile-time name lookup (as C `#include`, Rust `use`); a body-level `@import` is no longer marked `runsOnlyWhenCalled`. - Namespace imports record the MODULE as `importedName` (`zigModuleNameOf`: last path segment without `.zig`), per the shared contract; the local handle stays `localName`. - Keyword-less ` = @import(…)` (`_ = @import("x.zig")` in a test block) is a `side-effect` import: file edge, no binding. Only `const`/`var` declarations bind a name or feed alias promotion. - `extractZigFunctionName` doc: an empty name is falsy, so the enclosing- function walk skips the test node and continues to the File; it does not "end" there. Not re-fixed: "DIR_LANG has no zig entry" — carried over for the fourth pass in a row; `test/helpers/literal-collectors.ts` has had `zig` in `DIR_LANG` (line 91) and `BASENAME_LANGS` since the second-pass commit. Regression tests: absolute-path spellings, nested `.dependencies` decoy, namespace/side-effect interpretation, function-scoped `@import` not deferred (all four fail on the previous source). --- .../src/core/ingestion/language-config.ts | 58 ++++++++++++++++--- gitnexus/src/core/ingestion/languages/zig.ts | 6 ++ .../core/ingestion/languages/zig/captures.ts | 19 +++++- .../core/ingestion/languages/zig/interpret.ts | 32 ++++++++-- .../method-extractors/configs/zig.ts | 6 +- gitnexus/test/unit/zig-extractors.test.ts | 45 +++++++++++--- .../test/unit/zig-import-resolver.test.ts | 36 ++++++++++++ 7 files changed, 178 insertions(+), 24 deletions(-) diff --git a/gitnexus/src/core/ingestion/language-config.ts b/gitnexus/src/core/ingestion/language-config.ts index ee5f33954..5bfcff5f9 100644 --- a/gitnexus/src/core/ingestion/language-config.ts +++ b/gitnexus/src/core/ingestion/language-config.ts @@ -507,8 +507,8 @@ export async function loadSwiftPackageConfig(repoRoot: string): Promise = .{ ... }` * where `` is a bare identifier (no `@"…"` quoted form). * - Only `.path = "..."` is captured. `.url` deps are left unresolved @@ -561,7 +561,9 @@ export async function loadZigBuildZon(repoRoot: string): Promise 0) d--; + } + return depth; +} + /** * First match of a sticky-free global `re` in `text[from, to)` whose start - * lies outside a string literal (per `mask`). Null when none. + * lies outside a string literal (per `mask`) and, when `depthAt` is given, at + * exactly that brace depth (per `depth`). Null when none. */ function matchZonHeader( text: string, @@ -729,11 +756,15 @@ function matchZonHeader( mask: Uint8Array, from: number, to: number, + depth?: Uint8Array, + depthAt?: 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; + if (mask[m.index] !== 0) continue; + if (depth !== undefined && depthAt !== undefined && depth[m.index] !== depthAt) continue; + return m; } return null; } @@ -742,11 +773,22 @@ function matchZonHeader( export function parseZigBuildZon(raw: string): ZigBuildZonConfig | null { const text = stripZonComments(raw); const mask = zonStringMask(text); + const depth = zonDepthMask(text); // Locate the `.dependencies = .{ ... }` block. Use brace counting because // 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); + // early — and only accept a header outside string literals AND at brace + // depth 1 (a direct field of the file's top-level `.{`), so neither a + // `.name` value spelling `.dependencies = .{` nor a `.dependencies` field + // nested in some earlier anonymous struct can hijack it. + const depsHeader = matchZonHeader( + text, + /\.dependencies\s*=\s*\.\{/g, + mask, + 0, + text.length, + depth, + 1, + ); if (!depsHeader) return null; const start = depsHeader.index + depsHeader[0].length; const end = findZonBlockEnd(text, start); diff --git a/gitnexus/src/core/ingestion/languages/zig.ts b/gitnexus/src/core/ingestion/languages/zig.ts index f1b7d144c..fae047643 100644 --- a/gitnexus/src/core/ingestion/languages/zig.ts +++ b/gitnexus/src/core/ingestion/languages/zig.ts @@ -89,6 +89,12 @@ export const zigProvider = defineLanguage({ // ── RFC #909 Ring 3: scope-based resolution hooks ── emitScopeCaptures: emitZigScopeCaptures, interpretImport: interpretZigImport, + // `@import` is compile-time name lookup, not an executed statement: one + // written inside a function body is resolved exactly as one at file scope + // (same answer as C `#include` and Rust `use`). Without this, a + // function-scoped `@import` would be marked `runsOnlyWhenCalled` and a + // real import cycle through it would be hidden from `check --cycles`. + importsExecuteWhereWritten: false, interpretTypeBinding: interpretZigTypeBinding, bindingScopeFor: zigBindingScopeFor, receiverBinding: zigReceiverBinding, diff --git a/gitnexus/src/core/ingestion/languages/zig/captures.ts b/gitnexus/src/core/ingestion/languages/zig/captures.ts index d110c1472..0abdcded9 100644 --- a/gitnexus/src/core/ingestion/languages/zig/captures.ts +++ b/gitnexus/src/core/ingestion/languages/zig/captures.ts @@ -201,11 +201,14 @@ export function emitZigScopeCaptures( const byName = new Map(m.captures.map((c) => [c.name, c.node] as const)); const importName = byName.get('import.name'); const importSource = byName.get('import.source'); + const importStmt = byName.get('import.statement'); if ( importName !== undefined && importSource !== undefined && byName.get('import.imported') === undefined && - byName.get('import.statement')?.parent?.type === 'source_file' + importStmt !== undefined && + importStmt.parent?.type === 'source_file' && + isZigKeywordDeclaration(importStmt) ) { importSources.set(importName.text, importSource); } @@ -230,6 +233,20 @@ export function emitZigScopeCaptures( } if (Object.keys(grouped).length === 0) continue; + // `_ = @import("x.zig");` and any other keyword-less ` = + // @import(…)`: a statement (tree-sitter-zig reuses variable_declaration + // for assignments), not a declaration. It references the file without + // binding a name → side-effect import (file edge only). The keyword-less + // shape also never enters `importSources`, so it cannot promote aliases. + const importStmt = nodeMap['@import.statement']; + if (importStmt !== undefined && !isZigKeywordDeclaration(importStmt)) { + out.push({ + '@import.side-effect': nodeToCapture('@import.side-effect', importStmt), + '@import.source': grouped['@import.source']!, + }); + continue; + } + // Member aliases: promote to a named import when the object is one of // this file's @import bindings; otherwise the group is inert (the same // node is also matched by the plain-variable rule). diff --git a/gitnexus/src/core/ingestion/languages/zig/interpret.ts b/gitnexus/src/core/ingestion/languages/zig/interpret.ts index 5efe199cf..a173d538f 100644 --- a/gitnexus/src/core/ingestion/languages/zig/interpret.ts +++ b/gitnexus/src/core/ingestion/languages/zig/interpret.ts @@ -2,12 +2,28 @@ import type { CaptureMatch, ParsedImport, ParsedTypeBinding, TypeRef } from 'git const stripQuotes = (s: string): string => s.replace(/^["']|["']$/g, ''); +/** + * The module a `@import` target names, for the `importedName` of a namespace + * import (the shared contract wants the MODULE there — Go's `import foo + * "pkg/bar"` records `bar` — not the local handle): the last path segment + * without its `.zig` extension. `"std"` → `std`, `"./net/socket.zig"` → + * `socket`, `"mylib"` (a build.zig.zon dep) → `mylib`. + */ +export function zigModuleNameOf(targetRaw: string): string { + const last = targetRaw.replace(/\\/g, '/').split('/').filter(Boolean).pop() ?? targetRaw; + return last.endsWith('.zig') ? last.slice(0, -'.zig'.length) : last; +} + /** * `const std = @import("std");` binds the imported module to a const handle * accessed via qualified syntax — a namespace import (closest peers: Python - * `import numpy`, Go `import "pkg/bar"`). The local name and imported name - * are always the same identifier; Zig has no rename syntax at the import - * site (renames are ordinary const aliases handled as variable bindings). + * `import numpy`, Go `import "pkg/bar"`). `localName` is the handle the + * author chose, `importedName` the module (`zigModuleNameOf`). + * + * `_ = @import("x.zig");` (and any keyword-less ` = @import(…)`, a + * statement rather than a declaration in this grammar) references the file + * without binding a name — the `refAllDecls` / test-aggregation idiom. That + * is a `side-effect` import: file edge, no binding (TS `import './x'`). */ export function interpretZigImport(captures: CaptureMatch): ParsedImport | null { const source = captures['@import.source']?.text; @@ -21,6 +37,9 @@ export function interpretZigImport(captures: CaptureMatch): ParsedImport | null if (captures['@import.wildcard'] !== undefined) { return { kind: 'wildcard', targetRaw }; } + if (captures['@import.side-effect'] !== undefined) { + return { kind: 'side-effect', targetRaw }; + } const name = captures['@import.name']?.text; if (name === undefined) return null; @@ -36,7 +55,12 @@ export function interpretZigImport(captures: CaptureMatch): ParsedImport | null : { kind: 'alias', localName: name, importedName: imported, alias: name, targetRaw }; } - return { kind: 'namespace', localName: name, importedName: name, targetRaw }; + return { + kind: 'namespace', + localName: name, + importedName: zigModuleNameOf(targetRaw), + targetRaw, + }; } /** diff --git a/gitnexus/src/core/ingestion/method-extractors/configs/zig.ts b/gitnexus/src/core/ingestion/method-extractors/configs/zig.ts index 4b20667c3..fd4ada3ea 100644 --- a/gitnexus/src/core/ingestion/method-extractors/configs/zig.ts +++ b/gitnexus/src/core/ingestion/method-extractors/configs/zig.ts @@ -91,8 +91,10 @@ const extractZigReceiverType = (node: SyntaxNode): string | undefined => { * Anonymous `test {}` and decl-tests `test add {}` are not graph nodes. They * return `''`, not `null`: `null` falls through to `genericFuncName`, whose * first-identifier scan would name `test add {}` "add" — the REAL `fn add`'s - * id — and hang the test body's calls on it. The empty name ends the walk at - * this node and lets the caller fall back to the File. + * id — and hang the test body's calls on it. The empty name is falsy, so + * `findEnclosingFunctionId` skips this node WITHOUT attributing to it and + * keeps walking up; a test block can only sit at container level, so the walk + * reaches the file and the calls attribute to the File. */ const extractZigFunctionName = ( node: SyntaxNode, diff --git a/gitnexus/test/unit/zig-extractors.test.ts b/gitnexus/test/unit/zig-extractors.test.ts index 062241fc0..8c72d4283 100644 --- a/gitnexus/test/unit/zig-extractors.test.ts +++ b/gitnexus/test/unit/zig-extractors.test.ts @@ -32,6 +32,7 @@ import { createFieldExtractor } from '../../src/core/ingestion/field-extractors/ import { zigFieldConfig } from '../../src/core/ingestion/field-extractors/configs/zig.js'; import { zigProvider } from '../../src/core/ingestion/languages/zig.js'; import { createSemanticModel } from '../../src/core/ingestion/model/semantic-model.js'; +import { extract as extractScopes } from '../../src/core/ingestion/scope-extractor.js'; const _require = createRequire(import.meta.url); let Zig: unknown = null; @@ -467,26 +468,32 @@ const plain = 1; it('interprets namespace, named, alias, wildcard and namespace-member forms', () => { const src = ` -const counter = @import("counter.zig"); -const Counter = counter.Counter; +const c = @import("net/counter.zig"); +const Counter = c.Counter; const Renamed = @import("counter.zig").Counter; const Same = @import("counter.zig").Same; const Deep = @import("std").mem.Allocator; pub usingnamespace @import("mixin.zig"); const notAnImport = other.Thing; +test { + _ = @import("all_tests.zig"); + x = @import("keyword_less.zig"); +} `; const imports = emitZigScopeCaptures(src, 'x.zig') .filter((m) => m['@import.source'] !== undefined) .map((m) => interpretZigImport(m)); expect(imports).toEqual([ - { - kind: 'namespace', - localName: 'counter', - importedName: 'counter', - targetRaw: 'counter.zig', - }, + // namespace: localName is the handle, importedName the MODULE (contract: + // Go `import foo "pkg/bar"` records `bar`), never the handle. + { kind: 'namespace', localName: 'c', importedName: 'counter', targetRaw: 'net/counter.zig' }, // alias of a namespace member → promoted to a named import of that member - { kind: 'named', localName: 'Counter', importedName: 'Counter', targetRaw: 'counter.zig' }, + { + kind: 'named', + localName: 'Counter', + importedName: 'Counter', + targetRaw: 'net/counter.zig', + }, { kind: 'alias', localName: 'Renamed', @@ -503,6 +510,10 @@ const notAnImport = other.Thing; targetRaw: 'std', }, { kind: 'wildcard', targetRaw: 'mixin.zig' }, + // `_ = @import(...)` and any keyword-less ` = @import(...)` are + // statements (no `const`/`var`): a file reference, not a binding. + { kind: 'side-effect', targetRaw: 'all_tests.zig' }, + { kind: 'side-effect', targetRaw: 'keyword_less.zig' }, ]); // `other` is not an @import binding of this file → stays a variable. const vars = emitZigScopeCaptures(src, 'x.zig') @@ -511,6 +522,22 @@ const notAnImport = other.Thing; expect(vars).toEqual(['notAnImport']); }); + it('a function-scoped @import is not deferred: Zig imports are compile-time (importsExecuteWhereWritten: false)', () => { + // C `#include` and Rust `use` answer the same. Without the flag the scope + // extractor marks a body-level `@import` `runsOnlyWhenCalled`, hiding a + // real import cycle through it from `check --cycles`. + const src = ` +pub fn run() void { + const helper = @import("helper.zig"); + helper.go(); +} +`; + const result = extractScopes(emitZigScopeCaptures(src, 'x.zig'), 'x.zig', zigProvider); + const helper = result.parsedImports.find((i) => i.targetRaw === 'helper.zig'); + expect(helper).toBeDefined(); + expect(helper!.runsOnlyWhenCalled).toBeUndefined(); + }); + it('the provider skips the Const capture for container and @import bindings', () => { const root = parse(` const std = @import("std"); diff --git a/gitnexus/test/unit/zig-import-resolver.test.ts b/gitnexus/test/unit/zig-import-resolver.test.ts index bdb33dd7d..3c4dd4213 100644 --- a/gitnexus/test/unit/zig-import-resolver.test.ts +++ b/gitnexus/test/unit/zig-import-resolver.test.ts @@ -133,6 +133,20 @@ describe('resolveZigImportInternal', () => { ); }); + it('returns null for absolute `.path` deps, POSIX and Windows spellings alike', () => { + // `normalizeZigDepPath` promises null for anything outside the repo; a + // `/`-only check let `C:\\local_dep` through as the relative `C:/local_dep`. + const files = new Set([ + 'src/main.zig', + 'C:/local_dep/src/dep.zig', + 'local_dep/src/dep.zig', + ]); + for (const abs of ['/local_dep', 'C:\\local_dep', 'c:/local_dep', 'D:\\x\\local_dep']) { + const zon = { pathDeps: new Map([['dep', abs]]) }; + expect(resolveZigImportInternal('src/main.zig', 'dep', files, zon)).toBeNull(); + } + }); + it('returns null for `.path` deps that escape the repo root (`..`)', () => { const files = new Set(['src/main.zig']); const buildZon = { pathDeps: new Map([['ziggit', '../ziggit']]) }; @@ -249,6 +263,28 @@ describe('parseZigBuildZon', () => { expect([...cfg!.pathDeps.entries()]).toEqual([['real', 'vendor/real']]); }); + it('takes the top-level `.dependencies` block, not a same-named field nested in an earlier struct', () => { + // Only a direct field of the file's `.{ … }` (brace depth 1) is the + // manifest's dependency map; a nested `.dependencies = .{` seen first + // used to be selected and the real map ignored. + const zon = ` +.{ + .name = .pkg, + .metadata = .{ + .dependencies = .{ + .decoy = .{ .path = "decoy" }, + }, + }, + .dependencies = .{ + .real = .{ .path = "libs/real" }, + }, +} +`; + const cfg = parseZigBuildZon(zon); + expect(cfg?.pathDeps.get('real')).toBe('libs/real'); + expect(cfg?.pathDeps.has('decoy')).toBe(false); + }); + it('returns null when no `.dependencies` block is present', () => { const raw = `.{ .name = "x", .version = "0.0.0", .paths = .{""} }`; expect(parseZigBuildZon(raw)).toBeNull();