mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-11 03:38:07 +00:00
fix(zig): address third gitnexus-check review pass
- language-config `parseZigBuildZon`: the `.dependencies = .{` header and
the per-entry `.<name> = .{` 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.
This commit is contained in:
parent
e249f875fe
commit
b654b66261
8 changed files with 148 additions and 39 deletions
|
|
@ -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('/')) {
|
||||
|
|
|
|||
|
|
@ -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<string, string>();
|
||||
// Walk each `.<name> = .{ ... }` entry; the body ends at the matching brace
|
||||
// (string-aware), not at the first `}` in the text.
|
||||
// Walk each `.<name> = .{ ... }` 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]);
|
||||
|
|
|
|||
|
|
@ -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()) {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -112,6 +112,7 @@ const BASENAME_LANGS: Record<string, SupportedLanguages[]> = {
|
|||
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<string, SupportedLanguages[]> = {
|
|||
CPP: [SupportedLanguages.CPlusPlus],
|
||||
TS: [SupportedLanguages.TypeScript],
|
||||
JS: [SupportedLanguages.JavaScript],
|
||||
ZIG: [SupportedLanguages.Zig],
|
||||
};
|
||||
|
||||
/** Candidate grammar languages a CODE literal in `relPath` should be checked against. */
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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<string>(['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<string>(['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();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue