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',