mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-11 03:38:07 +00:00
fix(zig): address sixth gitnexus-check review pass
- 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 `<ident> = @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).
This commit is contained in:
parent
65c07c3d9a
commit
f8b631630f
7 changed files with 178 additions and 24 deletions
|
|
@ -507,8 +507,8 @@ export async function loadSwiftPackageConfig(repoRoot: string): Promise<SwiftPac
|
|||
* },
|
||||
*
|
||||
* Limitations (intentional — bail to null on anything weirder):
|
||||
* - Only the top-level `.dependencies = .{ ... }` block is parsed; nested
|
||||
* or aliased blocks are ignored.
|
||||
* - Only the top-level `.dependencies = .{ ... }` block is parsed (brace
|
||||
* depth 1); a same-named field nested in another struct is ignored.
|
||||
* - Each dep entry is matched by a single shape: `.<name> = .{ ... }`
|
||||
* where `<name>` 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<ZigBuildZonConf
|
|||
* resolver so both sides agree on which deps are in-repo.
|
||||
*/
|
||||
export function normalizeZigDepPath(depPath: string): string | null {
|
||||
if (depPath.startsWith('/')) return null;
|
||||
// POSIX (`/x`) and Windows (`C:\x`, `C:/x`) absolute paths both point
|
||||
// outside the repository.
|
||||
if (depPath.startsWith('/') || /^[A-Za-z]:[\\/]/.test(depPath)) return null;
|
||||
const parts: string[] = [];
|
||||
for (const part of depPath.replace(/\\/g, '/').split('/')) {
|
||||
if (part === '' || part === '.') continue;
|
||||
|
|
@ -719,9 +721,34 @@ function zonStringMask(text: string): Uint8Array {
|
|||
return mask;
|
||||
}
|
||||
|
||||
/**
|
||||
* Per-offset brace depth for comment-stripped ZON text, string-aware: the
|
||||
* depth AT an offset is the number of unclosed `{` before it. The file's
|
||||
* top-level `.{` puts every direct field at depth 1.
|
||||
*/
|
||||
function zonDepthMask(text: string): Uint8Array {
|
||||
const depth = new Uint8Array(text.length);
|
||||
let d = 0;
|
||||
let inString = false;
|
||||
for (let i = 0; i < text.length; i++) {
|
||||
const ch = text[i];
|
||||
depth[i] = d;
|
||||
if (inString) {
|
||||
if (ch === '\\' && i + 1 < text.length) depth[++i] = d;
|
||||
else if (ch === '"') inString = false;
|
||||
continue;
|
||||
}
|
||||
if (ch === '"') inString = true;
|
||||
else if (ch === '{') d++;
|
||||
else if (ch === '}' && d > 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);
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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 `<ident> =
|
||||
// @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).
|
||||
|
|
|
|||
|
|
@ -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 `<ident> = @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,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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 `<ident> = @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");
|
||||
|
|
|
|||
|
|
@ -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<string>([
|
||||
'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<string>(['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();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue