mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-11 03:38:07 +00:00
fix(zig): address eighth gitnexus-check review pass
- Named dependency modules: `parseZigBuildModuleRoots` scanned
`addModule("<name>", .{ … })` 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("<name>")`. 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 `#<arity>` 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.
This commit is contained in:
parent
4f223cc469
commit
fdfdd2e35f
5 changed files with 134 additions and 8 deletions
|
|
@ -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];
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 `#<arity>` 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?.({
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
[],
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue