diff --git a/CHANGELOG.md b/CHANGELOG.md index cdb6cb00..4de0eb4e 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 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). - `codegraph sync` no longer fails with "FOREIGN KEY constraint failed" in a project whose code lazy-loads a workspace package, like Supabase Studio's `dynamic(() => import('icons').then((mod) => mod.EdgeFunctions))` or vben's `() => import('@vben/layouts').then((m) => m.IFrameView)`. Before, every sync after an edit in such a project rolled back, so the index stopped following your changes until a full re-index. Thanks @bompus for the report and @inth3shadows. (#2565) diff --git a/__tests__/python-external-module-receiver.test.ts b/__tests__/python-external-module-receiver.test.ts new file mode 100644 index 00000000..6317a6f2 --- /dev/null +++ b/__tests__/python-external-module-receiver.test.ts @@ -0,0 +1,102 @@ +/** + * A Python call on a module the file imports from outside the project — + * `import subprocess; subprocess.run(…)`, `import numpy as np`, `from os import + * path` — is never a method of a project class that happens to share the + * method's name (#2599). A call on a project module, or on a project class's + * instance, keeps its edge. + */ + +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-2599-')); + 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') + .map((e) => cg!.getNode(e.target)) + .filter((n): n is NonNullable => !!n) + .map((n) => `${n.kind}:${n.qualifiedName}`) + .sort(); +} + +afterEach(() => { + cg?.close(); + cg = null; + fs.rmSync(tempDir, { recursive: true, force: true }); +}); + +const FAKE = 'class Fake:\n def run(self, name):\n return name\n\n def join(self, *parts):\n return parts\n'; + +describe('a Python call on an imported outside module (#2599)', () => { + it.each([ + ['import subprocess', 'import subprocess\n\n\ndef use():\n return subprocess.run(["ls"])\n'], + ['import ... as', 'import subprocess as sp\n\n\ndef use():\n return sp.run(["ls"])\n'], + ['an import inside the function', 'def use():\n import subprocess\n return subprocess.run(["ls"])\n'], + ])('does not link a project method: %s', async (_label, code) => { + const out = await calls({ 'app.py': `${code}\n\n${FAKE}` }, 'use'); + expect(out.filter((e) => e.startsWith('method:Fake::'))).toEqual([]); + }); + + it('keeps a call on a project instance', async () => { + const out = await calls( + { 'app.py': `import subprocess\n\n\ndef use():\n f = Fake()\n subprocess.run(["ls"])\n return f.run("x")\n\n\n${FAKE}` }, + 'use' + ); + expect(out).toEqual(['method:Fake::run']); + }); + + it('keeps a call where a module-level assignment replaces the import', async () => { + const out = await calls( + { 'app.py': `import subprocess\n\nsubprocess = Fake()\n\n\ndef use():\n return subprocess.run("x")\n\n\n${FAKE}` }, + 'use' + ); + expect(out).toEqual(['method:Fake::run']); + }); + + it.each([ + ['a tuple assignment', 'import subprocess\n\nsubprocess, other = Fake(), 1\n'], + ['a global in a function', 'import subprocess\n\n\ndef init():\n global subprocess\n subprocess = Fake()\n'], + ['a for loop', 'import subprocess\n\nfor subprocess in [Fake()]:\n pass\n'], + ['a fallback class', 'try:\n from fastlib import subprocess\nexcept ImportError:\n class subprocess:\n @staticmethod\n def run(name):\n return name\n'], + ])('keeps a call where %s rebinds the import', async (_label, prelude) => { + const code = `${prelude}\n\ndef use():\n return subprocess.run("x")\n\n\n${FAKE}`; + const out = await calls({ 'app.py': code }, 'use'); + expect(out.length).toBe(1); + }); + + it('keeps a call on a project module imported by its package name when the package is the indexed root', async () => { + const out = await calls( + { + '__init__.py': '', + 'tasks.py': 'class Runner:\n def go(self, x):\n return x\n\n\nrunner = Runner()\n', + 'app.py': 'from mypkg.tasks import runner\n\n\ndef use():\n return runner.go("x")\n', + 'other.py': 'class Other:\n def go(self, x):\n return None\n', + }, + 'use' + ); + expect(out).toEqual(['method:Runner::go']); + }); + + it('keeps a call on an imported project module', async () => { + const out = await calls( + { 'tasks.py': 'def run(name):\n return name\n', 'app.py': 'import tasks\n\n\ndef use():\n return tasks.run("x")\n' }, + 'use' + ); + expect(out).toEqual(['function:run']); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 8f8ed4d0..eec1eb77 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -3825,6 +3825,11 @@ function pythonFromImports(filePath: string, context: ResolutionContext): Map { + const keys = pythonImportKeys(name, ref.filePath, context); + if (keys.size === 0) return false; + if (pythonGlobalBindings(name, ref.filePath, context).some((b) => b.kind !== 'import')) return false; + if (context.getNodesInFile(ref.filePath).some((n) => n.name === name && (n.kind === 'class' || n.kind === 'function'))) return false; + // `import a.b as x` keys as `a.b:*`; `from a import x` as `a:x`, where only + // the module `a` tells that `x` comes from outside. + return [...keys].every((key) => { + const module = key.slice(0, key.indexOf(':')); + return !module.startsWith('.') && !isPythonProjectModuleAnyRoot(module, context); + }); + }); + return outside && !pythonBindsLocally(name, ref, context, false); +} + +/** + * Whether `a.b.c` is a project module from some import root: `a/b/c.py`, or — + * with the package itself indexed as the root — `b/c.py` or `c.py`. Erring + * toward "project" only leaves a call to the name strategies, as before. + */ +function isPythonProjectModuleAnyRoot(module: string, context: ResolutionContext): boolean { + const parts = module.split('.'); + return parts.some((_, i) => isPythonProjectModule(parts.slice(i).join('.'), context)); } const JAVA_TYPE_KINDS: ReadonlySet = new Set(['class', 'interface', 'enum', 'struct', 'record', 'trait']); @@ -15695,6 +15735,9 @@ export function matchMethodCall( // A pytest test's parameter is what its fixture returns: flaskbb's // `cli_runner.invoke(…)` is click's `CliRunner`, not the project's one `invoke`. if (!inferredType && ref.language === 'python' && dotMatch) inferredType = pythonFixtureReturnType(objectOrClass!, ref, context); + // `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 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: