mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-24 11:24:49 +08:00
fix(python): a call through a module-level instance links its class's method (#2608)
A Python call through a module-level instance of the same file, like _svc.delete(obj) after _svc = SceneService(), now links to the method its class has or inherits, as a method value through that global already did. On django and fastapi: 50 correct links added, 42 moved to the right class (DEBUG_ENGINE's Engine, the dispatch tests' Signal), confidence raised on about 400 more. CPU time on django is unchanged. Fixes issue 2582. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
cc1eb1f942
commit
e48565067e
@@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
### Fixes
|
||||
|
||||
- In Python, a call through a module-level instance, like `_svc.delete(obj)` after `_svc = SceneService()`, now links to the method its class has or inherits, even when other classes have a method of that name. (#2582)
|
||||
- In Python, a call on a module imported from outside your project, like `subprocess.run(…)` after `import subprocess`, no longer links to a project method that happens to share its name. Thanks @Egrn for the report. (#2599)
|
||||
- `codegraph_explore`, the CLI `codegraph explore` and the prompt hook no longer stall for a minute or more on a large project after a full index; the step that finds the project's busiest file no longer slows to a crawl when a few files are huge, like a generated Android `R.java` (#2571). Thanks @yangfengfan2011.
|
||||
- In JavaScript and TypeScript, a call written as the body of an `if`, `while`, `for` or `do` without braces, like `if (e.key === "Escape") setOpen(false);`, no longer links to a same-named property or method in another file when the name is the file's own, such as a `setOpen` from `useState` (#2585).
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
/**
|
||||
* A Python call through a module-level instance — `_svc = SceneService()` at
|
||||
* module scope, `_svc.delete(obj)` inside a function — reaches the method of
|
||||
* the instance's class, inherited ones included, even when other classes
|
||||
* share the method's name (#2582). The method-value form already did (#2074).
|
||||
*/
|
||||
|
||||
import { describe, it, expect, afterEach } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import CodeGraph from '../src/index';
|
||||
|
||||
let tempDir: string;
|
||||
let cg: CodeGraph | null = null;
|
||||
|
||||
async function calls(files: Record<string, string>, fromName: string): Promise<string[]> {
|
||||
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-2582-'));
|
||||
for (const [rel, source] of Object.entries(files)) {
|
||||
fs.mkdirSync(path.dirname(path.join(tempDir, rel)), { recursive: true });
|
||||
fs.writeFileSync(path.join(tempDir, rel), source);
|
||||
}
|
||||
cg = await CodeGraph.init(tempDir, { index: true });
|
||||
cg.resolveReferences();
|
||||
const from = cg.getNodesByKind('function').find((n) => n.name === fromName)!;
|
||||
expect(from).toBeDefined();
|
||||
return cg
|
||||
.getOutgoingEdges(from.id)
|
||||
.filter((e) => e.kind === 'calls' || e.kind === 'instantiates')
|
||||
.map((e) => [e.kind, cg!.getNode(e.target)] as const)
|
||||
.filter(([, n]) => !!n)
|
||||
.map(([kind, n]) => `${kind === 'calls' ? '' : `${kind} `}${n!.qualifiedName}@${n!.filePath}`)
|
||||
.sort();
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
cg?.close();
|
||||
cg = null;
|
||||
fs.rmSync(tempDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const BASE = 'class GenericCRUDService:\n def delete(self, obj):\n return obj\n';
|
||||
const OTHER = 'class Unrelated:\n def delete(self, obj):\n return None\n\n def purge(self):\n return None\n';
|
||||
const SVC = (body: string) =>
|
||||
`from base import GenericCRUDService\n\n\nclass SceneService(GenericCRUDService):\n def purge(self):\n return 1\n\n\n_svc = SceneService()\n\n\n${body}\n`;
|
||||
|
||||
describe('a Python call through a module-level instance (#2582)', () => {
|
||||
it.each([
|
||||
['an inherited method', 'def use(obj):\n _svc.delete(obj)', ['GenericCRUDService::delete@base.py']],
|
||||
['the class\'s own method', 'def use():\n _svc.purge()', ['SceneService::purge@svc.py']],
|
||||
])('reaches %s', async (_label, body, expected) => {
|
||||
const out = await calls({ 'base.py': BASE, 'other.py': OTHER, 'svc.py': SVC(body) }, 'use');
|
||||
expect(out).toEqual(expected);
|
||||
});
|
||||
|
||||
it('does not use the global where a local of the same name shadows it', async () => {
|
||||
const out = await calls(
|
||||
{ 'base.py': BASE, 'other.py': OTHER, 'svc.py': SVC('def use(_svc):\n _svc.purge()') },
|
||||
'use'
|
||||
);
|
||||
expect(out).not.toContain('SceneService::purge@svc.py');
|
||||
});
|
||||
|
||||
it.each([
|
||||
['an instance attribute', 'class SceneService:\n def __init__(self, h):\n self.handler = h\n\n\n_svc = SceneService(print)\n\n\ndef use():\n _svc.handler()\n', ['Unrelated::handler@other.py']],
|
||||
['a class attribute', 'class SceneService:\n handler = None\n\n\n_svc = SceneService()\n\n\ndef use():\n _svc.handler()\n', ['Unrelated::handler@other.py']],
|
||||
['a property', 'class SceneService:\n @property\n def handler(self):\n return len\n\n\n_svc = SceneService()\n\n\ndef use():\n _svc.handler()\n', []],
|
||||
['a base class from outside the project', 'from thirdparty import Session\n\n\nclass SceneService(Session):\n pass\n\n\n_svc = SceneService()\n\n\ndef use():\n _svc.handler()\n', ['Unrelated::handler@other.py']],
|
||||
['one of two candidate classes lacking it', 'class A:\n def handler(self):\n return 1\n\n\nclass B:\n pass\n\n\nif X:\n _svc = A()\nelse:\n _svc = B()\n\n\ndef use():\n _svc.handler()\n', []],
|
||||
])('falls back to name matching, never the class or its getter, when the member is %s', async (_label, code, expected) => {
|
||||
// Unrelated.handler is the project's only other `handler`, so upstream's
|
||||
// name matching links it when the global's own class can't supply one
|
||||
// (known limit); a getter competing with it leaves the name ambiguous.
|
||||
const other = 'class Unrelated:\n def handler(self):\n return None\n';
|
||||
expect(await calls({ 'other.py': other, 'svc.py': code }, 'use')).toEqual(expected);
|
||||
});
|
||||
|
||||
it('lets a nested class shadow the global', async () => {
|
||||
const code = 'class A:\n def run(self):\n return 1\n\n\n_svc = A()\n\n\ndef use():\n class _svc:\n def run(self):\n return 2\n\n _svc.run()\n';
|
||||
const out = await calls({ 'other.py': 'class Z:\n def run(self):\n return 3\n', 'svc.py': code }, 'use');
|
||||
expect(out).toEqual(['use::_svc::run@svc.py']);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['a multi-line base list', 'class Base:\n def run(self):\n return 1\n\n\nclass A(\n Base,\n):\n pass\n', 'Base::run@svc.py'],
|
||||
['a subscripted base', 'from typing import Generic, TypeVar\n\nT = TypeVar("T")\n\n\nclass Base(Generic[T]):\n def run(self):\n return 1\n\n\nclass A(Base[int]):\n pass\n', 'Base::run@svc.py'],
|
||||
['a member-like line in another method', 'class A:\n def run(self):\n return 1\n\n def other(self):\n return dict(\n run=1,\n )\n', 'A::run@svc.py'],
|
||||
])('keeps the name-matched edge through %s', async (_label, classes, expected) => {
|
||||
const code = `${classes}\n\n_svc = A()\n\n\ndef use():\n _svc.run()\n`;
|
||||
expect(await calls({ 'svc.py': code }, 'use')).toEqual([expected]);
|
||||
});
|
||||
|
||||
it('keeps a recursive call through the global', async () => {
|
||||
const code = 'class A:\n def run(self, n):\n if n:\n _svc.run(n - 1)\n\n\n_svc = A()\n';
|
||||
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-2582-'));
|
||||
fs.writeFileSync(path.join(tempDir, 'svc.py'), code);
|
||||
fs.writeFileSync(path.join(tempDir, 'other.py'), 'class Z:\n def run(self, n):\n return n\n');
|
||||
cg = await CodeGraph.init(tempDir, { index: true });
|
||||
cg.resolveReferences();
|
||||
const run = cg.getNodesByKind('method').find((n) => n.qualifiedName === 'A::run')!;
|
||||
const targets = cg.getOutgoingEdges(run.id).filter((e) => e.kind === 'calls').map((e) => cg!.getNode(e.target)?.qualifiedName);
|
||||
expect(targets).toEqual(['A::run']);
|
||||
});
|
||||
|
||||
it('is not shadowed by a class nested in a helper function', async () => {
|
||||
const code = 'class A:\n def run(self):\n return 1\n\n\n_svc = A()\n\n\ndef use():\n def helper():\n class _svc:\n def run(self):\n return 2\n\n return _svc\n\n _svc.run()\n';
|
||||
const out = await calls({ 'other.py': 'class Z:\n def run(self):\n return 3\n', 'svc.py': code }, 'use');
|
||||
expect(out).toEqual(['A::run@svc.py']);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -335,10 +335,9 @@ function matchMemberFunctionRef(ref: UnresolvedRef, context: ResolutionContext):
|
||||
type = pythonFieldType(receiver, owner, ref, context);
|
||||
} else {
|
||||
type = pythonLocalType(receiver, ref, context);
|
||||
const global = type === null
|
||||
? context.getNodesInFile(ref.filePath).find(n => n.name === receiver && isPythonModuleGlobal(n, context)) : undefined;
|
||||
if (global && !pythonBindsLocally(receiver, ref, context, true)) {
|
||||
return result(pythonGlobalMembers(global, member, ref, context));
|
||||
const viaGlobal = type === null ? pythonGlobalReceiverTarget(receiver, member, ref, context) : undefined;
|
||||
if (viaGlobal !== undefined) {
|
||||
return viaGlobal ? { original: ref, targetNodeId: viaGlobal.id, confidence: 0.9, resolvedBy: 'function-ref' } : null;
|
||||
}
|
||||
}
|
||||
// A type name used directly (`Store.fetch`) is scoped just like an annotation.
|
||||
@@ -931,6 +930,44 @@ function pythonDynamicGlobalWrite(name: string, filePath: string, context: Resol
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* What `receiver.member` reaches through a module-level instance of this
|
||||
* file — `_svc = SceneService()` at module scope, then `_svc.delete` (#2074)
|
||||
* or `_svc.delete(obj)` (#2582). Undefined when `receiver` is not that global
|
||||
* where the ref is: a parameter, a local, or a `def`/`class` written directly
|
||||
* in an enclosing function shadows it. Otherwise the one method or function every class the global
|
||||
* may hold has or inherits — never a property, an attribute or the referring
|
||||
* node itself (allowed for a `call`: recursion) — or null when there is none.
|
||||
*/
|
||||
function pythonGlobalReceiverTarget(
|
||||
receiver: string, member: string, ref: UnresolvedRef, context: ResolutionContext, call = false,
|
||||
): Node | null | undefined {
|
||||
const global = pythonNameScan(context, `global\0${ref.filePath}\0${receiver}`, () =>
|
||||
context.getNodesInFile(ref.filePath).find(n => n.name === receiver && isPythonModuleGlobal(n, context)) ?? null);
|
||||
if (!global || pythonBindsLocally(receiver, ref, context, true)) return undefined;
|
||||
// A `def _svc` / `class _svc` nested in an enclosing function binds the name there.
|
||||
const nodes = context.getNodesInFile(ref.filePath);
|
||||
const scopes = nodes.filter(n => (n.kind === 'function' || n.kind === 'method') && n.startLine <= ref.line && n.endLine >= ref.line);
|
||||
const innermost = (n: Node) => nodes
|
||||
.filter(c => c.id !== n.id && (c.kind === 'function' || c.kind === 'method' || c.kind === 'class') &&
|
||||
c.startLine < n.startLine && c.endLine >= n.startLine)
|
||||
.sort((a, b) => b.startLine - a.startLine)[0];
|
||||
if (nodes.some(n => n.name === receiver && (n.kind === 'class' || n.kind === 'function') &&
|
||||
scopes.some(f => f.id === innermost(n)?.id))) return undefined;
|
||||
if (!pythonOwnGlobalWrites(global, context) || pythonExternalWrites(global, context).writers.has(ref.filePath)) {
|
||||
return null;
|
||||
}
|
||||
const classes = pythonGlobalClasses(global, ref, context);
|
||||
if (!classes) return null;
|
||||
// A class without the member (a base from outside the project may supply it) leaves it unknown.
|
||||
if (classes.some(cls => pythonMembers(cls, member, ref, context).length === 0)) return null;
|
||||
const pool = pythonGlobalMembers(global, member, ref, context).filter(n => sameLanguageFamily(n.language, ref.language));
|
||||
const target = pool.length === 1 ? pool[0]! : undefined;
|
||||
// A method value naming its own method is no reference; a call through the global is recursion.
|
||||
return target && (target.kind === 'function' || target.kind === 'method') && (call || target.id !== ref.fromNodeId) &&
|
||||
!isPythonProperty(target, context) ? target : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The method a module global's value can dispatch to: one class resolves to
|
||||
* its own method; several bind to the nearest declaration they all inherit,
|
||||
@@ -15738,6 +15775,14 @@ export function matchMethodCall(
|
||||
// `subprocess.run(…)` after `import subprocess`: a function of a module
|
||||
// from outside the project, not a same-named project method (#2599).
|
||||
if (!inferredType && ref.language === 'python' && dotMatch && isPythonOutsideModuleReceiver(objectOrClass!, ref, context)) return null;
|
||||
// A module-level instance of this file — `_svc = SceneService()` — called
|
||||
// from a function: the method its class has or inherits (#2582), as for a
|
||||
// method value through the same global (#2074). Otherwise the call goes on
|
||||
// to the strategies below, as before.
|
||||
if (!inferredType && ref.language === 'python' && dotMatch && /^\w+$/.test(objectOrClass!)) {
|
||||
const viaGlobal = pythonGlobalReceiverTarget(objectOrClass!, methodName!, ref, context, true);
|
||||
if (viaGlobal) return { original: ref, targetNodeId: viaGlobal.id, confidence: 0.9, resolvedBy: 'instance-method' };
|
||||
}
|
||||
// A Go receiver's declaration in scope, as the scope reader finds it
|
||||
// (`declared`), decides over the patterns, which scan back for the name
|
||||
// without block scope and can read another declaration of it:
|
||||
|
||||
Reference in New Issue
Block a user