diff --git a/gitnexus/src/core/ingestion/languages/python/builtin-descriptors.ts b/gitnexus/src/core/ingestion/languages/python/builtin-descriptors.ts index 537c0608a..3cd795561 100644 --- a/gitnexus/src/core/ingestion/languages/python/builtin-descriptors.ts +++ b/gitnexus/src/core/ingestion/languages/python/builtin-descriptors.ts @@ -108,6 +108,7 @@ function bindingOf(identifier: SyntaxNode): Omit | null } if (parent === null) return null; const shadow = { node: identifier, effect: 'shadow' as const }; + if (inCasePattern(identifier)) return isCaseCapture(identifier) ? shadow : null; switch (parent.type) { case 'assignment': case 'augmented_assignment': @@ -144,10 +145,9 @@ function bindingOf(identifier: SyntaxNode): Omit | null : null; case 'dotted_name': { const owner = parent.parent; - // `import a.b` binds `a`; `case name:` captures a single name. + // `import a.b` binds `a`. if (owner?.type === 'import_statement') return parent.firstNamedChild?.id === node.id ? shadow : null; - if (owner?.type === 'case_pattern') return parent.namedChildCount === 1 ? shadow : null; if (owner?.type !== 'import_from_statement' || !isField(parent, 'name')) return null; return importsFromBuiltins(owner) ? { node: identifier, effect: 'builtin' } : shadow; } @@ -194,10 +194,140 @@ function descriptorBindings(node: SyntaxNode): ReadonlyMap, + redirected: ReadonlyMap, +): void { + // Helper function name -> its `def` and, per descriptor name, the restore + // its call performs. + const helpers = new Map }>(); + for (const [name, list] of bindings) { + const byHelper = new Map(); + for (const binding of list) { + if (binding.redirect !== 'global') continue; + const fn = scopeOf(binding.node); + if (fn === null) continue; + const entry = byHelper.get(fn.id); + if (entry === undefined) byHelper.set(fn.id, { fn, globals: [binding] }); + else entry.globals.push(binding); + } + for (const { fn, globals } of byHelper.values()) { + const effect = restoreOnCall(fn, globals); + const helper = fn.childForFieldName('name')?.text; + if (effect === null || helper === undefined) continue; + const entry = helpers.get(helper) ?? { fn, restores: new Map() }; + entry.restores.set(name, effect); + helpers.set(helper, entry); + } + } + if (helpers.size === 0) return; + // A call only denotes the helper when its `def` is the name's one module + // binding. A local of the same name elsewhere cannot rebind it. + if (root.descendantsOfType('wildcard_import').length > 0) return; + const bindingCounts = new Map(); + for (const found of root.descendantsOfType('identifier')) { + if (!helpers.has(found.text) || bindingOf(found) === null) continue; + const scope = scopeOf(found); + if (scope !== null && redirected.get(`${found.text}@${scope.id}`) !== 'global') continue; + bindingCounts.set(found.text, (bindingCounts.get(found.text) ?? 0) + 1); + } + for (const [helper, count] of bindingCounts) if (count > 1) helpers.delete(helper); + const touched = new Set(); + for (const statement of root.namedChildren) { + // Only a call that is the whole statement surely runs when reached. + if (statement.type !== 'expression_statement' || statement.namedChildCount !== 1) continue; + const call = statement.firstNamedChild; + const callee = call?.type === 'call' ? call.childForFieldName('function') : null; + const helper = callee?.type === 'identifier' ? helpers.get(callee.text) : undefined; + // The call reaches the helper only once its `def` statement has run, and + // an argument-free call binds only when every parameter is optional. + if (helper === undefined || statement.startIndex < helper.fn.endIndex) continue; + if ((call?.childForFieldName('arguments')?.namedChildCount ?? 0) > 0) continue; + for (const [name, effect] of helper.restores) { + const list = bindings.get(name); + if (list === undefined) continue; + list.push({ node: callee, effect, redirect: null }); + touched.add(list); + } + } + for (const list of touched) list.sort((a, b) => a.node.startIndex - b.node.startIndex); +} + +/** + * Does this name in a case pattern capture? Value patterns (`a.b`), class + * names and keyword keys are reads. Every other name is a capture. + */ +function isCaseCapture(identifier: SyntaxNode): boolean { + const parent = identifier.parent; + if (parent?.type === 'keyword_pattern') return parent.firstNamedChild?.id !== identifier.id; + if (parent?.type !== 'dotted_name') return true; + if (parent.namedChildCount > 1) return false; + return !( + parent.parent?.type === 'class_pattern' && parent.parent.firstNamedChild?.id === parent.id + ); +} + +/** Is `node` inside a `match` case pattern? */ +function inCasePattern(node: SyntaxNode): boolean { + for (let parent = node.parent; parent !== null; parent = parent.parent) { + if (parent.type === 'case_pattern') return true; + } + return false; +} + +/** + * The restore a call to `fn` always performs on a module global, or `null`. + * `fn` must be a plain module-level `def` whose body runs on call (not a + * generator or coroutine), and every `global` binding of the name in it must + * be a restore in a simple statement that no earlier `return` can skip. + */ +function restoreOnCall(fn: SyntaxNode, globals: readonly NameBinding[]): BindingEffect | null { + if (fn.type !== 'function_definition' || fn.parent?.type !== 'module') return null; + if (fn.children.some((child) => child.type === 'async')) return null; + const required = fn + .childForFieldName('parameters') + ?.namedChildren.some( + (param) => + param.type === 'identifier' || + (param.type === 'typed_parameter' && param.firstNamedChild?.type === 'identifier'), + ); + if (required === true) return null; + const body = fn.childForFieldName('body'); + // `globals` is in source order, so the last restore is the one that sticks. + const first = globals[0]; + const last = globals[globals.length - 1]; + if (body === null || first === undefined || last === undefined) return null; + for (const binding of globals) { + const statement = statementIn(binding.node, body); + if ( + binding.effect === 'shadow' || + statement === null || + !SIMPLE_STATEMENTS.has(statement.type) + ) { + return null; + } + } + const escapes = body + .descendantsOfType(['yield', 'return_statement']) + .some( + (node) => + scopeOf(node)?.id === fn.id && + (node.type === 'yield' || node.startIndex < first.node.startIndex), + ); + return escapes ? null : last.effect; +} + /** * The scope that owns names bound at `node`: a function or lambda body, a * class body, a comprehension, or the module (`null`). A walrus target skips @@ -225,6 +355,11 @@ function statementIn(node: SyntaxNode, body: SyntaxNode): SyntaxNode | null { return current.parent === null ? null : current; } +/** Does `outer` span `node`? */ +function contains(outer: SyntaxNode, node: SyntaxNode): boolean { + return node.startIndex >= outer.startIndex && node.endIndex <= outer.endIndex; +} + /** Can `binding` run before `use` in the same scope, including an earlier * iteration of an enclosing loop? */ function mayRunBefore(binding: SyntaxNode, use: SyntaxNode, scope: SyntaxNode | null): boolean { @@ -232,8 +367,7 @@ function mayRunBefore(binding: SyntaxNode, use: SyntaxNode, scope: SyntaxNode | for (let loop = use.parent; loop !== null && loop.id !== scope?.id; loop = loop.parent) { if ( (loop.type === 'for_statement' || loop.type === 'while_statement') && - binding.startIndex >= loop.startIndex && - binding.endIndex <= loop.endIndex + contains(loop, binding) ) { return true; } @@ -296,6 +430,15 @@ function lookupName(bindings: readonly NameBinding[], use: SyntaxNode): BindingE (binding) => binding.redirect === null && scopeOf(binding.node)?.id === fn.id, ); if (!owned) continue; + // A nested function can rebind this cell whenever it is called; that + // order is not modelled, so assume it ran. + const rebound = bindings.some( + (binding) => + binding.effect === 'shadow' && + binding.redirect === 'nonlocal' && + contains(fn, binding.node), + ); + if (rebound) return 'shadow'; // Any binding makes the name local to this function, so the class body // reads that cell. An unbound cell raises NameError, not the builtin. return namespaceState(bindings, fn, use) === 'builtin' ? 'builtin' : 'shadow'; @@ -313,16 +456,13 @@ function lookupName(bindings: readonly NameBinding[], use: SyntaxNode): BindingE // is called; that order is not modelled, so assume it ran. if (binding.redirect === 'nonlocal') { const outer = functions[functions.length - 1]; - const inside = - outer !== undefined && - binding.node.startIndex >= outer.startIndex && - binding.node.endIndex <= outer.endIndex; - if (inside) return 'shadow'; + if (outer !== undefined && contains(outer, binding.node)) return 'shadow'; } if (binding.redirect === 'global') { // The function can only be called once the top-level statement that - // defines it has run. Whether a call happens is unknown, so a restoring - // `global` delete is ignored and a rebinding one is assumed. + // defines it has run. Whether a call happens is unknown, so a rebinding + // one is assumed. A restore counts only at a proven call site (see + // `addHelperCalls`). const top = statementIn(binding.node, binding.node.tree.rootNode); if (deferred || (top !== null && mayRunBefore(top, use, null))) return 'shadow'; } diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 9e981b4e4..c969ed3c7 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -812,7 +812,10 @@ import { copyV8CacheIfPresent, tryLoadV8Cache, writeV8CacheFile } from './v8-sid // comments, honors rebinding of builtin descriptor names visible where the // decorator is evaluated, and withholds subtype capacity from descriptor // stacks. Warm v120 captures carry the old verdicts. -const SCHEMA_BUMP = 121; +// v122 (#3414): Python decorator identity models restoring helper calls and +// treats match-pattern captures and nested nonlocal rebinds as shadowing. +// Warm v121 captures carry the old verdicts. +const SCHEMA_BUMP = 122; const GITNEXUS_PKG_VERSION = (() => { try { // package.json sits at gitnexus/package.json — two levels up from diff --git a/gitnexus/test/unit/incremental-parse-cache.test.ts b/gitnexus/test/unit/incremental-parse-cache.test.ts index 2376e5857..138bbe858 100644 --- a/gitnexus/test/unit/incremental-parse-cache.test.ts +++ b/gitnexus/test/unit/incremental-parse-cache.test.ts @@ -299,8 +299,9 @@ describe('PARSE_CACHE_VERSION', () => { // Moved 116 -> 117 for #3390's private positional-count side-channel. // Moved 117 -> 118 for #3398, 118 -> 119 for #3396, and 119 -> 120 for #3394. // Moved 120 -> 121 for the #3399 decorator-identity follow-up. - it('pins SCHEMA_BUMP to 121 so concurrent bumps cannot silently collide (#2766, #3015, #3088, #2885, #3128, #2865, #3130, #1432, #3161, #3179, #3219, #3190, #3253, #3273, #3339, #3354, #3371, #2965, #3390, #3398, #3396, #3394, #3399)', () => { - expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(121); + // Moved 121 -> 122 for #3414 restoring helper calls. + it('pins SCHEMA_BUMP to 122 so concurrent bumps cannot silently collide (#2766, #3015, #3088, #2885, #3128, #2865, #3130, #1432, #3161, #3179, #3219, #3190, #3253, #3273, #3339, #3354, #3371, #2965, #3390, #3398, #3396, #3394, #3399, #3414)', () => { + expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(122); expect(PARSE_CACHE_BUCKET_COUNT).toBe(128); // The PREVIOUS version must fail the reuse gate, not merely differ from the // current one — a hardcoded number outside the conflict hunk rebases cleanly @@ -309,7 +310,7 @@ describe('PARSE_CACHE_VERSION', () => { for (const taken of [ 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, - 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, + 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, ]) { expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).not.toBe(taken); } diff --git a/gitnexus/test/unit/scope-resolution/python/python-builtin-descriptors.test.ts b/gitnexus/test/unit/scope-resolution/python/python-builtin-descriptors.test.ts index 823042b8d..d79cffdfe 100644 --- a/gitnexus/test/unit/scope-resolution/python/python-builtin-descriptors.test.ts +++ b/gitnexus/test/unit/scope-resolution/python/python-builtin-descriptors.test.ts @@ -16,11 +16,19 @@ const decoratesWithBuiltin = (source: string): boolean => { }; const method = [' @staticmethod', ' def t(v):', ' return v']; +const resetHelper = ['def reset():', ' global staticmethod', ' del staticmethod']; +const nonlocalRebind = [ + ' def rebind():', + ' nonlocal staticmethod', + ' staticmethod = lambda f: f', +]; -// `true` means CPython's `A().t(7)` returns 7, so the decorator evaluated to -// the builtin staticmethod. A wildcard import from a module this file cannot -// see is expected `false` because the resolver fails closed, not because -// CPython always shadows the builtin there. +// `true` means the resolver proves the decorator is the builtin staticmethod, +// and CPython's `A().t(7)` returns 7. `false` means the resolver does not +// prove it. Usually CPython shadows the builtin there too. Where CPython keeps +// the builtin but the resolver fails closed (an unknown wildcard module, an +// unproven call order), the case comment says so. `true` must never hold where +// CPython shadows, because that would be a false edge. describe('Python builtin descriptor identity', () => { it.each([ ['a later module assignment', ['class A:', ...method, 'staticmethod = lambda f: f'], true], @@ -148,13 +156,174 @@ describe('Python builtin descriptor identity', () => { false, ], [ - // CPython restores the builtin when reset() runs; whether a call runs is - // not modelled, so the resolver keeps the override (fail closed). + // A bare top-level call runs the helper's `global` delete at the call. 'a global del in a called helper', + ['staticmethod = lambda f: f', ...resetHelper, 'reset()', 'class A:', ...method], + true, + ], + [ + 'a global builtins import in a called helper', [ 'staticmethod = lambda f: f', 'def reset():', ' global staticmethod', + ' from builtins import staticmethod', + 'reset()', + 'class A:', + ...method, + ], + true, + ], + [ + 'a called helper before a deferred class body', + [ + 'staticmethod = lambda f: f', + ...resetHelper, + 'reset()', + 'def make():', + ' class A:', + ...method.map((line) => ` ${line}`), + ' return A', + ], + true, + ], + [ + // CPython restores the builtin; a conditional call is not modelled. + 'a called helper inside an if', + [ + 'staticmethod = lambda f: f', + ...resetHelper, + 'if True:', + ' reset()', + 'class A:', + ...method, + ], + false, + ], + [ + 'a called helper inside a boolean expression', + [ + 'staticmethod = lambda f: f', + ...resetHelper, + 'flag = False', + 'flag and reset()', + 'class A:', + ...method, + ], + false, + ], + [ + // The target binds after the call returns. + 'a called helper whose result rebinds the name', + [ + 'staticmethod = lambda f: f', + ...resetHelper, + ' return lambda f: f', + 'staticmethod = reset()', + 'class A:', + ...method, + ], + false, + ], + [ + 'an override after a called helper', + [ + 'staticmethod = lambda f: f', + ...resetHelper, + 'reset()', + 'staticmethod = lambda f: f', + 'class A:', + ...method, + ], + false, + ], + [ + // Calling a generator function does not run its body. + 'a called generator helper', + ['staticmethod = lambda f: f', ...resetHelper, ' yield', 'reset()', 'class A:', ...method], + false, + ], + [ + 'a called helper that may return first', + [ + 'staticmethod = lambda f: f', + 'def reset(flag=True):', + ' global staticmethod', + ' if flag:', + ' return', + ' del staticmethod', + 'reset()', + 'class A:', + ...method, + ], + false, + ], + [ + 'a called helper whose name is rebound', + [ + 'staticmethod = lambda f: f', + ...resetHelper, + 'reset = print', + 'reset()', + 'class A:', + ...method, + ], + false, + ], + [ + // The call runs the builtin print(); the helper is defined afterwards. + 'a call before the helper is defined', + [ + 'staticmethod = lambda f: f', + 'print()', + 'class A:', + ...method, + 'def print():', + ' global staticmethod', + ' del staticmethod', + ], + false, + ], + [ + 'a helper name captured by a match pattern', + [ + 'staticmethod = lambda f: f', + ...resetHelper, + 'match 1:', + ' case int() as reset:', + ' pass', + 'reset()', + 'class A:', + ...method, + ], + false, + ], + [ + 'a match capture in the helper after its restore', + [ + 'staticmethod = lambda f: f', + ...resetHelper, + ' match [1]:', + ' case [*staticmethod]:', + ' pass', + 'reset()', + 'class A:', + ...method, + ], + false, + ], + [ + 'a module-level match capture', + ['match 1:', ' case object() as staticmethod:', ' pass', 'class A:', ...method], + false, + ], + [ + // The call raises TypeError before the helper body runs. + 'a call missing a required helper argument', + [ + 'staticmethod = lambda f: f', + 'def reset(required):', + ' global staticmethod', ' del staticmethod', 'reset()', 'class A:', @@ -162,6 +331,250 @@ describe('Python builtin descriptor identity', () => { ], false, ], + [ + 'a call to a helper whose parameters all have defaults', + [ + 'staticmethod = lambda f: f', + 'def reset(flag=True, *args: int, **kw):', + ' global staticmethod', + ' del staticmethod', + 'reset()', + 'class A:', + ...method, + ], + true, + ], + [ + 'a match value pattern that reads the name', + [ + 'import builtins', + 'match 1:', + ' case builtins.staticmethod:', + ' pass', + 'class A:', + ...method, + ], + true, + ], + [ + 'a match class pattern that reads the name', + ['match 1:', ' case staticmethod():', ' pass', 'class A:', ...method], + true, + ], + [ + 'a match keyword capture of the name', + ['match 1:', ' case int(real=staticmethod):', ' pass', 'class A:', ...method], + false, + ], + [ + // A parameter named like the helper is local to its own function. + 'a function parameter named like the helper', + [ + 'staticmethod = lambda f: f', + ...resetHelper, + 'def unrelated(reset):', + ' pass', + 'reset()', + 'class A:', + ...method, + ], + true, + ], + [ + // CPython keeps the builtin because swap() is never called; the + // resolver cannot prove that, so it fails closed. + 'a global rebind of the helper name in another function', + [ + 'staticmethod = lambda f: f', + ...resetHelper, + 'def swap():', + ' global reset', + ' reset = print', + 'reset()', + 'class A:', + ...method, + ], + false, + ], + [ + // CPython restores the builtin; any `global` rebind in the helper keeps + // the resolver fail-closed, even after a return. + 'a helper that rebinds after returning', + [ + 'staticmethod = lambda f: f', + ...resetHelper, + ' return', + ' staticmethod = 1', + 'reset()', + 'class A:', + ...method, + ], + false, + ], + [ + 'a helper whose restore is conditional', + [ + 'staticmethod = lambda f: f', + 'def reset(flag=False):', + ' global staticmethod', + ' if flag:', + ' del staticmethod', + 'reset()', + 'class A:', + ...method, + ], + false, + ], + [ + // Calling a coroutine function does not run its body. + 'a called async helper', + [ + 'staticmethod = lambda f: f', + 'async def reset():', + ' global staticmethod', + ' del staticmethod', + 'reset()', + 'class A:', + ...method, + ], + false, + ], + [ + 'a called helper that returns after restoring', + [ + 'staticmethod = lambda f: f', + ...resetHelper, + ' return', + 'reset()', + 'class A:', + ...method, + ], + true, + ], + [ + // A nested function's return does not end the helper. + 'a called helper with a nested return before restoring', + [ + 'staticmethod = lambda f: f', + 'def reset():', + ' global staticmethod', + ' def g(): return 1', + ' del staticmethod', + 'reset()', + 'class A:', + ...method, + ], + true, + ], + [ + // CPython restores the builtin; the wildcard import could rebind the + // helper name, so the resolver fails closed. + 'a called helper after a wildcard import', + [ + 'staticmethod = lambda f: f', + ...resetHelper, + 'from os.path import *', + 'reset()', + 'class A:', + ...method, + ], + false, + ], + [ + // The helper writes module globals, not the class namespace. + 'a class-body call after a class-local override', + [ + 'staticmethod = lambda f: f', + ...resetHelper, + 'class A:', + ' staticmethod = lambda f: f', + ' reset()', + ...method, + ], + false, + ], + [ + 'a function-body call after a function-local override', + [ + 'def reset():', + ' global staticmethod', + ' from builtins import staticmethod', + 'def make():', + ' staticmethod = lambda f: f', + ' reset()', + ' class A:', + ...method.map((line) => ` ${line}`), + ], + false, + ], + [ + // CPython restores the builtin; the helper's earlier `global` rebind + // keeps the resolver fail-closed. + 'a called helper that rebinds before deleting', + [ + 'staticmethod = lambda f: f', + 'def reset():', + ' global staticmethod', + ' staticmethod = 1', + ' del staticmethod', + 'reset()', + 'class A:', + ...method, + ], + false, + ], + [ + // CPython keeps the builtin when rebind() is never called; whether it + // is called is not modelled, so the resolver fails closed. + 'a global rebind helper that is never called', + [ + 'def rebind():', + ' global staticmethod', + ' staticmethod = lambda f: f', + 'class A:', + ...method, + ], + false, + ], + [ + // CPython keeps the builtin; call order is not modelled (fail closed). + 'an uncalled nonlocal rebind over an enclosing builtins import', + [ + 'def make():', + ' from builtins import staticmethod', + ...nonlocalRebind, + ' class A:', + ...method.map((line) => ` ${line}`), + ], + false, + ], + [ + 'a called nonlocal rebind over an enclosing builtins import', + [ + 'def make():', + ' from builtins import staticmethod', + ...nonlocalRebind, + ' rebind()', + ' class A:', + ...method.map((line) => ` ${line}`), + ], + false, + ], + [ + // CPython keeps the builtin because make() runs after the del; call + // order is not modelled (fail closed). + 'a module del after a deferred class body, called afterwards', + [ + 'staticmethod = lambda f: f', + 'def make():', + ' class A:', + ...method.map((line) => ` ${line}`), + ' return A', + 'del staticmethod', + 'make()', + ], + false, + ], [ // Each class statement builds a fresh namespace, so the decorator runs // before that iteration's own class-body assignment.