From f9956d89e29d3e44793c1dfaad597ddd6a1280dc Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Wed, 22 Apr 2026 11:53:37 +0100 Subject: [PATCH] refactor(python-scope): remove as-unknown-as casts in scope-resolver (mirrors Unit 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replay the C# scope-resolver cleanup on the Python side so both providers share a single clean pattern: * Drop `ws as unknown as WorkspaceIndex` — `WorkspaceIndex` is `unknown` in the shared contract, so the narrow context assigns structurally without a cast. * Drop `{ id: scopeId } as unknown as Scope` — `pythonMergeBindings` never read the scope (the parameter was `_scope`), so the stub was a type-only ghost. Signature is now `(bindings)` and the LanguageProvider slot wraps with an arrow adapter. * Drop `allFilePaths as Set` — the orchestrator hands a `ReadonlySet`; we copy it into a `Set` at the resolver adapter so the legacy downstream `resolvePythonImportInternal` chain (typed for mutable `Set`) keeps working. The copy is O(N) once per import, trivial cost. Left intact on purpose: the `(callsite, def) → (def, callsite)` arrow wrapper on `arityCompatibility`. That's a documented shape difference between `LanguageProvider.arityCompatibility(def, callsite)` and `ScopeResolver.arityCompatibility(callsite, def)`; both providers (Python + C#) carry the same wrapper. Reconciling is a separate refactor across both contracts. No runtime behavior change. Verified: - npx tsc --noEmit clean - Python + C# unit + integration suites 529/529 passing --- .../src/core/ingestion/languages/python.ts | 2 +- .../languages/python/import-target.ts | 8 +++++ .../languages/python/merge-bindings.ts | 7 ++-- .../languages/python/scope-resolver.ts | 34 +++++++++---------- .../python/python-hooks.test.ts | 16 ++++----- 5 files changed, 35 insertions(+), 32 deletions(-) diff --git a/gitnexus/src/core/ingestion/languages/python.ts b/gitnexus/src/core/ingestion/languages/python.ts index fa1b60b94..8dcb5b2d2 100644 --- a/gitnexus/src/core/ingestion/languages/python.ts +++ b/gitnexus/src/core/ingestion/languages/python.ts @@ -98,7 +98,7 @@ export const pythonProvider = defineLanguage({ interpretTypeBinding: interpretPythonTypeBinding, bindingScopeFor: pythonBindingScopeFor, importOwningScope: pythonImportOwningScope, - mergeBindings: pythonMergeBindings, + mergeBindings: (_scope, bindings) => pythonMergeBindings(bindings), receiverBinding: pythonReceiverBinding, arityCompatibility: pythonArityCompatibility, resolveImportTarget: resolvePythonImportTarget, diff --git a/gitnexus/src/core/ingestion/languages/python/import-target.ts b/gitnexus/src/core/ingestion/languages/python/import-target.ts index c99620115..3905f3301 100644 --- a/gitnexus/src/core/ingestion/languages/python/import-target.ts +++ b/gitnexus/src/core/ingestion/languages/python/import-target.ts @@ -15,6 +15,10 @@ import { resolvePythonImportInternal } from '../../import-resolvers/python.js'; export interface PythonResolveContext { readonly fromFile: string; + /** Mutable `Set` because the legacy `resolvePythonImportInternal` + * chain downstream is typed to accept `Set`. Callers that + * only hold a `ReadonlySet` should copy via `new Set(...)` at the + * adapter boundary. */ readonly allFilePaths: Set; } @@ -22,6 +26,10 @@ export function resolvePythonImportTarget( parsedImport: ParsedImport, workspaceIndex: WorkspaceIndex, ): string | null { + // WorkspaceIndex is `unknown` in the shared contract (Ring 1 + // placeholder). The scope-resolution orchestrator hands us a + // PythonResolveContext-shaped object; narrow structurally rather + // than via a cast chain so unexpected shapes return null cleanly. const ctx = workspaceIndex as PythonResolveContext | undefined; if ( ctx === undefined || diff --git a/gitnexus/src/core/ingestion/languages/python/merge-bindings.ts b/gitnexus/src/core/ingestion/languages/python/merge-bindings.ts index cf37ed6b4..1487698fa 100644 --- a/gitnexus/src/core/ingestion/languages/python/merge-bindings.ts +++ b/gitnexus/src/core/ingestion/languages/python/merge-bindings.ts @@ -13,7 +13,7 @@ * purposes). */ -import type { BindingRef, Scope } from 'gitnexus-shared'; +import type { BindingRef } from 'gitnexus-shared'; const TIER_LOCAL = 0; const TIER_IMPORT = 1; @@ -35,10 +35,7 @@ function tierOf(b: BindingRef): number { } } -export function pythonMergeBindings( - _scope: Scope, - bindings: readonly BindingRef[], -): readonly BindingRef[] { +export function pythonMergeBindings(bindings: readonly BindingRef[]): readonly BindingRef[] { if (bindings.length === 0) return bindings; let bestTier = Number.POSITIVE_INFINITY; diff --git a/gitnexus/src/core/ingestion/languages/python/scope-resolver.ts b/gitnexus/src/core/ingestion/languages/python/scope-resolver.ts index a29abfe09..8f0ca23f5 100644 --- a/gitnexus/src/core/ingestion/languages/python/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/languages/python/scope-resolver.ts @@ -12,7 +12,7 @@ * the 2 booleans, and register in `scope-resolution/pipeline/registry.ts`. */ -import type { ParsedFile, Scope, WorkspaceIndex } from 'gitnexus-shared'; +import type { ParsedFile } from 'gitnexus-shared'; import { SupportedLanguages } from 'gitnexus-shared'; import { buildMro, defaultLinearize } from '../../scope-resolution/passes/mro.js'; import { populateClassOwnedMembers } from '../../scope-resolution/scope/walkers.js'; @@ -31,30 +31,30 @@ const pythonScopeResolver: ScopeResolver = { importEdgeReason: 'python-scope: import', resolveImportTarget: (targetRaw, fromFile, allFilePaths) => { - // PythonResolveContext expects a mutable Set; orchestrator hands us - // a ReadonlySet — safe to widen since the resolver only reads. - const ws: PythonResolveContext = { - fromFile, - allFilePaths: allFilePaths as Set, - }; + // Copy the orchestrator's `ReadonlySet` into a `Set` because the + // legacy Python resolver chain (`resolvePythonImportInternal` → + // `resolveAbsoluteFromFiles` / `hasRepoCandidate`) is typed to + // receive a mutable `Set`. The copy is O(N) but called + // once per import — trivial compared to the parser work. + const ws: PythonResolveContext = { fromFile, allFilePaths: new Set(allFilePaths) }; + // `WorkspaceIndex` is an opaque `unknown` placeholder in the + // shared contract, so `ws` passes structurally without a cast. return resolvePythonImportTarget( { kind: 'named', localName: '_', importedName: '_', targetRaw }, - ws as unknown as WorkspaceIndex, + ws, ); }, // Python LEGB precedence: local > import/namespace/reexport > wildcard. - mergeBindings: (existing, incoming, scopeId) => { - // pythonMergeBindings(scope, bindings) only consults BindingRef.origin - // for tier ordering, not scope.kind. A shape-stub satisfies the type - // contract without falsifying behavior. Widen the readonly result - // to a mutable BindingRef[] for the orchestrator's hook signature. - const fakeScope = { id: scopeId } as unknown as Scope; - return [...pythonMergeBindings(fakeScope, [...existing, ...incoming])]; - }, + // The per-scope id is unused by pythonMergeBindings (tier ordering + // is computed purely from BindingRef.origin), so we don't need to + // synthesize a Scope. + mergeBindings: (existing, incoming) => [...pythonMergeBindings([...existing, ...incoming])], // Adapter: pythonArityCompatibility predates RegistryProviders and - // uses (def, callsite). Contract is (callsite, def). + // uses (def, callsite). ScopeResolver contract is (callsite, def). + // Wrapper kept to honor both contracts without altering the legacy + // shape that LanguageProvider.arityCompatibility consumes. arityCompatibility: (callsite, def) => pythonArityCompatibility(def, callsite), buildMro: (graph, parsedFiles, nodeLookup) => diff --git a/gitnexus/test/unit/scope-resolution/python/python-hooks.test.ts b/gitnexus/test/unit/scope-resolution/python/python-hooks.test.ts index 389a9f448..1411b6548 100644 --- a/gitnexus/test/unit/scope-resolution/python/python-hooks.test.ts +++ b/gitnexus/test/unit/scope-resolution/python/python-hooks.test.ts @@ -144,48 +144,46 @@ describe('pythonReceiverBinding', () => { // ─── mergeBindings ───────────────────────────────────────────────────────── describe('pythonMergeBindings — LEGB precedence', () => { - const scope = fnScope(); - it('local shadows imported', () => { const local = binding('local', 'L'); const imp = binding('import', 'I'); - expect(pythonMergeBindings(scope, [imp, local])).toEqual([local]); + expect(pythonMergeBindings([imp, local])).toEqual([local]); }); it('explicit import shadows wildcard', () => { const imp = binding('import', 'I'); const wc = binding('wildcard', 'W'); - expect(pythonMergeBindings(scope, [wc, imp])).toEqual([imp]); + expect(pythonMergeBindings([wc, imp])).toEqual([imp]); }); it('local shadows BOTH imported and wildcard', () => { const local = binding('local', 'L'); const imp = binding('import', 'I'); const wc = binding('wildcard', 'W'); - expect(pythonMergeBindings(scope, [wc, imp, local])).toEqual([local]); + expect(pythonMergeBindings([wc, imp, local])).toEqual([local]); }); it('keeps multiple bindings within the same tier (overload-like)', () => { const a = binding('local', 'A'); const b = binding('local', 'B'); - expect(pythonMergeBindings(scope, [a, b])).toEqual([a, b]); + expect(pythonMergeBindings([a, b])).toEqual([a, b]); }); it('dedupes by DefId — same nodeId collapses', () => { const a = binding('local', 'A'); const a2 = binding('local', 'A'); - expect(pythonMergeBindings(scope, [a, a2])).toHaveLength(1); + expect(pythonMergeBindings([a, a2])).toHaveLength(1); }); it('returns empty when given empty', () => { - expect(pythonMergeBindings(scope, [])).toEqual([]); + expect(pythonMergeBindings([])).toEqual([]); }); it('namespace and reexport tie with explicit import (same tier)', () => { const ns = binding('namespace', 'N'); const re = binding('reexport', 'R'); const imp = binding('import', 'I'); - expect(pythonMergeBindings(scope, [ns, re, imp])).toHaveLength(3); + expect(pythonMergeBindings([ns, re, imp])).toHaveLength(3); }); });