From 506432017fcf4321b7eb51ffa0db001a2014312a Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 9 Sep 2026 22:12:17 +0200 Subject: [PATCH] fix(zig): model callable-value references, and stop reporting their absence as `exact` (#3219) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(impact): stop reporting 'exact' over unmodelled callable-value references A function named in VALUE position — `bridge.accessor(Element.getNamespaceUri, null, .{})`, `{ onClick: handler }`, a comparator handed to a sort — is registered somewhere rather than called. The registration is modelled (a `value-ref` site becomes a USES edge; Kythe `ref` vs `ref/call`, Joern METHOD_REF), but the invocation THROUGH the stored value is not: it happens later via a struct field, a registry lookup or comptime reflection. impact()/context() nonetheless reported such a target as `epistemic: 'exact'`. For lightpanda-io/browser that meant the DOM `Element.namespaceURI` accessor came back with two internal callers, LOW risk and a claim of completeness — worse than no answer, because 'exact' tells the reader not to look further. tools.ts defines 'lower-bound' as "the walk provably missed callers", which is precisely this case. computeEpistemicBoundary now probes for inbound USES edges stamped with the value-ref reason and hedges when it finds any, contributing a boundary note and a new `causes.callableValueReferences` (unit: distinct referrer symbols). Read from the graph, not from index metadata: unlike a dropped receiver — which leaves no edge to find and therefore needs a persisted summary — a value reference IS in the graph. So the signal needs no re-index and no analyzer change, and it works on indexes written before this commit. The cause gets its own slot rather than joining `dispatchBoundary`: a value referrer is neither an implementation nor an interface-level consumer, and the two differ in what the reader should do about them — a dispatch boundary is irreducible, a callable value usually becomes traceable once the provider models the store/load that carries it. Language-neutral: every provider that emits a `value-ref` capture participates. The writer and the reader now share VALUE_REF_EDGE_REASON, because drift between them would fail silently — the probe would match nothing and every answer would go back to claiming certainty. * fix(zig): emit `value-ref` for a callable named in value position `mapReferenceKindToEdgeType` has handled the registration-vs-invocation case since #2437, and TypeScript, JavaScript and C++ all emit `value-ref`. Zig emitted zero — so a Zig function handed somewhere as a VALUE was absent from the graph entirely. That is not a corner: Zig's JS bridge is built out of this one shape, pub const namespaceURI = bridge.accessor(Element.getNamespaceUri, null, .{}); and `bridge.{accessor,function,indexed,…}` appears 2,047 times across 257 files in lightpanda-io/browser — the project's whole JS<->Zig surface, none of it reaching the graph. `impact` on `Element.getNamespaceUri`, which IS the DOM `Element.namespaceURI` accessor, answered with its two in-file callers. Three query rules, tagging `@reference.value-ref` on: a bare identifier in argument position (`bridge.accessor(_tagName, …)`), a qualified one (`bridge.accessor(Element.getNamespaceUri, …)`), and a const binding initialiser (`pub const defaultHandler = onReset;`). Everything downstream already existed — no new edge type, no schema change, no capture-machinery change, no baseline edited. One grammar detail is load-bearing: in tree-sitter-zig, call arguments are DIRECT children of `call_expression` — there is no `arguments` node, only builtins have one — so the callee has to be consumed explicitly by `function:`. Without that binding the same rule also matches the callee of `foo(bar)` and mints a USES edge shadowing the call's own CALLS edge. There is a test for it. The rules are deliberately broad — `js.Bridge(Element)` and `register(count)` match too — because the callable gate in the property-dispatch pass (Function/Method/Constructor only) is the filter, the same design that keeps TypeScript's `{ port: DEFAULT_PORT }` from registering anything. Measured on the real corpus: all 3,169 emitted edges land on a Method (3,146) or Function (23). Deliberately NOT modelled: the terminal invoke. The value reaches `Accessor.init` -> a struct field -> `Factory.zig` reflection (`inline for`, `@typeInfo`) -> `Caller.zig`'s `@call(.auto, func, args)` over `func: anytype`. Resolving that needs comptime evaluation. The point is to stop dropping the reference; the shortfall is now reported as `epistemic: "lower-bound"` by the preceding commit instead of being papered over. Measured on lightpanda-io/browser (698 .zig files), wiped index both times: 30,222 nodes / 71,070 edges -> 30,222 nodes / 74,229 edges. The delta is entirely `USES` (3,169 after vs 10 before, and every one is a value-ref); CALLS, ACCESSES, IMPORTS, HAS_METHOD, DEFINES and the rest are unchanged — purely additive, no node invented. The fixture joins the existing `zig-idioms` corpus rather than adding a new one, because `bench/receiver-resolution` uses `test/fixtures/lang-resolution` as its `--check` corpus. That gate, `scope-capture` (15 languages), `zig-cross-file-resolution` and every other bench `--check` pass unchanged. * fix(review): resolve qualified value references through their owner, and stop hedging on registrations the analyzer already followed Five findings from the review bot on #3219; four valid, all addressed. 1. QUALIFIED VALUE REFERENCES RESOLVED BY TAIL NAME (the serious one). `bridge.accessor(Element.getNamespaceUri, …)` was resolved with `findCallableBindingInScope(site.inScope, site.name, …)`, which never sees the receiver and gives LOCAL bindings precedence. Reproduced: const Element = @This(); pub fn getThing(...) // main.getThing pub const JsApi = struct { fn getThing(...) // JsApi.getThing pub const thing = bridge.accessor(Element.getThing, null, .{}); }; emitted `USES JsApi → JsApi.getThing` — a WRONG edge, which is worse than the missing edge this PR set out to fix. `resolveValueRefTarget` now resolves a site carrying an explicit receiver through `findClassBindingInScope` → `findOwnedMember` (the machinery `receiver-bound-calls` already uses), gated on `CALL_TARGET_TYPES` because `findOwnedMember` also answers with fields. A bare site keeps the lexical walk, which is what an unqualified name means. When the owner cannot be resolved the site is DECLINED rather than falling back: declining costs a reference, falling back mints a confident edge to the wrong function, and the missing reference is now reported as `lower-bound` anyway. Cost on lightpanda-io/browser: value-ref edges 3,169 → 2,799 (−12%). Those 370 were tail-name coincidences, not registrations — all 94 `Element.zig` `JsApi` entries survive, the cross-container case resolves and is correctly attributed (`IntersectionObserverEntry.JsApi` → `IntersectionObserverEntry. getTarget#0`, not the enclosing observer), and both acceptance probes are unchanged: `getNamespaceUri` 7/3/LOW/lower-bound, `getTagNameLower` 31/10/HIGH/exact. 2. A FAILED PROBE READ AS "NO BOUNDARY". `.catch(() => [])` turned an unanswerable query into count 0 and no note, so `exact` could be published on the strength of a question that was never asked. It now returns `null` and emits a boundary note. This file's own `loadMeta` comment states the rule: a probe failing must never read as certainty. 3. NOT EVERY VALUE REFERENCE IS AN UNMODELLED INVOCATION. Where `emitPropertyDispatchCalls` sweep 2 synthesized the dispatch (reason `property-dispatch`), the walk did NOT provably miss the caller, so hedging was noise over an answer that was computed — and a signal that fires on every JS/TS hook table stops carrying information. A second probe excludes those targets. Zig never sets a property key, so the motivating case is untouched. The exclusion is symbol-level, not per-edge, because the graph does not record which registration produced which synthesized call; that residual is documented at the call site. 4. `LIMIT 50` SILENTLY UNDERSTATED THE COUNT. `rows.length` over a capped row set published a ceiling as the documented symbol count. Replaced with `COUNT(DISTINCT other.id)` — bounded work without a bounded answer, the shape `countByType` in the same file already uses. 5. THE TEST DID NOT PIN THE CALLER COUNT its own comment promised. Added `expect(result.impactedCount).toBe(2)`. New regression tests: qualified references bind the written owner and not the nearer lexical match; an unresolvable receiver emits nothing rather than a wrong edge; a dispatch-modelled registration stays `exact`; a probe that cannot run hedges instead of claiming certainty. Known limitation, pinned by a test rather than left implicit: when a `@This()` alias's NAME differs from its container's (`const Element = @This();` inside `main.zig`), the receiver does not resolve and the reference is declined. The provider has `rewriteZigThisAlias` for this, but it is applied to type nodes and extending it to reference receivers would change existing CALL-site behaviour. Lightpanda's `const Foo = @This();` in `Foo.zig` convention makes the names coincide, which is why the corpus is unaffected. * fix(review): bump the parse-cache schema and resolve module-owned value references Review round 2 on #3219. Four inline items, all reproduced against the worktree before deciding. P1 — the new captures could stay INERT on a warm parse cache. Adding `@reference.value-ref` rules to `ZIG_SCOPE_QUERY` changes `ParsedFile.referenceSites`, which is a PARSE-TIME fact, but `SCHEMA_BUMP` stayed at 93. A repo indexed before this branch and re-analyzed after it replays the old, empty site list for every unchanged `.zig` file — `--force` included, since shards are content-addressed — so no USES edge is emitted, the boundary probe measures a real zero, and `impact` on a registered accessor goes back to `epistemic: "exact"`. #3399 un-fixed on the incremental path most users are on, with every cold-run test still green. DECISIONS D1-1's "zero changes to … the schema" conflated the graph schema with the cache schema. Bumped 93 -> 98, not 94: #3190 claims 94 and #3179 claims 94 through 97 in one PR. `incremental-parse-cache.test.ts` re-pinned, with 93-97 added to the taken list. Re-check against origin/main and open PRs immediately before merging. P2 — a qualified value reference through a MODULE was declined with no hedge. R1-2 resolves a written receiver with `findClassBindingInScope`, which requires `isClassLike`; a namespace-only `@import` handle is not class-like, so const dom_utils = @import("dom_utils.zig"); // no `@This()` in that file pub const comparator = bridge.accessor(dom_utils.compare, null, .{}); resolved to nothing. That is not the conservative half of R1-2's trade-off: a declined site emits NO edge, so there is nothing for the probe to read and `impact` on `compare` reports `exact`. Silence, not a hedge — and the pass comment claiming otherwise was wrong on this path. `resolveValueRefTarget` now tries the second kind of owner a qualified name can have. `findNamespaceValueRefTarget` reads the file's `namespace` import edges for the handle and the target module's own `origin: 'local'` module-scope bindings for the member — the same channel `receiver-bound-calls` Case 1 already trusts for `dom_utils.compare()`, with Case 1's three guards for Case 1's reasons: `isNamespaceNameShadowed`, local-origin bindings only, and two distinct defs under one name resolve nothing. `CALL_TARGET_TYPES` gates module owners exactly as it gates container owners. Language-neutral: it reads generic namespace import edges, names no language. Still declined, deliberately: a receiver this index knows under no name at all (the `@This()`-alias case, owners outside the workspace). There the alternative is a confident edge to a lexically-nearer function the source did not name. P2 — `impact-callable-value-references.test.ts` was in neither vitest list. It opens a real engine via `withTestLbugDB(poolAdapter: true)`, so TESTING.md puts it in the `lbug-db` include list and the `default` exclude list; it was in neither, so `default` also collected it into the parallel pool. Added next to its `impact-epistemic-lower-bound` sibling in both arrays. P3 — DECISIONS.md was stale against HEAD and embedded host paths. D2-3 still described the `LIMIT 50` that R1-5 removed and D1-3 still described the receiver-blind resolution that R1-2 replaced; both now carry explicit "superseded by" pointers. The `~/code/...` and mise-node paths are replaced with placeholders. Two smaller corrections the new cause made necessary: - `formatImpactResult`'s `lower-bound` header hard-coded "callers binding via DI / dynamic dispatch", which now contradicts the value-reference bullet printed directly under it. The bullets carry the cause; the header only states that the count is a floor. - `tools.ts` said a `causes.callableValueReferences` of 0 means "nothing was missed". The dispatch exclusion is symbol-level, not edge-level, so a target with both a followed registration and an unfollowed escape also reads 0. The docs now say a 0 means "no unfollowed registration was proven". Fixture: `src/webapi/dom_utils.zig` (namespace-only) plus three cases in `Element.zig` — the module-qualified registration, a non-callable module member, and a `u8` parameter shadowing the handle. The shadow case was verified to FAIL with the guard disabled, so it is not passing for an unrelated reason. Gates: `tsc --noEmit` clean, `npm run build` clean, prettier clean. `resolvers/` 3,630 passed / 3 skipped; `unit/scope-resolution` 2,010 passed; `impact-callable-value-references` 7 passed under `lbug-db`; `incremental-parse-cache` 40 passed; eval formatters 104 passed. Bench --check all PASS with no baseline edited: receiver-resolution, zig-cross-file-resolution, scope-emission, callable-value-flow, scope-capture (15 languages), python-scope. * fix(review): guard the class receiver against a shadowing binding, and pin the dispatchability partition Review round 3 (`gitnexus-check` bot on `cf53bbaa`). Three findings, each reproduced against the code before deciding. R3-1 (Error, valid, REPRODUCED) — a CLASS receiver could resolve through a shadowing value binding. `findClassBindingInScope` is a class-only walk: it filters the scope chain by `isClassLike`, so it steps over a nearer binding that is a value and keeps climbing — and past the chain entirely, into a qualified-name fallback that answers with the unique workspace definition of the name. A `u8` parameter named `Ticker`, in a file that neither declares nor imports the `Ticker` container another file defines, therefore emitted `register(Ticker.fire)` as a confident USES edge to that container's method. That is exactly the wrong-edge failure R1-2 exists to prevent, arriving through the class channel instead of the lexical one. Fixed with `isOwnerNameShadowedBySomethingElse` — a sibling of `isNamespaceNameShadowed` with one extra clause. The plain namespace guard could NOT be reused: a container is often its own local declaration (`fn make() { const Local = struct {…}; register(Local.go); }`), and reading that binding as its own shadow suppresses precisely the resolutions this path exists to make — the #2723 mistake, one channel over. So a scope that binds the name answers immediately, and the answer is "not shadowed" only when one of that scope's own bindings IS the def just resolved. Both halves are pinned and both were verified to fail when the guard is weakened: the parameter case fails with no guard at all, the local-container case fails with the plain `isNamespaceNameShadowed`. R3-2 (Warning; mechanism correct, unreachable today; fragility fixed instead) — the dispatch exclusion could suppress an unfollowed registration. The bot is right about the code: sweep 2 synthesizes CALLS only for a registration whose site carried a `propertyKey`, while the exclusion zeroes the note on ANY inbound `property-dispatch` CALLS edge. It is not reachable in the current rule set, and the reason is measured rather than assumed: `@reference.value-ref` is emitted by exactly three languages — JavaScript (2 rules), TypeScript (2), Zig (3) — every JS/TS rule also captures `@reference.property-key` (both are object-literal shapes) and no Zig rule does. A dispatchable registration is therefore always a JS/TS one, an undispatchable one always a Zig one, and they cannot meet on one symbol. Rejected: splitting the edge `reason` into dispatchable / undispatchable. It is the precise fix, but it is a graph-content change that churns whichever side keeps the old literal — the Zig, TypeScript and probe suites all pin 'scope-resolution: value-ref' by hand as a drift canary — and it buys nothing against a case no rule can produce. What was actually wrong is that the exclusion's soundness rested on a coincidence recorded nowhere, in files nobody reading `local-backend.ts` would open. Fixed at both ends: the exclusion site now states the invariant, the three facts it rests on and the two options for when it breaks; and `value-ref-dispatchability.test.ts` fails the day it does — a JS/TS rule for a bare callback argument, a Zig rule that grows a key, or a fourth language emitting `value-ref` at all. Verified to fire (adding a property key to a Zig value-ref rule fails the Zig case), and its rule splitter has its own guard test so the suite cannot pass vacuously. `ZIG_SCOPE_QUERY` is exported for that test only. R3-3 (Nit, valid) — a test comment claimed the wrong epistemic result. The `declines a qualified reference whose receiver cannot be resolved` case said the shortfall shows up as `lower-bound`. It does not: with no edge there is no evidence and the target stays `exact`. The pass docstring was corrected in round 2 and this comment was missed. It now says the decline costs the reference AND the hedge, and why that is still the right trade. Gates: tsc --noEmit clean, npm run build clean, prettier clean. `test/integration/resolvers` 3,632 passed / 3 skipped (70 files); `test/unit/scope-resolution` 2,015 passed (120 files); `impact-callable-value-references` 7 passed under `lbug-db`. Bench --check: receiver-resolution, zig-cross-file-resolution, scope-capture (15 languages), scope-emission PASS with no baseline edited; callable-value-flow failed once on its TIMING budget (2.006 > 1.9) with a byte-identical fingerprint, then passed twice at 1.788 / 1.813 — machine load, not a regression. * fix(review): let a file's own `@import` outrank the workspace-wide class fallback Found by running the local preflight harness before pushing rather than after. One of its five findings is a real hole in R2-2; the rest are documentation that overclaimed. R4-1 (valid, REPRODUCED) — the container channel preempted the file's own `@import`. R2-2 added the module channel as a FALLBACK after `findClassBindingInScope`, and that order is wrong: `findClassBindingInScope` does not stop at the scope chain. When its `isClassLike` walk misses — and a namespace handle binds a Module, so it always misses — it falls back to `scopes.qualifiedNames`, a workspace-wide index, and answers with the unique def of that name anywhere in the repo. A container named `dom_utils` in a file `Element.zig` never imports therefore captured `bridge.accessor(dom_utils.compare, …)`, binding `Method:src/webapi/decoy.zig:dom_utils.compare#2` while the module channel that would have answered correctly was never reached. R3-1's shadow guard cannot catch it: the import binds at MODULE scope, which that guard treats as the floor. Fixed by trying the module channel FIRST. An `@import` written in this file is the strongest available statement about what the name means here and outranks a global uniqueness guess; when the handle is not an import of this file the channel answers nothing and the container path runs exactly as before. Pinned by `decoy.zig` and a strengthened assertion on the existing module-owner test, which fails on the old order. R4-2 — the cause documentation named shapes nothing captures. `tools.ts` illustrated `callableValueReferences` with "a callback argument", "a stored function pointer" and `qsort(xs, n, sz, compareItems)`. Only Zig captures a call argument or a const initialiser; JS/TS capture only object-literal property values, and C has no value-ref rule, so the `qsort` example is counted in no language. Both cause blocks now name the captured shapes and say that a bare JS/TS callback argument is not among them, so a 0 does not rule it out. R4-3 — the same block exempted itself from the re-index caveat this PR proves it needs. "Read from the graph, so it needs no index-time metadata" is true of the probe and false of the edges: an index built before a language emitted these captures has none and reports 0 — the warm-cache failure R2-1 bumped SCHEMA_BUMP for. It now says to re-analyze before reading a 0 as measured. R4-4 — the dispatchability canary was narrower than its own promise. Its header claimed it fails on "a fourth language emitting value-ref at all"; it reads `languages//query.ts`, so Vue — which owns no query and borrows `emitTsScopeCaptures` / `emitJsScopeCaptures` — emits value-refs while the assertion lists three languages, and a capture synthesized in code is invisible to it. The case now asserts on query OWNERS, which is what it checks and is sound because a delegating language inherits the rules it borrows; the header states the synthesized-capture gap instead of letting a green tick imply it away. R4-5 — the BARE docstring described a lexical walk that is not one. `findCallableBindingInScope` applies the callable predicate WHILE walking, so a nearer parameter or local is stepped over: the defect R3-1 fixed on the container channel, unguarded here, pre-existing since #2437 and reachable in JS/TS. Out of scope for #3399, so behaviour is unchanged and the sentence now says what the walk does rather than implying a guarantee it does not give. Gates: tsc --noEmit clean, npm run build clean, prettier clean. `test/integration/resolvers` 3,632 passed / 3 skipped (70 files); `test/unit/scope-resolution` 2,015 passed (120 files); `impact-callable-value-references` 7 passed under `lbug-db`. Bench --check: receiver-resolution, zig-cross-file-resolution, scope-capture (15 languages), scope-emission, callable-value-flow all PASS with no baseline edited. * fix(review): honour the hub opt-in for value references, so `hub.fn` means one thing Review round 5 (`gitnexus-check` bot on `1c7c05ff`). One finding, valid and reproduced before fixing. `findNamespaceValueRefTarget` accepted only `ref.origin === 'local'`. R2-2 recorded that as deliberate — "the `namespaceExportsIncludeImportedNames` hub opt-in is a provider decision this language-neutral pass does not make" — and that reasoning was wrong twice over. Zig sets the flag (`languages/zig/scope-resolver.ts:41`, measured on ghostty and tigerbeetle before it landed), and the pass does have the provider in scope: `runScopeResolution` takes one and already forwards several of its hooks to other passes. The result was the exact asymmetry R2-2 argued against in its own first paragraph. A Zig hub declares nothing — every name it publishes it imported — so requiring a local declaration declines every member reached through one: // hub.zig pub const scale = @import("dom_utils.zig").scale; // Element.zig const hub = @import("hub.zig"); pub fn callsThroughTheHub(v: u8) u8 { return hub.scale(v); } // resolved pub const scaled = bridge.accessor(hub.scale, null, .{}); // declined One name meaning two different things depending on whether a `(` follows it. Fixed by forwarding `provider.namespaceExportsIncludeImportedNames` into the pass and consulting the published channel when it is set — the same question `receiver-bound-calls` Case 1 asks, through the same `lookupBindingsAt` read `findExportedDefIncludingImportedNames` performs for the CALL form. The pass still names no language; it asks the provider, which is the sanctioned hook. Precedence is unchanged where it mattered: a locally declared member still wins over a republished one, two distinct defs under one name still resolve nothing, and `CALL_TARGET_TYPES` still gates the answer — pinned by `hub.DEFAULT_NS`, a re-exported CONSTANT, which stays unregistered. Languages that do not opt in are unaffected: the parameter defaults to `false`, and passing `false` was verified to fail the new hub test, so it is load-bearing. The fixture republishes a member (`scale`) that nothing else uses, so the hub assertion cannot be satisfied by an edge another case emitted. Gates: tsc --noEmit clean, npm run build clean, prettier clean. `test/integration/resolvers` 3,634 passed / 3 skipped (70 files); `test/unit/scope-resolution` 2,015 passed (120 files); `impact-callable-value-references` 7 passed under `lbug-db`. Bench --check: receiver-resolution, zig-cross-file-resolution, scope-capture (15 languages), scope-emission, callable-value-flow all PASS with no baseline edited. Also recorded in DECISIONS.md: `/autofix` returned "No successful autofix run" because the `PR Autofix` run on `1c7c05ff` failed at `actions/upload-artifact` with a 403 from GitHub's artifact storage, not because of anything in this branch. This push triggers a fresh run. * fix(review): inspect the module scope in the owner-shadow guard, and check the TSX suffix The `gitnexus-check` pass on `bd6e577e` carried three findings of its own, and I answered the later `1c7c05ff` pass without noticing them. Recording that as a process failure too: bot reviews are per-head, and a later pass does not necessarily repeat an earlier one's findings. R6-1 (Error, valid, REPRODUCED) — the shadow guard stopped one rung short. `isOwnerNameShadowedBySomethingElse` returned `false` on reaching the module scope, justified as "a container declared there IS the binding, and the caller already resolved it". That holds when the owner came from the scope chain and fails when it came from the workspace-wide qualified-name fallback: // Gauge.zig — never imported by Element.zig const Gauge = @This(); pub fn read(self: *Gauge) u8 { … } // Element.zig const Gauge = @import("dom_utils.zig").DEFAULT_NS; // NOT a container pub const level = bridge.accessor(Gauge.read, null, .{}); // → Gauge.zig's read `findClassBindingInScope` steps over the module-scope binding because it is not class-like, answers from `scopes.qualifiedNames`, and the guard waved it through. Worth recording: the first fixture attempt did NOT reproduce. A local `const Gauge: u8 = 3;` also claims the workspace qualified name `Gauge`, leaving two candidates, and the fallback refuses to guess between two — so the shape defeated itself. Binding the name by IMPORT claims no qualified name, the fallback stays unique, and it fires. A negative result on the first shape was not evidence the finding was wrong. Fixed by inspecting the module scope as the last rung instead of skipping it. The identity exemption is what makes that safe where `isNamespaceNameShadowed` cannot do it (#2723: a namespace import writes its own name into the module scope and would read as its own shadow) — the binding that IS the owner exempts itself, and only a binding to something else answers `true`. `lookupBindingsAt` is consulted at that scope and only there, because an imported alias lives in the finalized channel rather than in `scope.bindings`. R6-2 — the hub re-export finding, already fixed in `5d8fe9d8`; the same defect restated on the later head. R6-3 (valid, fixed) — the dispatchability canary omitted the TSX suffix. `getTsScopeQuery` analyzes a `.tsx` file with `TYPESCRIPT_SCOPE_QUERY + TSX_JSX_QUERY_SUFFIX`, and the test read only the base, so a `value-ref` rule added to the suffix would be emitted in TSX analysis with the canary green. The suffix is now exported and concatenated into the check; verified load-bearing by adding an unkeyed `jsx_expression` value-ref rule to it, which fails the TypeScript case. Gates: tsc --noEmit clean, npm run build clean, prettier clean. `test/integration/resolvers` 3,635 passed / 3 skipped (70 files); `test/unit/scope-resolution` 2,015 passed (120 files); `impact-callable-value-references` 7 passed under `lbug-db`. Bench --check: receiver-resolution, zig-cross-file-resolution, scope-capture (15 languages), scope-emission, callable-value-flow all PASS with no baseline edited. * perf(bench): gate callable-value reference resolution on linear scaling `resolveValueRefTarget` (#3399) replaced one lexical walk with four channels, and the last of them — a qualified receiver resolved through `findClassBindingInScope` — falls back to `scopes.qualifiedNames`, a WORKSPACE-WIDE index consulted once per site. Keyed that is O(1); scanned it is O(files) per site, and a registration table that costs O(sites) today costs O(sites x files) tomorrow. Nothing in the suite can see that: the fixtures are single-file, and the corpus it actually matters on is lightpanda-io/browser, where `bridge.{accessor,function,…}` appears 2,047 times across 257 files. `bench/value-ref-resolution/measure.mjs` builds two synthetic Zig corpora of identical shape 4x apart in file count, and times ONLY the per-site resolution loop — extraction, `reconcileOwnership` and finalize are setup. `linear_factor` is `(t_large/t_small)/(N_large/N_small)`: measured 1.00-1.09 across runs, with `us_per_site` flat at 1.4-1.5 between the arms. Correctness comes first, because a timing gate alone is satisfied by a fast wrong answer: exact site/resolved/declined counts per arm plus an order-independent sha256 over every (site -> resolved target) pair. The corpus exercises all four channels — CONTAINER, NAMESPACE, HUB and BARE — and carries two DECLINE controls per module (a non-callable namespace member, a non-callable bare argument), so a widened callable gate moves `declined` instead of hiding inside the timing. Verified load-bearing rather than assumed: making the qualified lookup scan a workspace-sized collection leaves the fingerprint IDENTICAL and takes `linear_factor` to 3.752 against a slack of 1.375 — the regression class this exists for is exactly the one no correctness gate can see. Zig is the corpus because it is the only language whose provider sets `namespaceExportsIncludeImportedNames`, so it is the only one that can exercise the hub channel at all; the pass itself names no language. `resolveValueRefTarget` is exported for the bench. Timing `emitPropertyDispatchCalls` instead would fold the signal into edge emission, and re-implementing the channel order in the bench would pin the bench's idea of the function rather than the function. * feat(zig): index every build package in the repo, not only the root one Zig already had workspace setup — `loadZigBuildConfig` has parsed `build.zig.zon` `.path` deps, the root `build.zig`'s named modules and each build module's own `addImport` table since #1432, threaded through `ScopeResolver.loadResolutionConfig` exactly as tsconfig is for TypeScript. What it did not have is the part `tsconfigFor` supplies: per-package scope. `loadZigBuildConfig` reads `/build.zig{,.zon}` and nothing else, so a repo laying its packages out as `packages//build.zig` — no root build files at all — got `null`, and EVERY bare `@import("")` in it went unresolved. Cross-file resolution silently degraded to relative imports. Measured on a two-package probe before this change: `config = null`, `@import("core")` from `packages/app/src/main.zig` → `null`. `loadZigWorkspaceIndex` discovers packages the way `findTsconfigFiles` discovers configs — one bounded breadth-first walk that skips the hardcoded ignore set — and `zigPackageFor` is the `tsconfigFor` analogue: the nearest enclosing package governs a file, deepest-first, with no fall-through to an outer package. Fall-through is what makes a vendored dependency's `@import("config")` resolve to the outer repo's `config` module, the same failure `loadTsconfigIndex` documents for a package declaring no `baseUrl`. `resolveZigImportInternal` is UNCHANGED — impact analysis puts it at HIGH risk with 6 dependents, and it does not need to move: it is handed one package's config, and which config it receives is the only difference. Its 48 existing tests pass untouched. A nested package's paths are rebased to repo-relative at load time (`.path = "../core"` → `packages/core`, which the root-relative reading rejects outright as an escape); the ROOT package keeps its raw spelling, which is what `parseZigBuildZon` promises and its tests pin, so a single-package repo is byte-identical. `loadImportConfigs` — which runs unconditionally for every repo, Zig or not — keeps calling the root-only loader, so no non-Zig repo pays for the walk. That is the split TypeScript already has between the cheap `loadTsconfigPaths` and the repo-walking `loadTsconfigIndex`, which `loadResolutionConfig` reaches only during a language pass. The `zig-monorepo` fixture carries the discriminating case rather than only the happy path: `tool` binds the alias `core` to its OWN `src/core.zig`, so a repo-wide flattened module map — the shape a workspace index invites — would point `measure` at `packages/core`, a confident edge into a package `tool` does not depend on. That is worse than the unresolved import this fixes, and only per-package scoping keeps them apart. Tests: 7 unit cases pinning the index (including `loadZigBuildConfig` answering `null` on the same fixture, side by side) and 3 integration cases pinning the EDGES — cross-package CALLS from the module root AND from a non-root file, and `tool` not crossing over. Verified load-bearing: restricting the index to the root package fails all three. Second consumer caught by the integration test and fixed with it: `populateZigWorkspaceStaticGating` reads the same `resolutionConfig`, per file because two files of that pass can belong to different packages. Gates: build clean, `tsc --noEmit` clean, prettier clean, eslint 0 errors. `test/integration/resolvers` 3,768 passed / 4 skipped, `test/unit/scope-resolution` 2,015 passed. Bench `--check` with NO baseline edited: `receiver-resolution` (whose corpus is `test/fixtures/lang-resolution`, where the fixture lands), `zig-cross-file-resolution`, `value-ref-resolution`, `scope-capture` (15 languages), `import-target`, `callable-value-flow`, `scope-emission`. * perf(zig): dequeue the package walk by head index, not `shift()` `findZigPackageDirs` pushes children while it drains the queue, which keeps the array in a mode where `Array.prototype.shift()` memmoves the whole remainder rather than taking V8's left-trimming fast path — so the walk is quadratic in the frontier, bounded only by `ZIG_SCAN_MAX_DIRS` (20,000). Measured at that bound rather than estimated from the move count: 53 ms at fan-out 4 and 81 ms at fan-out 20, against 0.8 ms with a head index — 66-106x, and paid before a single config is read. FIFO order is unchanged, and the package ordering never depended on the walk anyway: `loadZigWorkspaceIndex` sorts by (dir length desc, localeCompare). Memory is unchanged too — the entries were already retained by the pushes; `shift()` only dropped the head. `findTsconfigFiles` and `loadNodeWorkspacePackages` carry the same walk with the same bound and are equally affected. Deliberately NOT fixed here: they are pre-existing, outside this PR's diff, and reach TypeScript/Node import resolution, which nothing in this change set covers. Filed separately. Reported by gitnexus-check on aea0ab06. * chore: drop DECISIONS.md from the branch Review feedback: the working log does not belong in the repository. The reasoning it carried that is still load-bearing lives in the code comments and in the PR description. * perf(bench): gate value-ref resolution on scaling alone, not wall clock A millisecond ceiling measures the runner. This repo has been bitten by that twice already — bench/callable-value-flow's widening_overhead failed at 2.07 and 1.975 against a 1.9 budget on a shared runner while the code was correct, both times on a sub-11ms measurement — so the arm is dropped rather than loosened. `ms_budget` is gone from both arms and from `--check`; `min_ms` and `us_per_site` are still reported, and nothing compares them to anything. The remaining timing gate is the ratio, reshaped after bench/parse-dispatch-rounds/baselines.json: `_what` / `_triage` notes, a `_measured` block recording samples for context, and min-of-15 reps instead of 7 (bench/import-target measured N=5 tripping its own budget about one run in twenty, N=15 holding). Budget 1.6 — 1.40x the measured maximum over 12 runs (0.907 .. 1.146), the ~1.5x headroom its siblings use on ratios. Re-verified rather than re-quoted, and the previous note was wrong: replacing QualifiedNameIndex.get with a full scan moves linear_factor from ~1.0 to 2.07, not the ~3.7 recorded. Only 1 of the 17 value-ref sites per module reaches that workspace-wide fallback. 1.6 sits clear of both ends. The note also records the trap the first attempt fell into: patching gitnexus-shared/src changes nothing, because the bench resolves the built package. * fix(review): settle namespace precedence on the name, before the type gate findNamespaceValueRefTarget's local lookup applied CALL_TARGET_TYPES while selecting, so a target module declaring a NON-callable under the name answered nothing and fell through to the published channel — binding a re-exported callable under a name the module's own declaration owns. findExportedDef does not do that: it returns any local def and lets its caller's type gate reject it, so findExportedDefIncludingImportedNames never reaches the imported names for a name the file declares. `x.f` and `x.f()` must not disagree about which module owns f. Not reachable through valid Zig today (a container cannot declare a name twice, and Zig is the only provider setting namespaceExportsIncludeImportedNames), which is why the regression test builds the indexes directly instead of adding a fixture — there is no valid source to write. Its second case fails with the guard removed. Three smaller corrections from the same review: - ZigBuildZonConfig.pathDeps promised the raw `.path` string; a nested package stores the normalized repo-relative value. The interface now documents both spellings and why either is safe to hand to normalizeZigDepPath. Comment only — no behaviour change. - The 'single-package repo' compatibility test ran against zig-idioms, which declares libs/geo as a path dep and that directory has its own build.zig, so the walk finds two packages and the name was a claim rather than a check. It now runs against libs/geo itself and asserts the package list is exactly ['']; a second test pins the multi-package case, including that a file inside libs/geo is governed by that package and not by the root. - The lower-bound header asserted 'some callers are not traced'. callableValueReferenceBoundaries hedges when its probe could not RUN and says whether the symbol is registered is unknown, so the header claimed an omission nothing established. It now says the count may be incomplete and names no cause; the per-cause bullets underneath carry that. * feat(zig): bind a `@This()` alias to the container it names `@This()` IS the enclosing container, and `const Self = @This();` is how most Zig files say so. The container is minted under the FILE STEM and the alias bound nothing class-like: a file-level alias mints no Const at all (isZigFileThisAlias suppresses it so it cannot shadow the type for `w: *Widget`), a container-level one mints a Variable every isClassLike walk steps over. So `Self.member` resolved to nothing — not a wrong edge, no edge, and a caller list missing it is the false confidence #3399 is about. This was the PR's declared known limitation. bindZigThisAliases binds the alias name to its container definition in indexes.bindingAugmentations — the sanctioned post-finalize channel (I8), consulted only AFTER a scope's own bindings, so it can never outrank a real local declaration and can only answer where nothing answered before. Nothing is replaced or removed. No query, capture or SCHEMA_BUMP change. It runs inside populateZigRangeBindings, sharing that pass's parsed tree: a pass of its own would re-parse every Zig file on a cold tree cache. Only aliases declared directly in a container body or at file level are bound — the same set collectZigThisAliases recognizes — because a function-local alias belongs in that function's scope. Measured cold-index before/after, same command, same build: tigerbeetle (246 .zig) CALLS 17,066 -> 17,140 (+74), USES 437 -> 443, MEMBER_OF 5,131 -> 5,143; every other edge type unchanged, all 24 node-label counts identical mach (132 .zig) CALLS 7,600 -> 7,615 (+15), MEMBER_OF +1, USES flat The +74 matches a source census of 72 `Alias.member(` call sites in tigerbeetle files whose alias differs from the stem. Spot-checked end to end: src/aof.zig:636 writes `try AOF.init(io, output_path)` inside AOFType, and the edge AOFType.merge -> AOFType.init#2 is present after and absent before. Node totals move only through the derived layers (Community 524->527, Process 774->763) — no source symbol added or removed. Scale of what was dropped: 73 of ghostty's 185 `@This()` files, 93 of tigerbeetle's 94 and 8 of mach's 42 spell the alias differently from the stem, carrying 302 `Alias.member` references between them, 96 of those calls. Fixture zig-idioms/src/webapi/Widget.zig exercises both paths — a file-level `Self` and a container-level `Me` in Metrics — through a call and a registration. Five cases; three fail with the binding disabled, and the two that do not are the controls: Element.zig's stem-spelled alias must keep resolving byte-identically, and the alias name must not become resolvable from another file. * docs(review): stop claiming what the alias pass does not do Two comments asserted things the adjacent code does not support. The alias pass's call site said it ran before the payload walk "so a subject spelled through the alias resolves here too". It does not: the payload walk types subjects through findReceiverTypeBinding, which reads typeBindings plus the namespace/workspace type channels and never bindingAugmentations, where bindZigThisAliases writes. Measured on `for (Self.items) |it|` — `it` is bound neither before nor after. The pass is in that loop for the tree and nothing else, which is now what the comment says. Nor is that a gap to close by also writing a typeBinding: no container name has one, the file stem included, so a payload subject written `Type.member` resolves for no spelling at all. Giving the alias an entry would make it behave unlike the container it names. Recorded at the call site so the next reader does not re-derive it. The module-shadow test said Element.zig declares `const Gauge: u8 = 3;`. It binds `Gauge` by IMPORT, and the distinction is the point of the fixture: a local declaration would also claim the workspace qualified name `Gauge`, leaving two candidates, and the fallback refuses to guess between two — so the case the test exists for would never be reached. The comment now says which binding it is and why the other shape would be self-defeating. Comment-only: node and edge counts are byte-identical across a full re-index (52,083 / 165,261 both sides). * docs(zig): stop describing loadZigBuildConfig as root-only in the present tense It takes a `packageDir` since this branch and reads `path.join(repoRoot, packageDir, name)`; `loadZigWorkspaceIndex` is what supplies it one, per package. Two comments still described the historical root-only invocation as the function's current behaviour: - the monorepo integration-test header, flagged by review; - loadZigWorkspaceIndex's own docstring — the same sentence, in the function that calls it WITH a packageDir a few lines below, so fixing only the test copy would have left the worse of the two. Both now attribute root-only reading to the CALL (no `packageDir`), which is what the argument actually rests on: a monorepo has no root build files, so that call answers null and every bare @import goes unresolved. The trailing note about `loadImportConfigs` is reworded the same way — it calls the loader for the root package alone; the loader is not root-bound. Comment-only: node and edge counts byte-identical across a full re-index (52,083 / 165,261 both sides), and detect-changes reports the two hunks overlap no indexed symbol. * fix(zig): a written namespace handle owns its own decline `findNamespaceValueRefTarget` returning `undefined` conflated two different answers: "no namespace import named this receiver" and "the module this file named does not expose that member as a callable". Only the first should fall through to the container channel. The second did too, and `findClassBindingInScope`'s miss path answers from the WORKSPACE-wide qualified-name index — so a same-named container in a file this one never imported supplied the member the written module does not have. The owner-shadow guard does not stop it, which is the part that is not obvious: a plain `const utils = @import("utils.zig");` records a namespace IMPORT EDGE, not a module-scope binding, so the guard finds nothing bound under the name and reads the container as unshadowed. It catches `const Gauge = @import(x).MEMBER` (a real binding) and misses the handle form. Reproduced before fixing, not argued: `decoy.zig`'s `dom_utils` struct gains `onlyOnDecoy`, a callable `dom_utils.zig` does not have, and `Element.zig` registers `dom_utils.onlyOnDecoy`. That minted `JsApi -> onlyOnDecoy` — a confident USES edge into a file `Element.zig` never imports, the wrong-edge failure this PR exists to avoid, arriving through the container channel after the namespace channel said no. The channel now returns 'owned' for every outcome reached once the receiver is established as this file's unshadowed namespace handle, and the caller declines on it. A locally shadowed handle still falls through, because there the name does not mean the import at that site and the container channel's guard is the right decider. Bench fingerprint and counts unchanged; no baseline edited. * fix(zig): reject an absolute nested-package `.path` before rebasing it The nested-package branch prefixes the package directory and THEN normalizes, so an absolute `.path` stops looking absolute on the way: `packages/app/` plus `/src` is `packages/app//src`, which is relative by inspection. `normalizeZigDepPath` drops the empty segment and the dep lands on `packages/app/src` — a directory that really exists — so a dependency pointing outside the repository is fabricated into an in-repo resolution. The root package was never affected: its prefix is empty, so the value reached the check as written. `isAbsoluteZigDepPath` is now asked of the value AS WRITTEN, before any prefixing, and `normalizeZigDepPath` asks the same helper so the two cannot drift. `..` is deliberately not handled there: `../core` escapes the package but not the repo, and rebasing it is what the branch exists to do — `normalizeZigDepPath` still rejects what escapes the ROOT afterwards. Note for the reviewer: `path.posix.join(pkg, depPath)` does NOT fix this. `join('packages/app/', '/dep')` is `packages/app/dep` — it strips the leading slash too, producing the same fabricated path without rejecting anything. Measured before writing the fix. Regression test pins it through the real loader: `packages/app` declares `.escapes = .{ .path = "/src" }`, and the test fails without the guard. `impact normalizeZigDepPath` is HIGH (14 impacted, 4 direct, exact). The edit is behaviour-preserving for that function — the same two conditions moved into a named helper it calls — and its existing absolute-path suite, POSIX, Windows drive and UNC spellings included, passes unchanged. * docs(mcp): stop defining lower-bound as proof that callers were missed The CLI header was corrected in round 8; the MCP tool contract still made the assertion the CLI stopped making. `context` said lower-bound "means callers exist that this view provably does not list" and `impact` said "the walk provably missed callers" — but `callableValueReferenceBoundaries` also publishes lower-bound when its probe could not RUN, and says in its own note that whether the symbol is registered is unknown. A client following the contract would read an unanswered question as evidence of an omission. Both now define it as a FLOOR with two possible causes — the walk provably missed callers, or a probe that would have established completeness could not run — and point at `boundaries` for which. That keeps the common case exactly as strong as it was; it only stops the contract asserting the one case it cannot support. The `causes.callableValueReferences` bullet already documented the probe-failure branch, so the headline was contradicting the body. `local-backend.ts` quotes that definition to justify hedging; the quote is updated to name which half it relies on. --------- Co-authored-by: Gergő Magyar --- .github/workflows/ci-tests.yml | 12 + .../bench/value-ref-resolution/baseline.json | 37 ++ .../bench/value-ref-resolution/measure.mjs | 303 ++++++++++++++ gitnexus/src/cli/eval-server.ts | 16 +- .../src/core/ingestion/language-config.ts | 288 ++++++++++++- .../ingestion/languages/typescript/query.ts | 8 +- .../src/core/ingestion/languages/zig/query.ts | 62 ++- .../ingestion/languages/zig/range-binding.ts | 26 ++ .../ingestion/languages/zig/scope-resolver.ts | 24 +- .../languages/zig/this-alias-bindings.ts | 191 +++++++++ .../languages/zig/workspace-static-gating.ts | 12 +- .../passes/property-dispatch.ts | 316 ++++++++++++++- .../scope-resolution/pipeline/run.ts | 5 + .../scope-resolution/scope/walkers.ts | 76 ++++ .../scope-resolution/value-ref-edges.ts | 14 + gitnexus/src/mcp/local/local-backend.ts | 201 +++++++++ gitnexus/src/mcp/tools.ts | 10 +- gitnexus/src/storage/parse-cache.ts | 19 +- .../lang-resolution/zig-idioms/src/main.zig | 1 + .../zig-idioms/src/webapi/Element.zig | 224 ++++++++++ .../zig-idioms/src/webapi/Gauge.zig | 10 + .../zig-idioms/src/webapi/Ticker.zig | 13 + .../zig-idioms/src/webapi/Widget.zig | 70 ++++ .../zig-idioms/src/webapi/decoy.zig | 24 ++ .../zig-idioms/src/webapi/dom_utils.zig | 24 ++ .../zig-idioms/src/webapi/hub.zig | 10 + .../lang-resolution/zig-monorepo/README.md | 19 + .../zig-monorepo/packages/app/build.zig | 7 + .../zig-monorepo/packages/app/build.zig.zon | 19 + .../zig-monorepo/packages/app/src/main.zig | 18 + .../zig-monorepo/packages/app/src/util.zig | 8 + .../zig-monorepo/packages/core/build.zig | 5 + .../zig-monorepo/packages/core/src/root.zig | 13 + .../zig-monorepo/packages/tool/build.zig | 8 + .../zig-monorepo/packages/tool/src/core.zig | 5 + .../zig-monorepo/packages/tool/src/main.zig | 7 + .../impact-callable-value-references.test.ts | 231 +++++++++++ .../test/integration/resolvers/zig.test.ts | 383 ++++++++++++++++++ .../test/unit/incremental-parse-cache.test.ts | 19 +- .../value-ref-dispatchability.test.ts | 173 ++++++++ .../value-ref-namespace-precedence.test.ts | 170 ++++++++ .../test/unit/zig-import-resolver.test.ts | 137 +++++++ gitnexus/vitest.config.ts | 2 + 43 files changed, 3163 insertions(+), 57 deletions(-) create mode 100644 gitnexus/bench/value-ref-resolution/baseline.json create mode 100644 gitnexus/bench/value-ref-resolution/measure.mjs create mode 100644 gitnexus/src/core/ingestion/languages/zig/this-alias-bindings.ts create mode 100644 gitnexus/src/core/ingestion/scope-resolution/value-ref-edges.ts create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Element.zig create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Gauge.zig create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Ticker.zig create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Widget.zig create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/decoy.zig create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/dom_utils.zig create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/hub.zig create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-monorepo/README.md create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/build.zig create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/build.zig.zon create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/src/main.zig create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/src/util.zig create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/core/build.zig create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/core/src/root.zig create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/tool/build.zig create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/tool/src/core.zig create mode 100644 gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/tool/src/main.zig create mode 100644 gitnexus/test/integration/impact-callable-value-references.test.ts create mode 100644 gitnexus/test/unit/scope-resolution/value-ref-dispatchability.test.ts create mode 100644 gitnexus/test/unit/scope-resolution/value-ref-namespace-precedence.test.ts diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index 01cfa24b6..b19785cfa 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -774,6 +774,18 @@ jobs: run: node --import tsx bench/objective-c-resolution/measure.mjs --check working-directory: gitnexus + - name: Callable-value reference resolution guards (#3399) + if: ${{ !cancelled() }} + # Build-free: pins the resolved-target SET of `resolveValueRefTarget` + # (exact site/resolved/declined counts plus an order-independent + # fingerprint) and asserts its per-site cost stays independent of + # workspace size across a 4x file-count step. The pass resolves a + # qualified receiver through `scopes.qualifiedNames`, a workspace-wide + # index: keyed it is O(1) per site, scanned it is O(files) — a + # regression a fixture cannot see and a 257-file binding table can. + run: node --import tsx bench/value-ref-resolution/measure.mjs --check + working-directory: gitnexus + - name: CFG construction time / disk / memory guards (#2081 M1) if: ${{ !cancelled() }} # Build-free: asserts collectFunctionCfgs output is unchanged diff --git a/gitnexus/bench/value-ref-resolution/baseline.json b/gitnexus/bench/value-ref-resolution/baseline.json new file mode 100644 index 000000000..7b23bcae5 --- /dev/null +++ b/gitnexus/bench/value-ref-resolution/baseline.json @@ -0,0 +1,37 @@ +{ + "_what": "Baselines for bench/value-ref-resolution/measure.mjs --check (#3399). Guards the resolved-target SET of `resolveValueRefTarget` and the per-site cost of its four channels across a 4x file-count step, over a synthetic Zig corpus whose shape is fixed in measure.mjs. Per module: 12 CONTAINER registrations (`ElementN.getJ`), 1 NAMESPACE (`dom_utils.compare`), 1 HUB (`hub.compare`), 1 BARE (`register(onTick)`), and 2 declines (a non-callable namespace member `dom_utils.DEFAULT_NS`, a non-callable bare argument `Bridge(ElementN)`) — 17 sites, 15 resolved.", + + "_triage": "READ THIS BEFORE RE-RUNNING. modules, files, value_ref_sites, resolved, declined and fingerprint are DETERMINISTIC: a re-run never changes them, and none may be re-baselined to make CI green — drift means the resolved target set moved, which is a behaviour change to explain. linear_scaling_budget is the only timing arm; runner contention dominates it, so re-run alone on an idle machine and read `reps` in the report before investigating. If exactly one arm fails and it is that one, suspect the machine.", + + "small": { + "modules": 80, + "files": 240, + "value_ref_sites": 1360, + "resolved": 1200, + "declined": 160, + "fingerprint": "4ca2933e25cfa9c84518fca6608c9a6f318f81a1c5bad63cb2f09b7394bfb792" + }, + "large": { + "modules": 320, + "files": 960, + "value_ref_sites": 5440, + "resolved": 4800, + "declined": 640, + "fingerprint": "249266f180388e9f1240a7e81f1141afbc51ef25d5c4b3286a681992dab69e61" + }, + + "linear_scaling_budget": 1.6, + "_linear_scaling_note": "(t_large/t_small)/(320/80) over the per-site resolution loop; ~1.0 is linear. A RATIO rather than a millisecond ceiling, and there is deliberately no ms gate at all: wall-clock measures the runner, and this repo has been bitten twice by a fixed budget (bench/callable-value-flow's widening_overhead failed at 2.07 and 1.975 against 1.9 on a shared runner while the code was correct, both on a sub-11ms measurement). Same reasoning, same shape as bench/parse-dispatch-rounds' pack_scaling_budget. Budget is 1.6 — 1.40x the measured maximum, matching the ~1.5x its siblings use on ratios. What it catches: the four channels each consult a wider index than the last, and channel 4 (CONTAINER) reaches `scopes.qualifiedNames`, a WORKSPACE-WIDE index — keyed it is O(1) per site, scanned it is O(names) per site. Verified load-bearing rather than assumed: replacing `QualifiedNameIndex.get` with a full scan (in gitnexus-shared/dist — the bench resolves the built package, so patching src changes nothing) takes the factor from ~1.0 to 2.07, well clear of this budget. Only 1 of the 17 sites per module reaches that fallback (the `dom_utils.DEFAULT_NS` decline, whose module receiver is not class-like), which is why the signal is 2.07 and not the ~4 a per-site scan on every site would give.", + + "_measured": { + "linear_factor": 1.146, + "linear_factor_samples": [ + 0.907, 0.969, 0.988, 1.03, 1.043, 1.048, 1.05, 1.051, 1.052, 1.107, 1.123, 1.146 + ], + "small_ms": 2.078, + "large_ms_4x": 9.197, + "us_per_site": "1.36-1.53 small / 1.37-1.69 large", + "reps": 15 + }, + "_measured_note": "Maxima over 12 runs on a box that was NOT idle, so the spread is an upper bound on real noise. small_ms / large_ms_4x / us_per_site are recorded for context only — NOTHING gates on them, because an absolute millisecond is exactly the gate this file avoids. `us_per_site` staying flat between the two arms is the same property linear_scaling_budget gates, read directly." +} diff --git a/gitnexus/bench/value-ref-resolution/measure.mjs b/gitnexus/bench/value-ref-resolution/measure.mjs new file mode 100644 index 000000000..a1c14f448 --- /dev/null +++ b/gitnexus/bench/value-ref-resolution/measure.mjs @@ -0,0 +1,303 @@ +#!/usr/bin/env node +/** + * Build-free scaling and correctness guard for callable-value reference + * resolution (#3399). + * + * WHAT IS GUARDED. `resolveValueRefTarget` is the per-site half of + * `emitPropertyDispatchCalls`: for every `value-ref` reference site it names the + * callable the source handed over as a value. #3399 replaced a single lexical + * walk with four channels, and each one reaches for a wider index than the last: + * + * 1. BARE `register(onTick)` — `findCallableBindingInScope` + * 2. NAMESPACE `bridge.accessor(utils.compare, …)` — the file's own + * namespace `@import` edges, then the target's module scope + * 3. HUB `bridge.accessor(hub.compare, …)` — the same, through the + * finalized/augmented channel a re-export publishes into + * 4. CONTAINER `bridge.accessor(Element.getNamespaceUri, …)` — + * `findClassBindingInScope`, whose miss path falls back to + * `scopes.qualifiedNames`, a WORKSPACE-WIDE index + * + * Channel 4 is why this bench exists. A workspace-wide index consulted per site + * is linear only while the lookup is keyed; make it a scan — or make any of the + * three guards around it (`isOwnerNameShadowedBySomethingElse`, + * `isNamespaceNameShadowed`, `findOwnedMember`) walk a collection that grows + * with the repo — and a registration table that costs O(sites) today costs + * O(sites x files) tomorrow. That regression is invisible on a fixture and + * expensive on lightpanda-io/browser, where `bridge.{accessor,function,…}` + * appears 2,047 times across 257 files. + * + * HOW. Two corpora of identical shape, 4x apart in file count, and the per-site + * resolution loop is the ONLY thing timed — extraction, ownership reconciliation + * and finalize are setup. `linear_factor` is `(t_large/t_small) / (N_large/N_small)`: + * ~1.0 linear, ~4.x quadratic on this 4x step. + * + * A RATIO IS THE ONLY TIMING GATE — no millisecond ceiling, deliberately. + * `min_ms` and `us_per_site` are printed for context and nothing compares them + * to anything: a wall-clock budget measures the runner, and this repo has been + * bitten by that twice already (`bench/callable-value-flow`'s `widening_overhead` + * failed at 2.07 and 1.975 against a 1.9 budget on a shared runner while the + * code was correct, both times on a sub-11ms measurement). Dividing the large + * arm by the small one divides the machine out, which is what + * `bench/parse-dispatch-rounds` settled on for the same reason. + * + * A timing gate alone would be satisfied by a fast wrong answer, so the + * correctness half is exact and comes first: the site/resolved/declined counts + * per arm, plus an order-independent sha256 over every (site -> resolved target) + * pair. The fingerprint is a CORRECTNESS gate — drift means the resolved target + * set moved, which is a behaviour change to be explained, never re-baselined to + * make CI green. + * + * WHY A ZIG CORPUS for a language-neutral pass. Zig is the only language whose + * provider sets `namespaceExportsIncludeImportedNames`, so it is the only one + * that can exercise channel 3 at all; and the file-as-struct idiom puts channels + * 2 and 4 in one file, which is the shape #3399 was filed over. The corpus also + * carries two DECLINE controls — a non-callable namespace member and a + * non-callable bare argument — so a change that widened the callable gate would + * move `declined` rather than hiding inside the timing. + * + * Container naming is load-bearing in the corpus: a Zig file-as-struct is minted + * under the FILE STEM, so `ElementN.zig` must write `const ElementN = @This();` + * and register `ElementN.getJ`. Spelling the alias `Element` instead is the + * documented `@This()`-alias limitation, every channel-4 site declines, and the + * bench would time a corpus that resolves nothing. + * + * Usage: + * node --import tsx bench/value-ref-resolution/measure.mjs + * node --import tsx bench/value-ref-resolution/measure.mjs --check + */ +import { createHash } from 'node:crypto'; +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { performance } from 'node:perf_hooks'; + +import { extractParsedFile } from '../../src/core/ingestion/scope-extractor-bridge.ts'; +import { finalizeScopeModel } from '../../src/core/ingestion/finalize-orchestrator.ts'; +import { createSemanticModel } from '../../src/core/ingestion/model/semantic-model.ts'; +import { reconcileOwnership } from '../../src/core/ingestion/scope-resolution/pipeline/reconcile-ownership.ts'; +import { resolveValueRefTarget } from '../../src/core/ingestion/scope-resolution/passes/property-dispatch.ts'; +import { zigScopeResolver } from '../../src/core/ingestion/languages/zig/scope-resolver.ts'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const SMALL_MODULES = 80; +const LARGE_MODULES = 320; +/** Registrations per module through the CONTAINER channel. */ +const ACCESSORS_PER_MODULE = 12; +/** + * Min-of-N, and N is 15 rather than a handful: `bench/import-target` measured + * N=5 tripping its own budget about one run in twenty while N=15 held every + * language inside a 1.13-1.26x swing, and `bench/parse-dispatch-rounds` uses 15 + * on the same grounds. The whole run is ~5 s, so the reps are nearly free. + */ +const REPS = 15; + +/** + * One module = three files, mirroring `test/fixtures/lang-resolution/zig-idioms/ + * src/webapi/`: a namespace-only helper, a hub that re-exports one of its + * members and declares nothing, and a file-as-struct carrying the binding table. + */ +function moduleFiles(i) { + const accessors = Array.from( + { length: ACCESSORS_PER_MODULE }, + (_, j) => `pub fn get${j}(self: *Element${i}) u8 { return self._n; }`, + ).join('\n'); + const registrations = Array.from( + { length: ACCESSORS_PER_MODULE }, + (_, j) => ` pub const a${j} = bridge.accessor(Element${i}.get${j}, null, .{});`, + ).join('\n'); + return [ + { + path: `src/dom_utils${i}.zig`, + content: `pub const DEFAULT_NS: u8 = 7;\npub fn compare(a: u8, b: u8) u8 { return if (a > b) a else b; }\n`, + }, + { + path: `src/hub${i}.zig`, + content: `pub const compare = @import("dom_utils${i}.zig").compare;\n`, + }, + { + path: `src/Element${i}.zig`, + content: `const Element${i} = @This(); +const dom_utils = @import("dom_utils${i}.zig"); +const hub = @import("hub${i}.zig"); + +_n: u8 = 0, + +${accessors} + +fn onTick(self: *Element${i}) u8 { return self._n; } + +pub const JsApi = struct { + pub const bridge = Bridge(Element${i}); +${registrations} + pub const comparator = bridge.accessor(dom_utils.compare, null, .{}); + pub const hubbed = bridge.accessor(hub.compare, null, .{}); + pub const defaultNs = bridge.accessor(dom_utils.DEFAULT_NS, null, .{}); +}; + +pub fn boot() void { register(onTick); } + +pub fn register(comptime f: anytype) void { _ = f; } + +fn Bridge(comptime T: type) type { + _ = T; + return struct { + pub fn accessor(comptime g: anytype, comptime s: anytype, comptime o: anytype) u8 { + _ = g; + _ = s; + _ = o; + return 0; + } + }; +} +`, + }, + ]; +} + +/** + * Everything `resolveValueRefTarget` reads, built the way the pipeline builds it + * (`runScopeResolution` phases 1-2): real extraction through the Zig provider, + * `populateOwners`, `reconcileOwnership` into the SemanticModel, then finalize. + * Hand-assembling the indexes instead would pin this file's idea of their shape + * rather than the code's. + */ +function buildCorpus(modules) { + const parsedFiles = []; + for (let i = 0; i < modules; i++) { + for (const file of moduleFiles(i)) { + const parsed = extractParsedFile(zigScopeResolver.languageProvider, file.content, file.path); + if (parsed === undefined) { + throw new Error( + `scope extraction failed for ${file.path} — the vendored tree-sitter-zig ` + + `grammar is unavailable on this host, so this bench cannot run`, + ); + } + zigScopeResolver.populateOwners(parsed); + parsedFiles.push(parsed); + } + } + const model = createSemanticModel(); + reconcileOwnership(parsedFiles, model); + const allFilePaths = new Set(parsedFiles.map((p) => p.filePath)); + const scopes = finalizeScopeModel(parsedFiles, { + hooks: { + resolveImportTarget: (raw, from) => + zigScopeResolver.resolveImportTarget(raw, from, allFilePaths), + mergeBindings: (existing, incoming, scopeId) => + zigScopeResolver.mergeBindings(existing, incoming, scopeId), + expandsWildcardTo: (scope, files) => zigScopeResolver.expandsWildcardTo(scope, files), + }, + }); + return { parsedFiles, scopes, model }; +} + +/** The timed loop: every `value-ref` site, resolved exactly as the pass does. */ +function resolveAll({ parsedFiles, scopes, model }, pairs) { + let sites = 0; + let resolved = 0; + for (const parsed of parsedFiles) { + for (const site of parsed.referenceSites) { + if (site.kind !== 'value-ref') continue; + sites++; + const def = resolveValueRefTarget( + site, + parsed.filePath, + scopes, + model, + // The provider hook the pass is handed in `run.ts`; Zig sets it. + zigScopeResolver.namespaceExportsIncludeImportedNames === true, + ); + if (def === undefined) continue; + resolved++; + pairs?.push( + `${parsed.filePath}:${site.atRange.startLine}:${site.atRange.startCol}->${def.nodeId}`, + ); + } + } + return { sites, resolved }; +} + +function measure(modules) { + const corpus = buildCorpus(modules); + const pairs = []; + const counts = resolveAll(corpus, pairs); + let bestMs = Infinity; + for (let i = 0; i < REPS; i++) { + const start = performance.now(); + resolveAll(corpus, undefined); + bestMs = Math.min(bestMs, performance.now() - start); + } + // Order-independent: the walk order is an implementation detail, the resolved + // SET is the behaviour. + pairs.sort(); + return { + modules, + files: corpus.parsedFiles.length, + value_ref_sites: counts.sites, + resolved: counts.resolved, + declined: counts.sites - counts.resolved, + fingerprint: createHash('sha256').update(pairs.join('\n')).digest('hex'), + min_ms: Number(bestMs.toFixed(3)), + us_per_site: Number(((bestMs * 1000) / Math.max(counts.sites, 1)).toFixed(3)), + }; +} + +const report = { small: measure(SMALL_MODULES), large: measure(LARGE_MODULES) }; +report.reps = REPS; +report.workload_ratio = LARGE_MODULES / SMALL_MODULES; +report.scaling_ratio = Number( + (report.large.min_ms / Math.max(report.small.min_ms, 0.001)).toFixed(3), +); +report.linear_factor = Number((report.scaling_ratio / report.workload_ratio).toFixed(3)); + +if (!process.argv.includes('--check')) { + console.log(JSON.stringify(report, null, 2)); + process.exit(0); +} + +const baseline = JSON.parse(readFileSync(join(HERE, 'baseline.json'), 'utf8')); +const failures = []; +const requirePositiveNumber = (path, value) => { + if (typeof value !== 'number' || !Number.isFinite(value) || value <= 0) { + failures.push(`${path}: expected a finite positive number, got ${JSON.stringify(value)}`); + return false; + } + return true; +}; +for (const arm of ['small', 'large']) { + // Correctness first: counts AND the resolved-target set. + for (const key of [ + 'modules', + 'files', + 'value_ref_sites', + 'resolved', + 'declined', + 'fingerprint', + ]) { + if (report[arm][key] !== baseline[arm][key]) { + failures.push( + `${arm}.${key}: expected ${JSON.stringify(baseline[arm][key])}, got ${JSON.stringify(report[arm][key])}`, + ); + } + } + // `min_ms` / `us_per_site` are reported, never gated — see the header. +} +if ( + requirePositiveNumber('linear_scaling_budget', baseline.linear_scaling_budget) && + report.linear_factor > baseline.linear_scaling_budget +) { + failures.push( + `linear_factor ${report.linear_factor} exceeds budget ${baseline.linear_scaling_budget} ` + + `(runtime ${report.scaling_ratio}x for ${report.workload_ratio}x work; ` + + `~1.0 is linear). Re-run alone on an idle machine before investigating — ` + + `this is the only arm a busy runner can move.`, + ); +} + +console.log(JSON.stringify(report, null, 2)); +if (failures.length > 0) { + console.error('[value-ref-resolution --check] FAIL'); + for (const failure of failures) console.error(` - ${failure}`); + process.exit(1); +} +console.log('[value-ref-resolution --check] PASS'); diff --git a/gitnexus/src/cli/eval-server.ts b/gitnexus/src/cli/eval-server.ts index b2f656c39..fc6af8e63 100644 --- a/gitnexus/src/cli/eval-server.ts +++ b/gitnexus/src/cli/eval-server.ts @@ -609,10 +609,20 @@ export function formatImpactResult(result: any): string { } // #1858 — an interface / indirection boundary on the path makes this a lower // bound; surface it so the count is not read as exhaustive. + // + // The header names no cause AND asserts no omitted caller, because it cannot + // know either. DI / dynamic dispatch was the only producer of `lower-bound` + // when this was written; #3399 added callables named in VALUE position (a + // registration table, a callback argument), and one of its producers is a + // probe that could not RUN — `callableValueReferenceBoundaries` hedges on a + // failed query and says in so many words that whether the symbol is + // registered is unknown. A header claiming "some callers are not traced" + // would there assert an omission nothing established, and would contradict + // the bullet printed directly under it. The bullets carry the cause — they + // are generated per-cause by `computeEpistemicBoundary` — so the header only + // has to say the count is a floor. if (result.epistemic === 'lower-bound') { - lines.push( - '⚠️ Lower bound — unresolved indirection on the path (callers binding via DI / dynamic dispatch are not traced; actual impact may be higher):', - ); + lines.push('⚠️ Lower bound — impact may be incomplete and actual impact may be higher:'); for (const b of result.boundaries || []) lines.push(` • ${b}`); } pushCallgraphRiskLines(lines, result); diff --git a/gitnexus/src/core/ingestion/language-config.ts b/gitnexus/src/core/ingestion/language-config.ts index 5d8fc9061..390e6a32f 100644 --- a/gitnexus/src/core/ingestion/language-config.ts +++ b/gitnexus/src/core/ingestion/language-config.ts @@ -5,6 +5,7 @@ import path from 'path'; import type { CsharpStructureLineScanner } from './languages/csharp/namespace-siblings.js'; import { isDev } from './utils/env.js'; +import { isHardcodedIgnoredDirectoryAtPath } from '../../config/ignore-service.js'; import { mapConcurrent } from '../../lib/utils.js'; import { logger } from '../logger.js'; @@ -182,13 +183,24 @@ export interface SwiftPackageConfig { /** Zig package config parsed from build.zig.zon and the root build.zig */ export interface ZigBuildZonConfig { /** - * Map of dependency name -> the raw `.path = "..."` value, exactly as - * written in build.zig.zon (relative to the repo root, and possibly - * escaping it: `../local_dep`). Consumers normalize — see - * `normalizeZigDepPath` below, which rejects absolute - * and repo-escaping values. `.url`-based deps cannot be resolved to a - * repo-local file (they unpack into a build cache outside the repo) and so - * are not included here. + * Map of dependency name -> the dep's directory, in one of two spellings + * depending on which package this config describes: + * + * - ROOT package (`pkg === ''`): the raw `.path = "..."` value, exactly as + * written in build.zig.zon (relative to the repo root, and possibly + * escaping it: `../local_dep`). This is what `parseZigBuildZon` promises + * and what its tests pin. + * - NESTED package: repo-relative and already normalized, because a nested + * package's `.path` is written relative to ITS directory and means + * nothing against the repo-relative keys consumers match on + * (`packages/app`'s `../core` is stored as `packages/core`). A dep + * escaping the REPO root is dropped rather than stored. + * + * Either spelling is safe to hand to `normalizeZigDepPath` below — it rejects + * absolute and repo-escaping values and is idempotent on an already + * normalized one, which is what `resolveZigImportInternal` relies on. + * `.url`-based deps cannot be resolved to a repo-local file (they unpack into + * a build cache outside the repo) and so are not included here. */ pathDeps: Map; /** @@ -228,6 +240,29 @@ export interface ZigBuildZonConfig { buildModules?: readonly ZigBuildModule[]; } +/** + * One Zig build package: the directory whose `build.zig` / `build.zig.zon` + * declare the config, and that config with every path REPO-relative. + * + * A Zig module's import table is declared by the `build.zig` of the package it + * belongs to, so a repo holding several packages holds several import tables — + * the same shape a TypeScript monorepo has with a `tsconfig.json` per package. + */ +export interface ZigPackageScope { + /** Repo-relative directory the package governs (`''` for the repo root). */ + readonly dir: string; + readonly config: ZigBuildZonConfig; +} + +/** + * Every Zig build package in the repo, indexed so the nearest one to a file + * wins — the `TsconfigIndex` analogue, and for the same reason. + */ +export interface ZigWorkspaceIndex { + /** Deepest-first, so the first `dir` that prefixes a file path governs it. */ + readonly packages: readonly ZigPackageScope[]; +} + /** One build module of the root `build.zig` — see `ZigBuildZonConfig.buildModules`. */ export interface ZigBuildModule { /** The `addModule("", …)` name; absent for `createModule` bindings @@ -646,10 +681,21 @@ export async function loadSwiftPackageConfig(repoRoot: string): Promise { +export async function loadZigBuildConfig( + repoRoot: string, + packageDir = '', +): Promise { + // Every path this function returns is REPO-relative, because that is the + // keyspace `allFilePaths` uses. The parsers below answer package-relative, so + // a nested package rebases them through `inPackage`. For the root package + // (`packageDir === ''`) the prefix is empty and every value is byte-identical + // to what this function returned before nested packages existed. + const pkg = packageDir === '' ? '' : `${packageDir}/`; + const inPackage = (relToPackage: string): string => `${pkg}${relToPackage}`; + const packageFile = (name: string): string => path.join(repoRoot, packageDir, name); let config: ZigBuildZonConfig | null = null; try { - const raw = await fs.readFile(path.join(repoRoot, 'build.zig.zon'), 'utf-8'); + const raw = await fs.readFile(packageFile('build.zig.zon'), 'utf-8'); config = parseZigBuildZon(raw); } catch { // No zon (or unreadable): the root build.zig may still declare modules. @@ -661,9 +707,11 @@ export async function loadZigBuildConfig(repoRoot: string): Promise | undefined; let rootBuildZig: string | null = null; try { - rootBuildZig = await fs.readFile(path.join(repoRoot, 'build.zig'), 'utf-8'); + rootBuildZig = await fs.readFile(packageFile('build.zig'), 'utf-8'); const parsed = parseZigRootModules(rootBuildZig); - if (parsed.size > 0) rootModules = parsed; + if (parsed.size > 0) { + rootModules = new Map(Array.from(parsed, ([name, root]) => [name, inPackage(root)])); + } } catch { // No root build.zig — nothing to declare. } @@ -671,7 +719,7 @@ export async function loadZigBuildConfig(repoRoot: string): Promise>(); + // A nested package's `.path` values are written relative to ITS directory, so + // they are rebased here and stored repo-relative; `resolveZigImportInternal` + // then reads them through the same `normalizeZigDepPath`, which is idempotent + // on an already-normalized value. A dep that escapes the REPO root (not merely + // the package) resolves to nothing and is dropped. The root package keeps its + // raw spelling, which is what `parseZigBuildZon` promises and its tests pin. + const pathDeps = pkg === '' ? config.pathDeps : new Map(); for (const [depName, depPath] of config.pathDeps) { - const rel = normalizeZigDepPath(depPath); + // Asked of the value AS WRITTEN, before the package prefix goes on: an + // absolute `.path` points outside the repository whichever package declared + // it, and prefixing hides that from `normalizeZigDepPath`. See + // `isAbsoluteZigDepPath`. + if (isAbsoluteZigDepPath(depPath)) continue; + const rel = normalizeZigDepPath(`${pkg}${depPath}`); if (rel === null) continue; + if (pkg !== '') pathDeps.set(depName, rel); let buildZig: string; try { buildZig = await fs.readFile(path.join(repoRoot, rel, 'build.zig'), 'utf-8'); @@ -706,15 +767,177 @@ export async function loadZigBuildConfig(repoRoot: string): Promise 0) depModules.set(depName, named); } - const buildModules = rootBuildZig === null ? [] : parseZigBuildModules(rootBuildZig, depModules); + const buildModules = + rootBuildZig === null + ? [] + : rebaseZigBuildModules( + parseZigBuildModules(rootBuildZig, depModules), + inPackage, + depModules, + ); return { ...config, + pathDeps, ...(moduleRoots.size > 0 ? { moduleRoots } : {}), ...(rootModules ? { rootModules } : {}), ...(buildModules.length > 0 ? { buildModules } : {}), }; } +/** + * Rebase a package's own build modules to repo-relative paths. + * + * `parseZigBuildModules` answers package-relative for everything it read out of + * the `build.zig` it was handed, with one exception: an alias resolved through + * `depModules` (`addImport("api", dep.module("core"))`) is already repo-relative, + * because `depModules` was built that way. Prefixing that a second time would + * point the alias at a path no file has. The already-repo-relative values are + * therefore identified by membership in `depModules`, not guessed at from their + * shape. + */ +function rebaseZigBuildModules( + modules: readonly ZigBuildModule[], + inPackage: (relToPackage: string) => string, + depModules?: ReadonlyMap>, +): ZigBuildModule[] { + if (inPackage('') === '') return [...modules]; + const fromDep = new Set(); + for (const named of depModules?.values() ?? []) + for (const root of named.values()) fromDep.add(root); + return modules.map((mod) => ({ + ...(mod.name !== undefined ? { name: mod.name } : {}), + root: inPackage(mod.root), + imports: new Map( + Array.from(mod.imports, ([alias, root]) => [ + alias, + fromDep.has(root) ? root : inPackage(root), + ]), + ), + })); +} + +/** Bounds for the package walk, mirroring the tsconfig scan. */ +const ZIG_SCAN_MAX_DIRS = 20_000; +const ZIG_SCAN_MAX_DEPTH = 24; + +/** + * The Zig build package governing `filePath` — the nearest one at or above it. + * + * A Zig module's import table is declared by the `build.zig` of the package the + * file belongs to, so the nearest enclosing package is the faithful reading of + * `@import("name")` at that site, exactly as `tsconfigFor` reads a non-relative + * specifier against the nearest enclosing project. + * + * There is deliberately NO fall-through to an enclosing package when the nearest + * one does not bind the name. Falling through is how a vendored dependency's + * `@import("config")` silently resolved to the outer repo's `config` module — + * the same failure `loadTsconfigIndex` documents for a package whose own + * tsconfig declares no `baseUrl`, and the same failure the per-module import + * tables in `resolveZigImportInternal` already exist to prevent one level down. + */ +export function zigPackageFor( + index: ZigWorkspaceIndex | null | undefined, + filePath: string, +): ZigBuildZonConfig | null { + if (index === null || index === undefined) return null; + for (const scope of index.packages) { + if (scope.dir === '') return scope.config; + if (filePath.startsWith(`${scope.dir}/`)) return scope.config; + } + return null; +} + +/** + * Load every Zig build package in the repo, nearest-first. + * + * Called with no `packageDir` — which is how every call site read it before + * this function existed — `loadZigBuildConfig` reads the ROOT `build.zig` / + * `build.zig.zon` and nothing else. That is the whole configuration of a + * single-package repo and none of the configuration of a monorepo: a repo + * laying its packages out as `packages//build.zig` has no root build + * files at all, so the loader answers `null` and EVERY bare + * `@import("")` in it goes unresolved — cross-file resolution silently + * degrades to relative imports only. Measured on a two-package fixture: + * `config = null`, `@import("core")` → `null`. + * + * The loader itself is not root-bound any more: this function is what supplies + * it a `packageDir`, one per package below. + * + * So the packages are discovered the way tsconfigs are (`findTsconfigFiles`): + * one bounded breadth-first walk that skips the hardcoded ignore set, then + * deepest-first ordering so `zigPackageFor` can take the first match. + * + * Called from `ScopeResolver.loadResolutionConfig`, which the orchestrator runs + * once per LANGUAGE workspace pass — so the walk happens only for repos that + * actually contain Zig. `loadImportConfigs`, which runs unconditionally for + * every repo, keeps calling `loadZigBuildConfig` for the root package alone; + * that is the same split TypeScript already has between the cheap + * `loadTsconfigPaths` and the repo-walking `loadTsconfigIndex`. + */ +export async function loadZigWorkspaceIndex(repoRoot: string): Promise { + const dirs = await findZigPackageDirs(repoRoot); + if (dirs.length === 0) return null; + const packages: ZigPackageScope[] = []; + for (const dir of dirs) { + const config = await loadZigBuildConfig(repoRoot, dir); + // A `build.zig` that declares no module and no path dep contributes nothing + // a lookup could answer with. Keeping it as an empty scope would be worse + // than dropping it: it would shadow an enclosing package that DOES declare + // the name, and answer nothing in its place. + if (config !== null) packages.push({ dir, config }); + } + if (packages.length === 0) return null; + // Deepest first, so `zigPackageFor` takes the most specific package rather + // than whichever the walk reached first. + packages.sort((a, b) => b.dir.length - a.dir.length || a.dir.localeCompare(b.dir)); + return { packages }; +} + +/** Repo-relative directories holding a `build.zig` and/or a `build.zig.zon`. */ +async function findZigPackageDirs(repoRoot: string): Promise { + const found: string[] = []; + const queue: { dir: string; depth: number }[] = [{ dir: repoRoot, depth: 0 }]; + // A HEAD INDEX rather than `queue.shift()`. The queue is pushed to while it is + // drained, which keeps the array in a mode where `shift()` memmoves the whole + // remainder instead of taking V8's left-trimming fast path — so the walk is + // quadratic in the frontier, and `ZIG_SCAN_MAX_DIRS` is the bound on how bad + // that gets. Measured at that bound (20,000 dequeues): 53 ms at fan-out 4 and + // 81 ms at fan-out 20, against 0.8 ms here — 66-106x, paid before any config + // is read. Memory is unchanged: entries were already retained by the pushes, + // `shift()` only dropped the head. + let queueHead = 0; + let dirsScanned = 0; + + while (queueHead < queue.length && dirsScanned < ZIG_SCAN_MAX_DIRS) { + const { dir, depth } = queue[queueHead++]!; + dirsScanned++; + let entries: import('fs').Dirent[]; + try { + entries = await fs.readdir(dir, { withFileTypes: true }); + } catch { + continue; + } + let isPackage = false; + for (const entry of entries) { + if (entry.isDirectory()) { + const childDir = path.join(dir, entry.name); + if (isHardcodedIgnoredDirectoryAtPath(repoRoot, childDir)) continue; + if (depth < ZIG_SCAN_MAX_DEPTH) queue.push({ dir: childDir, depth: depth + 1 }); + continue; + } + if (!entry.isFile()) continue; + // Either marker declares a package: a `build.zig` with no zon still names + // modules, and a zon with no build.zig still names path deps. + if (entry.name === 'build.zig' || entry.name === 'build.zig.zon') isPackage = true; + } + if (isPackage) { + const rel = path.relative(repoRoot, dir).split(path.sep).join('/'); + found.push(rel === '.' || rel === '' ? '' : rel); + } + } + return found; +} + /** * Normalize a `.path` value from build.zig.zon into a repo-relative form. * Returns null for paths that escape the repo root (start with `..`) or @@ -722,13 +945,38 @@ export async function loadZigBuildConfig(repoRoot: string): Promise...\`) emit; the closing tag is intentionally NOT captured — * each JSX element should emit exactly one CALLS edge per use site. */ -const TSX_JSX_QUERY_SUFFIX = ` +/** + * Exported alongside `TYPESCRIPT_SCOPE_QUERY` so + * `value-ref-dispatchability.test.ts` checks the whole query a `.tsx` file is + * analyzed with. Checking the base alone would miss a `value-ref` rule added + * here. Not part of the provider surface — use `getTsScopeQuery`. + */ +export const TSX_JSX_QUERY_SUFFIX = ` ;; ((jsx_self_closing_element name: (identifier) @reference.name) @reference.call.free diff --git a/gitnexus/src/core/ingestion/languages/zig/query.ts b/gitnexus/src/core/ingestion/languages/zig/query.ts index 9aa8e106a..0b4540417 100644 --- a/gitnexus/src/core/ingestion/languages/zig/query.ts +++ b/gitnexus/src/core/ingestion/languages/zig/query.ts @@ -20,7 +20,13 @@ import { requireVendoredGrammar } from '../../../tree-sitter/vendored-grammars.j * container and import bindings — `emitZigScopeCaptures` filters those * groups out so a name binds exactly once. */ -const ZIG_SCOPE_QUERY = ` +/** + * Exported for `value-ref-dispatchability.test.ts`, which reads every language's + * scope query to enforce the keyed/unkeyed partition that + * `callableValueReferenceBoundaries`' dispatch exclusion depends on. Not part of + * the provider surface — nothing else should import it. + */ +export const ZIG_SCOPE_QUERY = ` ;; Scopes (source_file) @scope.module (struct_declaration) @scope.class @@ -359,6 +365,60 @@ const ZIG_SCOPE_QUERY = ` "const" . (identifier) @type-binding.name (call_expression) @type-binding.type .) @type-binding.alias +;; References — VALUE positions (#3399): a callable named where a value is +;; expected rather than where a callee is. Zig's JS bridge is built entirely +;; out of this shape — +;; +;; pub const namespaceURI = bridge.accessor(Element.getNamespaceUri, null, .{}); +;; +;; — 2,047 such declarations across 257 files in lightpanda-io/browser, the +;; project's whole JS↔Zig surface, and NONE of them reached the graph: Zig +;; emitted no \`value-ref\` capture at all, so a public DOM accessor's only +;; recorded callers were the two internal ones and \`impact\` called that \`exact\`. +;; +;; These become reference-class USES edges through the existing +;; \`mapReferenceKindToEdgeType\` mapping — a registration is not an invocation +;; (Kythe \`ref\` vs \`ref/call\`; Joern \`METHOD_REF\`) — resolved by the +;; property-dispatch pass, which keeps ONLY callable targets. That callable gate +;; is what makes these deliberately broad rules safe: \`js.Bridge(Element)\` and +;; \`register(count)\` match too, and emit nothing, exactly as TypeScript's +;; \`{ port: DEFAULT_PORT }\` does. +;; +;; No \`@reference.property-key\` is attached: Zig has no object-literal key to +;; dispatch through, so these register a reference and never synthesize CALLS. +;; The terminal invoke — \`Accessor.init\` → a struct field → \`Factory.zig\`'s +;; \`inline for\`/\`@typeInfo\` → \`@call(.auto, func, args)\` — needs comptime +;; evaluation and is deliberately NOT modelled; \`impact\` reports the shortfall +;; as \`epistemic: "lower-bound"\` instead of pretending to certainty. + +;; Call ARGUMENTS. In tree-sitter-zig arguments are direct children of +;; \`call_expression\`, NOT wrapped in an \`arguments\` node (only builtins have +;; one), so the callee has to be consumed explicitly by \`function:\` — without +;; that binding the same rule also matches the callee of \`foo(bar)\` and mints a +;; USES edge duplicating the call. +(call_expression + function: (_) + (identifier) @reference.name @reference.value-ref) + +;; Qualified argument — \`bridge.accessor(Element.getNamespaceUri, …)\`. The +;; RECEIVER is captured alongside the member so the site carries the owner it +;; was written with; \`@reference.name\` stays the member, which is the name the +;; scope walk resolves. +(call_expression + function: (_) + (field_expression + object: (_) @reference.receiver + member: (identifier) @reference.name) @reference.value-ref) + +;; Const binding initialiser — \`pub const defaultHandler = onReset;\`. Both +;; anchors are load-bearing: the leading \`.\` pins the bound name to the first +;; named child (see the declaration rules above), and the trailing \`.\` keeps the +;; initializer as the LAST child, so a \`const x: T = y\` annotation shape cannot +;; put the TYPE in value position. +(variable_declaration + "const" . (identifier) + (identifier) @reference.name @reference.value-ref .) + ;; References — free calls: foo(...) (call_expression function: (identifier) @reference.name) @reference.call.free diff --git a/gitnexus/src/core/ingestion/languages/zig/range-binding.ts b/gitnexus/src/core/ingestion/languages/zig/range-binding.ts index a0186242f..408386bdf 100644 --- a/gitnexus/src/core/ingestion/languages/zig/range-binding.ts +++ b/gitnexus/src/core/ingestion/languages/zig/range-binding.ts @@ -17,6 +17,14 @@ * * Runs after `propagateImportedReturnTypes`, so return bindings hoisted from * other files are visible when a subject is a call. + * + * This hook also carries `bindZigThisAliases` (`this-alias-bindings.ts`), which + * is a different job — binding `const Self = @This();` to its container — but + * needs the same thing this pass is already paying for: the file's parsed tree, + * post-finalize. Giving it a pass of its own would re-parse every Zig file in + * the repo whenever the tree cache is cold. That is the whole reason it is + * here; the two do not otherwise interact, for the reason recorded at the call + * site. */ import type { ParsedFile, Scope, ScopeId, TypeRef } from 'gitnexus-shared'; @@ -32,6 +40,7 @@ import { isClassLike, } from '../../scope-resolution/scope/walkers.js'; import { isZigKeywordDeclaration, zigUnwrapValue } from './captures.js'; +import { bindZigThisAliases } from './this-alias-bindings.js'; import { normalizeZigTypeName } from './interpret.js'; type ZigTree = ReturnType['parse']>; @@ -89,6 +98,23 @@ export function populateZigRangeBindings( } const scopes = parsed.scopes; if (scopes.length === 0) continue; + + // `const Self = @This();` — bind the alias to its container. It sits in this + // loop for ONE reason: the tree. A pass of its own would re-parse every Zig + // file in the repo whenever the tree cache is cold. + // + // Its position relative to the payload walk below is NOT load-bearing, and + // saying otherwise would be wrong in a checkable way: the payload walk types + // a subject through `findReceiverTypeBinding`, which reads `typeBindings` + // and the namespace/workspace type channels — never `bindingAugmentations`, + // where this writes. Measured on `for (Self.items) |it|`: `it` is bound + // neither before nor after. Nor is that a gap this should close by also + // writing a typeBinding — NO container name has one, the file stem included, + // so a payload subject written `Type.member` resolves for no spelling at + // all, and giving the alias an entry would make it behave unlike the very + // container it names. + bindZigThisAliases(parsed, tree.rootNode, indexes); + const resolver = new ZigSubjectTypeResolver(scopes, indexes, classScopeByDefId); // Pre-order: an outer payload is bound before an inner construct reads it diff --git a/gitnexus/src/core/ingestion/languages/zig/scope-resolver.ts b/gitnexus/src/core/ingestion/languages/zig/scope-resolver.ts index 86afcfbf2..675f6e1a8 100644 --- a/gitnexus/src/core/ingestion/languages/zig/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/languages/zig/scope-resolver.ts @@ -5,8 +5,9 @@ * Thin wiring: Zig has no inheritance (default MRO linearization over an * empty heritage set), no `super`, and is statically typed (field-fallback * heuristic off per the contract guidance). Import resolution reuses the - * same `resolveZigImportInternal` the legacy import-resolver config wraps, - * with `build.zig.zon` `.path` deps threaded through `loadResolutionConfig`. + * same `resolveZigImportInternal` the legacy import-resolver config wraps, with + * the repo's Zig build packages threaded through `loadResolutionConfig` and the + * one governing each file selected per-file by `zigPackageFor`. */ import type { ParsedFile } from 'gitnexus-shared'; @@ -14,7 +15,11 @@ import { SupportedLanguages } from 'gitnexus-shared'; import { buildMro, defaultLinearize } from '../../scope-resolution/passes/mro.js'; import { populateClassOwnedMembers } from '../../scope-resolution/scope/walkers.js'; import type { ScopeResolver } from '../../scope-resolution/contract/scope-resolver.js'; -import { loadZigBuildConfig, type ZigBuildZonConfig } from '../../language-config.js'; +import { + loadZigWorkspaceIndex, + zigPackageFor, + type ZigWorkspaceIndex, +} from '../../language-config.js'; import { resolveZigImportInternal } from '../../import-resolvers/zig.js'; import { zigProvider } from '../zig.js'; import { expandZigWildcardNames, zigArityCompatibility, zigMergeBindings } from './index.js'; @@ -46,14 +51,23 @@ export const zigScopeResolver: ScopeResolver = { // import; a one-hop split at the last dot resolved none of them. resolveNamespaceChains: true, - loadResolutionConfig: (repoPath: string) => loadZigBuildConfig(repoPath), + // The whole workspace, not just the root package. A Zig module's import table + // is declared by the `build.zig` of the package the file belongs to, so a repo + // laying its packages out as `packages//build.zig` has as many import + // tables as packages — and reading only the root one leaves every bare + // `@import("")` in such a repo unresolved. Same shape, same reason, as + // `loadTsconfigIndex` for a TypeScript monorepo. + loadResolutionConfig: (repoPath: string) => loadZigWorkspaceIndex(repoPath), resolveImportTarget: (targetRaw, fromFile, allFilePaths, resolutionConfig) => resolveZigImportInternal( fromFile, targetRaw, allFilePaths, - (resolutionConfig as ZigBuildZonConfig | null | undefined) ?? null, + // The package governing THIS file — `zigPackageFor` is the `tsconfigFor` + // analogue. `resolveZigImportInternal` is handed one package's config and + // is unchanged by this: which config it receives is the only difference. + zigPackageFor(resolutionConfig as ZigWorkspaceIndex | null | undefined, fromFile), ), // `pub usingnamespace @import("x.zig");` — target decls become local decls. diff --git a/gitnexus/src/core/ingestion/languages/zig/this-alias-bindings.ts b/gitnexus/src/core/ingestion/languages/zig/this-alias-bindings.ts new file mode 100644 index 000000000..8a124bf3e --- /dev/null +++ b/gitnexus/src/core/ingestion/languages/zig/this-alias-bindings.ts @@ -0,0 +1,191 @@ +/** + * Bind a Zig `const X = @This();` alias to the container it names (#3219 + * review round 8). + * + * `@This()` IS the enclosing container, and Zig code says so constantly: the + * single most common spelling is `const Self = @This();`, followed by + * `Alias.member` wherever the explicit form is wanted (`Self.width(self)`, a + * registration `bridge.accessor(Self.width, …)`, a nested type `Self.Node`). + * A compiler resolves `Self` and the container's own name to the same type. + * This index did not, for one specific reason: + * + * - A container-level alias (`const Self = @This();` inside `struct {…}`) + * mints a `Variable` beside the `Struct`, so `Self` binds to a VALUE. Every + * class-like lookup filters on `isClassLike` and walks straight past it. + * - A FILE-level alias in a file-as-struct mints nothing at all — + * `isZigFileThisAlias` suppresses the Const deliberately, so it cannot + * shadow the type for `x: *Page`. The name is then bound to nothing, while + * the container itself is bound under the FILE STEM. + * + * So `Self.width` resolved only when the alias happened to be spelled like its + * container. Measured on the three Zig corpora on hand: 73 of ghostty's 185 + * `@This()` files, 93 of tigerbeetle's 94 and 8 of mach's 42 spell it + * differently, carrying 302 `Alias.member` references between them, 96 of those + * calls. Those were not wrong edges — they were no edges, and a caller list + * missing them is exactly the false confidence #3399 is about. + * + * WHAT THIS ADDS, precisely: one binding of the alias NAME to the container's + * own definition, appended to `indexes.bindingAugmentations` (the sanctioned + * post-finalize channel, invariant I8 — `indexes.bindings` is frozen). The + * augmentation channel is consulted by `lookupBindingsAt` only AFTER a scope's + * own `Scope.bindings`, so this can never outrank a real local declaration: the + * `Variable` the container-level alias already mints still answers first for + * anything that wants a value, and the container answers for anything that + * wants a type. Nothing is replaced and nothing is removed. + * + * WHAT IT DELIBERATELY DOES NOT DO. Only aliases declared DIRECTLY in a + * container body or at file level are bound, which is the same set + * `collectZigThisAliases` recognizes for the type-rewrite path. A function-local + * `const Self = @This();` also names the enclosing container, but it belongs in + * that function's scope, not the container's, and binding it here would make it + * visible to sibling functions that never declared it. + */ + +import type { ParsedFile, Scope, ScopeId, SymbolDefinition } from 'gitnexus-shared'; +import type { BindingRef } from 'gitnexus-shared'; +import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; +import type { SyntaxNode } from '../../utils/ast-helpers.js'; +import { isClassLike } from '../../scope-resolution/scope/walkers.js'; +import { isZigKeywordDeclaration, ZIG_CONTAINER_TYPES } from './captures.js'; + +/** `const X = @This();` — the declaration shape, wherever it sits. */ +function thisAliasName(node: SyntaxNode): string | undefined { + if (node.type !== 'variable_declaration' || !isZigKeywordDeclaration(node)) return undefined; + const named = node.namedChildren.filter((c): c is SyntaxNode => c !== null); + if (named.length !== 2 || named[0]!.type !== 'identifier') return undefined; + const value = named[1]!; + if (value.type !== 'builtin_function' || value.namedChild(0)?.text !== '@This') return undefined; + return named[0]!.text; +} + +/** The class-like definition a Class scope is the body of. */ +function containerDefOf(scope: Scope): SymbolDefinition | undefined { + return scope.ownedDefs.find((d) => isClassLike(d.type)); +} + +/** + * The Class scope of the FILE-AS-STRUCT, if this file is one. + * + * Identified by range rather than by name: it is the only Class scope that both + * hangs directly off the module scope AND spans the same lines as it. A + * namespace-only file with a top-level `pub const Counter = struct {…}` also + * produces a Class scope under the module scope, and it also carries the file + * stem when the file is `Counter.zig` — matching on the name would bind that + * file's `@This()` alias to `Counter`, which is not what `@This()` means there. + */ +function fileStructScope(scopes: readonly Scope[], moduleScope: Scope): Scope | undefined { + return scopes.find( + (s) => + s.kind === 'Class' && + s.parent === moduleScope.id && + s.range.startLine === moduleScope.range.startLine && + s.range.endLine === moduleScope.range.endLine, + ); +} + +/** + * Innermost Class scope containing `node`. + * + * Compared on (line, column) rather than line alone, because a Zig container + * can nest inside another on ONE line — `const A = struct { const S = @This(); + * const B = struct { const T = @This(); }; };` gives both scopes the same start + * line, and picking between them by line would be a coin toss that binds an + * alias to the wrong container. A wrong edge is the one outcome this whole + * change set treats as worse than no edge. + */ +function enclosingClassScope(scopes: readonly Scope[], node: SyntaxNode): Scope | undefined { + const line = node.startPosition.row + 1; + const column = node.startPosition.column; + const startsAtOrBefore = (l: number, c: number): boolean => + l < line || (l === line && c <= column); + const endsAtOrAfter = (l: number, c: number): boolean => l > line || (l === line && c >= column); + let best: Scope | undefined; + for (const s of scopes) { + if (s.kind !== 'Class') continue; + if (!startsAtOrBefore(s.range.startLine, s.range.startCol)) continue; + if (!endsAtOrAfter(s.range.endLine, s.range.endCol)) continue; + if ( + best === undefined || + s.range.startLine > best.range.startLine || + (s.range.startLine === best.range.startLine && s.range.startCol > best.range.startCol) + ) { + best = s; + } + } + return best; +} + +/** + * Append `alias -> def` at `scopeId`, skipping the append when THIS def is + * already bound there. Idempotence, not precedence: a duplicate would be + * harmless for lookup (`lookupBindingsAt` dedupes by `def.nodeId`) but would + * make the channel's contents depend on how many times the hook ran. Other + * bindings under the same name are left alone — the channel is append-only per + * I8, and the whole point of the augmentation tier is that a scope's own + * `Scope.bindings` are consulted first and still win. + */ +function appendBinding( + indexes: ScopeResolutionIndexes, + scopeId: ScopeId, + alias: string, + def: SymbolDefinition, +): void { + const channel = indexes.bindingAugmentations as Map>; + let byName = channel.get(scopeId); + if (byName === undefined) { + byName = new Map(); + channel.set(scopeId, byName); + } + const bucket = byName.get(alias); + if (bucket === undefined) { + byName.set(alias, [{ def, origin: 'local' }]); + return; + } + if (bucket.some((ref) => ref.def.nodeId === def.nodeId)) return; + bucket.push({ def, origin: 'local' }); +} + +/** + * Bind every `@This()` alias declared in `parsed` at container or file level. + * + * Called per file from `populateZigRangeBindings`, which already holds the + * parsed tree — a second pass over `parsedFiles` would re-parse every file when + * the tree cache is cold. That is the only thing the two share. Ordering + * against the payload bindings that follow does not matter, twice over: the + * payload walk types its subjects through `findReceiverTypeBinding`, which + * never reads the channel written here, and a `@This()` alias is a + * container-private name no other file can import, so nothing outside this file + * reads it either. + */ +export function bindZigThisAliases( + parsed: ParsedFile, + root: SyntaxNode, + indexes: ScopeResolutionIndexes, +): void { + const moduleScope = parsed.scopes.find((s) => s.kind === 'Module'); + if (moduleScope === undefined) return; + const fileStruct = fileStructScope(parsed.scopes, moduleScope); + + const visit = (node: SyntaxNode): void => { + const alias = thisAliasName(node); + if (alias !== undefined) { + const parent = node.parent; + if (parent?.type === 'source_file') { + // The file-as-struct's own name. Bound at the MODULE scope, which is + // where the container is already bound under the file stem, so the + // alias and the stem are visible to exactly the same sites. + const def = fileStruct === undefined ? undefined : containerDefOf(fileStruct); + if (def !== undefined) appendBinding(indexes, moduleScope.id, alias, def); + } else if (parent !== null && ZIG_CONTAINER_TYPES.has(parent.type)) { + const scope = enclosingClassScope(parsed.scopes, node); + const def = scope === undefined ? undefined : containerDefOf(scope); + if (scope !== undefined && def !== undefined) appendBinding(indexes, scope.id, alias, def); + } + } + for (let i = 0; i < node.namedChildCount; i++) { + const child = node.namedChild(i); + if (child !== null) visit(child); + } + }; + visit(root); +} diff --git a/gitnexus/src/core/ingestion/languages/zig/workspace-static-gating.ts b/gitnexus/src/core/ingestion/languages/zig/workspace-static-gating.ts index 040c058be..11c13b259 100644 --- a/gitnexus/src/core/ingestion/languages/zig/workspace-static-gating.ts +++ b/gitnexus/src/core/ingestion/languages/zig/workspace-static-gating.ts @@ -1,6 +1,6 @@ import type { ParsedFile, ReferenceSite } from 'gitnexus-shared'; import { getTreeSitterBufferSize } from '../../constants.js'; -import type { ZigBuildZonConfig } from '../../language-config.js'; +import { zigPackageFor, type ZigWorkspaceIndex } from '../../language-config.js'; import { resolveZigImportInternal } from '../../import-resolvers/zig.js'; import { buildZigBoolConstMap, @@ -51,7 +51,11 @@ export function populateZigWorkspaceStaticGating( tree, parsed.filePath, knownPaths, - ctx.resolutionConfig as ZigBuildZonConfig | null | undefined, + // `resolutionConfig` is the whole Zig workspace; the package governing + // THIS file is what `resolveZigImportInternal` takes. Selected here rather + // than hoisted out of the loop because the answer is per-file: two files + // of this pass can belong to different packages. + zigPackageFor(ctx.resolutionConfig as ZigWorkspaceIndex | null | undefined, parsed.filePath), ); if (aliases.size === 0) continue; const ranges = collectZigStaticGatedRanges( @@ -76,7 +80,7 @@ function collectImportAliases( tree: ZigTree, fromFile: string, knownPaths: ReadonlySet, - resolutionConfig?: ZigBuildZonConfig | null, + packageConfig: ReturnType, ): ZigImportAliasMap { const candidates = new Map(); const declarationCounts = new Map(); @@ -91,7 +95,7 @@ function collectImportAliases( const raw = builtin?.descendantsOfType('string').at(0)?.text; if (raw === undefined) continue; const specifier = raw.replace(/^['"]|['"]$/g, ''); - const target = resolveZigImportInternal(fromFile, specifier, knownPaths, resolutionConfig); + const target = resolveZigImportInternal(fromFile, specifier, knownPaths, packageConfig); if (target !== null) candidates.set(binding, target); } diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/property-dispatch.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/property-dispatch.ts index fef9ed29d..6a23ca94d 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/property-dispatch.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/property-dispatch.ts @@ -17,9 +17,11 @@ * registries only consult pre-finalize local bindings — imported names live * in finalized bindings (the same reason free calls need * `emitFreeCallFallback`) — so `resolveReferenceSites` skips `value-ref` - * sites and this pass resolves them post-finalize via - * `findCallableBindingInScope` (Function/Method/Constructor only — the - * callable gate that keeps `{ port: DEFAULT_PORT }` from emitting anything). + * sites and this pass resolves them post-finalize (see + * `resolveValueRefTarget` — Function/Method/Constructor only, the callable + * gate that keeps `{ port: DEFAULT_PORT }` from emitting anything, and + * receiver-aware so a qualified reference binds the owner it was written + * with). * * Precision posture (mirrors `emitInterfaceDispatchFor`): * - reason `'property-dispatch'` keeps synthesized CALLS auditable; @@ -36,13 +38,29 @@ * emits the capture participates) and generic member-call sites. */ -import type { ParsedFile, SymbolDefinition } from 'gitnexus-shared'; +import type { + BindingRef, + ParsedFile, + ReferenceSite, + ScopeId, + SymbolDefinition, +} from 'gitnexus-shared'; import type { KnowledgeGraph } from '../../../graph/types.js'; import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; import { tryEmitEdge, type CalleeIdCaptureCtx } from '../graph-bridge/edges.js'; import type { GraphNodeLookup } from '../graph-bridge/node-lookup.js'; import type { CalleeIdSink } from '../graph-bridge/callee-id-sink.js'; -import { findCallableBindingInScope } from '../scope/walkers.js'; +import { + findCallableBindingInScope, + findClassBindingInScope, + findOwnedMember, + isNamespaceNameShadowed, + isOwnerNameShadowedBySomethingElse, + lookupBindingsAt, +} from '../scope/walkers.js'; +import { VALUE_REF_EDGE_REASON } from '../value-ref-edges.js'; +import type { SemanticModel } from '../../model/semantic-model.js'; +import { CALL_TARGET_TYPES } from '../../model/symbol-table.js'; /** * Keys registered by more than this many distinct functions are skipped — @@ -65,12 +83,282 @@ export const MAX_PROPERTY_DISPATCH_FANOUT = (() => { /** Below the 0.85 resolved baseline; same discount idea as interface-dispatch. */ export const PROPERTY_DISPATCH_CONFIDENCE = 0.7; +/** + * Resolve a `value-ref` site to the callable it names. + * + * Two shapes, and the difference matters: + * + * - BARE (`{ handler: onClick }`, `register(onTick)`) — the name is resolved + * up the lexical chain, which is what an unqualified name means. Be exact + * about what that walk does, because it is not a plain lexical lookup: + * `findCallableBindingInScope` applies the callable predicate WHILE walking, + * so a scope binding the name to a parameter or a local contributes nothing + * and the walk continues outward. A nearer non-callable binding is stepped + * over — the same shape the qualified path guards against below, unguarded + * here. Pre-existing (#2437) and out of scope for #3399; reachable in JS/TS + * (`function outer(handler) { return { h: handler }; }` beside a top-level + * `function handler`), and narrow in Zig, which rejects a local shadowing a + * container declaration. + * + * - QUALIFIED (`bridge.accessor(Element.getNamespaceUri, …)`) — the source + * WROTE the owner, so the lexical chain is the wrong instrument. It gives + * local bindings precedence (`walkScopeChain`), so a nested container + * holding its own `getNamespaceUri` answers first and the registration is + * attached to a DIFFERENT function than the one written. That is a wrong + * edge, not a missing one — strictly worse for a tool whose value is that + * its edges can be trusted — and it is reachable in any language that + * allows a same-named callable in a nested container, Zig included. + * + * So a qualified site resolves through its receiver: name the owner, then take + * the member off that owner. An owner is either a MODULE or a CLASS-like + * container — `utils.compare` and `Element.getNamespaceUri` are the same shape + * written against the two kinds of namespace a language has, and the member-call + * path already resolves both (receiver-bound-calls Case 1 / Case 2). Both are + * tried here, MODULE FIRST: see `findNamespaceValueRefTarget` for why a module + * receiver cannot simply be declined, and the comment on the call below for why + * the container channel must not go first. + * + * If neither channel names the owner, or the owner is named but owns no such + * callable, this DECLINES rather than falling back to the lexical walk. + * + * Be precise about what declining costs, because it is more than one reference: + * a site that emits NO edge leaves no evidence for `impact`'s value-reference + * probe to read, so the target keeps `epistemic: "exact"` — silence, not a + * hedge. That is why the module channel above exists rather than being waved + * through as "just a decline". What remains declined is the case where the + * written receiver names nothing this index knows at all (a Zig `@This()` alias + * whose name differs from its container's, an owner from outside the workspace): + * there the alternative is not a hedge either, it is a confident edge to a + * lexically-nearer function that the source did not name, and a wrong edge is + * strictly worse than a missing one for a tool whose value is that its edges can + * be trusted. + * + * `CALL_TARGET_TYPES`, not a hand-rolled label set: `findOwnedMember` also + * answers with FIELDS, and a field named like the member would otherwise + * register as if it were the callable. + * + * Exported for `bench/value-ref-resolution/measure.mjs`, which gates both the + * resolved-target SET and the per-site cost of the four channels above across a + * 4x file-count step. It is the per-site half of the pass, so timing it in + * isolation is what makes a workspace-size dependency visible; timing + * `emitPropertyDispatchCalls` would fold that signal into edge emission, and + * re-implementing the channel order in the bench would pin the bench's idea of + * this function rather than this function. + */ +export function resolveValueRefTarget( + site: ReferenceSite, + filePath: string, + scopes: ScopeResolutionIndexes, + model: SemanticModel, + publishesImportedNames: boolean, +): SymbolDefinition | undefined { + const receiverName = site.explicitReceiver?.name; + if (receiverName === undefined) { + return findCallableBindingInScope(site.inScope, site.name, scopes); + } + // NAMESPACE FIRST, and the order is load-bearing. `findClassBindingInScope` + // does not stop at the scope chain: when its `isClassLike` walk misses — and a + // namespace handle binds a Module, so it always misses — it falls back to + // `scopes.qualifiedNames`, a WORKSPACE-wide index, and answers with the unique + // def of that name anywhere in the repo. Trying it first therefore lets a + // same-named container in a file this one never imported preempt the `@import` + // this file actually wrote: + // + // const dom_utils = @import("dom_utils.zig"); // namespace-only module + // … bridge.accessor(dom_utils.compare, …) // → decoy.zig's compare + // + // The shadow guard below cannot catch it: the import binds at MODULE scope, + // which the guard treats as the floor. An import written in this file is the + // strongest statement about what the name means here, so it outranks a global + // guess — and when the handle is not an import of this file, this answers + // nothing and the container channel runs exactly as before. + const viaNamespace = findNamespaceValueRefTarget( + site, + filePath, + receiverName, + scopes, + publishesImportedNames, + ); + if (viaNamespace !== undefined) { + // `'owned'` is NOT "no answer" — it is "this receiver is a namespace handle + // this file wrote, and it names no callable member". The two must not be + // conflated, because falling through from the second one reaches + // `findClassBindingInScope`, whose miss path answers from the WORKSPACE-wide + // qualified-name index: a same-named container in a file this one never + // imported then supplies the member the written module does not have. That + // is a confident edge into an unrelated file, and the owner-shadow guard + // below does not stop it — a plain `const utils = @import("utils.zig");` + // records a namespace IMPORT EDGE, not a module-scope binding, so the guard + // sees nothing bound under the name and reads the container as unshadowed. + // Verified with a fixture rather than argued: `dom_utils.onlyOnDecoy`, where + // `dom_utils.zig` has no such member and `decoy.zig` declares a same-named + // struct that does, minted `JsApi → onlyOnDecoy` before this line existed. + // + // The file said which module it meant. If that module does not expose the + // name as a callable, the honest answer is no edge. + return viaNamespace === 'owned' ? undefined : viaNamespace; + } + + const owner = findClassBindingInScope(site.inScope, receiverName, scopes); + if (owner !== undefined) { + // The container lookup is a CLASS-ONLY walk: `walkScopeChain` filters by + // `isClassLike`, so it steps over a nearer binding that is a value and keeps + // climbing — and past the scope chain entirely, into a qualified-name + // fallback that answers with the unique workspace definition of the name. + // `fn f(Ticker: u8) { register(Ticker.fire) }` in a file that neither + // declares nor imports `Ticker` therefore resolves to some other file's + // `Ticker` container. That is the wrong-edge failure R1-2 exists to prevent, + // arriving through the class channel instead of the lexical one, and a + // registration pointing at a function the source never named is worse than + // no registration at all. + // + // So the name has to still MEAN that container at this site. The namespace + // channel below asks the same question through `isNamespaceNameShadowed`; + // a container needs the variant that exempts the container ITSELF, because + // `fn make() { const Local = struct {…}; register(Local.go); }` binds the + // name locally to the very def we resolved, and reading that as its own + // shadow would suppress the resolutions this path exists to make. + if (isOwnerNameShadowedBySomethingElse(receiverName, owner, site.inScope, scopes)) + return undefined; + const member = findOwnedMember(owner.nodeId, site.name, model); + if (member === undefined || !CALL_TARGET_TYPES.has(member.type)) return undefined; + return member; + } + return undefined; +} + +/** + * The second kind of owner: a namespace handle. + * + * `const utils = @import("utils.zig"); register(utils.compare);` — `utils` is a + * MODULE, not a class, so `findClassBindingInScope` answers nothing and the + * class path above declines. Declining here would be a silent hole rather than + * a conservative one: no USES edge is emitted, so + * `callableValueReferenceBoundaries` measures a real zero and `impact` on + * `compare` republishes `exact` — the very claim this feature exists to stop + * making. Nothing downstream can hedge on evidence that was never recorded. + * + * So resolve it, through the SAME channel the member-CALL path already trusts + * for `utils.compare()` (receiver-bound-calls Case 1): the file's namespace + * import edges name the target module, and the target module's own local + * module-scope bindings name its members. `utils.compare` and `utils.compare()` + * disagreeing about what `utils` is would be the anomaly. + * + * The same three guards Case 1 applies, for the same reasons: + * - a LOCAL declaration shadowing the handle suppresses the resolution + * (`isNamespaceNameShadowed`) — `fn f(utils: Decoy) { register(utils.compare) }` + * names the parameter's member, and resolving through the import would be a + * wrong edge rather than a missing one; + * - a locally declared member wins, and a name the target file merely IMPORTED + * counts only when the provider says its imports ARE its exports + * (`ScopeResolver.namespaceExportsIncludeImportedNames`). That opt-in is not + * a detail to skip: a Zig HUB — a file made only of re-exports, ghostty's + * `src/terminal/`, tigerbeetle's `stdx` — declares nothing, so requiring a + * local declaration declines every member reached through one. `hub.fn()` + * resolves and `register(hub.fn)` would not, and one name would mean two + * things depending on whether a `(` followed it. In languages that do not + * opt in, a module's imports are not its exports and this stays closed; + * - two distinct defs under one name resolve NOTHING. Never guess a namespace + * member — the whole point of reading the written receiver is precision. + * + * `CALL_TARGET_TYPES` gates the answer for the same reason the class path needs + * it: `utils.DEFAULT_PORT` is a module-scope binding too, and a registration + * table full of constants must keep emitting nothing. + */ +function findNamespaceValueRefTarget( + site: ReferenceSite, + filePath: string, + receiverName: string, + scopes: ScopeResolutionIndexes, + publishesImportedNames: boolean, +): SymbolDefinition | 'owned' | undefined { + const moduleScopeId = scopes.moduleScopes.get(filePath); + if (moduleScopeId === undefined) return undefined; + const targetFiles: string[] = []; + for (const edge of scopes.imports.get(moduleScopeId) ?? []) { + if (edge.kind !== 'namespace' || edge.localName !== receiverName) continue; + if (edge.targetFile === null) continue; + if (!targetFiles.includes(edge.targetFile)) targetFiles.push(edge.targetFile); + } + if (targetFiles.length === 0) return undefined; + if (isNamespaceNameShadowed(receiverName, site.inScope, scopes)) return undefined; + + /** The unique callable `select` finds across every target file, or nothing. */ + const uniqueMember = ( + select: (moduleScope: ScopeId) => readonly BindingRef[], + ): SymbolDefinition | undefined | 'ambiguous' => { + let picked: SymbolDefinition | undefined; + for (const targetFile of targetFiles) { + const targetScopeId = scopes.moduleScopes.get(targetFile); + if (targetScopeId === undefined) continue; + for (const ref of select(targetScopeId)) { + if (!CALL_TARGET_TYPES.has(ref.def.type)) continue; + if (picked !== undefined && picked.nodeId !== ref.def.nodeId) return 'ambiguous'; + picked = ref.def; + } + } + return picked; + }; + + // A locally declared member first — same precedence `findExportedDef` states + // and `walkScopeChain` applies: what the target file DECLARED beats what it + // merely re-published. + const localRefs = (scope: ScopeId): readonly BindingRef[] => + (scopes.bindings.get(scope)?.get(site.name) ?? []).filter((ref) => ref.origin === 'local'); + const local = uniqueMember(localRefs); + if (local === 'ambiguous') return 'owned'; + if (local !== undefined) return local; + if (!publishesImportedNames) return 'owned'; + + // PRECEDENCE IS DECIDED BEFORE THE TYPE GATE, not by it. `uniqueMember` + // applies `CALL_TARGET_TYPES` while it selects, so a target file declaring a + // NON-callable under this name answers `undefined` above and would otherwise + // fall through to the published channel — publishing a re-exported callable + // under a name the module's own declaration owns. `findExportedDef` does not + // do that: it returns any local def it finds and lets its caller's type gate + // reject it, so `findExportedDefIncludingImportedNames` never reaches the + // imported names for a name the file declares. Same rule here, so `x.f` and + // `x.f()` cannot disagree about which module owns the name. + // + // Not reachable through valid Zig today — a container cannot declare a name + // twice, so one target file cannot hold both spellings, and Zig is the only + // provider that sets `namespaceExportsIncludeImportedNames`. It becomes + // reachable the moment a second provider opts in, or a receiver binds more + // than one target file; the guard is one `some` and the alternative failure + // is a confident edge into the wrong module. + const declaredLocally = targetFiles.some((targetFile) => { + const targetScopeId = scopes.moduleScopes.get(targetFile); + return targetScopeId !== undefined && localRefs(targetScopeId).length > 0; + }); + if (declaredLocally) return 'owned'; + + // Then a name the target file publishes but did not declare — the hub case. + // `lookupBindingsAt`, not `scopes.bindings`, because a hub's module scope owns + // no local binding for these names and the finalized/augmented channel is the + // only place they exist; the same read `findExportedDefIncludingImportedNames` + // does for the CALL form. + const published = uniqueMember((scope) => + lookupBindingsAt(scope, site.name, scopes).filter( + (ref) => ref.origin === 'import' || ref.origin === 'namespace' || ref.origin === 'reexport', + ), + ); + return published === 'ambiguous' || published === undefined ? 'owned' : published; +} + export function emitPropertyDispatchCalls( graph: KnowledgeGraph, scopes: ScopeResolutionIndexes, parsedFiles: readonly ParsedFile[], nodeLookup: GraphNodeLookup, + model: SemanticModel, calleeIdSink?: CalleeIdSink, + /** + * `ScopeResolver.namespaceExportsIncludeImportedNames`, forwarded rather than + * re-derived. The pass names no language; it asks the provider the same + * question `receiver-bound-calls` asks before resolving a namespace member, + * so the CALL and the REGISTRATION forms of `hub.fn` cannot disagree. + */ + publishesImportedNames = false, ): { usesEmitted: number; callsEmitted: number; @@ -86,18 +374,16 @@ export function emitPropertyDispatchCalls( for (const parsed of parsedFiles) { for (const site of parsed.referenceSites) { if (site.kind !== 'value-ref') continue; - const def = findCallableBindingInScope(site.inScope, site.name, scopes); + const def = resolveValueRefTarget( + site, + parsed.filePath, + scopes, + model, + publishesImportedNames, + ); if (def === undefined) continue; - const ok = tryEmitEdge( - graph, - scopes, - nodeLookup, - site, - def, - 'scope-resolution: value-ref', - seen, - ); + const ok = tryEmitEdge(graph, scopes, nodeLookup, site, def, VALUE_REF_EDGE_REASON, seen); if (ok) usesEmitted++; if (site.propertyKey === undefined) continue; diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts index aa30acf91..15976130f 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts @@ -1210,7 +1210,12 @@ export function runScopeResolution( indexes, emitParsedFiles, postHeritageNodeLookup, + readonlyModel, calleeIdAccumulator, + // Same provider hook the receiver-bound pass consults for a namespace + // member (Case 1). Without it a hub module's re-exported callable + // resolves when CALLED and declines when REGISTERED. + provider.namespaceExportsIncludeImportedNames === true, ); if (propertyDispatch.skippedKeys > 0) { // Never drop dispatch coverage silently: a hook table larger than the diff --git a/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts b/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts index 38413f013..835f9a7a8 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts @@ -353,6 +353,82 @@ export function isNamespaceNameShadowed( return true; } +/** + * Does something between `inScope` and its module scope bind `name` to + * ANYTHING other than `def`? + * + * `isNamespaceNameShadowed` asks the same question for a namespace handle, + * where any local binding of the name is by definition not the import. A + * CONTAINER receiver needs the extra clause: the container may itself be the + * local declaration (`fn make() { const Local = struct {…}; … Local.go … }`), + * and reading that as its own shadow would suppress exactly the resolutions it + * is meant to permit — the #2723 mistake, one channel over. + * + * So a scope that binds the name answers immediately, and the answer is "not + * shadowed" only when one of that scope's bindings IS `def`. A name bound in a + * nearer scope to something else — a parameter, a local, a type binding — wins + * the lexical race, which is the whole point: `findClassBindingInScope` filters + * the chain by `isClassLike` and therefore cannot see that it lost it. + * + * The MODULE scope is inspected too, unlike `isNamespaceNameShadowed`, and the + * exemption is what makes that safe. That guard stops one rung short because a + * namespace import writes its own name into the module scope and would read as + * its own shadow (#2723); here the owner is compared by identity, so the binding + * that IS the owner exempts itself and only a binding to something ELSE answers + * `true`. Stopping short would leave the exact hole this walk exists to close: + * `findClassBindingInScope` steps over a module-scope binding that is not + * class-like and then answers from a WORKSPACE-wide qualified-name index, so + * `const Gauge = @import("other.zig").SOME_CONST;` in a file that never imports + * `Gauge.zig` would still resolve `Gauge.read` to that file's container. + * `lookupBindingsAt` is used at that scope and only there, because an imported + * alias lives in the finalized channel rather than in `scope.bindings`. + * + * Fail-closed like its sibling: a missing scope or a parent cycle answers + * `true`, because suppressing a resolution costs a missing edge while trusting a + * corrupt chain costs a wrong one. + */ +export function isOwnerNameShadowedBySomethingElse( + name: string, + def: SymbolDefinition, + inScope: ScopeId, + scopes: ScopeResolutionIndexes, +): boolean { + let currentId: ScopeId | null = inScope; + const visited = new Set(); + while (currentId !== null) { + if (visited.has(currentId)) return true; + visited.add(currentId); + const scope = scopes.scopeTree.getScope(currentId); + if (scope === undefined) return true; + if (scope.kind !== 'Object') { + const isModule = scope.kind === 'Module'; + const imported = isModule ? lookupBindingsAt(currentId, name, scopes) : []; + const bindsHere = + scope.bindings.has(name) || + scope.typeBindings.has(name) || + scope.lexicalNames?.has(name) === true || + imported.length > 0 || + scope.ownedDefs.some((d) => { + const qualifiedName = d.qualifiedName; + if (qualifiedName === undefined) return false; + const dot = qualifiedName.lastIndexOf('.'); + return (dot === -1 ? qualifiedName : qualifiedName.slice(dot + 1)) === name; + }); + if (bindsHere) { + if ((scope.bindings.get(name) ?? []).some((b) => b.def.nodeId === def.nodeId)) return false; + if (scope.ownedDefs.some((d) => d.nodeId === def.nodeId)) return false; + if (imported.some((b) => b.def.nodeId === def.nodeId)) return false; + return true; + } + } + // The module scope is the last rung, not a rung to skip: nothing above it + // can shadow a name for this file. + if (scope.kind === 'Module') return false; + currentId = scope.parent; + } + return true; +} + export function findReceiverTypeBinding( startScope: ScopeId, receiverName: string, diff --git a/gitnexus/src/core/ingestion/scope-resolution/value-ref-edges.ts b/gitnexus/src/core/ingestion/scope-resolution/value-ref-edges.ts new file mode 100644 index 000000000..5507f9d0e --- /dev/null +++ b/gitnexus/src/core/ingestion/scope-resolution/value-ref-edges.ts @@ -0,0 +1,14 @@ +/** + * The `reason` text stamped on the USES edge a `value-ref` site emits. + * + * A leaf module on purpose. Two modules need this string and they sit on + * opposite sides of the product: `passes/property-dispatch.ts` writes it at + * analysis time, and `mcp/local/local-backend.ts` reads it back at query time + * to decide whether an answer is `exact`. Re-typing the literal in the reader + * would make the epistemic signal fail SILENTLY the day the writer's text is + * reworded — the query would simply match nothing and every answer would go + * back to claiming certainty, which is the exact defect (#3399) this constant + * exists to close. Importing `property-dispatch.ts` for it instead would drag + * the whole scope-resolution emit graph into the MCP backend for one string. + */ +export const VALUE_REF_EDGE_REASON = 'scope-resolution: value-ref'; diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index d288d14f4..8b34a3716 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -129,6 +129,7 @@ import type { UnresolvedReceiverSummary } from '../../core/ingestion/scope-resol import type { UndecidedSatisfactionSummary } from '../../core/ingestion/scope-resolution/undecided-satisfaction.js'; import { scopeExtractionFailureTotal } from '../../core/ingestion/scope-resolution/scope-extraction-failures.js'; import { lookupCount } from '../../core/ingestion/scope-resolution/summary-maps.js'; +import { VALUE_REF_EDGE_REASON } from '../../core/ingestion/scope-resolution/value-ref-edges.js'; import { DEFERRED_IMPORT_REASON_SUFFIX, TYPE_ONLY_IMPORT_REASON_SUFFIX, @@ -711,6 +712,39 @@ export interface EpistemicCauses { * "nothing was undecided", and a re-index is what tells the two apart. */ readonly undecidedSatisfaction: number; + /** + * Symbols that name this callable in VALUE position rather than calling it + * (#3399) — a registration table (`bridge.accessor(Element.getNamespaceUri, + * …)`), a callback argument, a function pointer stored in a field. + * + * Unit: SYMBOLS — distinct referrers, the same unit and the same reason as + * `dispatchBoundary`: the reference edge is per-site but the walk's question + * is "who else might reach this", and a referrer that names the callable + * twice is still one place the value escapes from. + * + * Kept separate from `dispatchBoundary` even though both describe dispatch + * the walk cannot follow. That slot counts implementations and + * interface-level consumers found by the heritage probe; these are neither, + * and folding them in would tell a consumer branching on the numbers that an + * interface boundary exists where there is none. The distinction is also the + * actionable one: a dispatch boundary is irreducible, whereas a callable + * value CAN often be followed once the language models the store/load that + * carries it. + * + * The reference itself IS modelled — that is what makes it countable. What is + * missing is the invocation through the value: it happens later, through a + * struct field, a registry lookup, or comptime reflection, and no CALLS edge + * connects the eventual call site back to this symbol. + * + * Zero when the property-dispatch pass DID synthesize that invocation + * (`x.()` through a registered object-literal key): the walk followed + * the registration, so nothing was missed and the result stays `exact`. + * + * Also zero — WITH a boundary note — when the probe itself could not run. + * The note is the signal there; the count is not, which is why a reader must + * branch on `epistemic` first and read the causes as explanation. + */ + readonly callableValueReferences: number; } function epistemicFrom(dropped: { @@ -720,6 +754,7 @@ function epistemicFrom(dropped: { undecided: number; dispatch: number; scopeExtraction: number; + callableValueReferences: number; }): { epistemic: 'exact' | 'lower-bound'; boundaries?: string[]; @@ -738,6 +773,7 @@ function epistemicFrom(dropped: { dispatchBoundary: dropped.dispatch, externalBoundary: dropped.external, undecidedSatisfaction: 0, + callableValueReferences: dropped.callableValueReferences, }, } : { epistemic: 'exact' } @@ -754,6 +790,7 @@ function epistemicFrom(dropped: { dispatchBoundary: dropped.dispatch, externalBoundary: dropped.external, undecidedSatisfaction: dropped.undecided, + callableValueReferences: dropped.callableValueReferences, }, }; } @@ -853,6 +890,153 @@ function undecidedSatisfactionBoundaries( return { notes, undecided }; } +/** + * Boundary evidence for callables named in VALUE position (#3399). + * + * `bridge.accessor(Element.getNamespaceUri, null, .{})`, `{ onClick: handler }`, + * `qsort(xs, n, sz, compareItems)` — each REGISTERS a function somewhere + * instead of calling it. The registration is modelled (`value-ref` → a USES + * edge, Kythe `ref` / Joern `METHOD_REF`); the invocation through the stored + * value is not, because it happens later through a struct field, a registry + * lookup or comptime reflection. + * + * That gap is precisely the first half of `tools.ts`'s definition of + * `lower-bound` — the walk provably missed callers — and it was previously + * reported as `exact`. (The second half, a probe that could not run, is what + * the failure branch below publishes; the contract states both because this + * function can produce either.) A + * public DOM accessor bound into a JS bridge table came back LOW/exact with two + * internal callers, which is worse than no answer: `lower-bound` invites the + * reader to look further, `exact` tells them not to bother. + * + * Counted as DISTINCT REFERRERS rather than sites: the question the count + * serves is "how many places does this value escape from", and a table that + * registers the same callable twice is still one table. + * + * NOT every value reference is a gap. Where the property-dispatch pass + * synthesized the invocation side, the walk followed it and the answer stays + * `exact` — see the second probe below. + * + * Three failure modes, three different answers, none of them silence: + * - the query cannot run → hedge, count 0 (a probe that did not answer + * is not evidence of completeness); + * - the query returns nothing → no hedge (a real, measured zero); + * - the reference was followed → no hedge (nothing was missed). + * + * The probe reads the edge's `reason`, which is why writer and reader share + * {@link VALUE_REF_EDGE_REASON}. Language-neutral by construction — every + * provider that emits a `value-ref` capture participates, and one that emits + * none simply gets no rows. + */ +async function callableValueReferenceBoundaries( + lbugPath: string, + symId: string, +): Promise<{ notes: string[]; referrers: number }> { + // `COUNT(DISTINCT …)`, not a capped row list. A `LIMIT n` here would make the + // published cause silently understate a target with more than n + // registrations — and this number is documented as "how many symbols", so a + // reader comparing its magnitude against `receiverTyping` would be comparing + // a truth to a ceiling. Aggregating in the database keeps the work bounded + // without capping the answer; scalar `sym.id` equality plus an implicit + // group-by is the shape `countByType` below already relies on. + // + // `null`, not `[]`, on failure: see below — an empty result set and an + // unanswerable query must not be the same value. + const rows = await executeParameterized( + lbugPath, + `MATCH (other)-[r:CodeRelation]->(sym) + WHERE sym.id = $symId AND r.type = 'USES' AND r.reason = $reason + RETURN COUNT(DISTINCT other.id) AS cnt`, + { symId, reason: VALUE_REF_EDGE_REASON }, + ).catch(() => null); + + // A probe that could not run must never read as certainty — the same rule the + // `loadMeta` read above states, and the whole reason this function exists. + // Returning zero here would publish `exact` on the strength of a query that + // never answered. + if (rows === null) { + return { + referrers: 0, + notes: [ + 'The callable-value-reference probe could not be run against this index, so whether ' + + 'this symbol is registered somewhere as a value is unknown. Treat the caller list as ' + + 'incomplete until it can be re-checked.', + ], + }; + } + const referrers = rows.length > 0 ? Number((rows[0] as any).cnt ?? (rows[0] as any)[0] ?? 0) : 0; + if (!Number.isFinite(referrers) || referrers <= 0) return { notes: [], referrers: 0 }; + + // Registrations whose invocation side the analyzer ALREADY synthesized are + // not a gap. `emitPropertyDispatchCalls` sweep 2 connects `x.()` member + // calls to every function registered under `` and stamps those edges + // `property-dispatch`; where that happened, the walk did not "provably miss" + // the caller and `lower-bound` would be noise sprayed over an answer the + // analyzer actually computed. Zig — the case this was built for — never sets + // a property key (no object-literal key to dispatch through), so it is never + // excluded here; the exclusion exists to keep TypeScript/JavaScript hook + // tables that ARE followed from being downgraded. + // + // SYMBOL-LEVEL, NOT PER-EDGE, and that is only sound because of an invariant + // that lives nowhere near this line. The graph does not record which + // registration produced which synthesized call, so if one symbol could carry + // both a followed and an unfollowed registration, this would zero the note + // over a gap the analyzer provably did not close — #3399 returning through a + // side door. Today no symbol can: + // + // - sweep 2 synthesizes CALLS only for a registration whose site carried a + // `propertyKey` (sweep 1 skips the index when it is undefined); + // - every JS/TS `@reference.value-ref` rule also captures + // `@reference.property-key` — both are object-literal shapes; + // - no Zig `@reference.value-ref` rule captures one. + // + // So a dispatchable registration is always a JS/TS one, an undispatchable + // registration is always a Zig one, and the two never meet on one symbol. + // `test/unit/scope-resolution/value-ref-dispatchability.test.ts` FAILS the day + // that stops holding — a JS/TS rule for a bare callback argument + // (`register(handler)`), a Zig rule that grows a key. When it does, the + // choice to make here is between (a) splitting the edge `reason` into + // dispatchable / undispatchable so this probe can count them apart, and + // (b) hedging any symbol with an undispatchable registration regardless of + // dispatch. (a) is precise and costs a graph-content change; (b) is cheap and + // over-hedges. What is NOT acceptable is leaving this as-is, because a signal + // that quietly stops firing is the defect this whole feature removes. + // + // Given the invariant, the residual today is only the coarseness of the + // exclusion within JS/TS, and hedging every property-value registration in + // every JS/TS codebase is worse: a signal that fires on everything stops + // carrying information, and the fan-out cap warning still sits behind it. + const dispatched = await executeParameterized( + lbugPath, + `MATCH (other)-[r:CodeRelation]->(sym) + WHERE sym.id = $symId AND r.type = 'CALLS' AND r.reason = 'property-dispatch' + RETURN COUNT(r) AS cnt`, + { symId }, + ).catch(() => null); + // Failure here is NOT a reason to skip the hedge: we already know a value + // reference exists, and being unable to prove it was followed leaves the + // conservative answer standing. + const dispatchedCount = + dispatched === null || dispatched.length === 0 + ? 0 + : Number((dispatched[0] as any).cnt ?? (dispatched[0] as any)[0] ?? 0); + if (Number.isFinite(dispatchedCount) && dispatchedCount > 0) { + return { notes: [], referrers: 0 }; + } + + const one = referrers === 1; + return { + referrers, + notes: [ + `${referrers} ${one ? 'symbol references' : 'symbols reference'} this callable as a VALUE ` + + `rather than calling it (a registration table, a callback argument, a stored function ` + + `pointer). The reference is recorded, but the call made THROUGH that value is not: it is ` + + `dispatched later from wherever the value is stored. Callers reached that way are absent ` + + `from this result — actual impact may be higher.`, + ], + }; +} + interface RepoHandle { id: string; // unique key = repo name (basename) name: string; @@ -6998,6 +7182,19 @@ export class LocalBackend { direction === 'downstream' ? Promise.resolve(undefined) : queryConvexDispatchMetadata(repo.lbugPath, symId, symName, symType); + // #3399 — callables named in value position. Upstream only: the question + // "who can reach this symbol" is the one a registration makes unanswerable. + // A downstream walk asks what THIS symbol reaches, which a reference INTO + // it does not affect. + // + // Issued alongside the heritage probe rather than after it, and read into + // `droppedBoundaries` below, so it hedges even when that probe finds + // nothing AND when it throws — a value reference is an independent reason + // a count is short, exactly as the receiver drops above are. + const valueRefPromise = + direction === 'downstream' + ? Promise.resolve({ notes: [] as string[], referrers: 0 }) + : callableValueReferenceBoundaries(repo.lbugPath, symId); const interfaceRowsPromise = executeParameterized( repo.lbugPath, `MATCH (x)-[r:CodeRelation]->(iface) @@ -7018,12 +7215,14 @@ export class LocalBackend { : []), ]); const convexDispatch = await convexDispatchPromise; + const valueRefDrops = await valueRefPromise; const droppedBoundaries = { ...receiverDrops, notes: [ ...receiverDrops.notes, ...scopeExtractionDrops.notes, ...undecidedDrops.notes, + ...valueRefDrops.notes, ...(convexDispatch === undefined ? [] : [convexDispatch.boundary]), ], undecided: undecidedDrops.undecided, @@ -7032,6 +7231,7 @@ export class LocalBackend { // inventing one from the presence of a note. dispatch: 0, scopeExtraction: scopeExtractionDrops.files, + callableValueReferences: valueRefDrops.referrers, }; try { // Discover the interface / abstract supertypes on the target's boundary. @@ -7123,6 +7323,7 @@ export class LocalBackend { dispatchBoundary: droppedBoundaries.dispatch + dispatchBoundarySymbols, externalBoundary: droppedBoundaries.external, undecidedSatisfaction: droppedBoundaries.undecided, + callableValueReferences: droppedBoundaries.callableValueReferences, }, }; } catch { diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index d68b5b70b..01f2a6a31 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -291,14 +291,15 @@ Handles disambiguation: if multiple symbols share the same name, returns ranked NOTE: ACCESSES edges (field read/write tracking) are included in context results with reason 'read' or 'write'. CALLS edges resolve through field access chains and method-call chains (e.g., user.address.getCity().save() produces CALLS edges at each step). COMPLETENESS OF incoming: alongside symbol/incoming/outgoing the result carries the same epistemic envelope impact() returns: -- epistemic: 'exact' | 'lower-bound' — 'lower-bound' means callers exist that this view provably does not list. +- epistemic: 'exact' | 'lower-bound' — 'lower-bound' means incoming is a FLOOR: either the walk provably missed callers, or a probe that would have established completeness could not run. Do not read it as proof that an omitted caller exists — read boundaries for which of the two it is. - boundaries: string[] — one plain-language sentence per reason. Prose for humans; branch on causes instead. -- causes: { scopeExtractionFiles, receiverTyping, dispatchBoundary, externalBoundary, undecidedSatisfaction } — machine-readable WHY. Every field counts MISSING THINGS, never sentences: +- causes: { scopeExtractionFiles, receiverTyping, dispatchBoundary, externalBoundary, undecidedSatisfaction, callableValueReferences } — machine-readable WHY. Every field counts MISSING THINGS, never sentences: - causes.scopeExtractionFiles (unit: files) > 0 — scope extraction still failed after the fallback pass, so scope-resolution edges from those files are absent. A value of 0 does not prove completeness when epistemic is 'lower-bound' because an older or unverified index has no measured file count. Re-run \`gitnexus analyze --force\`; if the reason persists, inspect the extraction warnings. - causes.receiverTyping (unit: call sites) > 0 — RESOLVER GAP: the analyzer dropped that many call sites on this name because it could not type the receiver, so they are missing from incoming. Do not read an absent caller as proof none exists. - causes.externalBoundary (unit: call sites) > 0 — the calls left the indexed program (System.out.println, fetch(...)). NOT a defect: no in-graph node could have been reached. An epistemic:'exact' result can carry this. - causes.dispatchBoundary (unit: symbols) > 0 — DI or interface dispatch: that many symbols sit on or beyond a boundary static analysis cannot cross. Irreducible. A symbol count, not a site count — per-site multiplicity is not retained for these edges — so compare its magnitude with receiverTyping, not its exact value. A framework runtime-proxy boundary can make epistemic lower-bound while this value remains 0 because endpoint metadata proves the gap but cannot count omitted symbols. - causes.undecidedSatisfaction (unit: unjudged interface/type pairs) > 0 — the analyzer could not decide whether a type satisfies an interface, so no IMPLEMENTS edge exists and no dispatch boundary was left for the walk to notice. Usually fixable by making the missing dependency available to analysis. + - causes.callableValueReferences (unit: symbols) > 0 — that many symbols name this callable as a VALUE instead of calling it (a Zig registration table or const initialiser, a JS/TS object-literal property value). A bare callback argument in JS/TS is not captured today and is not counted, so a 0 does not rule that shape out; nor does it, on an index built before the language emitted these captures — re-analyze first. The reference is in the graph as a USES edge; the call made THROUGH the value is not, because it is dispatched later from wherever the value was stored. incoming.calls is therefore a floor. Follow the USES edges to find the registration, then the code that reads it. It is 0 when the analyzer DID synthesize the dispatch through a registered property key. That exclusion is per SYMBOL, not per registration: a target with BOTH a followed registration and an unfollowed escape reads 0 here, so a 0 means 'no unfollowed registration was proven', not 'this symbol escapes nowhere'. A 0 alongside epistemic 'lower-bound' can also mean the probe itself could not run — read boundaries for which. REQUIRES RE-INDEX: causes.scopeExtractionFiles, causes.receiverTyping, causes.externalBoundary, causes.undecidedSatisfaction, and framework runtime-proxy boundary detection depend on index-time metadata that only a current analyzer writes. Against an older index the metadata can be absent, which is indistinguishable from "nothing was dropped" unless the schema probe detects the stale index — re-run \`gitnexus analyze\` before trusting a zero or an apparently exact result. @@ -488,15 +489,16 @@ Output includes: - affected_processes: which execution flows break and at which step - affected_modules: which functional areas are hit (direct vs indirect; classification-unavailable when that secondary query fails) - byDepth: affected symbols grouped by traversal depth (paginated by limit/offset; omitted when summaryOnly:true — use byDepthCounts for totals per depth, pagination object when truncated). Each item includes a processes:[{id,label,processType,step}] field listing the execution flows that symbol participates in. Empty when the symbol has no process membership. Can ALSO be empty when partial:true is set — either the process-aggregation pass hit its cap before detecting affected processes, or per-symbol enrichment was capped on a very large page. When partial:true, do NOT treat processes:[] as proof of no participation; cross-check the top-level affected_processes list. An item carries staticGated:true only when the edge that reached it is provably unreachable at compile time from the indexed source (today: Zig calls inside an 'if (CONST_FALSE)' body or the else of 'if (CONST_TRUE)'); the field is absent when the edge is live or the language does not model it. Traversal and risk do NOT filter or rank on it: it is metadata for the caller to weigh. -- epistemic: 'exact' | 'lower-bound' — whether impactedCount is the whole story. 'lower-bound' means the walk provably missed callers, so the count is a floor. Absent only on skipped probes (ambiguous-candidate lists, group fan-out). +- epistemic: 'exact' | 'lower-bound' — whether impactedCount is the whole story. 'lower-bound' means the count is a FLOOR: either the walk provably missed callers, or a probe that would have established completeness could not run (a failed callable-value-reference query says so in boundaries). It is not itself proof that an omitted caller exists — branch on causes and read boundaries. Absent only on skipped probes (ambiguous-candidate lists, group fan-out). - boundaries: string[] — one plain-language sentence per reason the count is short. Prose for humans; branch on causes instead. -- causes: { scopeExtractionFiles, receiverTyping, dispatchBoundary, externalBoundary, undecidedSatisfaction } — the machine-readable split of WHY, so an agent gating its own edits can tell a fixable analyzer gap from an irreducible one. Every field counts MISSING THINGS, never sentences: +- causes: { scopeExtractionFiles, receiverTyping, dispatchBoundary, externalBoundary, undecidedSatisfaction, callableValueReferences } — the machine-readable split of WHY, so an agent gating its own edits can tell a fixable analyzer gap from an irreducible one. Every field counts MISSING THINGS, never sentences: - causes.scopeExtractionFiles (unit: files) > 0 — scope extraction still failed after the fallback pass, so scope-resolution edges from those files are absent. A value of 0 does not prove completeness when epistemic is 'lower-bound' because an older or unverified index has no measured file count. Re-run \`gitnexus analyze --force\`; if the reason persists, inspect the extraction warnings. - causes.receiverTyping (unit: call sites) > 0 — the RESOLVER GAP signal: the analyzer dropped that many call sites because it could not establish the receiver's type (unresolved constructor, factory, chained expression). Those callers are absent from byDepth. Treat the result as incomplete: grep the symbol name before deleting or renaming. - causes.externalBoundary (unit: call sites) > 0 — those calls left the indexed program (System.out.println, fetch(...), os.environ.*). NOT a defect and NOT a reason the count is short: there is no in-graph node any edge could have reached. An epistemic:'exact' result can carry this. - causes.dispatchBoundary (unit: symbols) > 0 — DI or interface dispatch: that many symbols sit on or beyond a boundary a static walk cannot cross. Irreducible. A symbol count, not a site count — per-site multiplicity is not retained for these edges — so compare its magnitude with receiverTyping, not its exact value. A framework runtime-proxy boundary can make epistemic lower-bound while this value remains 0 because endpoint metadata proves the gap but cannot count omitted symbols. - causes.undecidedSatisfaction (unit: unjudged interface/type pairs) > 0 — the analyzer could not DECIDE whether a type satisfies an interface (a type in a required signature named a package it could not resolve), so no IMPLEMENTS edge exists and no dispatch boundary was left for the walk to notice. Distinct from every cause above, which count decided facts that could not be attributed; this one counts questions never answered. It is the only cause that shortens a result WITHOUT leaving a trace in the graph, so an unhedged zero on a symbol reached only through such an interface would otherwise read as 'nobody calls this'. Usually fixable: it most often means a dependency is missing from the analyzed tree. + - causes.callableValueReferences (unit: symbols) > 0 — that many symbols name this callable as a VALUE rather than calling it: 'bridge.accessor(Element.getNamespaceUri, ...)' and 'pub const h = onReset;' in Zig, '{ onClick: handler }' in JS/TS. Those are the shapes actually captured today — a bare callback argument in JS/TS ('qsort'-style, 'setTimeout(tick)') is NOT one of them and is not counted, so a 0 here does not rule that shape out. The registration IS modelled (a USES edge); the invocation through the stored value is NOT, because it happens later via a struct field, a registry lookup or comptime reflection. So impactedCount is a floor and a LOW risk verdict on such a symbol is a floor too. Unlike dispatchBoundary this is often reducible — it usually means the language provider does not yet follow that store/load — but until it is, do NOT read an empty or small caller set as 'safe to change'. It is an exact count, not a capped sample. It is 0 when the analyzer DID synthesize the dispatch through a registered property key, and epistemic stays 'exact' on that account. That exclusion is symbol-level, not edge-level — the graph does not record which registration produced which synthesized call — so a symbol with a mix of followed and unfollowed registrations also reads 0: treat a 0 as 'no unfollowed registration was proven', not as proof the value escapes nowhere. A 0 alongside epistemic 'lower-bound' can instead mean the probe could not run at all, so read boundaries to tell those apart. Read from the graph, so it needs no index-time metadata BEYOND the edges being there: an index built by an analyzer that did not yet emit this language's value-ref captures has none, and reports 0. Re-analyze before reading a 0 as measured. REQUIRES RE-INDEX: causes.scopeExtractionFiles, causes.receiverTyping, causes.externalBoundary, causes.undecidedSatisfaction, and framework runtime-proxy boundary detection depend on index-time metadata that only a current analyzer writes. Against an older index the metadata can be absent, which is indistinguishable from "nothing was dropped" unless the schema probe detects the stale index — re-run \`gitnexus analyze\` before trusting a zero or an apparently exact result. diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 8a22b5038..a06f646df 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -749,7 +749,24 @@ import { copyV8CacheIfPresent, tryLoadV8Cache, writeV8CacheFile } from './v8-sid // v97: Objective-C macro-marker preprocessing recognizes comment-prefixed // directives and rejects invalid numeric marker prefixes. A warm v96 cache can // replay error-recovered facts from the previous normalization behavior. -const SCHEMA_BUMP = 97; +// v98 (#3219): `ZIG_SCOPE_QUERY` gained three `@reference.value-ref` rules — +// bare call argument, qualified call argument (with `@reference.receiver`), and +// const-binding initialiser — so a Zig callable named in VALUE position now +// produces a `value-ref` entry in `ParsedFile.referenceSites` where it produced +// none before. These captures are PARSE-TIME facts, so a warm pre-v98 cache +// replays unchanged `.zig` files with zero value-ref sites, `--force` included +// (shards are content-addressed): `emitPropertyDispatchCalls` then emits no +// USES edge, `callableValueReferenceBoundaries` measures a real zero, and +// `impact` on a registered accessor republishes `epistemic: "exact"` — the exact +// #3399 defect this change exists to close, silently un-fixed. +// +// 94-97 went to #3179 (Objective-C), which has since merged; 98 is the next +// value above it. Re-checked against origin/main and every open PR touching +// this file at merge time, which is the rule the paragraphs above were written +// by PRs that each checked only once: main is at 97, and the remaining open +// claims (#3190 at 94, #2840 at 71, #1616 at 2) all sit below it and must +// re-bump themselves. +const SCHEMA_BUMP = 98; const GITNEXUS_PKG_VERSION = (() => { try { // package.json sits at gitnexus/package.json — two levels up from diff --git a/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/main.zig b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/main.zig index 1cb77e2ee..15c44f34a 100644 --- a/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/main.zig +++ b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/main.zig @@ -21,6 +21,7 @@ pub usingnamespace @import("mixin.zig"); pub const Interfaces = .{ @import("webapi/AbortController.zig"), @import("webapi/AbortSignal.zig"), + @import("webapi/Element.zig"), }; pub fn main() void { diff --git a/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Element.zig b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Element.zig new file mode 100644 index 000000000..02bdeed1e --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Element.zig @@ -0,0 +1,224 @@ +// The JS-API binding-table idiom, as Lightpanda writes it (#3399). +// +// Every accessor below hands a Zig function to `bridge.accessor` AS A VALUE: +// the function is REGISTERED here, never called here. The eventual invocation +// runs through comptime reflection (`@call(.auto, func, args)` over a +// `func: anytype` field), which no static walk can follow — that terminal hop +// is out of scope. What was NOT acceptable is dropping the reference entirely: +// `impact` then reported the accessor as having only its two in-file callers +// and called that answer `exact`. +// +// A file-as-struct, like every webapi module in the real tree. +const Element = @This(); + +// A NAMESPACE-only module (no `@This()`), imported under a handle. The bridge +// table below registers one of its functions the same way it registers this +// file's own methods. +const dom_utils = @import("dom_utils.zig"); + +// A hub module that re-exports `dom_utils`' members without declaring any. +const hub = @import("hub.zig"); + +// A MODULE-LEVEL binding whose name collides with the container `Gauge.zig` +// declares, bound to something that is NOT a container. This file never imports +// `Gauge.zig`. `findClassBindingInScope` filters the scope chain by +// `isClassLike`, so it walks past this binding, and its workspace-wide +// qualified-name fallback answers with the other file's struct — while the +// shadow guard used to permit exactly this, treating the module scope as a floor +// it need not inspect. +// +// The name is bound by IMPORT rather than by a local `const Gauge: u8 = 3`, +// and that detail is the difference between a live case and a self-defeating +// one: a local declaration would also claim the workspace qualified name +// `Gauge`, leaving two candidates, and the fallback refuses to guess between +// two. An imported alias claims nothing, so the fallback stays unique and fires. +const Gauge = @import("dom_utils.zig").DEFAULT_NS; + +_namespace: u8 = 0, + +// ── Registered accessors ──────────────────────────────────────────────────── + +pub fn getNamespaceUri(self: *Element) u8 { + return self._namespace; +} + +// An ordinary in-file caller. The point of the defect is that the REGISTRATION +// was missing, not that the symbol looked like a leaf: a plausible-but-short +// caller list is exactly what makes `epistemic: "exact"` dangerous. +pub fn lookupNamespaceUri(self: *Element) u8 { + return self.getNamespaceUri(); +} + +// ── The control ───────────────────────────────────────────────────────────── + +// Called normally and registered NOWHERE. Its edges must not move: a change +// that hedges or re-links every method would be indistinguishable from one +// that models value references, and only the second is correct. +pub fn getTagNameLower(self: *Element) u8 { + return self._namespace; +} + +pub fn describe(self: *Element) u8 { + return self.getTagNameLower(); +} + +// ── Owner discrimination ──────────────────────────────────────────────────── + +// Shadowed below by a same-named sibling inside `JsApi`. The registration +// writes `Element.getLocalName`, so THIS is the one it must bind. +pub fn getLocalName(self: *Element) u8 { + return self._namespace; +} + +fn tick(self: *Element) u8 { + return self._namespace; +} + +// ── The binding table ─────────────────────────────────────────────────────── + +pub const JsApi = struct { + pub const bridge = Bridge(Element); + + // QUALIFIED value reference — the accessor names its container explicitly. + // This is the exact line from Element.zig:2296 that #3399 was filed over. + pub const namespaceURI = bridge.accessor(Element.getNamespaceUri, null, .{}); + + // BARE value reference to a sibling declared in this same container. + pub const tagName = bridge.accessor(_tagName, null, .{}); + + fn _tagName(self: *Element) u8 { + return self.getTagNameLower(); + } + + // A sibling with the SAME simple name as the file-struct method above. + // `walkScopeChain` gives a local binding precedence over the enclosing + // scope, so a registration resolved by TAIL NAME alone binds here — the + // wrong function, silently. Resolving `Element.getLocalName` through its + // written owner is what keeps them apart. + fn getLocalName(self: *Element) u8 { + return 0; + } + + pub const localName = bridge.accessor(Element.getLocalName, null, .{}); + + // A receiver this index cannot resolve. There is a file-level `tick`, and + // tail-name resolution would happily bind it even though the source says + // the function belongs to something else entirely. Declining is the only + // safe answer: a missing reference is recoverable, a confident wrong edge + // is not. + pub const ticker = bridge.accessor(unresolvable_ns.tick, null, .{}); + + // QUALIFIED value reference through a MODULE handle rather than a container. + // `dom_utils` is a namespace, not a class, so the class-owner lookup answers + // nothing here — and declining would be silent rather than safe: with no + // USES edge, `impact` on `compare` measures a real zero and reports `exact`, + // which is the claim this whole change exists to stop making. + pub const comparator = bridge.accessor(dom_utils.compare, null, .{}); + + // A namespace member that is NOT callable. Module receivers get the same + // callable gate as container receivers — a registration table full of + // constants must keep emitting nothing. + pub const defaultNs = bridge.accessor(dom_utils.DEFAULT_NS, null, .{}); + + // The receiver IS a known namespace import, but `dom_utils.zig` declares no + // `onlyOnDecoy` — so the namespace channel declines and the container + // channel runs, reaching `decoy.zig`'s same-named struct through the + // workspace-wide qualified-name index. A registration must NOT be minted + // there: the file wrote which module it meant. + pub const decoyed = bridge.accessor(dom_utils.onlyOnDecoy, null, .{}); + + // Through a HUB, whose published names are all imported ones. The CALL form + // resolves — `namespaceExportsIncludeImportedNames` is what makes a Zig hub + // work at all — so the REGISTRATION form has to resolve to the same def, or + // one name means two things depending on whether it is followed by `(`. + pub const scaled = bridge.accessor(hub.scale, null, .{}); + + // …and the callable gate still applies through the hub. + pub const hubNs = bridge.accessor(hub.DEFAULT_NS, null, .{}); + + // `Gauge` names this file's `const Gauge: u8`, not `Gauge.zig`'s container. + pub const level = bridge.accessor(Gauge.read, null, .{}); +}; + +// The CALL form of the same hub member, so the two are pinned side by side. +pub fn callsThroughTheHub(v: u8) u8 { + return hub.scale(v); +} + +// A LOCAL declaration shadowing the module handle. `dom_utils` here is a `u8` +// parameter with no member of its own; resolving `dom_utils.normalize` through +// the file-level import would attach the registration to a module the source +// did not name at this site — a wrong edge, the failure the same guard prevents +// on the member-CALL path. +pub fn shadowsTheModuleHandle(dom_utils: u8) u8 { + register(dom_utils.normalize); + return dom_utils; +} + +// A LOCAL declaration shadowing a CONTAINER name — the class-owner half of the +// same failure. `Ticker` here is a `u8` parameter, and this file neither +// declares nor imports the `Ticker` container that `Ticker.zig` defines. +// `findClassBindingInScope` filters the scope chain by `isClassLike`, so it +// walks straight past the parameter and its qualified-name fallback answers +// with a struct from a file this one never named. Resolving `Ticker.fire` +// through that is a confident edge to a function the source did not write. +pub fn shadowsAContainerName(Ticker: u8) u8 { + register(Ticker.fire); + return Ticker; +} + +// The positive half of the same guard: here the LOCAL declaration IS the +// container the registration names. A shadow test that only asked "is this name +// bound nearer than the module scope" would answer yes and decline — reading the +// declaration as its own shadow — and this whole class of local container would +// stop registering anything. +pub fn registersALocalContainer() u8 { + const Local = struct { + pub fn go() u8 { + return 3; + } + }; + return bridge.accessor(Local.go, null, .{}); +} + +// ── Const binding initialiser ─────────────────────────────────────────────── + +fn onReset(self: *Element) u8 { + return self._namespace; +} + +// A `const` whose initialiser IS a function value. Second value position, same +// class of drop. +pub const defaultHandler = onReset; + +// ── comptime anytype sink ─────────────────────────────────────────────────── + +var registered: ?*const fn (*Element) u8 = null; + +// Takes a callable by value into a `comptime … anytype` parameter and STORES +// it. Nothing in this file calls `f`. +pub fn register(comptime f: anytype) void { + registered = f; +} + +fn onTick(self: *Element) u8 { + return self._namespace; +} + +pub fn boot() void { + // `onTick` is never called in this file — it is handed over as a value and + // invoked later through `registered`. + register(onTick); +} + +fn Bridge(comptime T: type) type { + _ = T; + return struct { + pub fn accessor(comptime getter: anytype, comptime setter: anytype, comptime opts: anytype) u8 { + _ = getter; + _ = setter; + _ = opts; + return 0; + } + }; +} diff --git a/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Gauge.zig b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Gauge.zig new file mode 100644 index 000000000..5b6071db4 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Gauge.zig @@ -0,0 +1,10 @@ +// The unique workspace definition of the name `Gauge`. `Element.zig` never +// imports it, and declares a module-local `const Gauge` of its own — see +// `readsThroughAShadowedContainerName` there. +const Gauge = @This(); + +_level: u8 = 0, + +pub fn read(self: *Gauge) u8 { + return self._level; +} diff --git a/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Ticker.zig b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Ticker.zig new file mode 100644 index 000000000..58fb48aa5 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Ticker.zig @@ -0,0 +1,13 @@ +// A file-struct in a module `Element.zig` never imports. +// +// Its only job is to be the unique workspace definition of the name `Ticker`, +// so that `findClassBindingInScope`'s qualified-name fallback can reach it from +// a file that has no binding for that name at all. See the +// `shadowsAContainerName` case in `Element.zig`. +const Ticker = @This(); + +_ticks: u8 = 0, + +pub fn fire(self: *Ticker) u8 { + return self._ticks; +} diff --git a/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Widget.zig b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Widget.zig new file mode 100644 index 000000000..595c3aab9 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Widget.zig @@ -0,0 +1,70 @@ +// A file-as-struct whose `@This()` alias is spelled `Self`, NOT `Widget`. +// +// This is the ordinary Zig idiom, and the case #3219 originally declined. The +// container this file mints is named after the FILE STEM (`Widget`), so before +// `bindZigThisAliases` the name `Self` meant nothing class-like here: a +// file-level `@This()` alias mints no Const at all (`isZigFileThisAlias` +// suppresses it so it cannot shadow the type for `w: *Widget`), and a +// container-level one mints a Variable that every `isClassLike` walk steps +// over. Every qualified reference through the alias — a CALL as much as a +// REGISTRATION — resolved to nothing. +// +// Counted on the corpora on hand when this was fixed: 73 of ghostty's 185 +// `@This()` files, 93 of tigerbeetle's 94 and 8 of mach's 42 spell the alias +// differently from the file stem, carrying 302 `Alias.member` references, 96 +// of them calls. +// +// `Element.zig` next door is the OTHER half of the control: it writes +// `const Element = @This();` in `Element.zig`, so its qualified references +// resolved through the stem binding and must keep resolving exactly as before. +const Self = @This(); + +_w: u8 = 0, + +pub fn width(self: *Self) u8 { + return self._w; +} + +// A qualified CALL through the alias — the explicit spelling Zig allows beside +// `self.width()`, and the shape `Alias.member` takes 96 times in the corpora +// above. +pub fn describeWidth(self: *Self) u8 { + return Self.width(self); +} + +pub const WidgetApi = struct { + pub const binder = Binder(Self); + // A qualified REGISTRATION through the alias: the #3399 shape, written the + // way most Zig files spell their own type. + pub const w = binder.accessor(Self.width, null, .{}); +}; + +// A NESTED container with its own differently-spelled alias. The file-level and +// container-level aliases take different code paths — one binds at the module +// scope against the file-struct, the other at the container's own scope — so +// both are exercised. +pub const Metrics = struct { + const Me = @This(); + + _n: u8 = 0, + + pub fn read(self: *Me) u8 { + return self._n; + } + + pub fn readTwice(self: *Me) u8 { + return Me.read(self) +% Me.read(self); + } +}; + +fn Binder(comptime T: type) type { + _ = T; + return struct { + pub fn accessor(comptime g: anytype, comptime s: anytype, comptime o: anytype) u8 { + _ = g; + _ = s; + _ = o; + return 0; + } + }; +} diff --git a/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/decoy.zig b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/decoy.zig new file mode 100644 index 000000000..1197a40c8 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/decoy.zig @@ -0,0 +1,24 @@ +// A container whose NAME collides with `Element.zig`'s `dom_utils` import +// handle, in a file `Element.zig` never imports. +// +// `findClassBindingInScope` does not stop at the scope chain: when its +// `isClassLike` walk misses — and a namespace import binds a Module, not a +// class — it falls back to `scopes.qualifiedNames`, a WORKSPACE-wide index, and +// answers with the unique def of that name. This struct is that unique def. A +// registration written `dom_utils.compare` in `Element.zig` must still bind +// `dom_utils.zig`'s function, not this one: the file said which module it meant. +pub const dom_utils = struct { + pub fn compare(a: u8, b: u8) u8 { + return if (a < b) a else b; + } + + // A callable that exists ONLY here. `dom_utils.zig` has no member of this + // name, so a registration written `dom_utils.onlyOnDecoy` declines in the + // namespace channel and falls through to the container channel — where the + // workspace-wide qualified-name fallback answers with this struct. The + // owner-shadow guard is the only thing standing between that and a + // confident edge into a file `Element.zig` never imported. + pub fn onlyOnDecoy(v: u8) u8 { + return v; + } +}; diff --git a/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/dom_utils.zig b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/dom_utils.zig new file mode 100644 index 000000000..e8972ee41 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/dom_utils.zig @@ -0,0 +1,24 @@ +// A NAMESPACE-only module: no `const X = @This()`, so this file declares no +// container symbol of its own. Its members are reachable only through an +// `@import` handle — the second kind of owner a qualified name can have, and +// the one `findClassBindingInScope` cannot answer for. +// +// Lightpanda's `libdom.zig` / `parser.zig` helpers are written exactly this +// way, and they are registered into the same bridge tables as the file-struct +// methods next door. + +pub const DEFAULT_NS: u8 = 7; + +pub fn compare(a: u8, b: u8) u8 { + return if (a > b) a else b; +} + +pub fn normalize(v: u8) u8 { + return v; +} + +// Republished by `hub.zig`, and by nothing else — so an assertion about the +// hub cannot be satisfied by an edge some other case emitted. +pub fn scale(v: u8) u8 { + return v +% 1; +} diff --git a/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/hub.zig b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/hub.zig new file mode 100644 index 000000000..b99f60d9e --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/hub.zig @@ -0,0 +1,10 @@ +// A HUB module: a file made only of re-exports, the shape +// `ScopeResolver.namespaceExportsIncludeImportedNames` exists for (ghostty's +// `src/terminal/`, tigerbeetle's `stdx`). It declares nothing of its own — +// every name it publishes is a name it imported. +// +// A consumer writes `hub.scale(x)` to CALL through it, and +// `bridge.accessor(hub.scale, …)` to REGISTER through it. Those are the same +// name resolved by the same rule, so they must not disagree. +pub const scale = @import("dom_utils.zig").scale; +pub const DEFAULT_NS = @import("dom_utils.zig").DEFAULT_NS; diff --git a/gitnexus/test/fixtures/lang-resolution/zig-monorepo/README.md b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/README.md new file mode 100644 index 000000000..4ec85bc38 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/README.md @@ -0,0 +1,19 @@ +A Zig MONOREPO: several build packages, and no `build.zig` at the repo root. + +This is the layout the root-only config loader could not see at all. With no +root `build.zig` / `build.zig.zon`, `loadZigBuildConfig` answered `null`, and +every bare `@import("")` below went unresolved — so cross-file symbol +resolution in a repo like this degraded to relative imports only. + +The three packages are not interchangeable: + +- `core` declares module `core` and is imported by `app`. +- `app` declares module `app` and depends on `core` through a + `build.zig.zon` `.path = "../core"` — a spelling relative to the PACKAGE, + which has to be rebased to repo-relative `packages/core` before it can be + matched against indexed files. +- `tool` is the discriminating control. It binds the SAME alias `core` to its + OWN `src/core.zig`. A workspace that flattened every package's modules into + one repo-wide map would resolve `tool`'s `@import("core")` to `core`'s root + — a confident edge into a package `tool` never depends on. Only per-package + scoping (`zigPackageFor`, the `tsconfigFor` analogue) keeps them apart. diff --git a/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/build.zig b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/build.zig new file mode 100644 index 000000000..e237138a8 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/build.zig @@ -0,0 +1,7 @@ +const std = @import("std"); + +pub fn build(b: *std.Build) void { + const core_dep = b.dependency("core", .{}); + const app = b.addModule("app", .{ .root_source_file = b.path("src/main.zig") }); + app.addImport("core", core_dep.module("core")); +} diff --git a/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/build.zig.zon b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/build.zig.zon new file mode 100644 index 000000000..2b666b3d1 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/build.zig.zon @@ -0,0 +1,19 @@ +.{ + .name = "app", + .version = "0.1.0", + .dependencies = .{ + .core = .{ + .path = "../core", + }, + // ABSOLUTE, so it points outside the repository whichever package + // declares it. It sits in a NESTED package on purpose: that is the + // branch where the package prefix used to hide it. `packages/app/` + + // `/src` reads as the relative, in-repo `packages/app/src` — a real + // directory in this fixture — so the absolute dep silently became the + // package's own source tree instead of being rejected. + .escapes = .{ + .path = "/src", + }, + }, + .paths = .{""}, +} diff --git a/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/src/main.zig b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/src/main.zig new file mode 100644 index 000000000..26798d708 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/src/main.zig @@ -0,0 +1,18 @@ +// `core` is a PACKAGE dependency, declared by this package's build.zig.zon and +// wired in by its build.zig — not a relative path. Resolving it needs the +// package's own config, which lives two directories below the repo root. +const core = @import("core"); +const util = @import("util.zig"); + +pub fn run(attempt: u8) u8 { + return core.retryBudget(attempt); +} + +pub fn configured() u8 { + var cfg = core.Config{}; + return cfg.load(); +} + +pub fn delegated(attempt: u8) u8 { + return util.clamp(attempt); +} diff --git a/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/src/util.zig b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/src/util.zig new file mode 100644 index 000000000..5387442cd --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/app/src/util.zig @@ -0,0 +1,8 @@ +// A second file of the same package reaching the same dependency: membership in +// a build module is the root plus what it reaches, so a file that is not itself +// a module root must resolve the alias too. +const core = @import("core"); + +pub fn clamp(attempt: u8) u8 { + return core.retryBudget(attempt); +} diff --git a/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/core/build.zig b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/core/build.zig new file mode 100644 index 000000000..53ac256a7 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/core/build.zig @@ -0,0 +1,5 @@ +const std = @import("std"); + +pub fn build(b: *std.Build) void { + _ = b.addModule("core", .{ .root_source_file = b.path("src/root.zig") }); +} diff --git a/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/core/src/root.zig b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/core/src/root.zig new file mode 100644 index 000000000..b048ced4c --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/core/src/root.zig @@ -0,0 +1,13 @@ +pub const DEFAULT_RETRIES: u8 = 3; + +pub fn retryBudget(attempt: u8) u8 { + return DEFAULT_RETRIES - attempt; +} + +pub const Config = struct { + retries: u8 = DEFAULT_RETRIES, + + pub fn load(self: *Config) u8 { + return self.retries; + } +}; diff --git a/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/tool/build.zig b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/tool/build.zig new file mode 100644 index 000000000..8a3964bee --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/tool/build.zig @@ -0,0 +1,8 @@ +const std = @import("std"); + +pub fn build(b: *std.Build) void { + const tool = b.addModule("tool", .{ .root_source_file = b.path("src/main.zig") }); + // The SAME alias as `app` binds, pointing somewhere else entirely. + const own_core = b.createModule(.{ .root_source_file = b.path("src/core.zig") }); + tool.addImport("core", own_core); +} diff --git a/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/tool/src/core.zig b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/tool/src/core.zig new file mode 100644 index 000000000..1616db72b --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/tool/src/core.zig @@ -0,0 +1,5 @@ +// `tool`'s own `core`, unrelated to `packages/core`. Same module name, and that +// is the point: the name is only meaningful inside the package that binds it. +pub fn retryBudget(attempt: u8) u8 { + return attempt; +} diff --git a/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/tool/src/main.zig b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/tool/src/main.zig new file mode 100644 index 000000000..979d76d7d --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/zig-monorepo/packages/tool/src/main.zig @@ -0,0 +1,7 @@ +// Resolving this through a repo-wide module map would reach +// `packages/core/src/root.zig` — a package `tool` does not depend on. +const core = @import("core"); + +pub fn measure(attempt: u8) u8 { + return core.retryBudget(attempt); +} diff --git a/gitnexus/test/integration/impact-callable-value-references.test.ts b/gitnexus/test/integration/impact-callable-value-references.test.ts new file mode 100644 index 000000000..6124d76d3 --- /dev/null +++ b/gitnexus/test/integration/impact-callable-value-references.test.ts @@ -0,0 +1,231 @@ +/** + * Integration test: a callable named in VALUE position makes `impact` a lower + * bound (#3399). + * + * The rejected behaviour, in one sentence: `Element.getNamespaceUri` — the DOM + * `Element.namespaceURI` accessor — is bound into a JS bridge table as + * `bridge.accessor(Element.getNamespaceUri, null, .{})`, and `impact` answered + * "2 callers, LOW risk, epistemic: exact". Everything about that answer except + * the number 2 was wrong, and `exact` is the part that made it unusable: it + * tells the reader not to look further. + * + * The seed below is the shape that matters, not the language. A registration + * edge (`USES`, reason `scope-resolution: value-ref`) says a function was + * handed somewhere as a value. Where the value goes next — a struct field, a + * registry lookup, comptime reflection — is not modelled, so no CALLS edge + * connects the eventual invocation back to the target. `tools.ts` defines + * `lower-bound` as "the walk provably missed callers", and that is exactly this + * situation. + * + * WHY the assertions are what they are: + * - `impactedCount` must NOT move. Hedging is not inventing callers; a fix + * that made the number go up would be a different (and unearned) claim. + * - a plain CALLS-only target in the SAME index must stay `exact`, or the + * hedge is noise sprayed over every query and carries no information. + * - the cause has its own slot rather than being folded into + * `dispatchBoundary`: an agent branching on the numbers would otherwise be + * told an interface boundary exists where there is none, and would draw the + * opposite conclusion about whether the gap is reducible. + */ +import { it, expect, beforeAll, vi } from 'vitest'; +import path from 'node:path'; +import { LocalBackend } from '../../src/mcp/local/local-backend.js'; +import { withTestLbugDB } from '../helpers/test-indexed-db.js'; +import { VALUE_REF_EDGE_REASON } from '../../src/core/ingestion/scope-resolution/value-ref-edges.js'; + +/** + * Fail ONLY the value-reference probe, leaving every other query on the real + * database. Keyed on the bound `$reason` param rather than the query text, so + * it cannot accidentally match a different query that happens to mention USES. + */ +let failValueRefProbe = false; + +vi.mock('../../src/core/lbug/pool-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + executeParameterized: (...args: any[]) => { + const params = args[2] as { reason?: string } | undefined; + if (failValueRefProbe && params?.reason === 'scope-resolution: value-ref') { + return Promise.reject(new Error('simulated: index unreadable')); + } + return (actual as any).executeParameterized(...args); + }, + }; +}); + +vi.mock('../../src/storage/repo-manager.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + listRegisteredRepos: vi.fn().mockResolvedValue([]), + cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), + }; +}); +const { listRegisteredRepos, saveMeta } = await import('../../src/storage/repo-manager.js'); + +const SEED = [ + // The registered accessor, with one ordinary in-file caller so the walk has + // something real to report. This mirrors Element.zig: `getNamespaceUri` genuinely + // has internal callers, and it is the REGISTRATION that the answer omits. + `CREATE (:Method {id: 'Method:webapi/Element.zig:Element.getNamespaceUri', name: 'getNamespaceUri', filePath: 'webapi/Element.zig', startLine: 439, endLine: 442, isExported: true, content: '', description: ''})`, + `CREATE (:Method {id: 'Method:webapi/Element.zig:Element.lookupPrefixForElement', name: 'lookupPrefixForElement', filePath: 'webapi/Element.zig', startLine: 490, endLine: 520, isExported: true, content: '', description: ''})`, + `MATCH (a:Method {id:'Method:webapi/Element.zig:Element.lookupPrefixForElement'}), (b:Method {id:'Method:webapi/Element.zig:Element.getNamespaceUri'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:0.85, reason:'scope-resolution: local-call', step:0}]->(b)`, + + // The binding table entry: `pub const namespaceURI = bridge.accessor(Element.getNamespaceUri, null, .{});` + // A registration, NOT an invocation — hence USES, not CALLS (Kythe `ref` vs + // `ref/call`; Joern METHOD_REF). + `CREATE (:Struct {id: 'Struct:webapi/Element.zig:Element.JsApi', name: 'JsApi', filePath: 'webapi/Element.zig', startLine: 2281, endLine: 2400, content: '', description: ''})`, + `MATCH (a:Struct {id:'Struct:webapi/Element.zig:Element.JsApi'}), (b:Method {id:'Method:webapi/Element.zig:Element.getNamespaceUri'}) CREATE (a)-[:CodeRelation {type:'USES', confidence:0.85, reason:'${VALUE_REF_EDGE_REASON}', step:0}]->(b)`, + + // Control: same file, same shape of caller, but nothing registers it as a + // value. This is `getTagNameLower` — it must come back `exact`. + `CREATE (:Method {id: 'Method:webapi/Element.zig:Element.getTagNameLower', name: 'getTagNameLower', filePath: 'webapi/Element.zig', startLine: 400, endLine: 410, isExported: true, content: '', description: ''})`, + `MATCH (a:Method {id:'Method:webapi/Element.zig:Element.lookupPrefixForElement'}), (b:Method {id:'Method:webapi/Element.zig:Element.getTagNameLower'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:0.85, reason:'scope-resolution: local-call', step:0}]->(b)`, + + // Second control: an ordinary USES edge that is NOT a value registration (a + // type reference). The probe keys on the reason, so this must not hedge — + // otherwise every type mention in the repo would downgrade its target. + `CREATE (:Method {id: 'Method:webapi/Element.zig:Element.getInnerText', name: 'getInnerText', filePath: 'webapi/Element.zig', startLine: 600, endLine: 610, isExported: true, content: '', description: ''})`, + `MATCH (a:Struct {id:'Struct:webapi/Element.zig:Element.JsApi'}), (b:Method {id:'Method:webapi/Element.zig:Element.getInnerText'}) CREATE (a)-[:CodeRelation {type:'USES', confidence:0.85, reason:'scope-resolution: type-reference', step:0}]->(b)`, + + // Third control: a registration whose invocation the analyzer DID synthesize. + // `emitPropertyDispatchCalls` sweep 2 connects `x.()` member calls to + // every function registered under `` and stamps those edges + // `property-dispatch`. Where that happened the walk followed the + // registration, so hedging would be noise over an answer that was actually + // computed. This is the JS/TS hook-table shape, not the Zig one — Zig has no + // object-literal key to dispatch through and so is never excluded. + `CREATE (:Function {id: 'Function:hooks/registry.js:onClick', name: 'onClick', filePath: 'hooks/registry.js', startLine: 5, endLine: 8, isExported: true, content: '', description: ''})`, + `CREATE (:Function {id: 'Function:hooks/registry.js:registerAll', name: 'registerAll', filePath: 'hooks/registry.js', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + `CREATE (:Function {id: 'Function:hooks/consumer.js:runHandlers', name: 'runHandlers', filePath: 'hooks/consumer.js', startLine: 1, endLine: 6, isExported: true, content: '', description: ''})`, + `MATCH (a:Function {id:'Function:hooks/registry.js:registerAll'}), (b:Function {id:'Function:hooks/registry.js:onClick'}) CREATE (a)-[:CodeRelation {type:'USES', confidence:0.85, reason:'${VALUE_REF_EDGE_REASON}', step:0}]->(b)`, + `MATCH (a:Function {id:'Function:hooks/consumer.js:runHandlers'}), (b:Function {id:'Function:hooks/registry.js:onClick'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:0.7, reason:'property-dispatch', step:0}]->(b)`, +]; + +withTestLbugDB( + 'impact-callable-value-references', + (handle) => { + let backend: LocalBackend; + beforeAll(() => { + backend = (handle as any)._backend; + }); + + it('downgrades a registered accessor to lower-bound without inventing callers', async () => { + const result: any = await backend.callTool('impact', { + target: 'getNamespaceUri', + direction: 'upstream', + }); + expect(result).not.toHaveProperty('error'); + // Pinned, not implied: hedging must not INVENT callers. The seed gives + // this symbol exactly two inbound edges — one CALLS, one value-ref USES — + // and both are traversed. If a later change made the hedge also widen the + // walk, every other assertion here would still pass. + expect(result.impactedCount).toBe(2); + // The registration is a real inbound edge, so it IS traversed and counted + // — but the call THROUGH the registered value is not, which is why the + // count still cannot be the whole story. + expect(result.epistemic).toBe('lower-bound'); + expect(result.causes.callableValueReferences).toBe(1); + // Its own slot: an agent must not read this as an interface boundary. + expect(result.causes.dispatchBoundary).toBe(0); + expect(result.boundaries.join(' ')).toContain('as a VALUE'); + }); + + it('leaves a symbol with only ordinary calls exact', async () => { + const result: any = await backend.callTool('impact', { + target: 'getTagNameLower', + direction: 'upstream', + }); + expect(result).not.toHaveProperty('error'); + expect(result.epistemic).toBe('exact'); + }); + + it('does not hedge on a USES edge that is not a value registration', async () => { + const result: any = await backend.callTool('impact', { + target: 'getInnerText', + direction: 'upstream', + }); + expect(result).not.toHaveProperty('error'); + // A type reference is a use, not an escape: nothing can be invoked + // through it, so the answer stays complete. + expect(result.epistemic).toBe('exact'); + }); + + it('hedges context() for the same reason it hedges impact()', async () => { + const result: any = await backend.callTool('context', { name: 'getNamespaceUri' }); + expect(result).not.toHaveProperty('error'); + expect(result.epistemic).toBe('lower-bound'); + expect(result.causes.callableValueReferences).toBe(1); + }); + + it('stays exact when the analyzer already synthesized the dispatch', async () => { + // `onClick` is registered as a value AND reached by a synthesized + // property-dispatch CALLS edge. The walk did not "provably miss" that + // caller, so `lower-bound` would be wrong — and a signal that fires on + // every hook table in every JS codebase carries no information. + const result: any = await backend.callTool('impact', { + target: 'onClick', + direction: 'upstream', + }); + expect(result).not.toHaveProperty('error'); + expect(result.epistemic).toBe('exact'); + }); + + it('hedges — never reports exact — when the probe itself cannot run', async () => { + // The failure mode this whole feature exists to remove is silence reading + // as certainty. A probe that threw has not established that the symbol is + // unregistered; swallowing the error into a zero would publish `exact` on + // the strength of a question that was never answered. + failValueRefProbe = true; + try { + const result: any = await backend.callTool('impact', { + target: 'getTagNameLower', // the control: exact when the probe works + direction: 'upstream', + }); + expect(result).not.toHaveProperty('error'); + expect(result.epistemic).toBe('lower-bound'); + expect(result.boundaries.join(' ')).toContain('could not be run'); + // No count is claimed — the note is the signal, and inventing a + // magnitude from a failed query would be the same error inverted. + expect(result.causes.callableValueReferences).toBe(0); + } finally { + failValueRefProbe = false; + } + }); + + it('stays exact downstream — a reference INTO a symbol says nothing about what it reaches', async () => { + const result: any = await backend.callTool('impact', { + target: 'getNamespaceUri', + direction: 'downstream', + }); + expect(result).not.toHaveProperty('error'); + expect(result.epistemic).toBe('exact'); + }); + }, + { + seed: SEED, + poolAdapter: true, + afterSetup: async (h) => { + // Without a completeness receipt EVERY answer is hedged ("scope-extraction + // completeness was not recorded"), which would make the controls below + // pass for the wrong reason and prove nothing about this probe. + // `saveMeta` is the only writer production uses. + await saveMeta(path.dirname(h.dbPath), { scopeExtractionReceipt: 1 } as any); + vi.mocked(listRegisteredRepos).mockResolvedValue([ + { + name: 'test-repo', + path: '/test/repo', + storagePath: h.tmpHandle.dbPath, + indexedAt: new Date().toISOString(), + lastCommit: 'abc123', + stats: { files: 1, nodes: 5, communities: 0, processes: 0 }, + }, + ] as any); + const backend = new LocalBackend(); + await backend.init(); + (h as any)._backend = backend; + }, + }, +); diff --git a/gitnexus/test/integration/resolvers/zig.test.ts b/gitnexus/test/integration/resolvers/zig.test.ts index 724bd5368..ffdd22750 100644 --- a/gitnexus/test/integration/resolvers/zig.test.ts +++ b/gitnexus/test/integration/resolvers/zig.test.ts @@ -229,6 +229,328 @@ describe.skipIf(!zigAvailable)('Zig idioms (zig-idioms fixture)', () => { expect(calls).toContain('main → incr'); }); + /** + * #3399 — a callable named in VALUE position. + * + * `src/webapi/Element.zig` is the JS-API binding-table idiom verbatim: + * `pub const namespaceURI = bridge.accessor(Element.getNamespaceUri, null, .{});` + * registers a Zig function with the JS bridge instead of calling it. Zig + * emitted NO `value-ref` capture at all, so every one of those references was + * dropped — 2,047 of them across 257 files in lightpanda-io/browser, the whole + * JS<->Zig surface — and `impact` on a public DOM accessor answered with its + * two in-file callers and the verdict `epistemic: "exact"`. + * + * These assert USES and not CALLS on purpose. A registration is not an + * invocation (Kythe `ref` vs `ref/call`; Joern METHOD_REF), and the call that + * eventually happens goes through comptime reflection this analyzer cannot + * follow. The claim being pinned is the reference, not the dispatch. + */ + describe('callable values (#3399)', () => { + let uses: string[]; + let valueRefs: string[]; + let valueRefTargetIds: string[]; + beforeAll(() => { + const edges = getRelationships(result, 'USES'); + uses = edgeSet(edges); + valueRefTargetIds = edges + .filter((e) => e.rel.reason === 'scope-resolution: value-ref') + .map((e) => e.rel.targetId) + .sort(); + // The reason text is spelled out rather than imported from + // `VALUE_REF_EDGE_REASON`, deliberately and as `typescript-value-refs.test.ts` + // already does: `impact`'s epistemic probe matches this exact string in + // the stored graph, so a change to the constant's VALUE (as opposed to + // its name) must fail a test rather than quietly agree with itself on + // both sides. + valueRefs = edgeSet(edges.filter((e) => e.rel.reason === 'scope-resolution: value-ref')); + }); + + it('records a QUALIFIED function value handed to a call (`bridge.accessor(Element.getNamespaceUri, …)`)', () => { + expect(valueRefs).toContain('JsApi → getNamespaceUri'); + }); + + it('records a BARE function value handed to a call (`bridge.accessor(_tagName, …)`)', () => { + expect(valueRefs).toContain('JsApi → _tagName'); + }); + + it('records a function value in a const initialiser (`pub const defaultHandler = onReset;`)', () => { + expect(valueRefs).toContain('Element → onReset'); + }); + + it('records a function passed into a `comptime f: anytype` parameter and stored', () => { + // The shape the non-goal is about: `register(onTick)` stores the value in + // a module-level field and nothing in the file ever calls `onTick`. The + // terminal invoke needs comptime evaluation and is NOT modelled — but the + // reference must survive, or `onTick` reads as dead code. + expect(valueRefs).toContain('boot → onTick'); + expect(calls).not.toContain('boot → onTick'); + }); + + it('emits USES, never CALLS, for a registration', () => { + // The whole distinction: if these became CALLS, `impact` would claim the + // accessor is invoked from the binding table, which is not a fact the + // analyzer has. + expect(calls).not.toContain('JsApi → getNamespaceUri'); + expect(calls).not.toContain('JsApi → _tagName'); + }); + + it('does not mint a value reference for a non-callable argument', () => { + // `Bridge(Element)` passes a TYPE. The property-dispatch pass keeps only + // Function/Method/Constructor targets, which is what makes the broad + // capture rules safe — the same gate that stops TypeScript's + // `{ port: DEFAULT_PORT }` from registering anything. + expect(valueRefs).not.toContain('JsApi → Element'); + expect(uses.filter((u) => u === 'JsApi → Element')).toHaveLength(0); + }); + + it('leaves a method that is only ever CALLED untouched', () => { + // The control. `getTagNameLower` is called twice and registered nowhere; + // a change that sprayed USES edges over every method would satisfy every + // assertion above and still be wrong. + expect(calls).toContain('describe → getTagNameLower'); + expect(calls).toContain('_tagName → getTagNameLower'); + expect(valueRefs.filter((v) => v.endsWith(' → getTagNameLower'))).toEqual([]); + }); + + it('binds a QUALIFIED reference to the owner that was written, not the nearest lexical match', () => { + // `JsApi` declares its own `getLocalName` next to + // `bridge.accessor(Element.getLocalName, …)`. `walkScopeChain` gives that + // local binding precedence, so resolving the registration by tail name + // alone attaches it to the SIBLING — a confidently wrong edge, which is a + // worse failure than the missing edge this whole change is about. The + // written receiver is the only thing that tells them apart. + const local = valueRefTargetIds.filter((id) => id.endsWith('.getLocalName#0')); + expect(local).toEqual(['Method:src/webapi/Element.zig:Element.getLocalName#0']); + expect(local).not.toContain('Method:src/webapi/Element.zig:JsApi.getLocalName#0'); + }); + + it('declines a qualified reference whose receiver cannot be resolved', () => { + // `bridge.accessor(unresolvable_ns.tick, …)` names an owner this index + // does not have, while a file-level `tick` sits in the lexical chain + // waiting to be mis-bound. Emitting nothing is the safe direction, but be + // exact about what it buys: no edge means no evidence, and `impact` on + // `tick` therefore stays `epistemic: "exact"` — this decline costs the + // reference AND the hedge. It is still the right trade, because the + // alternative is a confident edge to a function the source did not name, + // and a wrong edge is worse than a missing one. See + // `resolveValueRefTarget`'s docstring for the same distinction, and the + // module-owner case below for the half of it that IS recoverable. + expect(valueRefTargetIds.filter((id) => id.includes('.tick#'))).toEqual([]); + expect(valueRefs).not.toContain('JsApi → tick'); + }); + + it('records a QUALIFIED function value owned by a MODULE, not a container (`bridge.accessor(dom_utils.compare, …)`)', () => { + // `dom_utils` is a namespace-only file — no `@This()`, so no container + // symbol to look the member up on. Resolving only through class-like + // owners declines here, and a decline is SILENT: with no USES edge the + // boundary probe measures a real zero and `impact` on `compare` goes back + // to `epistemic: "exact"`, which is the defect, not a conservative answer. + // The member-CALL path already resolves `dom_utils.compare()` through the + // file's namespace import; the registration reads the same channel. + expect(valueRefs).toContain('JsApi → compare'); + // And it must be dom_utils.zig's `compare`, not `decoy.zig`'s. That file + // declares a CONTAINER also called `dom_utils`, with its own `compare`, + // and `Element.zig` never imports it. `findClassBindingInScope` does not + // stop at the scope chain: a namespace handle binds a Module, so the + // `isClassLike` walk misses and its workspace-wide `qualifiedNames` + // fallback answers with that unique container — preempting the `@import` + // this very file wrote. The written import has to outrank a global guess. + expect(valueRefTargetIds.filter((id) => id.includes('compare'))).toEqual([ + 'Function:src/webapi/dom_utils.zig:compare', + ]); + }); + + it('resolves a value reference through a HUB the same way a call through it resolves', () => { + // `hub.zig` declares nothing: every name it publishes it imported + // (`pub const normalize = @import("dom_utils.zig").normalize;`). That is + // the shape `ScopeResolver.namespaceExportsIncludeImportedNames` exists + // for, and `receiver-bound-calls` honours it — so `hub.scale(v)` + // resolves. Accepting only locally-declared members here would make + // `bridge.accessor(hub.scale, …)` decline, and one name would mean + // two different things depending on whether a `(` followed it. + expect(calls).toContain('callsThroughTheHub → scale'); + expect(valueRefs).toContain('JsApi → scale'); + expect(valueRefTargetIds.filter((id) => id.includes('scale'))).toEqual([ + 'Function:src/webapi/dom_utils.zig:scale', + ]); + }); + + it('applies the callable gate through a HUB too', () => { + expect(valueRefs).not.toContain('JsApi → DEFAULT_NS'); + }); + + it('applies the callable gate to a MODULE owner too', () => { + // `dom_utils.DEFAULT_NS` is a module-scope constant. Widening the owner + // channel must not widen what counts as a registration, or every + // `bridge.accessor(mod.SOME_CONST, …)` in a binding table starts claiming + // a callable was registered. + expect(valueRefs).not.toContain('JsApi → DEFAULT_NS'); + expect(valueRefTargetIds.filter((id) => id.includes('DEFAULT_NS'))).toEqual([]); + }); + + it('declines a module-qualified reference whose handle is locally shadowed', () => { + // `shadowsTheModuleHandle(dom_utils: u8)` names a PARAMETER, not the + // file-level `@import`. Reading through the import here would attach the + // registration to a module this site never named — the same wrong-edge + // failure `isNamespaceNameShadowed` prevents on the member-call path, and + // the reason the module channel is guarded rather than merely added. + expect(valueRefs).not.toContain('shadowsTheModuleHandle → normalize'); + expect(valueRefTargetIds.filter((id) => id.includes('normalize'))).toEqual([]); + }); + + it('declines a container-qualified reference whose owner name is locally shadowed', () => { + // `shadowsAContainerName(Ticker: u8)` names a PARAMETER. This file neither + // declares nor imports `Ticker.zig`'s container, so + // `findClassBindingInScope` walks past the parameter (it filters by + // `isClassLike`) and its qualified-name fallback answers with the unique + // workspace `Ticker` — a struct the source never named at this site. + // Verified to emit `shadowsAContainerName → fire` without the guard. + expect(valueRefs).not.toContain('shadowsAContainerName → fire'); + expect(valueRefTargetIds.filter((id) => id.includes('Ticker'))).toEqual([]); + }); + + it('still binds a reference whose container IS the local declaration', () => { + // `registersALocalContainer` declares `Local` in its own body and registers + // `Local.go`. The shadow guard above must exempt the container it just + // resolved, or the nearer binding — which is that container — reads as its + // own shadow and every function-local registry stops registering. + expect(valueRefs).toContain('registersALocalContainer → go'); + }); + + it('declines a container-qualified reference shadowed at MODULE scope', () => { + // `Element.zig` binds `Gauge` at module scope to something that is NOT a + // container, and never imports `Gauge.zig`, which declares one. The class + // walk filters by `isClassLike`, steps over that binding, and its + // workspace-wide qualified-name fallback answers with the other file's + // struct. The shadow guard has to inspect the MODULE scope to catch it — + // stopping one rung short, as it did, permitted precisely this case. + // + // The binding is an IMPORT (`const Gauge = @import("dom_utils.zig").DEFAULT_NS;`), + // not a local `const Gauge: u8 = 3;`, and that is the difference between a + // live case and a self-defeating one: a local declaration would ALSO claim + // the workspace qualified name `Gauge`, leaving two candidates, and the + // fallback refuses to guess between two — so the case this test exists for + // would never be reached. See the fixture's own note at `Element.zig:30-35`. + expect(valueRefs).not.toContain('JsApi → read'); + expect(valueRefTargetIds.filter((id) => id.includes('Gauge'))).toEqual([]); + }); + + it('declines a namespace member the written module does not have, rather than reaching a same-named container', () => { + // The fall-through the channel order creates, and the guard that closes + // it. `dom_utils` IS a namespace import here, but `dom_utils.zig` has no + // `onlyOnDecoy`, so the namespace channel declines — and declining is not + // the end: `findClassBindingInScope` runs next, its `isClassLike` walk + // misses (an import binds a Module), and its WORKSPACE-WIDE + // `qualifiedNames` fallback answers with `decoy.zig`'s same-named struct, + // which does declare `onlyOnDecoy`. Only `isOwnerNameShadowedBySomethingElse` + // stands between that and a confident edge into a file this one never + // imported — the wrong-edge failure, arriving through the container + // channel after the namespace channel said no. + expect(valueRefs).not.toContain('JsApi → onlyOnDecoy'); + expect(valueRefTargetIds.filter((id) => id.includes('onlyOnDecoy'))).toEqual([]); + }); + + it('does not mint a value reference for the CALLEE of an ordinary call', () => { + // `register(onTick)` must produce ONE value reference (the argument), not + // two: without binding the callee to the `function:` field the same rule + // also matches `register` itself and every call in the repo would emit a + // USES edge shadowing its own CALLS edge. + expect(valueRefs).not.toContain('boot → register'); + expect(calls).toContain('boot → register'); + }); + }); + + /** + * `@This()` aliases (#3219 review round 8). + * + * `@This()` IS the enclosing container, and `const Self = @This();` is how + * most Zig files say so. The container itself is minted under the FILE STEM, + * and the alias bound nothing class-like — a file-level alias mints no Const + * at all, a container-level one mints a Variable that every `isClassLike` + * walk steps over — so `Self.member` resolved to nothing at all: not a wrong + * edge, no edge. On the corpora at hand that is 302 `Alias.member` + * references (ghostty 73 files, tigerbeetle 93, mach 8), 96 of them calls. + * + * `bindZigThisAliases` binds the alias name to its container in the + * post-finalize augmentation channel, so a compiler's answer and this + * index's answer agree. Both spellings of the alias are exercised: + * `Widget.zig`'s file-level `Self` and `Metrics`' container-level `Me`. + */ + describe('@This() aliases (#3219)', () => { + let valueRefs: string[]; + let valueRefTargetIds: string[]; + beforeAll(() => { + // Recomputed here rather than shared with the block above: these are + // sibling describes, and a shared `beforeAll` would make the order of + // the two blocks load-bearing. + const edges = getRelationships(result, 'USES').filter( + (e) => e.rel.reason === 'scope-resolution: value-ref', + ); + valueRefs = edgeSet(edges); + valueRefTargetIds = edges.map((e) => e.rel.targetId).sort(); + }); + + it('resolves a qualified CALL written through a file-level alias', () => { + // `Widget.zig` writes `const Self = @This();` and calls `Self.width(self)`. + expect(calls).toContain('describeWidth → width'); + expect( + getRelationships(result, 'CALLS') + .filter((e) => e.source === 'describeWidth') + .map((e) => e.rel.targetId), + ).toEqual(['Method:src/webapi/Widget.zig:Widget.width#0']); + }); + + it('resolves a qualified REGISTRATION written through a file-level alias', () => { + // The #3399 shape spelled the ordinary way: `binder.accessor(Self.width, …)`. + // Declining it cost the reference AND the hedge — no edge means no + // evidence, so `impact` on `width` went back to claiming `exact`. + expect(valueRefs).toContain('WidgetApi → width'); + expect(valueRefTargetIds.filter((id) => id.includes('Widget.width'))).toEqual([ + 'Method:src/webapi/Widget.zig:Widget.width#0', + ]); + }); + + it('resolves a qualified call through a CONTAINER-level alias', () => { + // `Metrics` declares `const Me = @This();`, which mints a Variable beside + // the Struct — the binding is there, it is just not class-like, so the + // walk stepped over it and kept climbing. + expect(calls).toContain('readTwice → read'); + expect( + getRelationships(result, 'CALLS') + .filter((e) => e.source === 'readTwice') + .map((e) => e.rel.targetId), + ).toEqual([ + 'Method:src/webapi/Widget.zig:Metrics.read#0', + 'Method:src/webapi/Widget.zig:Metrics.read#0', + ]); + }); + + it('leaves a stem-spelled alias resolving exactly as it did', () => { + // The control. `Element.zig` writes `const Element = @This();`, so its + // qualified references already resolved through the stem binding. The + // alias binding is an ADDITION to the augmentation channel, consulted + // only after a scope's own bindings, so it must move nothing here. + expect(valueRefTargetIds.filter((id) => id.endsWith('.getNamespaceUri#0'))).toEqual([ + 'Method:src/webapi/Element.zig:Element.getNamespaceUri#0', + ]); + expect(valueRefs).toContain('JsApi → getNamespaceUri'); + }); + + it('does not make the alias name resolvable from another file', () => { + // `Self` and `Me` are container-private: Zig has no way to import them, + // and the binding is appended at the declaring scope only. If it leaked + // to the workspace channels, every file in a repo would see one + // arbitrary `Self` — 66 files in ghostty declare that exact name. + const targets = valueRefTargetIds.concat( + getRelationships(result, 'CALLS').map((e) => e.rel.targetId), + ); + // The alias's OWN def (`Metrics.Me`, a Variable) must never be an edge + // target — matched on the last segment so `Metrics.read` is not read as + // a hit on `Me`. + expect(targets.filter((id) => /[:.](Self|Me)(#\d+)?$/.test(id))).toEqual([]); + }); + }); + it('types a receiver from its ANNOTATION (`var b: Counter = undefined; b.twice()`, `const c: Counter = .init(); c.get()`)', () => { // The declared type is the ONLY type source for `= undefined` and for // 0.14+ decl literals (`.init`, `.empty`), which current std uses for @@ -1207,3 +1529,64 @@ describe.skipIf(!zigAvailable)( }); }, ); + +// ── Monorepo: several build packages, no build.zig at the repo root ────────── +// +// The layout the root-only config loader could not see. `loadZigBuildConfig` +// read `/build.zig{,.zon}` and nothing else, so a repo whose packages +// live under `packages//` had no config at all and every bare +// `@import("")` in it went unresolved — cross-file resolution silently +// degraded to relative imports. It now takes a `packageDir` and +// `loadZigWorkspaceIndex` walks the repo for the packages to hand it, which is +// the change these assert. +// +// They assert the EDGES, not the config: the unit tests in +// `test/unit/zig-import-resolver.test.ts` pin the index, and this pins that the +// index actually reaches symbol resolution. +describe.skipIf(!zigAvailable)('Zig monorepo package resolution', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'zig-monorepo'), () => {}); + }, 60000); + + it('resolves a cross-package @import declared by the package’s own build files', () => { + // `packages/app` depends on `packages/core` through `.path = "../core"` — + // package-relative, and rejected outright when read from the repo root. + const imports = getRelationships(result, 'IMPORTS').filter((e) => + e.sourceFilePath.includes('packages/app/src/main.zig'), + ); + expect(imports.map((e) => e.targetFilePath).join('\n')).toContain('packages/core/src/root.zig'); + }); + + it('emits CALLS across the package boundary, from the module root and from a non-root file', () => { + const calls = getRelationships(result, 'CALLS'); + const crossPackage = calls.filter( + (e) => + e.sourceFilePath.includes('packages/app/') && + e.targetFilePath.includes('packages/core/src/root.zig'), + ); + // `run` is in the module ROOT, `clamp` in a sibling file of the same + // package: membership is the root plus what it reaches, so both must + // resolve the alias — a fix that only worked for module roots would pass an + // assertion on `run` alone. + expect(edgeSet(crossPackage)).toContain('run → retryBudget'); + expect(edgeSet(crossPackage)).toContain('clamp → retryBudget'); + }); + + it('binds one alias to two different roots in two packages without crossing them', () => { + // The discriminating case, and the reason the index is scoped per package + // rather than flattened repo-wide: `tool` binds `core` to its OWN + // src/core.zig. Flattened, `measure` would call into `packages/core` — a + // confident edge into a package `tool` does not depend on, which is worse + // than the unresolved import this change set out to fix. + const fromTool = getRelationships(result, 'CALLS').filter((e) => + e.sourceFilePath.includes('packages/tool/src/main.zig'), + ); + expect(edgeSet(fromTool)).toContain('measure → retryBudget'); + expect(fromTool.map((e) => e.targetFilePath).join('\n')).toContain( + 'packages/tool/src/core.zig', + ); + expect(fromTool.map((e) => e.targetFilePath).join('\n')).not.toContain('packages/core/'); + }); +}); diff --git a/gitnexus/test/unit/incremental-parse-cache.test.ts b/gitnexus/test/unit/incremental-parse-cache.test.ts index d0657607f..7ae7f152d 100644 --- a/gitnexus/test/unit/incremental-parse-cache.test.ts +++ b/gitnexus/test/unit/incremental-parse-cache.test.ts @@ -266,12 +266,25 @@ describe('PARSE_CACHE_VERSION', () => { // recognizes form feed and vertical tab as C preprocessing whitespace. // Moved 96 -> 97 for #3179: comment-prefixed directives and invalid numeric // marker prefixes change the parse-time normalization result. - it('pins SCHEMA_BUMP to 97 so concurrent bumps cannot silently collide (#2766, #3015, #3088, #2885, #3128, #2865, #3130, #1432, #3161, #3179)', () => { - expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(97); + // Moved 97 -> 98 for #3219 (Zig callable-value references): `ZIG_SCOPE_QUERY` + // gained three `@reference.value-ref` rules, so a `.zig` file now yields + // `value-ref` entries in `ParsedFile.referenceSites` where it yielded none. + // A warm pre-v98 cache replays the old, empty site list for every unchanged + // file — `--force` included, since shards are content-addressed — so no USES + // edge is emitted, the boundary probe measures a real zero, and `impact` on a + // registered accessor goes back to `epistemic: "exact"`: the #3399 defect, + // silently un-fixed on exactly the incremental path most users are on. + // 94-97 belong to #3179, which merged first; 98 is the next value above it. + // Re-checked against origin/main and every open PR touching parse-cache.ts at + // merge time — the rule the paragraphs above were written by PRs that each + // checked only once — and the remaining open claims (#3190 at 94, #2840 at + // 71, #1616 at 2) all sit below main and must re-bump themselves. + it('pins SCHEMA_BUMP to 98 so concurrent bumps cannot silently collide (#2766, #3015, #3088, #2885, #3128, #2865, #3130, #1432, #3161, #3179, #3219)', () => { + expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(98); expect(PARSE_CACHE_BUCKET_COUNT).toBe(128); for (const taken of [ 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, - 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, + 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, ]) { expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).not.toBe(taken); } diff --git a/gitnexus/test/unit/scope-resolution/value-ref-dispatchability.test.ts b/gitnexus/test/unit/scope-resolution/value-ref-dispatchability.test.ts new file mode 100644 index 000000000..3527587af --- /dev/null +++ b/gitnexus/test/unit/scope-resolution/value-ref-dispatchability.test.ts @@ -0,0 +1,173 @@ +/** + * Canary for the invariant that `callableValueReferenceBoundaries`' dispatch + * exclusion silently depends on (#3219 review round 3). + * + * The exclusion, in `mcp/local/local-backend.ts`: a target with an inbound + * `property-dispatch` CALLS edge is NOT hedged, because the analyzer followed + * the registration and nothing was missed. It is symbol-level, not edge-level — + * the graph does not record which registration produced which synthesized call. + * + * That is only sound while no single symbol can carry BOTH kinds of + * registration, and today none can, for a reason that lives nowhere near the + * exclusion: + * + * - `emitPropertyDispatchCalls` synthesizes a CALLS edge only for a + * registration whose site carries a `propertyKey` (sweep 1 skips the + * registration index when it is undefined; sweep 2 reads only that index). + * - Every JS/TS `@reference.value-ref` rule also captures + * `@reference.property-key` — both are object-literal shapes. + * - No Zig `@reference.value-ref` rule captures one: Zig has no + * object-literal key to dispatch through. + * + * So a dispatchable registration is always a JS/TS one and an undispatchable + * registration is always a Zig one, and the two cannot meet on one symbol. + * + * The day that stops being true — a JS/TS rule for a bare callback argument + * (`register(handler)`), a Zig rule that grows a key — a symbol CAN have both, + * and the exclusion starts publishing `exact` over a registration the analyzer + * provably did not follow. That is the #3399 defect returning through a side + * door, and it would not fail a single existing test. + * + * This test fails instead. If it fails, do not relax it: go and decide what + * `callableValueReferenceBoundaries` should do about a mixed symbol (the + * options are recorded at the exclusion site), then update this file. + * + * WHAT IT DOES NOT COVER, stated so the green tick is not read as more than it + * is. It reads query SOURCES, so a capture synthesized in code rather than + * matched by a rule — the mechanism `@reference.static-gated` uses — can break + * the partition with this test green. A provider adding one has to come here by + * hand. Languages that own no query and delegate to another's captures (Vue → + * `emitTsScopeCaptures` / `emitJsScopeCaptures`) are covered transitively, by + * the rules they borrow, which is why the last case asserts on query OWNERS + * rather than on the set of languages that can emit a value-ref. + */ +import { describe, it, expect } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { + TSX_JSX_QUERY_SUFFIX, + TYPESCRIPT_SCOPE_QUERY, +} from '../../../src/core/ingestion/languages/typescript/query.js'; +import { JAVASCRIPT_SCOPE_QUERY } from '../../../src/core/ingestion/languages/javascript/query.js'; +import { ZIG_SCOPE_QUERY } from '../../../src/core/ingestion/languages/zig/query.js'; + +const VALUE_REF = '@reference.value-ref'; +const PROPERTY_KEY = '@reference.property-key'; + +/** + * Split a tree-sitter scope query into its top-level s-expression rules. + * + * `;;` comments are dropped first — they discuss the very tags this test + * matches on (Zig's rules carry a paragraph explaining why they attach no + * property key), so leaving them in would make every Zig rule look keyed. + * Double-quoted anonymous nodes (`"const"`, `"("`) are skipped while counting + * depth: a query that matches a literal paren would otherwise unbalance it. + */ +function topLevelRules(query: string): string[] { + const src = query + .split('\n') + .map((line) => { + const comment = line.indexOf(';;'); + return comment === -1 ? line : line.slice(0, comment); + }) + .join('\n'); + + const rules: string[] = []; + let depth = 0; + let start = -1; + let inString = false; + for (let i = 0; i < src.length; i++) { + const ch = src[i]; + if (inString) { + if (ch === '\\') i++; + else if (ch === '"') inString = false; + continue; + } + if (ch === '"') { + inString = true; + continue; + } + if (ch === '(') { + if (depth === 0) start = i; + depth++; + } else if (ch === ')') { + depth--; + if (depth === 0 && start !== -1) { + rules.push(src.slice(start, i + 1)); + start = -1; + } + if (depth < 0) depth = 0; + } + } + return rules; +} + +function valueRefRules(query: string): string[] { + return topLevelRules(query).filter((rule) => rule.includes(VALUE_REF)); +} + +describe('value-ref dispatchability partition', () => { + it('splits a query into rules without being confused by comments or literal parens', () => { + // Guards the guard: a splitter that silently returned [] would make every + // assertion below vacuously true. + const rules = topLevelRules(` +;; a comment mentioning (parens) and ${PROPERTY_KEY} +(call_expression + function: (_) + (identifier) @reference.name) + +(variable_declaration + "const" . (identifier) @a .) +`); + expect(rules).toHaveLength(2); + expect(rules[0]).toContain('call_expression'); + expect(rules[1]).toContain('variable_declaration'); + expect(rules.join('\n')).not.toContain(PROPERTY_KEY); + }); + + it('every TypeScript value-ref rule is DISPATCHABLE (carries a property key)', () => { + // The BASE query plus the TSX suffix, because `getTsScopeQuery` concatenates + // them for a `.tsx` file: a value-ref rule added to the suffix alone would + // be emitted in TSX analysis while a base-only check stayed green. + const rules = valueRefRules(TYPESCRIPT_SCOPE_QUERY + TSX_JSX_QUERY_SUFFIX); + expect(rules.length).toBeGreaterThan(0); + expect(rules.filter((r) => !r.includes(PROPERTY_KEY))).toEqual([]); + }); + + it('every JavaScript value-ref rule is DISPATCHABLE (carries a property key)', () => { + const rules = valueRefRules(JAVASCRIPT_SCOPE_QUERY); + expect(rules.length).toBeGreaterThan(0); + expect(rules.filter((r) => !r.includes(PROPERTY_KEY))).toEqual([]); + }); + + it('every Zig value-ref rule is UNDISPATCHABLE (carries no property key)', () => { + const rules = valueRefRules(ZIG_SCOPE_QUERY); + expect(rules.length).toBeGreaterThan(0); + expect(rules.filter((r) => r.includes(PROPERTY_KEY))).toEqual([]); + }); + + it('no OTHER language OWNS a value-ref rule', () => { + // The three above are hand-classified. A fourth query declaring + // `value-ref` has not been classified by anyone, so the exclusion's premise + // is unverified for it — classify it here and in the exclusion's comment. + // "Owns", not "emits": Vue has no query of its own and borrows TypeScript's + // and JavaScript's captures, so it inherits their classification rather than + // needing one. A capture synthesized in code owns no rule either and is + // invisible here — see the header. + const languagesDir = path.join( + path.dirname(fileURLToPath(import.meta.url)), + '../../../src/core/ingestion/languages', + ); + const emitting = fs + .readdirSync(languagesDir, { withFileTypes: true }) + .filter((e) => e.isDirectory()) + .filter((e) => { + const query = path.join(languagesDir, e.name, 'query.ts'); + return fs.existsSync(query) && fs.readFileSync(query, 'utf8').includes(VALUE_REF); + }) + .map((e) => e.name) + .sort(); + expect(emitting).toEqual(['javascript', 'typescript', 'zig']); + }); +}); diff --git a/gitnexus/test/unit/scope-resolution/value-ref-namespace-precedence.test.ts b/gitnexus/test/unit/scope-resolution/value-ref-namespace-precedence.test.ts new file mode 100644 index 000000000..67c3019a9 --- /dev/null +++ b/gitnexus/test/unit/scope-resolution/value-ref-namespace-precedence.test.ts @@ -0,0 +1,170 @@ +/** + * `findNamespaceValueRefTarget` precedence: a name the target module DECLARES + * beats a name it merely re-publishes, and it beats it whether or not the + * declaration happens to be callable (#3219 review round 8). + * + * The channel resolves `bridge.accessor(hub.scale, …)` by reading the target + * module's own module-scope bindings first and its re-published ones second. + * The order is only meaningful if the FIRST lookup decides on the NAME: it + * applies `CALL_TARGET_TYPES` while it selects, so a target module declaring a + * non-callable `scale` answers nothing there and — before this guard — fell + * through to the published channel and bound an imported callable under a name + * the module's own declaration owns. `findExportedDef` does not behave that + * way: it returns any local def and lets the caller's type gate reject it, so + * `findExportedDefIncludingImportedNames` never reaches the imported names for + * a name the file declares. `hub.scale` and `hub.scale()` must not disagree + * about which module owns `scale`. + * + * WHY THE INDEXES ARE HAND-BUILT, stated so this is not read as a fixture that + * "just happens" to be synthetic. Zig forbids declaring a name twice in one + * container, and Zig is today the only provider that sets + * `namespaceExportsIncludeImportedNames`, so no valid Zig source can put a + * local non-callable and a published callable under one name in one module — + * there is no source-level fixture to write. The shape becomes reachable the + * moment a second provider opts in, or a receiver name binds more than one + * target file. Building the indexes directly is what lets the guard be pinned + * before that happens; the middle case below fails without it. + */ + +import { describe, it, expect } from 'vitest'; +import { resolveValueRefTarget } from '../../../src/core/ingestion/scope-resolution/passes/property-dispatch.js'; +import type { BindingRef, ReferenceSite, Scope, ScopeId, SymbolDefinition } from 'gitnexus-shared'; +import type { ScopeResolutionIndexes } from '../../../src/core/ingestion/model/scope-resolution-indexes.js'; +import type { SemanticModel } from '../../../src/core/ingestion/model/semantic-model.js'; + +const CONSUMER_FILE = 'src/Element.zig'; +const HUB_FILE = 'src/hub.zig'; +const CONSUMER = 'scope:consumer:module' as ScopeId; +const HUB = 'scope:hub:module' as ScopeId; + +const def = (nodeId: string, type: string, filePath: string): SymbolDefinition => + ({ nodeId, name: 'scale', type, filePath }) as unknown as SymbolDefinition; + +const ref = (d: SymbolDefinition, origin: string): BindingRef => + ({ def: d, origin }) as unknown as BindingRef; + +/** The callable `hub.zig` re-publishes from `dom_utils.zig`. */ +const PUBLISHED = def('Function:src/dom_utils.zig:scale', 'Function', 'src/dom_utils.zig'); +/** A `scale` the hub declares ITSELF, in the two kinds that matter. */ +const LOCAL_CONST = def('Const:src/hub.zig:scale', 'Const', HUB_FILE); +const LOCAL_FN = def('Function:src/hub.zig:scale', 'Function', HUB_FILE); + +function moduleScope(id: ScopeId, filePath: string): Scope { + return { + id, + kind: 'Module', + parent: null, + filePath, + range: { startLine: 1, startCol: 0, endLine: 99, endCol: 0 }, + bindings: new Map(), + typeBindings: new Map(), + imports: [], + ownedDefs: [], + } as unknown as Scope; +} + +/** + * Everything `resolveValueRefTarget` reads for a qualified site whose receiver + * is a namespace handle: the consumer's namespace import edge, and the target + * module's local (`scopes.bindings`) and published (`bindingAugmentations`) + * channels. `qualifiedNames` answers nothing so the CONTAINER channel — which + * runs only if this one declines — cannot supply the resolution instead and + * make a declining assertion pass for the wrong reason. + */ +function indexes(opts: { + local?: readonly BindingRef[]; + published?: readonly BindingRef[]; +}): ScopeResolutionIndexes { + const scopesById = new Map([ + [CONSUMER, moduleScope(CONSUMER, CONSUMER_FILE)], + [HUB, moduleScope(HUB, HUB_FILE)], + ]); + const bindings = new Map>(); + if (opts.local !== undefined) bindings.set(HUB, new Map([['scale', opts.local]])); + const augmentations = new Map>(); + if (opts.published !== undefined) augmentations.set(HUB, new Map([['scale', opts.published]])); + return { + scopeTree: { getScope: (id: ScopeId) => scopesById.get(id) }, + moduleScopes: new Map([ + [CONSUMER_FILE, CONSUMER], + [HUB_FILE, HUB], + ]), + imports: new Map([[CONSUMER, [{ kind: 'namespace', localName: 'hub', targetFile: HUB_FILE }]]]), + bindings, + bindingAugmentations: augmentations, + workspaceFqnBindings: new Map(), + namespaceFqnBindings: new Map(), + accessibleNamespacesByScope: new Map(), + defs: new Map(), + qualifiedNames: { get: () => [], has: () => false, size: 0 }, + } as unknown as ScopeResolutionIndexes; +} + +/** `bridge.accessor(hub.scale, …)` written at the consumer's module scope. */ +const SITE = { + name: 'scale', + kind: 'value-ref', + inScope: CONSUMER, + explicitReceiver: { name: 'hub' }, + atRange: { startLine: 10, startCol: 4, endLine: 10, endCol: 20 }, +} as unknown as ReferenceSite; + +const MODEL = {} as unknown as SemanticModel; + +const resolve = (scopes: ScopeResolutionIndexes): SymbolDefinition | undefined => + resolveValueRefTarget(SITE, CONSUMER_FILE, scopes, MODEL, true); + +describe('findNamespaceValueRefTarget — local declarations outrank re-published ones', () => { + it('resolves a re-published callable when the hub declares nothing under the name', () => { + // The control: this is the HUB case the channel exists for, and every + // assertion below is only meaningful while it holds. If the guard were + // written as "decline whenever anything is bound in the target module", + // this is what would break. + expect(resolve(indexes({ published: [ref(PUBLISHED, 'reexport')] }))?.nodeId).toBe( + PUBLISHED.nodeId, + ); + }); + + it('declines when the hub declares a NON-callable under the name', () => { + // The regression. The local lookup type-gates before precedence is settled, + // so `scale` answered nothing locally and the published channel bound + // `dom_utils.scale` — a USES edge to a member the hub does not expose under + // that name, and the opposite of what `hub.scale()` resolves to. + expect( + resolve( + indexes({ + local: [ref(LOCAL_CONST, 'local')], + published: [ref(PUBLISHED, 'reexport')], + }), + ), + ).toBeUndefined(); + }); + + it('still prefers a local CALLABLE over a re-published one', () => { + // The guard suppresses the published channel; it must not suppress the + // local answer that made the precedence rule worth stating. + expect( + resolve( + indexes({ + local: [ref(LOCAL_FN, 'local')], + published: [ref(PUBLISHED, 'reexport')], + }), + )?.nodeId, + ).toBe(LOCAL_FN.nodeId); + }); + + it('declines a re-published callable for a provider that does not publish its imports', () => { + // `namespaceExportsIncludeImportedNames` is opt-in: in a language where a + // module's imports are NOT its exports, the hub channel stays closed and + // the guard above is never consulted. + expect( + resolveValueRefTarget( + SITE, + CONSUMER_FILE, + indexes({ published: [ref(PUBLISHED, 'reexport')] }), + MODEL, + false, + ), + ).toBeUndefined(); + }); +}); diff --git a/gitnexus/test/unit/zig-import-resolver.test.ts b/gitnexus/test/unit/zig-import-resolver.test.ts index 1c23b9f3d..932012d18 100644 --- a/gitnexus/test/unit/zig-import-resolver.test.ts +++ b/gitnexus/test/unit/zig-import-resolver.test.ts @@ -8,10 +8,12 @@ import { fileURLToPath } from 'node:url'; import { resolveZigImportInternal } from '../../src/core/ingestion/import-resolvers/zig.js'; import { loadZigBuildConfig, + loadZigWorkspaceIndex, parseZigBuildModules, parseZigRootModules, parseZigBuildModuleRoots, parseZigBuildZon, + zigPackageFor, } from '../../src/core/ingestion/language-config.js'; const FIXTURES = path.resolve( @@ -816,3 +818,138 @@ describe('loadZigBuildConfig (zig-idioms fixture)', () => { expect(config!.moduleRoots?.has('oldlib')).toBe(false); }); }); + +describe('loadZigWorkspaceIndex / zigPackageFor (zig-monorepo fixture)', () => { + const MONOREPO = path.join(FIXTURES, 'zig-monorepo'); + /** Every `.zig` the fixture indexes, repo-relative — what `allFilePaths` holds. */ + const files = new Set([ + 'packages/core/build.zig', + 'packages/core/src/root.zig', + 'packages/app/build.zig', + 'packages/app/src/main.zig', + 'packages/app/src/util.zig', + 'packages/tool/build.zig', + 'packages/tool/src/core.zig', + 'packages/tool/src/main.zig', + ]); + + it('finds the packages the root-only loader cannot see at all', async () => { + // The regression this exists for: with no build.zig at the repo root the + // root-only loader has nothing to read and answers null, so every bare + // `@import` in the repo is unresolvable. Asserted side by side so the + // difference is the test rather than a claim in a comment. + expect(await loadZigBuildConfig(MONOREPO)).toBeNull(); + + const index = await loadZigWorkspaceIndex(MONOREPO); + expect(index).not.toBeNull(); + expect(index!.packages.map((p) => p.dir)).toEqual([ + 'packages/core', + 'packages/tool', + 'packages/app', + ]); + }); + + it('rebases a package-relative `.path` dep to repo-relative', async () => { + // `app/build.zig.zon` writes `.path = "../core"`, which means nothing + // against `allFilePaths` (repo-relative keys) and is rejected outright by + // `normalizeZigDepPath` as an escape when read from the repo root. + const app = zigPackageFor(await loadZigWorkspaceIndex(MONOREPO), 'packages/app/src/main.zig'); + expect(app!.pathDeps.get('core')).toBe('packages/core'); + expect(app!.moduleRoots?.get('core')).toEqual(['packages/core/src/root.zig']); + }); + + it('resolves a cross-package dependency from every file of the dependent package', async () => { + const index = await loadZigWorkspaceIndex(MONOREPO); + // The module root, and a file that is NOT the module root: membership is + // the root plus what it reaches, so both must resolve the alias. + for (const from of ['packages/app/src/main.zig', 'packages/app/src/util.zig']) { + expect(resolveZigImportInternal(from, 'core', files, zigPackageFor(index, from))).toBe( + 'packages/core/src/root.zig', + ); + } + }); + + it('keeps one alias bound to two different roots in two packages apart', async () => { + // The discriminating case. `tool` binds `core` to its OWN src/core.zig. A + // repo-wide module map — the shape a workspace index invites — would answer + // `packages/core/src/root.zig` here: a confident edge into a package `tool` + // does not depend on, which is worse than the unresolved import this change + // set out to fix. + const index = await loadZigWorkspaceIndex(MONOREPO); + const from = 'packages/tool/src/main.zig'; + expect(resolveZigImportInternal(from, 'core', files, zigPackageFor(index, from))).toBe( + 'packages/tool/src/core.zig', + ); + }); + + it('does not leak one package’s modules to files outside it', async () => { + // `core` declares module `core`; `tool` never depends on `app`. A file in + // one package must not resolve another package's module name. + const index = await loadZigWorkspaceIndex(MONOREPO); + const from = 'packages/core/src/root.zig'; + expect(resolveZigImportInternal(from, 'app', files, zigPackageFor(index, from))).toBeNull(); + }); + + it('governs a file by the NEAREST enclosing package', async () => { + const index = await loadZigWorkspaceIndex(MONOREPO); + expect(zigPackageFor(index, 'packages/core/src/root.zig')!.rootModules?.get('core')).toBe( + 'packages/core/src/root.zig', + ); + // A path under no package at all resolves to no config rather than to the + // first package in the list. + expect(zigPackageFor(index, 'docs/notes.zig')).toBeNull(); + }); + + it('keeps a genuinely single-package repo byte-identical to the root-only loader', async () => { + // `libs/geo` is one directory with one build.zig and no nested marker + // anywhere below it, so the workspace walk finds exactly one package and + // this really does pin the no-op case. `zig-idioms` cannot: it declares + // `libs/geo` as a path dep and that directory has its own build.zig, so the + // walk finds TWO packages there — the assertion below is the one the next + // test makes, and naming this file "single-package" would be the claim, not + // the check. + const single = path.join(FIXTURES, 'zig-idioms', 'libs', 'geo'); + const index = await loadZigWorkspaceIndex(single); + expect(index!.packages.map((p) => p.dir)).toEqual(['']); + expect(index!.packages[0]!.config).toEqual(await loadZigBuildConfig(single)); + expect(zigPackageFor(index, 'src/root.zig')).toEqual(await loadZigBuildConfig(single)); + }); + + it('leaves the ROOT package of a multi-package repo as the root-only loader saw it', async () => { + // The root package is a scope like any other and `dir: ''` matches every + // file no deeper package claims, so a file outside `libs/geo` must still + // get exactly what `loadZigBuildConfig` alone used to answer. + const idioms = path.join(FIXTURES, 'zig-idioms'); + const index = await loadZigWorkspaceIndex(idioms); + expect(index!.packages.map((p) => p.dir)).toEqual(['libs/geo', '']); + const root = index!.packages.find((p) => p.dir === ''); + expect(root).toBeDefined(); + expect(root!.config).toEqual(await loadZigBuildConfig(idioms)); + expect(zigPackageFor(index, 'src/idioms.zig')).toEqual(await loadZigBuildConfig(idioms)); + // …and a file INSIDE the nested package is governed by that package, not + // by the root — the regression the misnamed version of this test could not + // have caught, because it never looked below `src/`. + expect(zigPackageFor(index, 'libs/geo/src/root.zig')).toEqual( + await loadZigBuildConfig(idioms, 'libs/geo'), + ); + }); + + it('rejects an ABSOLUTE `.path` in a nested package instead of rebasing it in', async () => { + // `packages/app` declares `.escapes = .{ .path = "/src" }`. Absolute, so it + // names something outside this repository — but the nested branch prefixes + // the package directory before normalizing, and `packages/app/` + `/src` is + // `packages/app//src`, which is relative by inspection. The empty segment is + // then dropped and the dep lands on `packages/app/src`, a directory that + // really exists here: an out-of-repo dependency fabricated into an in-repo + // resolution. `isAbsoluteZigDepPath` asks the question of the value AS + // WRITTEN, before any prefixing. + // + // `path.posix.join` would NOT have fixed this: it strips the leading slash + // too, producing the same `packages/app/src` without rejecting anything. + const app = zigPackageFor(await loadZigWorkspaceIndex(MONOREPO), 'packages/app/src/main.zig'); + expect(app!.pathDeps.has('escapes')).toBe(false); + expect(app!.moduleRoots?.has('escapes') ?? false).toBe(false); + // The legitimate package-relative dep beside it is untouched. + expect(app!.pathDeps.get('core')).toBe('packages/core'); + }); +}); diff --git a/gitnexus/vitest.config.ts b/gitnexus/vitest.config.ts index d2534c025..7d69f1ce1 100644 --- a/gitnexus/vitest.config.ts +++ b/gitnexus/vitest.config.ts @@ -64,6 +64,7 @@ export default defineConfig({ test: { name: 'lbug-db', include: [ + 'test/integration/impact-callable-value-references.test.ts', 'test/integration/impact-epistemic-lower-bound.test.ts', 'test/integration/impact-scope-omission-persistence.test.ts', 'test/integration/lbug-core-adapter.test.ts', @@ -143,6 +144,7 @@ export default defineConfig({ sequence: { groupOrder: 3 }, include: ['test/**/*.test.ts'], exclude: [ + 'test/integration/impact-callable-value-references.test.ts', 'test/integration/impact-epistemic-lower-bound.test.ts', 'test/integration/impact-scope-omission-persistence.test.ts', 'test/integration/lbug-core-adapter.test.ts',