mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-28 14:08:56 +08:00
fix(python): calls like reg.get() on a typed local or parameter reach the class's method (#2821)
In Python, a call of a method named like a builtin dict, list, string or file method (get, update, items, pop, close and the rest of the builtin-method list) was dropped by the builtin filter before any strategy could read its receiver. Only a receiver whose capitalized name is a project class, or an imported project module, got past it. So `reg = SystemPackRegistry(); reg.get(k)`, `registry = load_registry()` with `def load_registry() -> SystemPackRegistry`, and a parameter `reg: SystemPackRegistry` gave no edge, while `self.reg.get()` linked through the attribute reader of PR #1926. A call the filter drops now gets one way back, by evidence only: matchPythonTypedLocalCall. It reads the def that binds the receiver where the call reads it (the calling def, or one it is nested in). It types the name only when every binding there gives it the same project class: - a parameter gives what its annotation names ("T", Optional[T], T | None); - an assignment gives its own annotation, or what its whole value makes: a constructor, or a factory typed by its return annotation or by its returns (the attribute reader's pythonValueEvidence and pythonFactoryType); - `x = None` gives nothing either way; - each class is the one the file's imports name (pythonClassNamed), so a class imported from outside the project is none of the project's; - the method must be the class's own or a declared base's. Everything else stays dropped: any other binding of the name (a loop, with, except or comprehension target, a tuple or augmented assignment, a walrus, an import, del, a nested def or class of the name, a nonlocal write from a nested def), a value that names no class ({}, json.load(p), Foo().data(), a union), a rebound callee, a module-level name. These are the shapes the reporter's follow-up found making wrong edges with a looser reader. Like an attribute call, the edge records the other files its answer read (typeFrom), so a sync that edits the factory or a base re-opens it. Validation: - New __tests__/python-typed-builtin-method-calls.test.ts, 20 tests: - the issue's three calls; annotated, quoted and Optional parameters; items, pop and close; a module-qualified constructor; typed, untyped and awaited factories; a None placeholder; a local annotation; a closure; typeFrom; two same-named classes; an inherited method; - the reporter's trap shapes, which stay unlinked; - three sync cases, each matching a fresh index. With main's source the 13 positive tests fail and the 7 trap tests pass. - A/B against main3f58e343. Both arms were built from their own sources with the kernel. Rebased ontoe32021f6(PR #2820, outside Python), the fix arm's graphs are byte-identical to before the rebase: - requests: +15 edges; - sqlalchemy: +52 edges; - django's 2,938 Python files: +73 edges; - 0 removed and no other change in any of them; - express and gin (controls): byte-identical. All 140 added edges were read at their source: each is a constructed local, a factory-typed local or an annotated parameter calling its own class's method, or one a declared base has. The issue's fixtures link all three calls, and the verification kit's nine trap call sites stay unresolved, kernel and wasm alike. - Cost: a CPU profile puts the new reader at about 1.0 s on django and 1.2 s on sqlalchemy, around 1% of index CPU. Index CPU medians of 3 interleaved runs, main vs fix: sqlalchemy 98.6 s vs 98.5 s, django 97.9 s vs 99.1 s (single runs vary by several seconds on this shared machine). - kernel-parity.mjs: 295/295 Python files at parity. Fixes #2583 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
e32021f6fb
commit
d5fc1fbb21
+16
-3
@@ -21,7 +21,7 @@ import {
|
||||
isImportableKind,
|
||||
CPP_DEFINE_SIGNATURE,
|
||||
} from './types';
|
||||
import { isPythonSelfCall, matchJsStoreBindingCall, isUnresolvedJsMemberCall, matchObjectPathCall, thisScopeCaller, isVisibleAcrossFiles, matchReference, matchByFilePath, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesCodeBoundary, gateLanguageMatch, dumpNameMatcherProfile, clearNameMatcherMemos, isRustNameInScope, isRustForeignImplCallable, CASE_INSENSITIVE_LANGUAGES, isDartMemberRead, matchDartMemberRead, isDartChainLink, matchDartChainLink, isDartAnnotation, matchDartAnnotation, isStdMethodName, isGoUnknownQualified, isGoProjectLocalCall, GO_STDLIB_PACKAGES, isGoBareName, goTypePositionTarget, goDotImportedSupertype, GO_TYPE_KINDS, matchGoAssertedCall, matchGoIndexedCall, matchGoChainedCall, preferGoBuildTwin, isGoAssertedLocal, isGoLocallyBound, isPythonFromImportModule, pythonAttrRefResolves, isImportShadowedByJsBinding, isManualMockPath } from './name-matcher';
|
||||
import { isPythonSelfCall, matchJsStoreBindingCall, isUnresolvedJsMemberCall, matchObjectPathCall, thisScopeCaller, isVisibleAcrossFiles, matchReference, matchByFilePath, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesCodeBoundary, gateLanguageMatch, dumpNameMatcherProfile, clearNameMatcherMemos, isRustNameInScope, isRustForeignImplCallable, CASE_INSENSITIVE_LANGUAGES, isDartMemberRead, matchDartMemberRead, isDartChainLink, matchDartChainLink, isDartAnnotation, matchDartAnnotation, isStdMethodName, isGoUnknownQualified, isGoProjectLocalCall, GO_STDLIB_PACKAGES, isGoBareName, goTypePositionTarget, goDotImportedSupertype, GO_TYPE_KINDS, matchGoAssertedCall, matchGoIndexedCall, matchGoChainedCall, preferGoBuildTwin, isGoAssertedLocal, isGoLocallyBound, isPythonFromImportModule, pythonAttrRefResolves, matchPythonTypedLocalCall, isImportShadowedByJsBinding, isManualMockPath } from './name-matcher';
|
||||
import { isVisibleCppMacro, clearCppMacroVisibility } from './cpp-macro-visibility';
|
||||
import { isCppConstructorRef, matchCppConstructor } from './cpp-constructor';
|
||||
import { isCppSupertypeRef, matchCppSupertype, clearCppSupertypeMemos } from './cpp-supertypes';
|
||||
@@ -1522,9 +1522,14 @@ export class ReferenceResolver {
|
||||
const superCall = matchJsSuperCall(ref, this.context);
|
||||
if (superCall !== undefined) return superCall;
|
||||
|
||||
// Skip built-in/external references
|
||||
// Skip built-in/external references — except a python call of a
|
||||
// builtin-named method through a local or a parameter its def types as a
|
||||
// project class (`reg = Registry(); reg.get(k)`, #2583): that reading,
|
||||
// by evidence only, is the one way back, and nothing guesses past it.
|
||||
if (this.isBuiltInOrExternal(ref)) {
|
||||
return null;
|
||||
return this.isPythonBuiltinNamedMethodCall(ref)
|
||||
? this.gateLanguage(matchPythonTypedLocalCall(ref, this.context), ref)
|
||||
: null;
|
||||
}
|
||||
|
||||
// CFML component paths in inheritance (#1152): `extends="coldbox.system.web.
|
||||
@@ -3216,6 +3221,14 @@ export class ReferenceResolver {
|
||||
return false;
|
||||
}
|
||||
|
||||
/** A python call `x.get(…)` of a method named like a builtin collection, string or file method. */
|
||||
private isPythonBuiltinNamedMethodCall(ref: UnresolvedRef): boolean {
|
||||
if (ref.language !== 'python' || ref.referenceKind !== 'calls') return false;
|
||||
const dot = ref.referenceName.indexOf('.');
|
||||
return dot > 0 && PYTHON_BUILT_IN_METHODS.has(ref.referenceName.slice(dot + 1)) &&
|
||||
!PYTHON_BUILT_IN_TYPES.has(ref.referenceName.slice(0, dot));
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if reference is to a built-in or external symbol
|
||||
*/
|
||||
|
||||
@@ -17856,6 +17856,9 @@ export function clearNameMatcherMemos(context: ResolutionContext): void {
|
||||
PY_BASES_MEMO.delete(context);
|
||||
PY_FILE_CLASSES.delete(context);
|
||||
PY_ATTR_CALL_MEMO.delete(context);
|
||||
PY_FILE_DEFS.delete(context);
|
||||
PY_SCOPE_BINDINGS.delete(context);
|
||||
PY_LOCAL_CLASS_MEMO.delete(context);
|
||||
}
|
||||
|
||||
function memoPatterns(key: string, build: () => RegExp[]): RegExp[] {
|
||||
@@ -23407,6 +23410,255 @@ function pythonAttrCallOn(
|
||||
return hit.answer ? { ...hit.answer, original: ref } : hit.answer;
|
||||
}
|
||||
|
||||
/**
|
||||
* A call of a builtin-named method — `get`, `update`, `items`, `close` —
|
||||
* through a local or a parameter (#2583): `reg = SystemPackRegistry();
|
||||
* reg.get(k)`, `registry = load_registry()` with `def load_registry() ->
|
||||
* SystemPackRegistry`, `def f(reg: SystemPackRegistry)`. The builtin filter
|
||||
* drops such a call by its method's name before any strategy reads the
|
||||
* receiver, and this is its one way back, by evidence only: the def that binds
|
||||
* the name where the call reads it (the calling one, or one it is nested in)
|
||||
* gives it one project class everywhere it binds it (pythonLocalClass), and
|
||||
* the method is that class's own or a declared base's. A module-level name,
|
||||
* and one its def leaves unknown or contested, stay dropped. The edge records
|
||||
* the other files its answer was read from (`typeFrom`), as an attribute
|
||||
* call's does, so a sync that edits the factory or a base re-opens it.
|
||||
*/
|
||||
export function matchPythonTypedLocalCall(ref: UnresolvedRef, context: ResolutionContext): ResolvedRef | null {
|
||||
if (ref.language !== 'python' || ref.referenceKind !== 'calls') return null;
|
||||
const m = /^([A-Za-z_]\w*)\.(\w+)$/.exec(ref.referenceName);
|
||||
if (!m || m[1] === 'self' || m[1] === 'cls') return null;
|
||||
const receiver = m[1]!;
|
||||
const scope = pythonBindingScope(receiver, ref, context);
|
||||
if (!scope) return null;
|
||||
const read = new Set<string>();
|
||||
const cls = pythonLocalClass(scope, receiver, ref, context, read);
|
||||
if (!cls || cls === 'runtime') return null;
|
||||
const hit = pythonMethodOnClass(cls, m[2]!, ref, context, read);
|
||||
if (!hit) return null;
|
||||
read.delete(ref.filePath);
|
||||
return read.size > 0 ? { ...hit, metadata: { typeFrom: [...read].sort() } } : hit;
|
||||
}
|
||||
|
||||
/** Per context: a python file's functions and methods, the latest-starting first, for the defs around a call. */
|
||||
const PY_FILE_DEFS = new WeakMap<ResolutionContext, Map<string, Node[]>>();
|
||||
|
||||
function pythonFileDefs(filePath: string, context: ResolutionContext): Node[] {
|
||||
const memo = pyMemo(PY_FILE_DEFS, context);
|
||||
let hit = memo.get(filePath);
|
||||
if (!hit) {
|
||||
hit = context.getNodesInFile(filePath)
|
||||
.filter((n) => n.kind === 'function' || n.kind === 'method')
|
||||
.sort((a, b) => b.startLine - a.startLine);
|
||||
if (memo.size >= PY_MEMO_CAP) memo.clear();
|
||||
memo.set(filePath, hit);
|
||||
}
|
||||
return hit;
|
||||
}
|
||||
|
||||
/**
|
||||
* Per context: a def and a name → what the def's parameter of that name is
|
||||
* annotated with (pythonParamAnnotation), and every binding of the name in the
|
||||
* def's own body.
|
||||
*/
|
||||
const PY_SCOPE_BINDINGS = new WeakMap<ResolutionContext, Map<string, { param: string | null | undefined; bindings: PythonBinding[] }>>();
|
||||
|
||||
function pythonScopeBindings(scope: Node, name: string, context: ResolutionContext): { param: string | null | undefined; bindings: PythonBinding[] } {
|
||||
const memo = pyMemo(PY_SCOPE_BINDINGS, context);
|
||||
const key = `${scope.id}\0${name}`;
|
||||
let hit = memo.get(key);
|
||||
if (!hit) {
|
||||
const lines = pythonMemberLines(scope.filePath, context);
|
||||
const statements = pythonStatementStarts(scope.filePath, context);
|
||||
// pythonLineBindings binds nothing on a statement that names the name
|
||||
// only as a receiver or a container (`reg.get(…)`, `reg[k]`), its
|
||||
// continuation lines included: its own word test rules those out first.
|
||||
// An import binds the head of a dotted name (`import os.path`), and a star
|
||||
// import, which binds any name, can't be written in a def. So the lines
|
||||
// that can't bind skip its per-line work.
|
||||
const n = escapeRegExp(name);
|
||||
const word = new RegExp(`(?<![\\w.])${n}\\b(?!\\s*[.\\[])`);
|
||||
const mention = new RegExp(`(?<![\\w.])${n}\\b`);
|
||||
const names = (i: number): boolean => {
|
||||
const test = /^\s*(?:from|import)\b/.test(lines[i]!) ? mention : word;
|
||||
if (test.test(lines[i]!)) return true;
|
||||
if (!statements[i]) return false;
|
||||
for (let j = i + 1; j < lines.length && !statements[j]; j++) if (test.test(lines[j]!)) return true;
|
||||
return false;
|
||||
};
|
||||
const bindings: PythonBinding[] = [];
|
||||
for (const i of pythonOwnLines(scope, scope.filePath, context)) {
|
||||
if (names(i)) bindings.push(...pythonLineBindings(lines, i, statements, name));
|
||||
}
|
||||
hit = { param: pythonParamAnnotation(scope.signature ?? '', name), bindings };
|
||||
if (memo.size >= PY_MEMO_CAP) memo.clear();
|
||||
memo.set(key, hit);
|
||||
}
|
||||
return hit;
|
||||
}
|
||||
|
||||
/**
|
||||
* The def whose local `name` is the one `ref` reads: the innermost def around
|
||||
* the reference that binds the name, by a parameter or anywhere in its own
|
||||
* body. Null when no def around it does (a module global), or when one
|
||||
* declares it `global`.
|
||||
*/
|
||||
function pythonBindingScope(name: string, ref: UnresolvedRef, context: ResolutionContext): Node | null {
|
||||
for (const scope of pythonFileDefs(ref.filePath, context)) {
|
||||
if (scope.startLine > ref.line || scope.endLine < ref.line) continue;
|
||||
const { param, bindings } = pythonScopeBindings(scope, name, context);
|
||||
if (param !== undefined) return scope;
|
||||
if (bindings.some((b) => b.kind === 'global')) return null;
|
||||
if (bindings.length > 0) return scope;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* What the parameter `name` of a def with this signature (`(a, b: T = d)`)
|
||||
* says of its type: the annotation as written, null for a parameter without
|
||||
* one and for `*name` / `**name` (a tuple, a dict), undefined when the def has
|
||||
* no parameter of that name.
|
||||
*/
|
||||
function pythonParamAnnotation(signature: string, name: string): string | null | undefined {
|
||||
const open = signature.indexOf('(');
|
||||
if (open < 0 || !signature.includes(name)) return undefined;
|
||||
// Parameters split at the commas directly inside the parentheses; strings
|
||||
// are blanked (same length) so a quoted comma or bracket doesn't count.
|
||||
const blank = pythonBlankStrings(signature);
|
||||
const params: Array<[number, number]> = [];
|
||||
let depth = 0;
|
||||
let from = open + 1;
|
||||
for (let i = open; i < blank.length; i++) {
|
||||
const c = blank[i]!;
|
||||
if (c === '(' || c === '[' || c === '{') depth++;
|
||||
else if (c === ')' || c === ']' || c === '}') {
|
||||
if (--depth === 0) { params.push([from, i]); break; }
|
||||
} else if (c === ',' && depth === 1) {
|
||||
params.push([from, i]);
|
||||
from = i + 1;
|
||||
}
|
||||
}
|
||||
for (const [start, end] of params) {
|
||||
const head = /^\s*(\*{0,2})([A-Za-z_]\w*)\s*(:?)/.exec(signature.slice(start, end));
|
||||
if (!head || head[2] !== name) continue;
|
||||
if (head[1] || !head[3]) return null;
|
||||
// The annotation runs up to the default's `=`, outside brackets.
|
||||
const at = start + head[0].length;
|
||||
let d = 0;
|
||||
let to = end;
|
||||
for (let i = at; i < end; i++) {
|
||||
const c = blank[i]!;
|
||||
if (c === '(' || c === '[' || c === '{') d++;
|
||||
else if (c === ')' || c === ']' || c === '}') d--;
|
||||
else if (c === '=' && d === 0) { to = i; break; }
|
||||
}
|
||||
return signature.slice(at, to).trim() || null;
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/** Per context: a def and a name → the class pythonLocalClass reads, and the files read for it. */
|
||||
const PY_LOCAL_CLASS_MEMO = new WeakMap<ResolutionContext, Map<string, { answer: PythonTypeAnswer; read: string[] }>>();
|
||||
|
||||
/**
|
||||
* The project class the local or parameter `name` of `scope` holds, the same
|
||||
* for every binding of it there. A parameter holds what its annotation names
|
||||
* (`"T"`, `Optional[T]` and `T | None` too); an assignment, what its own
|
||||
* annotation names, or else what its whole value makes — a class's
|
||||
* constructor, or a factory, typed by its `-> T` or by what it returns
|
||||
* (pythonValueEvidence, pythonFactoryType) — or what an annotated parameter it
|
||||
* copies holds; `name = None` says nothing either way. Each class is the one
|
||||
* the file's imports name (pythonClassNamed), so one from outside the project,
|
||||
* as `from queue import Queue` gives, is none of the project's ('runtime').
|
||||
*
|
||||
* Anything else that binds the name leaves its type unknown (null): a
|
||||
* parameter without an annotation, a `for` / `with` / `except` / comprehension
|
||||
* target, a tuple or augmented assignment, a walrus, an import, `del`, a
|
||||
* nested `def` or `class` of the name, a `nonlocal` write from a def nested in
|
||||
* the scope. So does a value that names no class — `{}` is a dict, and
|
||||
* `json.load(p)`, `Foo().data()` or a union are unknown — and a callee the
|
||||
* scope or a def around it rebinds (`Store = load(); reg = Store()`).
|
||||
*/
|
||||
function pythonLocalClass(
|
||||
scope: Node,
|
||||
name: string,
|
||||
ref: UnresolvedRef,
|
||||
context: ResolutionContext,
|
||||
read: Set<string>,
|
||||
): PythonTypeAnswer {
|
||||
const memo = pyMemo(PY_LOCAL_CLASS_MEMO, context);
|
||||
const key = `${scope.id}\0${name}`;
|
||||
let hit = memo.get(key);
|
||||
if (!hit) {
|
||||
const own = new Set<string>();
|
||||
hit = { answer: pythonLocalClassUncached(scope, name, ref, context, own), read: [...own] };
|
||||
if (memo.size >= PY_MEMO_CAP) memo.clear();
|
||||
memo.set(key, hit);
|
||||
}
|
||||
for (const f of hit.read) read.add(f);
|
||||
return hit.answer;
|
||||
}
|
||||
|
||||
function pythonLocalClassUncached(
|
||||
scope: Node,
|
||||
name: string,
|
||||
ref: UnresolvedRef,
|
||||
context: ResolutionContext,
|
||||
read: Set<string>,
|
||||
): PythonTypeAnswer {
|
||||
const filePath = scope.filePath;
|
||||
const { param, bindings } = pythonScopeBindings(scope, name, context);
|
||||
const evidence: PythonEvidence[] = [];
|
||||
if (param !== undefined) {
|
||||
const ev = param === null ? null : pythonAnnotationEvidence(param);
|
||||
if (!ev) return null;
|
||||
evidence.push(ev);
|
||||
}
|
||||
for (const b of bindings) {
|
||||
if (b.kind !== 'assign') return null;
|
||||
let ev: PythonAttrEvidence;
|
||||
if (b.type) {
|
||||
// `reg: Base = make()` trusts the annotation; `reg: A = B()` contradicts it.
|
||||
const constructor = b.value && b.value !== 'None' ? pythonConstructorCall(b.value) : null;
|
||||
if (constructor && constructor.split('.').pop() !== b.type.split('.').pop()) return null;
|
||||
ev = pythonAnnotationEvidence(b.type);
|
||||
} else if (b.value === 'None') {
|
||||
continue;
|
||||
} else if (/^[A-Za-z_]\w*$/.test(b.value)) {
|
||||
// `reg = registry`: what the parameter's annotation names, unless the def rebinds it.
|
||||
const copied = pythonScopeBindings(scope, b.value, context);
|
||||
if (!copied.param || copied.bindings.length > 0) return null;
|
||||
ev = pythonAnnotationEvidence(copied.param);
|
||||
} else {
|
||||
const at: UnresolvedRef = { ...ref, line: b.line + 1 };
|
||||
if (pythonValueCallees(b.value).some((c) => pythonBindsLocally(c.split('.')[0]!, at, context, false))) return null;
|
||||
ev = pythonValueEvidence(b.value, null);
|
||||
}
|
||||
if (!ev) return null;
|
||||
evidence.push(ev);
|
||||
}
|
||||
// Nothing here can be a project class: a builtin, or no evidence.
|
||||
if (!evidence.some((ev) => ev !== 'runtime')) return evidence.length > 0 ? 'runtime' : null;
|
||||
|
||||
// A def or class of the name nested in the scope binds it there, a def
|
||||
// nested in it can rebind the scope's own through `nonlocal`, and `with … as
|
||||
// (a, name)` binds it in a way pythonLineBindings doesn't read.
|
||||
if (pythonFileDefs(filePath, context).some((n) => n.id !== scope.id && n.name === name &&
|
||||
n.startLine > scope.startLine && n.endLine <= scope.endLine) ||
|
||||
pythonFileClasses(filePath, context).some((n) => n.name === name &&
|
||||
n.startLine > scope.startLine && n.endLine <= scope.endLine)) return null;
|
||||
const lines = pythonMemberLines(filePath, context);
|
||||
const n = escapeRegExp(name);
|
||||
const nonlocal = new RegExp(`^\\s*nonlocal\\b.*(?<![\\w.])${n}\\b`);
|
||||
const asTuple = new RegExp(`\\bas\\s*\\([^)]*(?<![\\w.])${n}\\b`);
|
||||
for (let i = scope.startLine - 1; i < Math.min(scope.endLine, lines.length); i++) {
|
||||
const line = lines[i]!;
|
||||
if (line.includes(name) && (nonlocal.test(line) || asTuple.test(line))) return null;
|
||||
}
|
||||
return pythonAgree(evidence.map((ev) => pythonEvidenceClass(ev, filePath, context, read, 0, null)));
|
||||
}
|
||||
|
||||
/**
|
||||
* Python chained factory call — `Client.from_env().send()`,
|
||||
* `make_client().send()`, `Client().send()` — the python instance of the #750
|
||||
|
||||
Reference in New Issue
Block a user