From 996ea4f9e4c4d3089f6bdfe50a4a490f8dffaa5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Sat, 18 Apr 2026 16:09:54 +0100 Subject: [PATCH] feat(shared): resolveTypeRef strict single-return type resolver (#916, RFC #909 Ring 2 SHARED) (#959) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements RFC §4.6: a strict, pure resolver for `TypeRef`s used by `Registry.lookup` Step 2 (type-binding propagation) and by any caller that wants the single best type-target for an annotation without paying for the full evidence pipeline. Algorithm (strict): 1. Walk the scope chain from `ref.declaredAtScope`: - Return the first binding for `rawName` whose origin is in `{'local','import','namespace','reexport'}` AND whose `def.type` is a type-kind (class-like, interface-like, enum-like, alias-like). - If bindings exist but none qualify (non-type shadow, wildcard-only origin), return null immediately — do NOT fall through to the global qualified-name index. 2. If `rawName` is dotted and the scope walk produced no match, consult `QualifiedNameIndex.byQualifiedName`. Only accept a UNIQUE type-kind hit; ambiguous or non-type results return null. `'wildcard'` is deliberately excluded from strict origins — a wildcard-expanded name is too loose to anchor type resolution. Module placement: `gitnexus-shared/src/scope-resolution/resolve-type-ref.ts` (alongside sibling indexes) rather than the issue's suggested `gitnexus-shared/src/resolve-type-ref.ts`, for consistency with the rest of the RFC §2/§3 surface. A minimal `ScopeLookup` interface is declared inline so #916 ships standalone; #912's `ScopeTree` will satisfy this contract without change. Closes part of #909. --- gitnexus-shared/src/index.ts | 4 + .../src/scope-resolution/resolve-type-ref.ts | 152 +++++++++ .../scope-resolution/resolve-type-ref.test.ts | 318 ++++++++++++++++++ 3 files changed, 474 insertions(+) create mode 100644 gitnexus-shared/src/scope-resolution/resolve-type-ref.ts create mode 100644 gitnexus/test/unit/scope-resolution/resolve-type-ref.test.ts diff --git a/gitnexus-shared/src/index.ts b/gitnexus-shared/src/index.ts index 0f4bf8e299..b572716060 100644 --- a/gitnexus-shared/src/index.ts +++ b/gitnexus-shared/src/index.ts @@ -71,6 +71,10 @@ export type { ModuleScopeIndex, ModuleScopeEntry } from './scope-resolution/modu export { buildQualifiedNameIndex } from './scope-resolution/qualified-name-index.js'; export type { QualifiedNameIndex } from './scope-resolution/qualified-name-index.js'; +// Strict type-reference resolver (RFC §4.6; Ring 2 SHARED #916) +export { resolveTypeRef } from './scope-resolution/resolve-type-ref.js'; +export type { ResolveTypeRefContext, ScopeLookup } from './scope-resolution/resolve-type-ref.js'; + // Shadow-mode diff + aggregation (RFC §6.3; Ring 2 SHARED #918) export { diffResolutions } from './scope-resolution/shadow/diff.js'; export type { diff --git a/gitnexus-shared/src/scope-resolution/resolve-type-ref.ts b/gitnexus-shared/src/scope-resolution/resolve-type-ref.ts new file mode 100644 index 0000000000..63bd6a8da2 --- /dev/null +++ b/gitnexus-shared/src/scope-resolution/resolve-type-ref.ts @@ -0,0 +1,152 @@ +/** + * `resolveTypeRef` — strict single-return resolver for `TypeRef`s + * (RFC §4.6; Ring 2 SHARED #916). + * + * Narrower contract than `Registry.lookup`: no name-only global fallback, no + * confidence ranking, no arity check. Used by `Registry.lookup` Step 2 (type- + * binding propagation) and by any caller that wants the single best type- + * target for an annotation without paying for the full evidence pipeline. + * + * **Algorithm (strict).** Walk the scope chain from `ref.declaredAtScope`: + * + * 1. At each scope, inspect `bindings.get(ref.rawName)`: + * - If one of the bindings is a **type-kind** def with a **strict origin** + * (`'local' | 'import' | 'namespace' | 'reexport'`), return it. + * - If any binding for this name exists at this scope but none qualifies + * (e.g., a local variable named `User` shadows an outer import of class + * `User`), return `null`. The nearer binding shadows; we do NOT fall + * through to the global qualified-name index. + * - Otherwise continue to the parent scope. + * 2. If the raw name is a dotted path (e.g., `'models.User'`) and the scope + * walk produced no match, consult `QualifiedNameIndex.byQualifiedName`. + * Only accept **exactly one** type-kind hit — anything ambiguous returns + * `null` rather than a guess. + * 3. Return `null`. + * + * **What `'strict' origins' means.** `'wildcard'` is intentionally excluded. + * A wildcard-expanded name (`from x import *`) is too loose to use as an + * anchor for type resolution — it gives no signal about whether the name was + * actually imported. `Registry.lookup` may accept wildcard bindings at its + * own discretion (with lower evidence weight); `resolveTypeRef` does not. + * + * **What 'type-kind' means.** The subset of `NodeLabel` that a type annotation + * may legitimately reference: class-like, interface-like, enum-like, and + * alias-like kinds. See `TYPE_KINDS` below. + * + * Pure function — safe to call repeatedly; no side effects. + */ + +import type { NodeLabel } from '../graph/types.js'; +import type { SymbolDefinition } from './symbol-definition.js'; +import type { BindingRef, Scope, ScopeId, TypeRef } from './types.js'; +import type { DefIndex } from './def-index.js'; +import type { QualifiedNameIndex } from './qualified-name-index.js'; + +// ─── Public contracts ─────────────────────────────────────────────────────── + +/** + * Minimal scope-lookup contract required by `resolveTypeRef`. Implemented by + * the `ScopeTree` from #912; declared here so #916 can ship as a standalone + * piece without a hard dependency on the full scope-tree implementation. Any + * structure that hands back a `Scope` by `ScopeId` satisfies this contract. + */ +export interface ScopeLookup { + getScope(id: ScopeId): Scope | undefined; +} + +/** + * All inputs `resolveTypeRef` needs from the semantic model. Bundled into a + * context object so the call site stays short and the interface is stable as + * additional indexes get threaded through in later rings. + */ +export interface ResolveTypeRefContext { + readonly scopes: ScopeLookup; + readonly defIndex: DefIndex; + readonly qualifiedNameIndex: QualifiedNameIndex; +} + +// ─── Strict policy constants ──────────────────────────────────────────────── + +/** `'wildcard'` is deliberately absent. See file header. */ +const STRICT_ORIGINS: ReadonlySet = new Set([ + 'local', + 'import', + 'namespace', + 'reexport', +]); + +/** + * `NodeLabel` values that may appear on the RHS of a type annotation. + * + * Includes the usual class-like and interface-like kinds plus the alias-like + * ones (`TypeAlias`, `Typedef`). `Namespace` is excluded — it is a scope + * container, not a value type. `Function` / `Method` / `Variable` are + * excluded by design: a `rawName` bound to them at a strict origin is a + * *shadowing* binding, which the algorithm short-circuits to `null`. + */ +const TYPE_KINDS: ReadonlySet = new Set([ + 'Class', + 'Interface', + 'Enum', + 'Struct', + 'Union', + 'Trait', + 'TypeAlias', + 'Typedef', + 'Record', + 'Delegate', + 'Annotation', + 'Template', +]); + +// ─── Main entry point ────────────────────────────────────────────────────── + +export function resolveTypeRef(ref: TypeRef, ctx: ResolveTypeRefContext): SymbolDefinition | null { + // Phase 1: scope-chain walk anchored at the declaration site. + let currentId: ScopeId | null = ref.declaredAtScope; + const visited = new Set(); + + while (currentId !== null) { + // Cycle guard — a well-formed scope tree never loops, but a bug in the + // construction path should fail fast here rather than hanging. + if (visited.has(currentId)) return null; + visited.add(currentId); + + const scope = ctx.scopes.getScope(currentId); + if (scope === undefined) return null; // broken chain = unresolvable + + const bindings = scope.bindings.get(ref.rawName); + if (bindings !== undefined && bindings.length > 0) { + // At least one binding exists at this scope → it is the shadowing site. + // Either one of them qualifies, or the name is shadowed by a non-type. + for (const binding of bindings) { + if (!STRICT_ORIGINS.has(binding.origin)) continue; + if (TYPE_KINDS.has(binding.def.type)) { + return binding.def; + } + } + // Shadowed by a non-type / non-strict-origin binding. Fail fast — no + // global fallback, no walk to the parent. + return null; + } + + currentId = scope.parent; + } + + // Phase 2: dotted fallback via `QualifiedNameIndex`. Only accept a unique + // type-kind hit; anything ambiguous returns null (strict: no guesses). + if (ref.rawName.includes('.')) { + const candidates = ctx.qualifiedNameIndex.get(ref.rawName); + let onlyTypeDef: SymbolDefinition | null = null; + for (const defId of candidates) { + const def = ctx.defIndex.get(defId); + if (def === undefined) continue; + if (!TYPE_KINDS.has(def.type)) continue; + if (onlyTypeDef !== null) return null; // ambiguous + onlyTypeDef = def; + } + if (onlyTypeDef !== null) return onlyTypeDef; + } + + return null; +} diff --git a/gitnexus/test/unit/scope-resolution/resolve-type-ref.test.ts b/gitnexus/test/unit/scope-resolution/resolve-type-ref.test.ts new file mode 100644 index 0000000000..2e2c082631 --- /dev/null +++ b/gitnexus/test/unit/scope-resolution/resolve-type-ref.test.ts @@ -0,0 +1,318 @@ +/** + * Unit tests for `resolveTypeRef` (RFC #909 Ring 2 SHARED #916). + * + * Covers: local type, parameter/return annotation via scope walk, aliased + * import, re-exported type, shadowing by local variable, wildcard-origin + * ignored, qualified-name fallback (unique + ambiguous), broken scope chain, + * cycle guard. + */ + +import { describe, it, expect } from 'vitest'; +import { + resolveTypeRef, + buildDefIndex, + buildQualifiedNameIndex, + type ResolveTypeRefContext, + type ScopeLookup, + type BindingRef, + type ImportEdge, + type Scope, + type ScopeId, + type SymbolDefinition, + type TypeRef, +} from 'gitnexus-shared'; + +// ─── Test helpers ─────────────────────────────────────────────────────────── + +const mkDef = (overrides: Partial & { nodeId: string }): SymbolDefinition => ({ + nodeId: overrides.nodeId, + filePath: overrides.filePath ?? 'src/test.ts', + type: overrides.type ?? 'Class', + ...overrides, +}); + +const mkBinding = ( + def: SymbolDefinition, + origin: BindingRef['origin'], + via?: ImportEdge, +): BindingRef => ({ def, origin, ...(via !== undefined ? { via } : {}) }); + +const mkScope = ( + id: ScopeId, + parent: ScopeId | null, + bindings: Record = {}, + filePath = 'src/test.ts', +): Scope => ({ + id, + parent, + kind: 'Module', + range: { startLine: 1, startCol: 0, endLine: 100, endCol: 0 }, + filePath, + bindings: new Map(Object.entries(bindings)), + ownedDefs: [], + imports: [], + typeBindings: new Map(), +}); + +const mkLookup = (scopes: Scope[]): ScopeLookup => { + const byId = new Map(scopes.map((s) => [s.id, s])); + return { getScope: (id) => byId.get(id) }; +}; + +const mkCtx = (scopes: Scope[], defs: SymbolDefinition[]): ResolveTypeRefContext => ({ + scopes: mkLookup(scopes), + defIndex: buildDefIndex(defs), + qualifiedNameIndex: buildQualifiedNameIndex(defs), +}); + +const typeRef = ( + rawName: string, + declaredAtScope: ScopeId, + source: TypeRef['source'] = 'parameter-annotation', +): TypeRef => ({ rawName, declaredAtScope, source }); + +// ─── Tests ───────────────────────────────────────────────────────────────── + +describe('resolveTypeRef', () => { + describe('scope-chain walk', () => { + it('resolves a local type defined in the same scope', () => { + const userClass = mkDef({ nodeId: 'def:User', type: 'Class' }); + const moduleScope = mkScope('scope:module', null, { + User: [mkBinding(userClass, 'local')], + }); + const ctx = mkCtx([moduleScope], [userClass]); + const result = resolveTypeRef(typeRef('User', 'scope:module'), ctx); + expect(result).toBe(userClass); + }); + + it('walks parent scopes when the name is not bound locally (return-annotation case)', () => { + const userClass = mkDef({ nodeId: 'def:User', type: 'Class' }); + const moduleScope = mkScope('scope:module', null, { + User: [mkBinding(userClass, 'local')], + }); + const functionScope = mkScope('scope:fn', 'scope:module'); + const ctx = mkCtx([moduleScope, functionScope], [userClass]); + const result = resolveTypeRef(typeRef('User', 'scope:fn', 'return-annotation'), ctx); + expect(result).toBe(userClass); + }); + + it('returns the closest binding when the name is bound at multiple levels', () => { + const outerUser = mkDef({ nodeId: 'def:outer', type: 'Class' }); + const innerUser = mkDef({ nodeId: 'def:inner', type: 'Class' }); + const moduleScope = mkScope('scope:module', null, { + User: [mkBinding(outerUser, 'local')], + }); + const classScope = mkScope('scope:class', 'scope:module', { + User: [mkBinding(innerUser, 'local')], + }); + const ctx = mkCtx([moduleScope, classScope], [outerUser, innerUser]); + const result = resolveTypeRef(typeRef('User', 'scope:class'), ctx); + expect(result).toBe(innerUser); // inner shadows outer + }); + }); + + describe('import origins', () => { + it('resolves a plain named import', () => { + const userClass = mkDef({ nodeId: 'def:User', filePath: 'models.ts', type: 'Class' }); + const moduleScope = mkScope('scope:module', null, { + User: [mkBinding(userClass, 'import')], + }); + const ctx = mkCtx([moduleScope], [userClass]); + expect(resolveTypeRef(typeRef('User', 'scope:module'), ctx)).toBe(userClass); + }); + + it('resolves an aliased import under the alias (e.g., `import { User as Account }`)', () => { + // In `def save_user(user: Account)` where `Account` is an aliased import + // of `User`, we resolve `Account` to the underlying `User` class def. + const userClass = mkDef({ nodeId: 'def:User', filePath: 'models.ts', type: 'Class' }); + const importEdge: ImportEdge = { + localName: 'Account', + targetFile: 'models.ts', + targetExportedName: 'User', + targetDefId: 'def:User', + kind: 'alias', + }; + const moduleScope = mkScope('scope:module', null, { + Account: [mkBinding(userClass, 'import', importEdge)], + }); + const ctx = mkCtx([moduleScope], [userClass]); + expect(resolveTypeRef(typeRef('Account', 'scope:module'), ctx)).toBe(userClass); + }); + + it('resolves a namespace-origin binding (e.g., `import * as np`)', () => { + const numpyMod = mkDef({ nodeId: 'def:numpy-mod', type: 'Namespace' }); + // A namespace binding must resolve to a type-kind def to satisfy strict + // mode. Here the binding is the namespace module itself — treat it as a + // shadowing non-type and expect null. + const moduleScope = mkScope('scope:module', null, { + np: [mkBinding(numpyMod, 'namespace')], + }); + const ctx = mkCtx([moduleScope], [numpyMod]); + // Namespace is NOT a type kind → strict returns null. + expect(resolveTypeRef(typeRef('np', 'scope:module'), ctx)).toBeNull(); + }); + + it('resolves a re-exported type (`export { X } from `./y`)', () => { + const userClass = mkDef({ nodeId: 'def:User', filePath: 'y.ts', type: 'Class' }); + const moduleScope = mkScope('scope:module', null, { + User: [mkBinding(userClass, 'reexport')], + }); + const ctx = mkCtx([moduleScope], [userClass]); + expect(resolveTypeRef(typeRef('User', 'scope:module'), ctx)).toBe(userClass); + }); + + it('ignores wildcard origin — not in the strict set', () => { + const userClass = mkDef({ nodeId: 'def:User', filePath: 'models.ts', type: 'Class' }); + const moduleScope = mkScope('scope:module', null, { + User: [mkBinding(userClass, 'wildcard')], + }); + const ctx = mkCtx([moduleScope], [userClass]); + // Binding exists but wildcard is not strict → return null (shadow). + expect(resolveTypeRef(typeRef('User', 'scope:module'), ctx)).toBeNull(); + }); + + it('prefers a strict-origin type binding over a non-strict binding at the same scope', () => { + const wildcardClass = mkDef({ nodeId: 'def:wild', type: 'Class' }); + const importClass = mkDef({ nodeId: 'def:import', type: 'Class' }); + const moduleScope = mkScope('scope:module', null, { + // The strict-origin binding wins regardless of input order. + User: [mkBinding(wildcardClass, 'wildcard'), mkBinding(importClass, 'import')], + }); + const ctx = mkCtx([moduleScope], [wildcardClass, importClass]); + expect(resolveTypeRef(typeRef('User', 'scope:module'), ctx)).toBe(importClass); + }); + }); + + describe('shadowing (non-type binding fails fast)', () => { + it('returns null when a local variable shadows an outer imported type', () => { + // Outer module has `import { User }`; inner function declares `User = 5`. + // Per RFC §4.6, the local non-type shadows; strict resolver returns null. + const importedUser = mkDef({ nodeId: 'def:imp', type: 'Class' }); + const localUser = mkDef({ nodeId: 'def:local', type: 'Variable' }); + const moduleScope = mkScope('scope:module', null, { + User: [mkBinding(importedUser, 'import')], + }); + const functionScope = mkScope('scope:fn', 'scope:module', { + User: [mkBinding(localUser, 'local')], + }); + const ctx = mkCtx([moduleScope, functionScope], [importedUser, localUser]); + expect(resolveTypeRef(typeRef('User', 'scope:fn'), ctx)).toBeNull(); + }); + + it('returns null when the only binding at the declaration scope is a Method', () => { + const method = mkDef({ nodeId: 'def:m', type: 'Method' }); + const scope = mkScope('scope:class', null, { + save: [mkBinding(method, 'local')], + }); + const ctx = mkCtx([scope], [method]); + expect(resolveTypeRef(typeRef('save', 'scope:class'), ctx)).toBeNull(); + }); + + it('does NOT fall through to the qualified-name index when shadowed', () => { + // Even if `app.User` is a unique type in the qualified-name index, a + // shadowing non-type binding at the declaration scope should short-circuit. + const shadowVar = mkDef({ nodeId: 'def:v', type: 'Variable' }); + const globalType = mkDef({ + nodeId: 'def:g', + qualifiedName: 'app.User', + type: 'Class', + }); + const scope = mkScope('scope:s', null, { + 'app.User': [mkBinding(shadowVar, 'local')], + }); + const ctx = mkCtx([scope], [shadowVar, globalType]); + expect(resolveTypeRef(typeRef('app.User', 'scope:s'), ctx)).toBeNull(); + }); + }); + + describe('dotted qualified-name fallback', () => { + it('resolves a unique qualified name when no scope binding matches', () => { + const userClass = mkDef({ + nodeId: 'def:appUser', + qualifiedName: 'app.models.User', + type: 'Class', + }); + const scope = mkScope('scope:s', null); // no bindings for `app.models.User` + const ctx = mkCtx([scope], [userClass]); + expect(resolveTypeRef(typeRef('app.models.User', 'scope:s'), ctx)).toBe(userClass); + }); + + it('does NOT apply the qualified-name fallback for non-dotted names', () => { + const simple = mkDef({ nodeId: 'def:s', qualifiedName: 'User', type: 'Class' }); + const scope = mkScope('scope:s', null); + const ctx = mkCtx([scope], [simple]); + // Even though `User` is in the qname index as `User`, the fallback only + // fires for dotted names (scope walk is the answer for simple names). + expect(resolveTypeRef(typeRef('User', 'scope:s'), ctx)).toBeNull(); + }); + + it('returns null when the qualified name is ambiguous across type defs', () => { + const a = mkDef({ nodeId: 'def:a', qualifiedName: 'app.User', type: 'Class' }); + const b = mkDef({ nodeId: 'def:b', qualifiedName: 'app.User', type: 'Class' }); + const scope = mkScope('scope:s', null); + const ctx = mkCtx([scope], [a, b]); + expect(resolveTypeRef(typeRef('app.User', 'scope:s'), ctx)).toBeNull(); + }); + + it('ignores non-type-kind hits and accepts the single type hit', () => { + const cls = mkDef({ nodeId: 'def:c', qualifiedName: 'app.User', type: 'Class' }); + const fn = mkDef({ nodeId: 'def:f', qualifiedName: 'app.User', type: 'Function' }); + const scope = mkScope('scope:s', null); + const ctx = mkCtx([scope], [cls, fn]); + expect(resolveTypeRef(typeRef('app.User', 'scope:s'), ctx)).toBe(cls); + }); + + it('returns null when no qualified-name hit is a type kind', () => { + const fn = mkDef({ nodeId: 'def:f', qualifiedName: 'app.User', type: 'Function' }); + const scope = mkScope('scope:s', null); + const ctx = mkCtx([scope], [fn]); + expect(resolveTypeRef(typeRef('app.User', 'scope:s'), ctx)).toBeNull(); + }); + }); + + describe('robustness', () => { + it('returns null for a missing type (no scope binding, no qname hit)', () => { + const scope = mkScope('scope:s', null); + const ctx = mkCtx([scope], []); + expect(resolveTypeRef(typeRef('User', 'scope:s'), ctx)).toBeNull(); + }); + + it('returns null when the declaredAtScope id is not known to the lookup', () => { + const ctx = mkCtx([], []); + expect(resolveTypeRef(typeRef('User', 'scope:missing'), ctx)).toBeNull(); + }); + + it('returns null when a parent pointer references an unknown scope (broken chain)', () => { + const functionScope = mkScope('scope:fn', 'scope:ghost'); + const ctx = mkCtx([functionScope], []); + expect(resolveTypeRef(typeRef('User', 'scope:fn'), ctx)).toBeNull(); + }); + + it('terminates cleanly on a cyclic parent chain (defensive guard)', () => { + // A well-formed scope tree is acyclic; construction bugs shouldn't hang. + const a: Scope = mkScope('scope:a', 'scope:b'); + const b: Scope = mkScope('scope:b', 'scope:a'); + const ctx = mkCtx([a, b], []); + expect(resolveTypeRef(typeRef('User', 'scope:a'), ctx)).toBeNull(); + }); + + it('accepts all annotation source flavors uniformly', () => { + const userClass = mkDef({ nodeId: 'def:User', type: 'Class' }); + const scope = mkScope('scope:s', null, { + User: [mkBinding(userClass, 'local')], + }); + const ctx = mkCtx([scope], [userClass]); + for (const source of [ + 'annotation', + 'parameter-annotation', + 'return-annotation', + 'self', + 'assignment-inferred', + 'constructor-inferred', + 'receiver-propagated', + ] as const) { + expect(resolveTypeRef(typeRef('User', 'scope:s', source), ctx)).toBe(userClass); + } + }); + }); +});