refactor(scope-resolution): remove unused shouldShadow / shouldCreateScope hooks

Both LanguageProvider hooks were dead weight:

- `shouldShadow` had zero call sites — the interface declared it,
  Python implemented a trivial always-true no-op, but no consumer
  ever read it. The shadowing decision lives in pythonMergeBindings
  and the central merge algorithm, not in a per-scope predicate.
- `shouldCreateScope` had one call site in pass1BuildScopes but the
  only language implementing it (Python) always returned true. No
  producer ever emits a `@scope.block` for Python, so the hook's
  "declines to create" branch was unreachable. Other languages
  didn't implement it at all.

Removing both:

- Drops the interface declarations in language-provider.ts.
- Drops `shouldCreateScope` from ScopeExtractorHooks Pick and from
  the pass1BuildScopes conditional — the stack-based parent-resolve
  loop becomes unconditional.
- Drops pythonShouldShadow / pythonShouldCreateScope from simple-hooks,
  the Python index barrel, and the python.ts provider wiring.
- Drops the tests that exercised the removed hooks: one block-
  suppression scenario in scope-extractor.test.ts, one shouldCreateScope
  test in parse-worker-scope-integration.test.ts, and the
  pythonShouldShadow / pythonShouldCreateScope always-true assertions
  in python-hooks.test.ts. pythonBindingScopeFor's delegate-to-default
  test is preserved in its own describe block.

Shadowing itself is unchanged: pythonMergeBindings still runs, LEGB
ordering still applies, wildcard transparency is still handled via
the merge precedence rules. The hook API just no longer has a
vestigial per-scope toggle we decided not to use.

Verification: 204/204 test/integration/resolvers/python.test.ts both
REGISTRY_PRIMARY_PYTHON=0 and =1. 335/335 scope-resolution + graph
unit tests (was 339, net -4 after removing the hook-specific
assertions). tsc clean.
This commit is contained in:
Gergo Magyar 2026-04-21 13:50:08 +01:00
parent c3adf4ac7c
commit 97e1a05a51
8 changed files with 9 additions and 120 deletions

View file

@ -386,19 +386,6 @@ interface LanguageProviderConfig {
*/
readonly resolveScopeKind?: (captures: CaptureMatch) => ScopeKind | null;
/**
* Should this scope capture materialize as a real `Scope` node? Return
* `false` to skip scope creation while still emitting declarations that
* would have gone inside (they attach to the enclosing real scope).
*
* Example: Python `if`/`for`/`while` bodies capture as `@scope.block` but
* Python has no block scope hook returns `false` and child declarations
* lift to the enclosing function/module.
*
* Default: undefined (treated as `true` always create).
*/
readonly shouldCreateScope?: (captures: CaptureMatch) => boolean;
/**
* Override where a declaration's name becomes visible. By default the name
* is bound in the innermost enclosing scope; return a different `ScopeId`
@ -510,19 +497,6 @@ interface LanguageProviderConfig {
// ── Resolution phase (RFC §4v2) ────────────────────────────────────
/**
* Does a binding at this scope shadow bindings of the same name in outer
* scopes? Default: any binding shadows (standard lexical scoping). Return
* `false` for transparent-scope edge cases (Python `from x import *`
* contexts, JS `var` hoisting quirks, COBOL PARAGRAPH transparency).
*
* Consulted by `Registry.lookup` Step 1 and by `resolveTypeRef` for
* shadowing decisions during the lexical chain walk.
*
* Default: undefined (treated as `true` any binding shadows).
*/
readonly shouldShadow?: (scope: Scope, bindings: readonly BindingRef[]) => boolean;
/**
* Is this callable definition compatible with the given call-site arity?
* Language-specific rules: Python `*args`/`**kwargs`/defaults, JS default

View file

@ -38,8 +38,6 @@ import {
pythonImportOwningScope,
pythonMergeBindings,
pythonReceiverBinding,
pythonShouldCreateScope,
pythonShouldShadow,
resolvePythonImportTarget,
} from './python/index.js';
@ -98,11 +96,9 @@ export const pythonProvider = defineLanguage({
emitScopeCaptures: emitPythonScopeCaptures,
interpretImport: interpretPythonImport,
interpretTypeBinding: interpretPythonTypeBinding,
shouldCreateScope: pythonShouldCreateScope,
bindingScopeFor: pythonBindingScopeFor,
importOwningScope: pythonImportOwningScope,
mergeBindings: pythonMergeBindings,
shouldShadow: pythonShouldShadow,
receiverBinding: pythonReceiverBinding,
arityCompatibility: pythonArityCompatibility,
resolveImportTarget: resolvePythonImportTarget,

View file

@ -80,9 +80,7 @@ export { pythonMergeBindings } from './merge-bindings.js';
export { pythonArityCompatibility } from './arity.js';
export { resolvePythonImportTarget, type PythonResolveContext } from './import-target.js';
export {
pythonShouldCreateScope,
pythonBindingScopeFor,
pythonImportOwningScope,
pythonShouldShadow,
pythonReceiverBinding,
} from './simple-hooks.js';

View file

@ -7,7 +7,6 @@
*/
import type {
BindingRef,
CaptureMatch,
ParsedImport,
Scope,
@ -16,15 +15,6 @@ import type {
TypeRef,
} from 'gitnexus-shared';
// ─── shouldCreateScope ────────────────────────────────────────────────────
/** We never emit `@scope.block` for Python (no block scope in the
* language), so this hook only ever sees scopes we explicitly want to
* materialize. Always-on. */
export function pythonShouldCreateScope(_captures: CaptureMatch): boolean {
return true;
}
// ─── bindingScopeFor ──────────────────────────────────────────────────────
/** Python has no block scope, so the central extractor's "innermost
@ -55,18 +45,6 @@ export function pythonImportOwningScope(
return null;
}
// ─── shouldShadow ─────────────────────────────────────────────────────────
/** Standard Python lexical scoping. The central default (`true` any
* binding shadows) is correct. Wildcard transparency is handled by
* `pythonMergeBindings`, not by toggling shadowing here.
*
* Implemented as an explicit pass-through so reviewers don't have to
* re-derive the analysis from absence. */
export function pythonShouldShadow(_scope: Scope, _bindings: readonly BindingRef[]): boolean {
return true;
}
// ─── receiverBinding ──────────────────────────────────────────────────────
/** Look up `self` or `cls` in the function scope's type bindings.

View file

@ -25,7 +25,6 @@
* ## The five passes
*
* 1. **Build scope tree.** Walk `@scope.*` matches. For each, consult
* `provider.shouldCreateScope` (default true) and
* `provider.resolveScopeKind` (default: suffix of the capture name).
* Derive parent by lexical-range containment. Hand the resulting
* `Scope[]` to `buildScopeTree` for validation.
@ -95,7 +94,6 @@ import type { LanguageProvider } from './language-provider.js';
*/
export type ScopeExtractorHooks = Pick<
LanguageProvider,
| 'shouldCreateScope'
| 'resolveScopeKind'
| 'bindingScopeFor'
| 'interpretImport'
@ -308,9 +306,7 @@ function draftToScope(draft: ScopeDraft): Scope {
/**
* Convert `@scope.*` matches into `ScopeDraft[]`. Parent relationships
* are derived from range containment (outermost scope containing `range`
* becomes the parent). Scopes with `shouldCreateScope === false` are
* silently omitted their children reparent to the next enclosing
* real scope.
* becomes the parent).
*/
function pass1BuildScopes(
matches: readonly CaptureMatch[],
@ -321,7 +317,6 @@ function pass1BuildScopes(
readonly match: CaptureMatch;
readonly range: Range;
readonly kind: ScopeKind;
readonly create: boolean;
readonly id: ScopeId;
}
@ -331,9 +326,8 @@ function pass1BuildScopes(
if (anchor === undefined) continue;
const kind = resolveKindForScopeMatch(match, anchor, provider);
if (kind === null) continue;
const create = provider.shouldCreateScope?.(match) ?? true;
const id = makeScopeId({ filePath, range: anchor.range, kind });
candidates.push({ match, range: anchor.range, kind, create, id });
candidates.push({ match, range: anchor.range, kind, id });
}
// Sort by (startLine, startCol) ASC, (endLine, endCol) DESC so outer
@ -354,13 +348,9 @@ function pass1BuildScopes(
stack.pop();
}
if (cand.create) {
const parent = stack.length > 0 ? stack[stack.length - 1]!.id : null;
drafts.push(makeDraft(cand.id, parent, cand.kind, cand.range, filePath));
stack.push(cand);
}
// If `cand.create === false`, we don't push it onto the stack — child
// scopes will reparent to whatever's below it.
const parent = stack.length > 0 ? stack[stack.length - 1]!.id : null;
drafts.push(makeDraft(cand.id, parent, cand.kind, cand.range, filePath));
stack.push(cand);
}
return drafts;

View file

@ -42,9 +42,7 @@ const moduleScopeMatch = (): CaptureMatch => ({
* `ScopeExtractorHooks`); the real worker always has a full provider.
*/
function fakeProvider(
hooks: Partial<
Pick<LanguageProvider, 'emitScopeCaptures' | 'shouldCreateScope' | 'resolveScopeKind'>
>,
hooks: Partial<Pick<LanguageProvider, 'emitScopeCaptures' | 'resolveScopeKind'>>,
): LanguageProvider {
return hooks as unknown as LanguageProvider;
}
@ -94,21 +92,6 @@ describe('extractParsedFile', () => {
expect(seenText).toBe('the real text');
expect(seenPath).toBe('deep/path/file.ts');
});
it('honors provider hooks beyond emitScopeCaptures (shouldCreateScope)', () => {
// A Block scope the provider declines to create — the resulting
// ParsedFile should have only the Module scope, not the Block.
const provider = fakeProvider({
emitScopeCaptures: () => [
moduleScopeMatch(),
{ '@scope.block': cap('@scope.block', 10, 0, 20, 0) },
],
shouldCreateScope: (match) => match['@scope.block'] === undefined,
});
const result = extractParsedFile(provider, 'src', 'a.ts');
expect(result!.scopes).toHaveLength(1);
expect(result!.scopes[0]!.kind).toBe('Module');
});
});
describe('error resilience — never breaks legacy parsing', () => {

View file

@ -23,8 +23,6 @@ import {
pythonImportOwningScope,
pythonMergeBindings,
pythonReceiverBinding,
pythonShouldShadow,
pythonShouldCreateScope,
pythonBindingScopeFor,
resolvePythonImportTarget,
} from '../../../../src/core/ingestion/languages/python/index.js';
@ -217,18 +215,10 @@ describe('pythonImportOwningScope', () => {
});
});
// ─── shouldShadow / shouldCreateScope / bindingScopeFor — defensive ───────
// ─── bindingScopeFor — defensive ──────────────────────────────────────────
describe('pythonShouldShadow / pythonShouldCreateScope / pythonBindingScopeFor', () => {
it('shouldShadow always returns true (standard LEGB)', () => {
expect(pythonShouldShadow(fnScope(), [])).toBe(true);
});
it('shouldCreateScope always returns true (no @scope.block emitted)', () => {
expect(pythonShouldCreateScope({})).toBe(true);
});
it('bindingScopeFor delegates to default for every input', () => {
describe('pythonBindingScopeFor', () => {
it('delegates to default for every input', () => {
expect(pythonBindingScopeFor({}, fnScope(), {} as never)).toBeNull();
});
});

View file

@ -157,26 +157,6 @@ describe('Pass 1: scope tree', () => {
for (const fn of fns) expect(fn.parent).toBe(mod.id);
});
it('honors `provider.shouldCreateScope === false` by reparenting children to next real scope', () => {
// Block at [10:0..25:0] is SUPPRESSED; inner function at [12:0..20:0]
// should reparent to the enclosing Module instead of the Block.
const result = extract(
[
scopeMatch('module', 1, 0, 100, 0),
scopeMatch('block', 10, 0, 25, 0),
scopeMatch('function', 12, 0, 20, 0),
],
'a.ts',
mockProvider({
shouldCreateScope: (match) => match['@scope.block'] === undefined,
}),
);
const mod = result.scopes.find((s) => s.kind === 'Module')!;
const fn = result.scopes.find((s) => s.kind === 'Function')!;
expect(result.scopes).toHaveLength(2); // block suppressed
expect(fn.parent).toBe(mod.id);
});
it('uses `provider.resolveScopeKind` to override the default kind from the suffix', () => {
// Provider upgrades a `@scope.block` to `Expression` for a comprehension-
// style use case.