refactor(resolution): collapse duplicated walkers, codec inverse, fold state (#2766)

Three consolidations from `/simplify`, none behavioural.

ONE SCOPE WALK, NOT TWO COPIES. `walkers.ts` carried six hand-rolled
copies of the same scope-chain walk, and the one this branch added had
already DIVERGED: it `break`-ed on a cycle or missing scope and fell
through to the qualified-name tail, where `findAllCallableBindingsInScope`
`return []`s — identical malformed input, different answer depending on
which copy the caller reached. Extracted `findAllBindingsInScope(start,
name, scopes, predicate)`; both "collect all at the nearest binding scope"
functions now delegate. Six copies to five, and the divergence is gone.
The remaining four are pre-existing and out of this change's scope.

CODEC DERIVES BOTH DIRECTIONS FROM ONE TABLE. `SIGIL_BY_KIND` carried the
comment "one table so encoder and decoder cannot drift" — but only the
encoder read it. The decoder hand-wrote `sigil === 'c' ? 'call' : 'field'`
plus its own await/index comparison, so the decoder was precisely the side
that could drift from the table meant to prevent drift. `KIND_BY_SIGIL`
and `NAME_FREE_KINDS` are now derived from it.

ONE SIGNAL FOR "NO CLASS HERE". `FoldState` carried
`unresolvedDeclaredType` alongside `def`, and `typeOfMemberOnClass` wrote
`def: owner` — the PREVIOUS position — next to the marker. That value is
never read on any path, because all three reads test the marker first. Two
sources of truth for one fact, one of them a knowingly inconsistent state
that reads as intentional. `def === undefined` is now the only signal; the
trailing guard disappeared because `return current.def` already yields
undefined there.

Also moved the construction-selector veto BELOW the name-free continues
instead of keeping the `step.name !== undefined` guard added earlier. The
guard patched a reachability problem that position solves outright — only
named steps reach the veto now. That line is the one that silently vetoed
every await/index fold, so removing the need for the guard is the better
resolution than keeping it.

`DecorationStripper` is now used at all five sites rather than declared
once and spelled longhand at four.

Shape matrix unchanged at RESOLVES 55 / VISIBLE-GAP 23 / INVISIBLE-GAP 18.
4397 tests green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Gergo Magyar 2026-08-01 06:09:45 +00:00
parent e74803dc76
commit a8cec54789
4 changed files with 110 additions and 96 deletions

View file

@ -268,6 +268,7 @@
* `docs/plans/2026-04-20-001-refactor-emit-pipeline-generalization-plan.md`.
*/
import type { DecorationStripper } from '../scope/walkers.js';
import type {
BindingRef,
Callsite,
@ -1145,7 +1146,7 @@ export interface ScopeResolver {
* decoration. Measured: only Go needs it for a receiver base; Rust, C#, Swift,
* TypeScript and C++ need it for field types.
*/
readonly stripTypePreservingDecoration?: (typeName: string) => string | undefined;
readonly stripTypePreservingDecoration?: DecorationStripper;
/**
* Unwrap a COLLECTION spelling to its element type — `User[]` -> `User`,
@ -1166,7 +1167,7 @@ export interface ScopeResolver {
* both kinds of path land on the element type without the core needing to know
* which is which.
*/
readonly unwrapCollectionElement?: (typeName: string) => string | undefined;
readonly unwrapCollectionElement?: DecorationStripper;
/**
* Whether the compound-receiver resolver should strip C-style cast

View file

@ -27,6 +27,7 @@ import type { WorkspaceResolutionIndex } from '../workspace-index.js';
import { stripTemplateArguments } from '../../utils/template-arguments.js';
import type { DecodedReceiverChain } from '../../utils/receiver-chain-codec.js';
import { decodeReceiverChain } from '../../utils/receiver-chain-codec.js';
import type { DecorationStripper } from '../scope/walkers.js';
import {
findClassBindingInScope,
findEnclosingClassDef,
@ -127,11 +128,11 @@ interface ResolveCompoundReceiverOptions {
* languages whose declared types carry no such decoration, and never applied
* by the shared lookup's other callers — see the contract's own note on why
* this is opt-in rather than global. */
readonly stripTypePreservingDecoration?: (typeName: string) => string | undefined;
readonly stripTypePreservingDecoration?: DecorationStripper;
/** Collection -> element unwrap, consulted ONLY by an index step. See the
* `ScopeResolver` field of the same name for why this is separate from the
* type-preserving stripper. */
readonly unwrapCollectionElement?: (typeName: string) => string | undefined;
readonly unwrapCollectionElement?: DecorationStripper;
}
/** Is this hop the language's construction selector applied to the class
@ -319,19 +320,20 @@ function resolveConstructionExpressionClass(
* than guessing.
*/
interface FoldState {
/** Absent when the position's declared type named no class — see
* `unresolvedDeclaredType`. Only an unwrapping step can advance from there. */
/**
* Absent when the position's declared type named no class — `Promise<User>`
* and `[]Repo` name nothing in the workspace. ONE signal, not two: an earlier
* version carried a separate `unresolvedDeclaredType` flag alongside a `def`
* holding the PREVIOUS position, which no path ever read. Two sources of
* truth for one fact, and the dead `def` read as intentional.
*
* Only an unwrapping step (await, index) can advance from an absent `def`;
* every other step declines, because folding on against the previous class
* would look the next member up on the wrong owner.
*/
readonly def: SymbolDefinition | undefined;
readonly declaredTypeName?: string;
readonly declaredAtScope?: ScopeId;
/**
* The declared type named no class in the workspace, so `def` is the PREVIOUS
* position rather than this member's type. Only an unwrapping step (await,
* index) can make progress from here; any other step must decline, because
* folding on against the previous class would silently look the next member up
* on the wrong owner.
*/
readonly unresolvedDeclaredType?: boolean;
}
function typeOfMemberOnClass(
@ -358,11 +360,12 @@ function typeOfMemberOnClass(
// await or index step unwrapping them is exactly how they become
// resolvable. Returning `undefined` here would strand those shapes.
if (def === undefined) {
// `def` absent, NOT the previous owner: nothing may fold a member off
// this position except an unwrapping step.
return {
def: owner,
def: undefined,
declaredTypeName: memberType.rawName,
declaredAtScope: memberType.declaredAtScope,
unresolvedDeclaredType: true,
};
}
return {
@ -472,33 +475,12 @@ export function foldReceiverChain(
def: undefined,
declaredTypeName: baseBinding.rawName,
declaredAtScope: baseBinding.declaredAtScope,
unresolvedDeclaredType: true,
};
} else {
return undefined;
}
for (const step of chain.steps) {
// Construction is NOT an ordinary member lookup. `Factory.new` on a class
// constant denotes an instance of Factory, and the cascade already encodes
// that (`isConstructionSelectorHop`) along with the class-constant test that
// separates it from an instance method genuinely named `new`. The fold
// carries no such distinction — a chain step records a name, not whether its
// base was a class reference or a value — so folding one would resolve
// `Factory.new.run` against whatever member named `new` the lookup reaches
// first. That turned a correct edge into a WRONG one (Ruby
// `Factory.new.run` → `Product.run`), which is the failure mode this whole
// line of work exists to avoid. Decline and let the cascade answer.
// `step.name !== undefined` is load-bearing, not defensive. The name-free
// step kinds (`await`, `index`) carry no name, and a language with no
// `constructionSyntax` has no selector — so the bare equality below was
// `undefined === undefined`, which matched EVERY name-free step and vetoed
// the whole fold before it ran. That is why subscript and await receivers
// minted a chain, fired the gate, and still produced no edge.
if (step.name !== undefined && options.constructionSyntax?.selector === step.name) {
return undefined;
}
// A position whose declared type named no class can only be advanced by an
// unwrapping step. Folding an ordinary member off it would look the member
// up on the PREVIOUS class — a wrong owner, silently.
@ -537,20 +519,39 @@ export function foldReceiverChain(
);
if (elementClass === undefined) return undefined;
current = { def: elementClass, declaredTypeName: element, declaredAtScope: scopeForLookup };
} else if (current.unresolvedDeclaredType === true) {
} else if (current.def === undefined) {
// No unwrap available and the position never named a class: nothing to
// fold on. Decline rather than continue against a stale owner.
return undefined;
}
continue;
}
if (current.unresolvedDeclaredType === true || current.def === undefined) return undefined;
// Construction is NOT an ordinary member lookup. `Factory.new` on a class
// constant denotes an instance of Factory, and the cascade already encodes
// that (`isConstructionSelectorHop`) along with the class-constant test that
// separates it from an instance method genuinely named `new`. The fold
// carries no such distinction — a chain step records a name, not whether its
// base was a class reference or a value — so folding one would resolve
// `Factory.new.run` against whatever member named `new` the lookup reaches
// first. That turned a correct edge into a WRONG one (Ruby
// `Factory.new.run` → `Product.run`), which is the failure mode this whole
// line of work exists to avoid. Decline and let the cascade answer.
//
// Placed AFTER the name-free continues above, deliberately. When it sat
// first, `options.constructionSyntax?.selector === step.name` compared
// `undefined === undefined` for every await/index step in a language with no
// construction selector, vetoing the entire fold before it ran — which is
// why those receivers minted a chain, fired the gate, and produced no edge.
// Position, not a guard, is what makes that unreachable: only named steps
// get here.
if (options.constructionSyntax?.selector === step.name) return undefined;
if (current.def === undefined) return undefined;
const next = typeOfMemberOnClass(current.def, step.name, scopes, index, options);
if (next === undefined) return undefined;
current = next;
}
// A chain that ended on an unresolved declared type never reached a class.
if (current.unresolvedDeclaredType === true) return undefined;
// A chain that ended without a class returns undefined naturally — no
// separate guard, because `def` IS the signal.
return current.def;
}

View file

@ -336,36 +336,12 @@ export function findAllClassBindingsInScope(
name: string,
scopes: ScopeResolutionIndexes,
): readonly SymbolDefinition[] {
const inScope = findAllBindingsInScope(startScope, name, scopes, (def) => isClassLike(def.type));
// The scope chain wins outright when it binds the name: an inner binding
// shadows anything the qualified-name index would contribute.
if (inScope.length > 0) return inScope;
const byNodeId = new Map<string, SymbolDefinition>();
let currentId: ScopeId | null = startScope;
const visited = new Set<ScopeId>();
while (currentId !== null) {
if (visited.has(currentId)) break;
visited.add(currentId);
const scope = scopes.scopeTree.getScope(currentId);
if (scope === undefined) break;
// `Object` scopes are a hoist boundary only — see walkScopeChain (#2545).
if (scope.kind !== 'Object') {
const found: SymbolDefinition[] = [];
for (const b of scope.bindings.get(name) ?? []) {
if (isClassLike(b.def.type)) found.push(b.def);
}
for (const b of lookupBindingsAt(currentId, name, scopes)) {
if (isClassLike(b.def.type)) found.push(b.def);
}
// Stop at the first scope that binds the name at all: an inner binding
// SHADOWS an outer one, so continuing would report a shadowed outer
// definition as a competing candidate and decline a name that is actually
// unambiguous at this point.
if (found.length > 0) {
for (const def of found) byNodeId.set(def.nodeId, def);
return [...byNodeId.values()];
}
}
currentId = scope.parent;
}
for (const id of scopes.qualifiedNames.get(name)) {
const def = scopes.defs.get(id);
if (def !== undefined && isClassLike(def.type)) byNodeId.set(def.nodeId, def);
@ -887,10 +863,28 @@ export function findAllCallableBindingCandidatesInScope(
* `findCallableBindingInScope`: once any callable binding is found in a
* scope, outer scopes are not consulted.
*/
export function findAllCallableBindingsInScope(
/**
* Every definition visible for `name` at the NEAREST scope that binds it,
* filtered by `predicate` and deduped by `nodeId`.
*
* THE shared "collect all at the nearest binding scope" walk. `walkScopeChain`
* answers the first-match question; this answers the how-many question, which is
* what a caller needs before it can decline on ambiguity.
*
* Stops at the first scope that binds the name at all: an inner binding SHADOWS
* an outer one, so continuing would report a shadowed outer definition as a
* competing candidate and decline a name that is unambiguous at this point.
*
* Returns `[]` on a cycle or a missing scope. That is deliberate and matters:
* an earlier copy of this walk `break`-ed instead and fell through to a
* qualified-name fallback, so the same malformed input produced a different
* answer depending on which copy the caller happened to reach.
*/
function findAllBindingsInScope(
startScope: ScopeId,
callableName: string,
name: string,
scopes: ScopeResolutionIndexes,
predicate: (def: SymbolDefinition) => boolean,
): readonly SymbolDefinition[] {
let currentId: ScopeId | null = startScope;
const visited = new Set<ScopeId>();
@ -905,24 +899,16 @@ export function findAllCallableBindingsInScope(
if (scope.kind !== 'Object') {
const out: SymbolDefinition[] = [];
const seen = new Set<string>();
const pushCallable = (def: SymbolDefinition): void => {
if (def.type !== 'Function' && def.type !== 'Method' && def.type !== 'Constructor') return;
const push = (def: SymbolDefinition): void => {
if (!predicate(def)) return;
if (seen.has(def.nodeId)) return;
seen.add(def.nodeId);
out.push(def);
};
const localBindings = scope.bindings.get(callableName);
if (localBindings !== undefined) {
for (const b of localBindings) {
pushCallable(b.def);
}
}
const importedBindings = lookupBindingsAt(currentId, callableName, scopes);
for (const b of importedBindings) {
pushCallable(b.def);
}
// Local first: a binding in this scope shadows an imported one.
for (const b of scope.bindings.get(name) ?? []) push(b.def);
for (const b of lookupBindingsAt(currentId, name, scopes)) push(b.def);
if (out.length > 0) return out;
}
@ -931,6 +917,19 @@ export function findAllCallableBindingsInScope(
return [];
}
export function findAllCallableBindingsInScope(
startScope: ScopeId,
callableName: string,
scopes: ScopeResolutionIndexes,
): readonly SymbolDefinition[] {
return findAllBindingsInScope(
startScope,
callableName,
scopes,
(def) => def.type === 'Function' || def.type === 'Method' || def.type === 'Constructor',
);
}
/**
* ISO C++ `[basic.lookup.unqual]` §7: ADL is suppressed when ordinary
* unqualified lookup finds:

View file

@ -69,17 +69,27 @@ const VERSION = '2';
const SEPARATOR = '|';
const TRUNCATED = '~';
const AWAIT_SIGIL = 'a';
const INDEX_SIGIL = 'i';
/** Sigil per step kind. One table so encoder and decoder cannot drift. */
/** Sigil per step kind. ONE table, and both directions derive from it — the
* decoder used to hand-write `sigil === 'c' ? 'call' : 'field'` and its own
* await/index comparison, which meant the decoder was exactly the side that
* could drift from the table claiming to prevent drift. */
const SIGIL_BY_KIND = {
call: 'c',
field: 'f',
await: AWAIT_SIGIL,
index: INDEX_SIGIL,
await: 'a',
index: 'i',
} as const;
type StepKind = keyof typeof SIGIL_BY_KIND;
/** Kinds that encode as a BARE sigil, because they have no member name to
* carry. Derived from the step union rather than listed twice. */
const NAME_FREE_KINDS = new Set<StepKind>(['await', 'index']);
const KIND_BY_SIGIL: ReadonlyMap<string, StepKind> = new Map(
Object.entries(SIGIL_BY_KIND).map(([kind, sigil]) => [sigil, kind as StepKind]),
);
/** Hard cap on the encoded payload. `MAX_CHAIN_DEPTH` already bounds the step
* COUNT; this bounds the total bytes so a pathological identifier cannot grow
* a shard without limit. Generous against real identifiers — the encoding for
@ -129,7 +139,7 @@ export function encodeReceiverChain(
// Name-free kinds encode as a bare sigil. They are exempt from the
// non-empty-name guard because they HAVE no name to check — not because the
// guard is relaxed: an empty-name `call` or `field` is still refused below.
if (step.kind === 'await' || step.kind === 'index') {
if (NAME_FREE_KINDS.has(step.kind)) {
parts.push(SIGIL_BY_KIND[step.kind]);
continue;
}
@ -172,14 +182,17 @@ export function decodeReceiverChain(value: unknown): DecodedReceiverChain | unde
// Name-free kinds must be EXACTLY their sigil. Rejecting a trailing tail is
// what keeps an accidentally empty-name call or field from decoding as one
// of these: `c` alone stays malformed, it does not become an await.
if (sigil === AWAIT_SIGIL || sigil === INDEX_SIGIL) {
const kind = sigil === undefined ? undefined : KIND_BY_SIGIL.get(sigil);
if (kind === undefined) return undefined;
if (NAME_FREE_KINDS.has(kind)) {
// Must be EXACTLY the sigil. Rejecting a trailing tail is what keeps an
// accidentally empty-name call or field from decoding as one of these.
if (name.length > 0) return undefined;
steps.push({ kind: sigil === AWAIT_SIGIL ? 'await' : 'index' });
steps.push({ kind: kind as 'await' | 'index' });
continue;
}
if (sigil !== 'c' && sigil !== 'f') return undefined;
if (!isEncodableSegment(name)) return undefined;
steps.push({ kind: sigil === 'c' ? 'call' : 'field', name });
steps.push({ kind: kind as 'call' | 'field', name });
}
return { baseReceiverName, steps, truncated };