mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-07 02:58:02 +00:00
fix(resolution): correct receiver-shape census and chain descent (#2766)
From `/simplify`. Two of these are correctness defects that three of four
review agents caught independently, not cleanups.
CENSUS MISLABELLED THE SHAPES IT EXISTS TO EXPOSE. `classifyReceiverShape`
took a duck-typed `{ kind: string }[]` instead of `DecodedReceiverChain`,
so `await` and `index` fell into its `else` and were counted as FIELDS.
`repos[0].save()` drops censused as `chain-field`; the U10 breakdown was
quietly wrong about exactly the two kinds this branch added. Now typed as
the discriminated union with an exhaustive switch and a `chain-unwrap`
bucket, so a future step kind is a compile error rather than a silent
miscount.
CHAIN DESCENT HAD A HOLE. The `call` and `field` branches of
`extractMixedChain` still tested only the call/field node sets; the two
new branches tested all four. So `x[0].f().g()` stopped at the subscript
and returned the literal text `x[0]` as the base — which
`isEncodableSegment` ACCEPTS, minting a chain whose base binds to nothing
— and `(await f()).g.h()` returned `await f()`, rejected on whitespace,
minting no chain at all. Adding one field hop turned the feature off. All
four branches now share one `isChainableReceiverNode` predicate, so a
fifth step kind is one edit rather than four.
REUSE. Two helpers already existed and I had hand-rolled both:
- `extractElementTypeFromString` (type-extractors/shared.ts) is
bracket-balanced, used by seven language extractors, and handles
`Map<K,V>` / `Record<K,V>` by returning the VALUE type — which is what a
subscript yields. My local regex returned undefined for those, so
`cache["k"].save()` declined.
- `subscriptBase` (callable-flow-captures.ts) already had the correct
per-grammar operand table including Python's `value` and Java's `array`,
which my ladder omitted; those worked only because the container
happened to be the first named child, an ordering coincidence rather
than a contract. Exported and reused, so one table answers the question.
EFFICIENCY. The base type binding was looked up on every fold even though
`resolveCompoundReceiverClass` had just walked the same scope chain for
the same name, and 86% of chains (U10 census: 59% field, 27% call) never
read it. Now conditional on the base having failed or an index step being
present. Also dropped a redundant undefined-ternary around
`decodeReceiverChain`, which already guards non-strings.
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:
parent
10d872529e
commit
e74803dc76
6 changed files with 96 additions and 50 deletions
|
|
@ -23,6 +23,7 @@ import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexe
|
|||
import { typescriptProvider } from '../typescript.js';
|
||||
import { loadTsconfigPaths, type TsconfigPaths } from '../../language-config.js';
|
||||
import { buildSuffixIndex, type SuffixIndex } from '../../import-resolvers/utils.js';
|
||||
import { extractElementTypeFromString } from '../../type-extractors/shared.js';
|
||||
import {
|
||||
typescriptArityCompatibility,
|
||||
typescriptMergeBindings,
|
||||
|
|
@ -107,24 +108,16 @@ function makeTsResolveImportTarget(): ScopeResolver['resolveImportTarget'] {
|
|||
|
||||
const typescriptScopeResolver: ScopeResolver = {
|
||||
// Collections only, consulted ONLY by an index step (see the contract field).
|
||||
// A TypeScript parameter annotation carries `User[]` through to the binding,
|
||||
// so `repos[0].save()` needs exactly one unwrap here; a binding path that
|
||||
// already reduced the container returns undefined and the step falls back to
|
||||
// identity. Deliberately NOT part of `stripTypePreservingDecoration`: a
|
||||
// container changes the member set, so unwrapping it at the bare class lookup
|
||||
// would let `repos.find(x)` fold to `User.find`.
|
||||
unwrapCollectionElement: (typeName) => {
|
||||
const trimmed = typeName.trim();
|
||||
if (trimmed.endsWith('[]')) {
|
||||
const inner = trimmed.slice(0, -2).trim();
|
||||
const unparen =
|
||||
inner.startsWith('(') && inner.endsWith(')') ? inner.slice(1, -1).trim() : inner;
|
||||
return unparen.length > 0 ? unparen : undefined;
|
||||
}
|
||||
const generic =
|
||||
/^(?:Readonly)?(?:Array|Set|ReadonlySet|Iterable|AsyncIterable)<([^,<>]+)>$/.exec(trimmed);
|
||||
return generic === null ? undefined : generic[1].trim();
|
||||
},
|
||||
// Delegates to the shared, bracket-balanced extractor rather than a local
|
||||
// regex: that helper is already used by seven language type-extractors, and it
|
||||
// covers `Map<K,V>` / `Record<K,V>` (returning the VALUE type, which is what a
|
||||
// subscript yields) where a hand-rolled single-arg regex returned undefined
|
||||
// and made `cache["k"].save()` decline.
|
||||
//
|
||||
// Deliberately NOT part of `stripTypePreservingDecoration`: a container
|
||||
// changes the member set, so unwrapping it at the bare class lookup would let
|
||||
// `repos.find(x)` fold to `User.find`.
|
||||
unwrapCollectionElement: (typeName) => extractElementTypeFromString(typeName),
|
||||
|
||||
// Construction is keyword-prefixed: `new Service(db).doWork()` (#2708).
|
||||
constructionSyntax: { keyword: 'new' },
|
||||
|
|
|
|||
|
|
@ -442,7 +442,17 @@ export function foldReceiverChain(
|
|||
// The base's own declared type is carried too, so a chain whose FIRST step is
|
||||
// an unwrap (`repos[0].save()` — index applied directly to the base) has the
|
||||
// container spelling available. Without it that shape declines at step 1.
|
||||
const baseBinding = findReceiverTypeBinding(inScope, chain.baseReceiverName, scopes);
|
||||
// Looked up ONLY when something will read it: the base failed to resolve (so
|
||||
// an unwrap step is the last chance), or an index step will unwrap the
|
||||
// container. `resolveCompoundReceiverClass` already walked the scope chain for
|
||||
// this same name above, so doing it unconditionally duplicated that walk on
|
||||
// every fold — and the U10 census says 86% of chains are pure field/call and
|
||||
// never read it.
|
||||
const needsBaseDeclaredType =
|
||||
baseDef === undefined || chain.steps.some((step) => step.kind === 'index');
|
||||
const baseBinding = needsBaseDeclaredType
|
||||
? findReceiverTypeBinding(inScope, chain.baseReceiverName, scopes)
|
||||
: undefined;
|
||||
|
||||
// A base whose declared type names no class is NOT automatically a dead end:
|
||||
// `repos: User[]` binds to the literal `User[]`, which matches no class
|
||||
|
|
|
|||
|
|
@ -1378,9 +1378,9 @@ export function emitReceiverBoundCalls(
|
|||
siteKind: site.kind,
|
||||
// Structural, from the AST-derived chain the emitter minted — never
|
||||
// re-derived from the source line.
|
||||
receiverShape: classifyReceiverShape(
|
||||
site.receiverChain === undefined ? undefined : decodeReceiverChain(site.receiverChain),
|
||||
),
|
||||
// `decodeReceiverChain` opens with a non-string guard, so the
|
||||
// undefined case needs no ternary here.
|
||||
receiverShape: classifyReceiverShape(decodeReceiverChain(site.receiverChain)),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import type { Range, ReferenceKind } from 'gitnexus-shared';
|
||||
import type { DecodedReceiverChain } from '../utils/receiver-chain-codec.js';
|
||||
|
||||
export type ResolutionSuppressionReason =
|
||||
| 'adl-ordinary-lookup-blocked'
|
||||
|
|
@ -94,25 +95,53 @@ export type ResolutionOutcome =
|
|||
* - `chain-call` every recorded step is a call — `svc.getUser().save()`
|
||||
* - `chain-field` every recorded step is a field — `h.repo.save()`
|
||||
* - `chain-mixed` the chain interleaves both — `svc.getUser().addr.save()`
|
||||
* - `chain-unwrap` the chain contains an `await` or `index` step — the shapes
|
||||
* this work exists to expose. They were previously counted as
|
||||
* FIELDS, because the classifier's parameter widened `kind` to
|
||||
* `string` and they fell into its `else`.
|
||||
* - `no-chain` the site carried no chain, so the receiver was a compound
|
||||
* expression the capture walk could not reduce to a nameable
|
||||
* base (it stopped early, or the base was unencodable)
|
||||
*/
|
||||
export type ReceiverShape = 'chain-call' | 'chain-field' | 'chain-mixed' | 'no-chain';
|
||||
export type ReceiverShape =
|
||||
| 'chain-call'
|
||||
| 'chain-field'
|
||||
| 'chain-mixed'
|
||||
| 'chain-unwrap'
|
||||
| 'no-chain';
|
||||
|
||||
/** Classify a dropped receiver from its encoded chain. `undefined` chain ⇒
|
||||
* `no-chain`; an undecodable one is also `no-chain`, since what we know about
|
||||
* it is exactly that no usable structure survived. */
|
||||
export function classifyReceiverShape(
|
||||
decoded: { readonly steps: readonly { readonly kind: string }[] } | undefined,
|
||||
): ReceiverShape {
|
||||
* it is exactly that no usable structure survived.
|
||||
*
|
||||
* Takes `DecodedReceiverChain` rather than a structural duck-type: widening
|
||||
* `kind` to `string` let `await` and `index` fall into an `else` branch and be
|
||||
* counted as FIELDS, so the two shapes this work exists to expose were
|
||||
* censused as `chain-field`. The discriminated union makes a new step kind a
|
||||
* compile error instead of a silent bucket. */
|
||||
export function classifyReceiverShape(decoded: DecodedReceiverChain | undefined): ReceiverShape {
|
||||
if (decoded === undefined || decoded.steps.length === 0) return 'no-chain';
|
||||
let calls = 0;
|
||||
let fields = 0;
|
||||
let unwraps = 0;
|
||||
for (const step of decoded.steps) {
|
||||
if (step.kind === 'call') calls++;
|
||||
else fields++;
|
||||
switch (step.kind) {
|
||||
case 'call':
|
||||
calls++;
|
||||
break;
|
||||
case 'field':
|
||||
fields++;
|
||||
break;
|
||||
case 'await':
|
||||
case 'index':
|
||||
unwraps++;
|
||||
break;
|
||||
}
|
||||
}
|
||||
// An unwrap step dominates: a chain containing one fails for reasons a pure
|
||||
// field or call chain does not, so folding it into either bucket would
|
||||
// misattribute the population a fix has to target.
|
||||
if (unwraps > 0) return 'chain-unwrap';
|
||||
if (calls > 0 && fields > 0) return 'chain-mixed';
|
||||
return calls > 0 ? 'chain-call' : 'chain-field';
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import type { MixedChainStep } from 'gitnexus-shared';
|
|||
|
||||
import type { SyntaxNode } from './ast-helpers.js';
|
||||
import { CALL_ARGUMENT_LIST_TYPES } from './ast-helpers.js';
|
||||
import { subscriptBase } from './callable-flow-captures.js';
|
||||
|
||||
/** Node types representing call expressions across supported languages. */
|
||||
export const CALL_EXPRESSION_TYPES = new Set([
|
||||
|
|
@ -396,6 +397,26 @@ const SUBSCRIPT_NODE_TYPES = new Set([
|
|||
'indexing_expression', // Kotlin
|
||||
]);
|
||||
|
||||
/**
|
||||
* Can the chain walk descend into this node, or is it the base?
|
||||
*
|
||||
* ONE predicate for all four branches. The call and field branches previously
|
||||
* tested only the call/field sets, so a receiver like `x[0].f().g()` stopped at
|
||||
* the subscript and returned the literal text `x[0]` as the base — which
|
||||
* `isEncodableSegment` accepts, minting a chain whose base binds to nothing. And
|
||||
* `(await f()).g.h()` returned `await f()`, rejected on whitespace, minting no
|
||||
* chain at all. Adding a fifth step kind must not require remembering four
|
||||
* separate call sites.
|
||||
*/
|
||||
function isChainableReceiverNode(node: SyntaxNode): boolean {
|
||||
return (
|
||||
CALL_EXPRESSION_TYPES.has(node.type) ||
|
||||
FIELD_ACCESS_NODE_TYPES.has(node.type) ||
|
||||
AWAIT_EXPRESSION_NODE_TYPES.has(node.type) ||
|
||||
SUBSCRIPT_NODE_TYPES.has(node.type)
|
||||
);
|
||||
}
|
||||
|
||||
const FIELD_ACCESS_NODE_TYPES = new Set([
|
||||
'member_expression', // TS/JS
|
||||
'member_access_expression', // C#
|
||||
|
|
@ -635,10 +656,7 @@ export function extractMixedChain(
|
|||
}
|
||||
if (!innerReceiver) break;
|
||||
|
||||
if (
|
||||
CALL_EXPRESSION_TYPES.has(innerReceiver.type) ||
|
||||
FIELD_ACCESS_NODE_TYPES.has(innerReceiver.type)
|
||||
) {
|
||||
if (isChainableReceiverNode(innerReceiver)) {
|
||||
current = innerReceiver;
|
||||
} else {
|
||||
return {
|
||||
|
|
@ -687,10 +705,7 @@ export function extractMixedChain(
|
|||
|
||||
if (!innerObject) break;
|
||||
|
||||
if (
|
||||
CALL_EXPRESSION_TYPES.has(innerObject.type) ||
|
||||
FIELD_ACCESS_NODE_TYPES.has(innerObject.type)
|
||||
) {
|
||||
if (isChainableReceiverNode(innerObject)) {
|
||||
current = innerObject;
|
||||
} else {
|
||||
return {
|
||||
|
|
@ -708,12 +723,7 @@ export function extractMixedChain(
|
|||
current.namedChildren?.find((c: SyntaxNode) => c !== null) ??
|
||||
null;
|
||||
if (!inner) break;
|
||||
if (
|
||||
CALL_EXPRESSION_TYPES.has(inner.type) ||
|
||||
FIELD_ACCESS_NODE_TYPES.has(inner.type) ||
|
||||
AWAIT_EXPRESSION_NODE_TYPES.has(inner.type) ||
|
||||
SUBSCRIPT_NODE_TYPES.has(inner.type)
|
||||
) {
|
||||
if (isChainableReceiverNode(inner)) {
|
||||
current = inner;
|
||||
} else {
|
||||
return {
|
||||
|
|
@ -725,7 +735,13 @@ export function extractMixedChain(
|
|||
// Name-free: a subscript key is a VALUE, not an identifier the resolver
|
||||
// could look up, so there is no member name to record.
|
||||
chain.unshift({ kind: 'index' });
|
||||
// Shared per-grammar table — it knows Python's `value` and Java's `array`,
|
||||
// which a locally-written ladder omitted (they worked only because the
|
||||
// container happened to be the first named child, an ordering coincidence
|
||||
// rather than a contract). Falls back for grammars whose subscript node
|
||||
// carries no `index` field.
|
||||
const obj =
|
||||
subscriptBase(current) ??
|
||||
current.childForFieldName?.('object') ??
|
||||
current.childForFieldName?.('argument') ??
|
||||
current.childForFieldName?.('operand') ??
|
||||
|
|
@ -733,12 +749,7 @@ export function extractMixedChain(
|
|||
current.namedChildren?.find((c: SyntaxNode) => c !== null) ??
|
||||
null;
|
||||
if (!obj) break;
|
||||
if (
|
||||
CALL_EXPRESSION_TYPES.has(obj.type) ||
|
||||
FIELD_ACCESS_NODE_TYPES.has(obj.type) ||
|
||||
AWAIT_EXPRESSION_NODE_TYPES.has(obj.type) ||
|
||||
SUBSCRIPT_NODE_TYPES.has(obj.type)
|
||||
) {
|
||||
if (isChainableReceiverNode(obj)) {
|
||||
current = obj;
|
||||
} else {
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -1092,7 +1092,10 @@ function unaryOperator(node: SyntaxNode): string | undefined {
|
|||
* `tbl[i]()` join (#2522 review). Field names cover the grammars that field
|
||||
* their subscript nodes; others keep the generic traversal.
|
||||
*/
|
||||
function subscriptBase(node: SyntaxNode): SyntaxNode | null {
|
||||
/** The container operand of a subscript node, per grammar. Exported because the
|
||||
* receiver-chain walk needs the same per-grammar answer — two divergent field
|
||||
* tables for one question is how a new grammar gets half-supported. */
|
||||
export function subscriptBase(node: SyntaxNode): SyntaxNode | null {
|
||||
if (node.childForFieldName('index') === null) return null;
|
||||
return (
|
||||
node.childForFieldName('argument') ?? // C/C++ subscript_expression
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue