mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-07 02:58:02 +00:00
feat(scope-resolution): resolve cross-file value references, skip block-locals
Two halves of the same question, "who uses this constant?".
CROSS-FILE. `resolveReferenceSites` runs against the registries and, as its
own comment says, "imports live in finalized bindings the registries can't
see" — which is why free CALLS need `emitFreeCallFallback`. Reads had no
counterpart, so `import { LIMIT }` followed by a bare use resolved to nothing
while a CALL through the very same import statement resolved fine. This adds
the read/write counterpart, reusing `findValueBindingInScope` (which walks the
FINALIZED chain) rather than inventing a lookup. Confidence 0.9: the import
names the def, so this is precise resolution, not inference.
BLOCK-LOCALS. Bare-identifier capture also matches a read of a block-local
`const`, and an edge to one keeps alive exactly the inert locals
`pruneLocalSymbols` exists to drop — a pruned node becomes a retained node
plus an edge, in every function of every indexed repo. Emission now takes the
set of value defs bound at MODULE scope and drops ACCESSES to
Const/Variable/Static outside it. The cross-file pass carries the same
guarantee structurally: a def in another file cannot be a block-local of this
one, so it skips same-file hits entirely.
The block-local leak was already shipped in the intra-file A2 commit and was
found only because a test was written for the guard rather than the feature —
the same way the object-literal id collision surfaced.
Verified on the full resolver matrix: 3172 tests, golden unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
6664e5c0a5
commit
1dbdbf5f23
5 changed files with 180 additions and 11 deletions
|
|
@ -35,6 +35,13 @@ import type { CalleeIdSink } from '../graph-bridge/callee-id-sink.js';
|
|||
*/
|
||||
type ReferenceSiteSkipSet = ReadonlySet<string>;
|
||||
|
||||
/**
|
||||
* Value labels whose defs may be BLOCK-LOCAL. A reference to one of these is
|
||||
* only worth an edge when the def lives at module scope — see
|
||||
* `moduleScopeValueDefIds`.
|
||||
*/
|
||||
const LOCALIZABLE_VALUE_LABELS: ReadonlySet<string> = new Set(['Const', 'Variable', 'Static']);
|
||||
|
||||
export function emitReferencesViaLookup(
|
||||
graph: KnowledgeGraph,
|
||||
scopes: ScopeResolutionIndexes,
|
||||
|
|
@ -45,6 +52,22 @@ export function emitReferencesViaLookup(
|
|||
* `--pdg`; `undefined` ⇒ zero overhead, byte-identity (R4). Captured at the
|
||||
* CALLS emit below BEFORE this loop's `seen` dedup (KTD6/R8). */
|
||||
calleeIdSink?: CalleeIdSink,
|
||||
/**
|
||||
* Def ids of value symbols bound at MODULE scope. When supplied, a
|
||||
* read/write whose target is a `Const`/`Variable`/`Static` outside this set
|
||||
* emits no edge.
|
||||
*
|
||||
* Bare-identifier reads (A2) made module-scope constants answerable, but the
|
||||
* same capture also matches a read of a BLOCK-LOCAL `const`. An edge to one
|
||||
* of those keeps alive precisely the inert local symbols `pruneLocalSymbols`
|
||||
* exists to drop — turning a pruned node into a retained node plus an edge,
|
||||
* in every function of every indexed repo. "Who uses this constant?" is a
|
||||
* question about a module's surface; a local's uses are the three lines
|
||||
* around it.
|
||||
*
|
||||
* Optional so callers that never capture bare identifiers are unchanged.
|
||||
*/
|
||||
moduleScopeValueDefIds?: ReadonlySet<string>,
|
||||
): { emitted: number; skipped: number } {
|
||||
let emitted = 0;
|
||||
let skipped = 0;
|
||||
|
|
@ -85,6 +108,17 @@ export function emitReferencesViaLookup(
|
|||
continue;
|
||||
}
|
||||
|
||||
// Block-local value reference — see `moduleScopeValueDefIds`.
|
||||
if (
|
||||
moduleScopeValueDefIds !== undefined &&
|
||||
edgeType === 'ACCESSES' &&
|
||||
LOCALIZABLE_VALUE_LABELS.has(targetDef.type) &&
|
||||
!moduleScopeValueDefIds.has(targetDef.nodeId)
|
||||
) {
|
||||
skipped++;
|
||||
continue;
|
||||
}
|
||||
|
||||
// Resolved-callee-id capture (#2227 U2/KTD6/R8): record this CALLS site's
|
||||
// resolved target BEFORE the `seen` dedup, keyed on `ref.atRange`
|
||||
// (byte-equal to U1's SiteRecord.at: 1-based line / 0-based col). Only
|
||||
|
|
|
|||
|
|
@ -0,0 +1,88 @@
|
|||
/**
|
||||
* Cross-file value references, resolved post-finalize (A2).
|
||||
*
|
||||
* `resolveReferenceSites` runs against the registries, and — as its own
|
||||
* comment says — "imports live in finalized bindings the registries can't
|
||||
* see". That is why free CALLS need `emitFreeCallFallback`. Reads had no
|
||||
* equivalent, so a module-scope `const` read from another file resolved to
|
||||
* nothing: `import { DEFAULT_FETCH_LIMIT } from './config.js'` followed by a
|
||||
* bare use produced no edge, while a CALL through the very same import
|
||||
* statement resolved fine. "Who imports this constant?" — the question behind
|
||||
* every constants refactor and dead-code trim — was unanswerable across files.
|
||||
*
|
||||
* This is the read/write counterpart to that call fallback, and it reuses the
|
||||
* walker built for finalized bindings rather than inventing a lookup.
|
||||
*
|
||||
* DELIBERATELY CROSS-FILE ONLY. Same-file reads already resolve through the
|
||||
* registries, so re-resolving them here would add nothing — and would add
|
||||
* something unwanted: `findValueBindingInScope` accepts `Const`/`Variable`,
|
||||
* which includes BLOCK-LOCAL values. Emitting an edge to one of those keeps
|
||||
* alive exactly the inert local symbols `pruneLocalSymbols` exists to drop,
|
||||
* inflating every indexed repo. A def in another file cannot be a block-local
|
||||
* of this one, so the file guard is what keeps this pass proportional.
|
||||
*/
|
||||
|
||||
import type { ParsedFile } from 'gitnexus-shared';
|
||||
import type { KnowledgeGraph } from '../../../graph/types.js';
|
||||
import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js';
|
||||
import type { GraphNodeLookup } from '../graph-bridge/node-lookup.js';
|
||||
import { tryEmitEdge } from '../graph-bridge/edges.js';
|
||||
import { findValueBindingInScope } from '../scope/walkers.js';
|
||||
|
||||
/**
|
||||
* Confidence for a reference resolved through a finalized import binding.
|
||||
* This is a PRECISE resolution — the import names the def — so it carries the
|
||||
* ordinary emission confidence, not the reduced tier used for name inference.
|
||||
*/
|
||||
const IMPORTED_VALUE_CONFIDENCE = 0.9;
|
||||
|
||||
export interface ImportedValueRefStats {
|
||||
/** Cross-file value references resolved through finalized bindings. */
|
||||
readonly emitted: number;
|
||||
}
|
||||
|
||||
export function emitImportedValueReferences(
|
||||
graph: KnowledgeGraph,
|
||||
indexes: ScopeResolutionIndexes,
|
||||
parsedFiles: readonly ParsedFile[],
|
||||
nodeLookup: GraphNodeLookup,
|
||||
/** Sites an earlier pass already owns — never re-resolved here. */
|
||||
skipSites: ReadonlySet<string>,
|
||||
): ImportedValueRefStats {
|
||||
let emitted = 0;
|
||||
const seen = new Set<string>();
|
||||
|
||||
for (const parsed of parsedFiles) {
|
||||
for (const site of parsed.referenceSites) {
|
||||
if (site.kind !== 'read' && site.kind !== 'write') continue;
|
||||
// A member read (`obj.field`) is the receiver-bound passes' business;
|
||||
// this pass exists for the BARE identifier an import binds.
|
||||
if (site.explicitReceiver !== undefined) continue;
|
||||
const siteKey = `${parsed.filePath}:${site.atRange.startLine}:${site.atRange.startCol}`;
|
||||
if (skipSites.has(siteKey)) continue;
|
||||
|
||||
const def = findValueBindingInScope(site.inScope, site.name, indexes);
|
||||
if (def === undefined) continue;
|
||||
// See the header: same-file hits are already resolved, and accepting
|
||||
// them here would start emitting edges to block-local values.
|
||||
if (def.filePath === parsed.filePath) continue;
|
||||
|
||||
if (
|
||||
tryEmitEdge(
|
||||
graph,
|
||||
indexes,
|
||||
nodeLookup,
|
||||
site,
|
||||
def,
|
||||
'import-resolved',
|
||||
seen,
|
||||
IMPORTED_VALUE_CONFIDENCE,
|
||||
)
|
||||
) {
|
||||
emitted++;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return { emitted };
|
||||
}
|
||||
|
|
@ -77,6 +77,7 @@ import {
|
|||
} from '../passes/property-dispatch.js';
|
||||
import { emitReferencesViaLookup } from '../graph-bridge/references-to-edges.js';
|
||||
import { emitUniqueNamePropertyAccesses } from '../passes/unique-name-properties.js';
|
||||
import { emitImportedValueReferences } from '../passes/imported-value-refs.js';
|
||||
import {
|
||||
createCalleeIdAccumulator,
|
||||
type CalleeIdAccumulator,
|
||||
|
|
@ -443,6 +444,8 @@ interface RunScopeResolutionStats {
|
|||
* ACCESSES edges recovered by workspace-unique property name (A1/A5) — the
|
||||
* last-resort pass for receivers no precise pass could type.
|
||||
*/
|
||||
/** Cross-file value references resolved through finalized import bindings. */
|
||||
readonly importedValueRefEdges: number;
|
||||
readonly uniqueNamePropertyEdges: number;
|
||||
/**
|
||||
* Read/write sites left unresolved because two or more `Property` defs share
|
||||
|
|
@ -578,6 +581,7 @@ export function runScopeResolution(
|
|||
referenceEdgesEmitted: 0,
|
||||
referenceSkipped: 0,
|
||||
propertyDispatchSkippedKeys: 0,
|
||||
importedValueRefEdges: 0,
|
||||
uniqueNamePropertyEdges: 0,
|
||||
uniqueNamePropertyAmbiguous: 0,
|
||||
resolutionOutcomes,
|
||||
|
|
@ -609,6 +613,7 @@ export function runScopeResolution(
|
|||
referenceEdgesEmitted: 0,
|
||||
referenceSkipped: 0,
|
||||
propertyDispatchSkippedKeys: 0,
|
||||
importedValueRefEdges: 0,
|
||||
uniqueNamePropertyEdges: 0,
|
||||
uniqueNamePropertyAmbiguous: 0,
|
||||
resolutionOutcomes,
|
||||
|
|
@ -906,6 +911,22 @@ export function runScopeResolution(
|
|||
);
|
||||
const referenceSkipSites = new Set(handledSites);
|
||||
for (const key of deferredIndirectSites) referenceSkipSites.add(key);
|
||||
// Value defs bound at MODULE scope. A read of a block-local `const` must not
|
||||
// mint an edge — that would retain the inert locals `pruneLocalSymbols`
|
||||
// drops. Built once here, where the parsed scopes are already in hand.
|
||||
const moduleScopeValueDefIds = new Set<string>();
|
||||
for (const parsed of emitParsedFiles) {
|
||||
const moduleScope = parsed.scopes.find((sc) => sc.kind === 'Module');
|
||||
if (moduleScope === undefined) continue;
|
||||
for (const [, refs] of moduleScope.bindings) {
|
||||
for (const ref of refs) {
|
||||
const t = ref.def.type;
|
||||
if (t === 'Const' || t === 'Variable' || t === 'Static') {
|
||||
moduleScopeValueDefIds.add(ref.def.nodeId);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
const { emitted, skipped } = callableFlowOnly
|
||||
? { emitted: 0, skipped: 0 }
|
||||
: emitReferencesViaLookup(
|
||||
|
|
@ -915,6 +936,7 @@ export function runScopeResolution(
|
|||
postHeritageNodeLookup,
|
||||
referenceSkipSites,
|
||||
calleeIdAccumulator,
|
||||
moduleScopeValueDefIds,
|
||||
);
|
||||
// Last-resort property resolution by workspace-unique name (A1/A5). Runs
|
||||
// after every precise pass and only sees what they left behind, so a
|
||||
|
|
@ -936,6 +958,19 @@ export function runScopeResolution(
|
|||
// matching a member by name over-connects when a real type system could have
|
||||
// answered exactly; inferring an ACCESSES edge by name is the same claim, so
|
||||
// it must obey the same opt-out rather than route around it.
|
||||
// Cross-file value references (A2): the read/write counterpart to
|
||||
// `emitFreeCallFallback`. Runs BEFORE unique-name inference so a precise
|
||||
// import-resolved target always wins over a name guess.
|
||||
const importedValueRefs = callableFlowOnly
|
||||
? { emitted: 0 }
|
||||
: emitImportedValueReferences(
|
||||
graph,
|
||||
indexes,
|
||||
emitParsedFiles,
|
||||
postHeritageNodeLookup,
|
||||
uniqueNameSkipSites,
|
||||
);
|
||||
|
||||
const uniqueNameProperties =
|
||||
callableFlowOnly || provider.fieldFallbackOnMethodLookup === false
|
||||
? { emitted: 0, ambiguous: 0 }
|
||||
|
|
@ -1425,6 +1460,7 @@ export function runScopeResolution(
|
|||
propertyDispatch.callsEmitted,
|
||||
referenceSkipped: skipped,
|
||||
propertyDispatchSkippedKeys: propertyDispatch.skippedKeys,
|
||||
importedValueRefEdges: importedValueRefs.emitted,
|
||||
uniqueNamePropertyEdges: uniqueNameProperties.emitted,
|
||||
uniqueNamePropertyAmbiguous: uniqueNameProperties.ambiguous,
|
||||
resolutionOutcomes,
|
||||
|
|
|
|||
|
|
@ -14,3 +14,12 @@ export function consumerInline() {
|
|||
export function consumerCall() {
|
||||
return pageSize();
|
||||
}
|
||||
|
||||
// Guard: a BLOCK-LOCAL const read in this same file must NOT gain an edge.
|
||||
// findValueBindingInScope accepts Const/Variable, so without the same-file
|
||||
// guard this pass would resurrect exactly the inert locals pruneLocalSymbols
|
||||
// exists to drop.
|
||||
export function localOnly() {
|
||||
const localScratchValue = 7;
|
||||
return Math.max(localScratchValue, 1);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -39,17 +39,19 @@ describe('JavaScript module-scope const references (A2)', () => {
|
|||
expect(readers).toContain('pageSize');
|
||||
});
|
||||
|
||||
// Cross-file is NOT yet covered. The reference site exists (the capture
|
||||
// fires on `return DEFAULT_FETCH_LIMIT` in consumer.js, verified against the
|
||||
// raw query) and a CALL through the very same import statement resolves
|
||||
// (`consumerCall → pageSize`, reason `import-resolved`), so the gap is
|
||||
// specific to linking a value-kind def across the import edge. Prime
|
||||
// suspect: exported-def resolution is callable-only — `findExportedDefByName`
|
||||
// returns a def only when `def.type` is `Function`/`Method`
|
||||
// (scope/walkers.ts:1323) and its workspace fallback index is
|
||||
// `exportedCallableByName`. Both export spellings (`export const X` and
|
||||
// `export { X }`) fail identically, so it is not the export syntax.
|
||||
it.todo('emits an edge for the cross-file named-import reader');
|
||||
it('emits an edge for the cross-file named-import reader', () => {
|
||||
expect(readersOfConst()).toContain('consumerLimit');
|
||||
});
|
||||
|
||||
it('does not emit edges to block-local values', () => {
|
||||
// The cross-file pass resolves through finalized bindings, which include
|
||||
// Const/Variable — block-locals among them. Same-file hits are skipped so
|
||||
// an inert local cannot gain an edge and survive pruning.
|
||||
const toLocal = getRelationships(result, 'ACCESSES').filter(
|
||||
(e) => e.target === 'localScratchValue',
|
||||
);
|
||||
expect(toLocal).toEqual([]);
|
||||
});
|
||||
|
||||
it('targets the Const node itself, not a same-named local', () => {
|
||||
const toConst = getRelationships(result, 'ACCESSES').filter(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue