mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-11 03:38:07 +00:00
fix(review): guard the class receiver against a shadowing binding, and pin the dispatchability partition
Review round 3 (`gitnexus-check` bot on `cf53bbaa`). Three findings, each
reproduced against the code before deciding.
R3-1 (Error, valid, REPRODUCED) — a CLASS receiver could resolve through a
shadowing value binding. `findClassBindingInScope` is a class-only walk: it
filters the scope chain by `isClassLike`, so it steps over a nearer binding that
is a value and keeps climbing — and past the chain entirely, into a
qualified-name fallback that answers with the unique workspace definition of the
name. A `u8` parameter named `Ticker`, in a file that neither declares nor
imports the `Ticker` container another file defines, therefore emitted
`register(Ticker.fire)` as a confident USES edge to that container's method.
That is exactly the wrong-edge failure R1-2 exists to prevent, arriving through
the class channel instead of the lexical one.
Fixed with `isOwnerNameShadowedBySomethingElse` — a sibling of
`isNamespaceNameShadowed` with one extra clause. The plain namespace guard could
NOT be reused: a container is often its own local declaration
(`fn make() { const Local = struct {…}; register(Local.go); }`), and reading that
binding as its own shadow suppresses precisely the resolutions this path exists
to make — the #2723 mistake, one channel over. So a scope that binds the name
answers immediately, and the answer is "not shadowed" only when one of that
scope's own bindings IS the def just resolved.
Both halves are pinned and both were verified to fail when the guard is
weakened: the parameter case fails with no guard at all, the local-container
case fails with the plain `isNamespaceNameShadowed`.
R3-2 (Warning; mechanism correct, unreachable today; fragility fixed instead) —
the dispatch exclusion could suppress an unfollowed registration. The bot is
right about the code: sweep 2 synthesizes CALLS only for a registration whose
site carried a `propertyKey`, while the exclusion zeroes the note on ANY inbound
`property-dispatch` CALLS edge. It is not reachable in the current rule set, and
the reason is measured rather than assumed: `@reference.value-ref` is emitted by
exactly three languages — JavaScript (2 rules), TypeScript (2), Zig (3) — every
JS/TS rule also captures `@reference.property-key` (both are object-literal
shapes) and no Zig rule does. A dispatchable registration is therefore always a
JS/TS one, an undispatchable one always a Zig one, and they cannot meet on one
symbol.
Rejected: splitting the edge `reason` into dispatchable / undispatchable. It is
the precise fix, but it is a graph-content change that churns whichever side
keeps the old literal — the Zig, TypeScript and probe suites all pin
'scope-resolution: value-ref' by hand as a drift canary — and it buys nothing
against a case no rule can produce.
What was actually wrong is that the exclusion's soundness rested on a
coincidence recorded nowhere, in files nobody reading `local-backend.ts` would
open. Fixed at both ends: the exclusion site now states the invariant, the three
facts it rests on and the two options for when it breaks; and
`value-ref-dispatchability.test.ts` fails the day it does — a JS/TS rule for a
bare callback argument, a Zig rule that grows a key, or a fourth language
emitting `value-ref` at all. Verified to fire (adding a property key to a Zig
value-ref rule fails the Zig case), and its rule splitter has its own guard test
so the suite cannot pass vacuously. `ZIG_SCOPE_QUERY` is exported for that test
only.
R3-3 (Nit, valid) — a test comment claimed the wrong epistemic result. The
`declines a qualified reference whose receiver cannot be resolved` case said the
shortfall shows up as `lower-bound`. It does not: with no edge there is no
evidence and the target stays `exact`. The pass docstring was corrected in round
2 and this comment was missed. It now says the decline costs the reference AND
the hedge, and why that is still the right trade.
Gates: tsc --noEmit clean, npm run build clean, prettier clean.
`test/integration/resolvers` 3,632 passed / 3 skipped (70 files);
`test/unit/scope-resolution` 2,015 passed (120 files);
`impact-callable-value-references` 7 passed under `lbug-db`. Bench --check:
receiver-resolution, zig-cross-file-resolution, scope-capture (15 languages),
scope-emission PASS with no baseline edited; callable-value-flow failed once on
its TIMING budget (2.006 > 1.9) with a byte-identical fingerprint, then passed
twice at 1.788 / 1.813 — machine load, not a regression.
This commit is contained in:
parent
cf53bbaaf6
commit
3bd1337a3a
9 changed files with 425 additions and 11 deletions
90
DECISIONS.md
90
DECISIONS.md
|
|
@ -447,3 +447,93 @@ exists for. Documented in the code at the exclusion site.
|
|||
- `tsc --noEmit -p tsconfig.json` — clean. `npm run build` — clean.
|
||||
- `resolvers/zig.test.ts` — 98 passed / 1 skipped, including the three new
|
||||
module-owner cases.
|
||||
|
||||
---
|
||||
|
||||
## Review round 3 — `gitnexus-check` bot on PR #3219 (head `cf53bbaa`)
|
||||
|
||||
Three findings: one Error, one Warning, one Nit. Each reproduced against the
|
||||
code before deciding; two were valid, one is correct about the mechanism but
|
||||
unreachable in the current rule set.
|
||||
|
||||
**R3-1 (valid, REPRODUCED, fixed) — a CLASS receiver could resolve through a
|
||||
shadowing value binding.** R1-2 resolves a written receiver with
|
||||
`findClassBindingInScope`, which 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. So:
|
||||
|
||||
// Ticker.zig — a file-struct `Element.zig` never imports
|
||||
const Ticker = @This();
|
||||
pub fn fire(self: *Ticker) u8 { … }
|
||||
|
||||
// Element.zig
|
||||
pub fn shadowsAContainerName(Ticker: u8) u8 {
|
||||
register(Ticker.fire); // ← USES → Ticker.zig's `fire`
|
||||
}
|
||||
|
||||
emitted a confident edge to a function from a file this one neither declares nor
|
||||
imports. That is precisely the wrong-edge failure R1-2 exists to prevent,
|
||||
arriving through the class channel instead of the lexical one. Reproduced with
|
||||
the fixture above before any fix; the test fails without the guard.
|
||||
|
||||
Fixed with `isOwnerNameShadowedBySomethingElse` in `scope/walkers.ts` — a
|
||||
sibling of `isNamespaceNameShadowed` with one extra clause. The plain namespace
|
||||
guard could NOT be reused: a container is often its own local declaration
|
||||
(`fn make() { const Local = struct {…}; register(Local.go); }`), and reading that
|
||||
binding as its own shadow suppresses exactly the resolutions the path exists to
|
||||
make. So a scope that binds the name answers immediately, and the answer is "not
|
||||
shadowed" only when one of that scope's own bindings IS the def just resolved.
|
||||
Both halves are pinned, and both were verified to fail when the guard is
|
||||
weakened: the parameter case fails with no guard, the local-container case fails
|
||||
with the plain `isNamespaceNameShadowed`.
|
||||
|
||||
**R3-2 (mechanism correct, not reachable today; fragility fixed instead) — the
|
||||
dispatch exclusion could suppress an unfollowed registration.** The bot is right
|
||||
about the code: sweep 2 synthesizes CALLS only for a registration whose site
|
||||
carried a `propertyKey`, so an unkeyed registration can never be followed, while
|
||||
the exclusion zeroes the note on ANY inbound `property-dispatch` CALLS edge.
|
||||
|
||||
It is not reachable in the current rule set, and the reason is measured rather
|
||||
than assumed: `@reference.value-ref` is emitted by exactly three languages —
|
||||
JavaScript (2 rules), TypeScript (2), Zig (3) — and every JS/TS rule also
|
||||
captures `@reference.property-key` (both are object-literal shapes) while no Zig
|
||||
rule does (Zig has no object-literal key). So a dispatchable registration is
|
||||
always a JS/TS one, an undispatchable registration is always a Zig one, and the
|
||||
two cannot meet on one symbol.
|
||||
|
||||
*Rejected:* splitting the edge `reason` into dispatchable / undispatchable now.
|
||||
It is the precise fix, but it is a graph-content change that churns whichever
|
||||
side keeps the old literal — the Zig, TypeScript and probe suites all pin
|
||||
`'scope-resolution: value-ref'` by hand as a drift canary — and it buys nothing
|
||||
against a case no rule can produce.
|
||||
|
||||
*What was actually wrong* is that the exclusion's soundness rested on a
|
||||
coincidence recorded nowhere, in files that no one reading `local-backend.ts`
|
||||
would open. Fixed at both ends: the exclusion site now states the invariant, the
|
||||
three facts it rests on, and the two options for when it breaks; and
|
||||
`test/unit/scope-resolution/value-ref-dispatchability.test.ts` FAILS the day it
|
||||
does — a JS/TS rule for a bare callback argument, a Zig rule that grows a key, or
|
||||
a fourth language emitting `value-ref` at all. Verified to fire: adding a
|
||||
property key to a Zig value-ref rule fails the Zig case, and the splitter has its
|
||||
own guard test so it cannot pass vacuously. `ZIG_SCOPE_QUERY` is exported for
|
||||
that test only.
|
||||
|
||||
**R3-3 (valid, fixed) — a test comment claimed the wrong epistemic result.**
|
||||
The `declines a qualified reference whose receiver cannot be resolved` case said
|
||||
the shortfall shows up as `lower-bound`. It does not: with no edge there is no
|
||||
evidence, and the target stays `exact`. The pass docstring was corrected in round
|
||||
2 and the test comment was missed. It now states that the decline costs the
|
||||
reference AND the hedge, and why that is still the right trade.
|
||||
|
||||
### Gates after review round 3
|
||||
|
||||
- `tsc --noEmit` clean, `npm run build` clean, `prettier --check` clean.
|
||||
- `test/integration/resolvers` — 3,632 passed / 3 skipped (70 files).
|
||||
- `test/unit/scope-resolution` — 2,015 passed (120 files).
|
||||
- `impact-callable-value-references` under `lbug-db` — 7 passed.
|
||||
- Bench `--check`: `receiver-resolution`, `zig-cross-file-resolution`,
|
||||
`scope-capture` (15 languages), `scope-emission` PASS, no baseline edited.
|
||||
`callable-value-flow` failed once on its TIMING budget (widening overhead
|
||||
2.006 > 1.9) with a byte-identical fingerprint, then passed twice at 1.788 /
|
||||
1.813 — machine load, not a regression.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -49,6 +49,7 @@ import {
|
|||
findClassBindingInScope,
|
||||
findOwnedMember,
|
||||
isNamespaceNameShadowed,
|
||||
isOwnerNameShadowedBySomethingElse,
|
||||
} from '../scope/walkers.js';
|
||||
import { VALUE_REF_EDGE_REASON } from '../value-ref-edges.js';
|
||||
import type { SemanticModel } from '../../model/semantic-model.js';
|
||||
|
|
@ -131,6 +132,25 @@ function resolveValueRefTarget(
|
|||
}
|
||||
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;
|
||||
|
|
|
|||
|
|
@ -351,6 +351,65 @@ 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.
|
||||
*
|
||||
* Same floor and the same fail-closed posture as `isNamespaceNameShadowed`: the
|
||||
* module scope is not inspected (a container declared there IS the binding, and
|
||||
* the caller already resolved it), and 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<ScopeId>();
|
||||
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 === 'Module') return false;
|
||||
if (scope.kind !== 'Object') {
|
||||
const bindsHere =
|
||||
scope.bindings.has(name) ||
|
||||
scope.typeBindings.has(name) ||
|
||||
scope.lexicalNames?.has(name) === true ||
|
||||
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;
|
||||
return true;
|
||||
}
|
||||
}
|
||||
currentId = scope.parent;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
export function findReceiverTypeBinding(
|
||||
startScope: ScopeId,
|
||||
receiverName: string,
|
||||
|
|
|
|||
|
|
@ -972,13 +972,35 @@ async function callableValueReferenceBoundaries(
|
|||
// excluded here; the exclusion exists to keep TypeScript/JavaScript hook
|
||||
// tables that ARE followed from being downgraded.
|
||||
//
|
||||
// Symbol-level, not per-edge: the graph does not record which registration
|
||||
// produced which synthesized call. A target with a mix of followed and
|
||||
// unfollowed registrations is therefore NOT hedged, which is the one place
|
||||
// this errs toward confidence. Preferred over the alternative — hedging every
|
||||
// property-value registration in every JS/TS codebase — because a signal that
|
||||
// fires on everything stops carrying information, and the unfollowed half
|
||||
// still has the `property-dispatch` fan-out cap warning behind it.
|
||||
// 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)
|
||||
|
|
|
|||
|
|
@ -113,6 +113,32 @@ pub fn shadowsTheModuleHandle(dom_utils: u8) u8 {
|
|||
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 {
|
||||
|
|
|
|||
13
gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Ticker.zig
vendored
Normal file
13
gitnexus/test/fixtures/lang-resolution/zig-idioms/src/webapi/Ticker.zig
vendored
Normal file
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -327,9 +327,14 @@ describe.skipIf(!zigAvailable)('Zig idioms (zig-idioms fixture)', () => {
|
|||
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: the
|
||||
// shortfall then shows up as `epistemic: "lower-bound"` rather than as a
|
||||
// confident edge pointing at the wrong function.
|
||||
// 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');
|
||||
});
|
||||
|
|
@ -367,6 +372,25 @@ describe.skipIf(!zigAvailable)('Zig idioms (zig-idioms fixture)', () => {
|
|||
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('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
|
||||
|
|
|
|||
|
|
@ -0,0 +1,154 @@
|
|||
/**
|
||||
* 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.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { 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)', () => {
|
||||
const rules = valueRefRules(TYPESCRIPT_SCOPE_QUERY);
|
||||
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 emits a value-ref capture', () => {
|
||||
// The three above are hand-classified. A fourth language emitting
|
||||
// `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.
|
||||
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']);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue