mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-10 03:27:59 +00:00
feat(scope-resolution): resolve members through a call result's return shape
The question three rounds of reports could not answer, and the one narrowing
must refuse by design: a field produced by SEVERAL functions. A read of
`spike.wickRatio` could mean any producer, so name inference correctly declines
and no amount of tier-tuning changes that. It needs evidence, not inference.
The evidence existed in two halves that had never been joined. The call-result
type binding (`const alert = formatSpikeAlert(row)` binds `alert` to a TypeRef
whose rawName is the callee) predates all of this work; it simply had nothing to
resolve to when the callee returned an anonymous literal, because an anonymous
literal named nothing. R3-4 gave it a name. Joining them:
const alert = formatSpikeAlert(row);
alert.wickRatio -> Property:...:formatSpikeAlert.wickRatio
Precise, at ordinary emission confidence, and it works EXACTLY where narrowing
cannot: several producers sharing a field name stop being competitors because
the receiver says which one. Runs before the name fallback and claims its sites,
so a precise answer is never second-guessed by a name match.
Measured on the reporting repo: 1,410 precise edges, and all six fields round 3
verified OUT-OF-SAMPLE go from 0 backend readers to 7, 11, 10, 7, 6 and 14.
Round 3 scored 0/6 on that set; this is 6/6.
The bound is asserted, not just documented: a read off a BARE PARAMETER has no
binding here, because typing it needs the caller's type to flow in — that is
inter-procedural and genuinely larger. Those reads still fall through to name
inference and are still reported when it declines. The fixture has two producers
sharing a field name precisely so the test cannot pass by name matching, and
mutation-checking the owner lookup fails it.
No SCHEMA_BUMP: this is scope resolution, not parse-time capture, so a warm
cache already carries everything it reads. Noted in the ledger because the
reflex on this branch has been to bump, and an unnecessary bump costs every user
a full re-parse.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
305b7f879f
commit
0880bbb97d
5 changed files with 252 additions and 1 deletions
|
|
@ -0,0 +1,151 @@
|
|||
/**
|
||||
* PRECISE member resolution through a call result's RETURN SHAPE (R3-5).
|
||||
*
|
||||
* The last unanswered question from three rounds of blind-spot reports was
|
||||
* "who reads `wickRatio`?", where the field is produced by several functions
|
||||
* that each return an anonymous object containing it. Name inference must
|
||||
* refuse that — a read of `spike.wickRatio` could mean any producer, and a
|
||||
* wrong edge in the pre-edit safety gate is worse than a missing one — so no
|
||||
* amount of narrowing gets there. It needs EVIDENCE instead of inference.
|
||||
*
|
||||
* The evidence already exists in two halves that had never been joined:
|
||||
*
|
||||
* 1. The call-result type binding. `const alert = formatSpikeAlert(row)`
|
||||
* binds `alert` to a `TypeRef` whose `rawName` is the callee. That
|
||||
* machinery predates this work; it simply had nothing to resolve to when
|
||||
* the callee returned an anonymous literal, because an anonymous literal
|
||||
* named nothing.
|
||||
* 2. R3-4 gave it a name. A returned literal's keys are now owned by the
|
||||
* producing function, so `formatSpikeAlert.wickRatio` is a real symbol.
|
||||
*
|
||||
* Joining them turns a refusal into a precise answer:
|
||||
*
|
||||
* const alert = formatSpikeAlert(row);
|
||||
* alert.wickRatio → Property:…:formatSpikeAlert.wickRatio
|
||||
*
|
||||
* and it works for exactly the case narrowing cannot: several producers sharing
|
||||
* a field name are no longer competitors, because the receiver says WHICH one.
|
||||
* That is why this runs before the unique-name fallback and registers its sites
|
||||
* as handled — a precise answer must never be second-guessed by a name match.
|
||||
*
|
||||
* BOUND, deliberately. This only fires where the value is BOUND to a name the
|
||||
* type binding could attach to. A field read off a bare parameter
|
||||
* (`function f(spike) { return spike.wickRatio }`) still has no receiver type
|
||||
* here, because typing it requires the CALLER's type to flow in — that is
|
||||
* inter-procedural and genuinely larger. Those reads keep falling through to
|
||||
* name inference, and keep being reported when it declines.
|
||||
*/
|
||||
|
||||
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 { resolveCallerGraphId } from '../graph-bridge/ids.js';
|
||||
import { findReceiverTypeBinding } from '../scope/walkers.js';
|
||||
import { callableFlowSiteKey } from './callable-value-flow.js';
|
||||
import type { PropertyNameIndex } from './unique-name-properties.js';
|
||||
|
||||
/**
|
||||
* Confidence for a return-shape member. This is a PRECISE resolution — the
|
||||
* receiver's binding names the producing function and the member is owned by
|
||||
* it — so it carries the ordinary emission confidence, not the reduced tier
|
||||
* name inference uses. Nothing here is guessed.
|
||||
*/
|
||||
const RETURN_SHAPE_CONFIDENCE = 0.9;
|
||||
|
||||
const EDGE_REASON = 'scope-resolution: return-shape member';
|
||||
|
||||
export interface ReturnShapeMemberStats {
|
||||
/** ACCESSES edges resolved through a call result's return shape. */
|
||||
readonly emitted: number;
|
||||
/**
|
||||
* Sites where the receiver WAS typed to a producer but that producer owns no
|
||||
* member of this name. Reported rather than dropped: it means the read and
|
||||
* the shape disagree, which is either a stale field name or a producer this
|
||||
* pass mis-attributed, and both are worth seeing.
|
||||
*/
|
||||
readonly memberNotOnShape: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Does this Property node id name `<owner>.<member>`?
|
||||
*
|
||||
* Ids carry an optional position suffix for function-local symbols
|
||||
* (`…:buildFlat.field@33:4`), so the owner segment is matched up to a `@` or
|
||||
* the end rather than by equality.
|
||||
*/
|
||||
function idNamesMember(id: string, owner: string, member: string): boolean {
|
||||
const needle = `:${owner}.${member}`;
|
||||
const at = id.indexOf(needle);
|
||||
if (at === -1) return false;
|
||||
const after = id.slice(at + needle.length);
|
||||
return after.length === 0 || after.startsWith('@');
|
||||
}
|
||||
|
||||
export function emitReturnShapeMemberAccesses(
|
||||
graph: KnowledgeGraph,
|
||||
indexes: ScopeResolutionIndexes,
|
||||
parsedFiles: readonly ParsedFile[],
|
||||
nodeLookup: GraphNodeLookup,
|
||||
/** Sites a precise pass already owns — never re-resolved here. */
|
||||
skipSites: ReadonlySet<string>,
|
||||
propertyNameIndex: PropertyNameIndex,
|
||||
/** Sites this pass resolves, so the name fallback leaves them alone. */
|
||||
handledSink: Set<string>,
|
||||
): ReturnShapeMemberStats {
|
||||
let emitted = 0;
|
||||
let memberNotOnShape = 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;
|
||||
const receiver = site.explicitReceiver?.name;
|
||||
if (receiver === undefined || receiver.length === 0) continue;
|
||||
const siteKey = callableFlowSiteKey(parsed.filePath, site.atRange);
|
||||
if (skipSites.has(siteKey)) continue;
|
||||
|
||||
// The receiver's binding names the PRODUCER, not a class. That is the
|
||||
// whole point: `formatSpikeAlert` is a function, and before R3-4 there
|
||||
// was nothing named after it to look a member up on.
|
||||
const typeRef = findReceiverTypeBinding(site.inScope, receiver, indexes);
|
||||
const producer = typeRef?.rawName;
|
||||
if (producer === undefined || producer.length === 0) continue;
|
||||
|
||||
const candidates = propertyNameIndex.get(site.name);
|
||||
if (candidates === undefined) continue;
|
||||
const owned = candidates.filter((c) => idNamesMember(c.id, producer, site.name));
|
||||
// Exactly one, or nothing. Two nodes claiming `<producer>.<member>` would
|
||||
// mean the id qualifier failed to separate them, and picking between them
|
||||
// would be the guess this pass exists to avoid.
|
||||
if (owned.length !== 1) {
|
||||
if (owned.length === 0) memberNotOnShape++;
|
||||
continue;
|
||||
}
|
||||
const target = owned[0]!;
|
||||
|
||||
const callerGraphId = resolveCallerGraphId(site.inScope, indexes, nodeLookup, site.atRange);
|
||||
if (callerGraphId === undefined) continue;
|
||||
if (callerGraphId === target.id) continue;
|
||||
|
||||
const dedupKey = `ACCESSES:${callerGraphId}->${target.id}:${site.atRange.startLine}:${site.atRange.startCol}`;
|
||||
if (seen.has(dedupKey)) continue;
|
||||
seen.add(dedupKey);
|
||||
|
||||
graph.addRelationship({
|
||||
id: `rel:${dedupKey}`,
|
||||
sourceId: callerGraphId,
|
||||
targetId: target.id,
|
||||
type: 'ACCESSES',
|
||||
confidence: RETURN_SHAPE_CONFIDENCE,
|
||||
reason: `${EDGE_REASON}: ${site.kind}`,
|
||||
evidence: [],
|
||||
});
|
||||
// Claim the site so the name fallback cannot re-answer it differently.
|
||||
handledSink.add(siteKey);
|
||||
emitted++;
|
||||
}
|
||||
}
|
||||
|
||||
return { emitted, memberNotOnShape };
|
||||
}
|
||||
|
|
@ -77,9 +77,11 @@ import {
|
|||
} from '../passes/property-dispatch.js';
|
||||
import { emitReferencesViaLookup } from '../graph-bridge/references-to-edges.js';
|
||||
import {
|
||||
buildPropertyNameIndex,
|
||||
emitUniqueNamePropertyAccesses,
|
||||
type PropertyNameIndex,
|
||||
} from '../passes/unique-name-properties.js';
|
||||
import { emitReturnShapeMemberAccesses } from '../passes/return-shape-members.js';
|
||||
import { emitImportedValueReferences } from '../passes/imported-value-refs.js';
|
||||
import {
|
||||
createCalleeIdAccumulator,
|
||||
|
|
@ -1053,6 +1055,23 @@ export function runScopeResolution(
|
|||
// TS-anchor case R3-1 was filed about — the identical defect, mirrored.
|
||||
// `reportOnly` counts without emitting: no edge, no inference, no change to
|
||||
// what the opt-out protects.
|
||||
// PRECISE first (R3-5). A call result's return shape names WHICH producer a
|
||||
// receiver holds, so it answers exactly the case name inference must refuse:
|
||||
// several functions returning the same field name. Sites it resolves are
|
||||
// added to the skip set, so the fallback below never second-guesses them.
|
||||
const sharedPropertyIndex = input.prebuiltPropertyNameIndex ?? buildPropertyNameIndex(graph);
|
||||
const returnShapeMembers = callableFlowOnly
|
||||
? { emitted: 0, memberNotOnShape: 0 }
|
||||
: emitReturnShapeMemberAccesses(
|
||||
graph,
|
||||
indexes,
|
||||
emitParsedFiles,
|
||||
postHeritageNodeLookup,
|
||||
uniqueNameSkipSites,
|
||||
sharedPropertyIndex,
|
||||
uniqueNameSkipSites,
|
||||
);
|
||||
|
||||
const nameFallbackDisabled = provider.fieldFallbackOnMethodLookup === false;
|
||||
const uniqueNameProperties = callableFlowOnly
|
||||
? {
|
||||
|
|
@ -1070,7 +1089,7 @@ export function runScopeResolution(
|
|||
postHeritageNodeLookup,
|
||||
uniqueNameSkipSites,
|
||||
finalized,
|
||||
input.prebuiltPropertyNameIndex,
|
||||
sharedPropertyIndex,
|
||||
nameFallbackDisabled,
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -300,6 +300,10 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j
|
|||
// is a different capture set wearing the same number — which is exactly what
|
||||
// the note above records for 33/34.
|
||||
// RE-CHECK AGAINST origin/main IMMEDIATELY BEFORE MERGING.
|
||||
// 49 -> 50 is NOT needed for R3-5: that pass is scope-resolution, not
|
||||
// parse-time capture, so a warm cache replays ParsedFiles that already carry
|
||||
// everything it reads. Recorded because the reflex on this branch has been to
|
||||
// bump, and a bump nobody needs still forces every user a full re-parse.
|
||||
const SCHEMA_BUMP = 49;
|
||||
const GITNEXUS_PKG_VERSION = (() => {
|
||||
try {
|
||||
|
|
|
|||
32
gitnexus/test/fixtures/lang-resolution/javascript-object-properties/return-shape-typed.js
vendored
Normal file
32
gitnexus/test/fixtures/lang-resolution/javascript-object-properties/return-shape-typed.js
vendored
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
// R3-5: TWO producers returning the same field name — the case name inference
|
||||
// must refuse, because `x.ambiguousProducedField` alone cannot say which shape
|
||||
// is meant. The receiver's binding says which, so this resolves precisely.
|
||||
export function producerAlpha(row) {
|
||||
return {
|
||||
ambiguousProducedField: row.a,
|
||||
};
|
||||
}
|
||||
|
||||
export function producerBeta(row) {
|
||||
return {
|
||||
ambiguousProducedField: row.b,
|
||||
};
|
||||
}
|
||||
|
||||
// BOUND to the call result, so the type binding attaches.
|
||||
export function readsAlpha(row) {
|
||||
const shaped = producerAlpha(row);
|
||||
return shaped.ambiguousProducedField;
|
||||
}
|
||||
|
||||
export function readsBeta(row) {
|
||||
const shaped = producerBeta(row);
|
||||
return shaped.ambiguousProducedField;
|
||||
}
|
||||
|
||||
// THE BOUND of the mechanism: a bare parameter has no binding here, because
|
||||
// typing it needs the CALLER's type to flow in. This must stay unresolved and
|
||||
// fall through to name inference, which will refuse it (two producers).
|
||||
export function readsUnbound(shaped) {
|
||||
return shaped.ambiguousProducedField;
|
||||
}
|
||||
|
|
@ -235,6 +235,51 @@ describe('JavaScript plain-object property access (A1/A5)', () => {
|
|||
});
|
||||
});
|
||||
|
||||
// R3-5. The question three rounds of reports could not answer: a field
|
||||
// produced by SEVERAL functions. Name inference must refuse it — the name
|
||||
// alone cannot say which shape is meant — so this replaces inference with
|
||||
// evidence, joining the call-result type binding (which already existed) to
|
||||
// the return-shape owner (which R3-4 created).
|
||||
describe('return-shape members via the call result (R3-5)', () => {
|
||||
const targetOf = (source: string): string[] =>
|
||||
getRelationships(result, 'ACCESSES')
|
||||
.filter((e) => e.target === 'ambiguousProducedField' && e.source === source)
|
||||
.map((e) => String((e.rel as { targetId?: string }).targetId ?? ''));
|
||||
|
||||
it('resolves to the producer the receiver actually holds', () => {
|
||||
expect(targetOf('readsAlpha').some((id) => id.includes('producerAlpha.'))).toBe(true);
|
||||
expect(targetOf('readsBeta').some((id) => id.includes('producerBeta.'))).toBe(true);
|
||||
});
|
||||
|
||||
// The discriminator. Both producers own a field of this name, so a name
|
||||
// match cannot tell them apart — getting the RIGHT one is only possible
|
||||
// because the receiver's binding names the producer.
|
||||
it('does not cross the two producers', () => {
|
||||
expect(targetOf('readsAlpha').some((id) => id.includes('producerBeta.'))).toBe(false);
|
||||
expect(targetOf('readsBeta').some((id) => id.includes('producerAlpha.'))).toBe(false);
|
||||
});
|
||||
|
||||
it('marks the edge as precise, not as name inference', () => {
|
||||
const reasons = getRelationships(result, 'ACCESSES')
|
||||
.filter((e) => e.target === 'ambiguousProducedField')
|
||||
.map((e) => String(e.rel.reason ?? ''));
|
||||
expect(reasons.every((r) => r.includes('return-shape member'))).toBe(true);
|
||||
for (const e of getRelationships(result, 'ACCESSES').filter(
|
||||
(x) => x.target === 'ambiguousProducedField',
|
||||
)) {
|
||||
expect(e.rel.confidence).toBeGreaterThan(0.85);
|
||||
}
|
||||
});
|
||||
|
||||
// THE BOUND, asserted so the mechanism is not mistaken for something it is
|
||||
// not. A bare parameter has no binding here — typing it needs the CALLER's
|
||||
// type to flow in, which is inter-procedural — so it falls through to name
|
||||
// inference, which refuses because two producers share the name.
|
||||
it('leaves an unbound receiver to name inference, which refuses it', () => {
|
||||
expect(targetOf('readsUnbound')).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
// R2. Strict workspace uniqueness was measurably too blunt: in the reporting
|
||||
// repo `exitMinAtrMult` had 26 definitions, 16 of them in one-off scripts the
|
||||
// backend has no relationship with, so every backend read was refused because
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue