diff --git a/CHANGELOG.md b/CHANGELOG.md index 4de0eb4e..16faabba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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). diff --git a/__tests__/python-module-global-receiver-call.test.ts b/__tests__/python-module-global-receiver-call.test.ts new file mode 100644 index 00000000..73203519 --- /dev/null +++ b/__tests__/python-module-global-receiver-call.test.ts @@ -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, fromName: string): Promise { + 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']); + }); +}); + diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index eec1eb77..e6a58383 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -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: