From 23a80a847ae2a90bf195af2ee030c00cc094bb00 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 23 Oct 2026 02:38:35 +0000 Subject: [PATCH] fix(python): a from-import of a module with no dot in its name links the module's file (#2823) In Python, `from loader import X` with loader.py at a package root left the module reference `loader` unresolved, so the importing file had no imports edge to loader.py. `import loader` and `from pkg.loader import X` both link their module's file. The from-import's names (X) linked, but `codegraph affected` and impact could miss the file's dependency on the module. v1.6.2 behaves the same. Root cause: resolvePythonAbsoluteModule resolves only dotted module refs. It leaves bare names to resolveModuleImportToFile, because a bare imported name like `authentication` in `from app.api.routes import authentication` is ambiguous on its own. But resolveModuleImportToFile matches an import by its local name, which for `from loader import X` is X, never `loader`. So the module of an undotted from-import was never looked up by its path. When the file also bound the module's name some other way, the generic import loop resolved it through that binding: next to `from django.core import mail`, the module of `from mail import ...` became django/core/mail/__init__.py. Django's tests/mail/__init__.py is the module meant, and the statement's own names already resolve there. Fix: resolvePythonAbsoluteModule also takes the module a from-import names (isPythonFromImportModule), dotted or not. It resolves it from the package roots, as `import loader` is resolved. Imported names keep their path, and a module from outside the project still links nothing. Validation: - New __tests__/python-from-import-module-file.test.ts, 7 tests: a root-level module, a top-level package, a src layout, a module named on a continuation line, a submodule imported by name, sync after the module file is added (matching a fresh index), and an outside module with a same-named nested file (links nothing). With main's import-resolver.ts the 6 positive tests fail and the guard passes. Seven assertions in python-module-resolution.test.ts listed a file's import edges exactly while the module edge was missing; they now include the module's file. What they test is unchanged: where the imported name binds, and no cycle. - A/B against main d5fc1fbb. Both arms were built from their own sources with the kernel. Rebased onto 628f69e2 (PR #2822, Kotlin only), the fix arm's graphs are byte-identical to before the rebase: - requests: +2 edges; - sqlalchemy: +3,189; - django's 2,938 Python files: +103 edges and 2 retargeted; - every change is a from-import's module linking its file, and nothing else changed; - express and gin (controls): byte-identical. Triage of all 3,296 added edges: - 3,284 point at the file, or the package folder, where the same statement's imported names already resolve; - the other 12 were read by hand: multi-line statements, re-exported constants the name resolver doesn't follow, and a deliberately broken name in a Django test fixture, each on the right module file; - the 2 django retargets are the mail case above. - Cost: index CPU medians of 3 interleaved runs, main vs fix: sqlalchemy 100.7 s vs 100.3 s, django 98.4 s vs 97.8 s. Found while verifying another Python issue in the sweep; no issue was filed for it. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + .../python-from-import-module-file.test.ts | 129 ++++++++++++++++++ __tests__/python-module-resolution.test.ts | 10 +- src/resolution/import-resolver.ts | 14 +- 4 files changed, 145 insertions(+), 9 deletions(-) create mode 100644 __tests__/python-from-import-module-file.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 07c21e8d..dd8a4d1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In JavaScript and TypeScript, react-dom's `root.render()` no longer links to your own class component's `render()`, such as an error boundary's, when `root` is declared outside the function making the call, like a module's `let root: Root | null`, or holds what `createRoot(el)` returned. A call on a variable declared outside the calling function now goes by the type that declaration gives it, so one declared with your own class, like `const store = new Store()` at the top of a file, links to that class's method, and a class component's `render()` and lifecycle methods, which only React calls, are no longer a guess for an untyped value. Re-index JavaScript and TypeScript projects after upgrading. Thanks @inth3shadows for the report. (#2589) - In Python, `import a.b as X` written inside a function now links its calls like `X.f()`, as the same import at module level already did. Thanks @Egrn for the report. (#2600) - In Python, `from app import email` no longer links the importing file to a project variable named `app`, such as a FastAPI or Flask `app = FastAPI()`. (#2584) +- In Python, a from-import of a module whose name has no dot, like `from loader import Registry` with `loader.py` at the repository root, or `from mypackage import helper` in a `src/` layout, now links the importing file to that module's file, as `import loader` and `from pkg.loader import Registry` already did. Before, the file's dependency on the module itself was left out, so `codegraph affected` and impact could miss the files that import it. When the file also imported something else under the module's name, like `from django.core import mail` beside `from mail import …`, the module was linked to that instead. Re-index Python projects after upgrading. - 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 of your own class's method named like a dictionary, list or file method, such as `get`, `update`, `items`, `pop` or `close`, now links to that method when the function says what the variable holds: `reg = SystemPackRegistry()` then `reg.get(…)`, `registry = load_registry()` with `def load_registry() -> SystemPackRegistry`, or a parameter written `reg: SystemPackRegistry`. Before, these calls were dropped as built-in methods unless the variable was named after the class. A variable the function also sets to something else, like `{}` or what `json.load` returns, a class imported from another package, like `queue.Queue`, and a union type still link nothing. Re-index Python projects after upgrading. Thanks @inth3shadows for the report. (#2583) - 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) diff --git a/__tests__/python-from-import-module-file.test.ts b/__tests__/python-from-import-module-file.test.ts new file mode 100644 index 00000000..a258f7a4 --- /dev/null +++ b/__tests__/python-from-import-module-file.test.ts @@ -0,0 +1,129 @@ +/** + * The module a Python `from X import y` names is a file of the project when X + * is one, dotted or not: `from loader import X` with `loader.py` at a package + * root links the importing file to it, as `import loader` and `from + * pkg.loader import X` already did. Before, an undotted module of a + * from-import was left unresolved, so the file lost its dependency on the + * module and `codegraph affected` missed it. A module from outside the + * project still links nothing. + */ + +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; + +function write(files: Record): void { + 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); + } +} + +/** `kind:name@file` of every target of an `imports` edge from `fromFile`'s file node, sorted. */ +function importEdges(fromFile: string): string[] { + const file = cg!.getNodesByKind('file').find((n) => n.filePath === fromFile)!; + expect(file).toBeDefined(); + return cg! + .getOutgoingEdges(file.id) + .filter((e) => e.kind === 'imports') + .map((e) => cg!.getNode(e.target)) + .filter((n): n is NonNullable => !!n) + .map((n) => `${n.kind}:${n.name}@${n.filePath}`) + .sort(); +} + +async function indexed(files: Record): Promise { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-from-module-')); + write(files); + cg = await CodeGraph.init(tempDir, { index: true }); +} + +afterEach(() => { + cg?.close(); + cg = null; + fs.rmSync(tempDir, { recursive: true, force: true }); +}); + +const LOADER = 'class Registry:\n def get(self, k):\n return k\n\n\ndef load_registry():\n return Registry()\n'; + +describe('the module of a Python from-import links its file', () => { + it('links a top-level module at the project root, as `import loader` does', async () => { + await indexed({ + 'loader.py': LOADER, + 'packs.py': 'from loader import Registry, load_registry\n\n\ndef use():\n return load_registry()\n', + 'plain.py': 'import loader\n\n\ndef use():\n return loader.load_registry()\n', + }); + expect(importEdges('packs.py')).toEqual([ + 'class:Registry@loader.py', + 'file:loader.py@loader.py', + 'function:load_registry@loader.py', + ]); + expect(importEdges('plain.py')).toEqual(['file:loader.py@loader.py']); + }); + + it('links a top-level package to its __init__.py, as a dotted from-import links its package', async () => { + await indexed({ + 'pkg/__init__.py': 'def helper():\n return 1\n', + 'pkg/sub/__init__.py': 'def deep():\n return 2\n', + 'app.py': 'from pkg import helper\nfrom pkg.sub import deep\n', + }); + expect(importEdges('app.py')).toEqual([ + 'file:__init__.py@pkg/__init__.py', + 'file:__init__.py@pkg/sub/__init__.py', + 'function:deep@pkg/sub/__init__.py', + 'function:helper@pkg/__init__.py', + ]); + }); + + it('finds the module from the package roots, as in a src layout', async () => { + await indexed({ + 'pyproject.toml': '[project]\nname = "app"\n', + 'src/app/__init__.py': 'def helper():\n return 1\n', + 'tests/test_app.py': 'from app import helper\n\n\ndef test_helper():\n assert helper() == 1\n', + }); + expect(importEdges('tests/test_app.py')).toEqual([ + 'file:__init__.py@src/app/__init__.py', + 'function:helper@src/app/__init__.py', + ]); + }); + + it('links a module named on a continuation line of the statement', async () => { + await indexed({ 'loader.py': LOADER, 'packs.py': 'from \\\n loader import Registry\n' }); + expect(importEdges('packs.py')).toEqual(['class:Registry@loader.py', 'file:loader.py@loader.py']); + }); + + it('links nothing for a module from outside the project, even one a nested file is named like', async () => { + await indexed({ + 'tests/fixtures/requests.py': 'class Session:\n pass\n', + 'client.py': 'from requests import Session\nimport requests\n', + }); + expect(importEdges('client.py')).toEqual([]); + }); + + it('keeps a submodule the statement imports by name', async () => { + await indexed({ + 'pkg/__init__.py': '', + 'pkg/loader.py': LOADER, + 'app.py': 'from pkg import loader\n', + }); + expect(importEdges('app.py')).toEqual(['file:__init__.py@pkg/__init__.py', 'file:loader.py@pkg/loader.py']); + }); + + it('links the module once it is added, on sync as on a fresh index', async () => { + await indexed({ 'packs.py': 'from loader import Registry\n' }); + expect(importEdges('packs.py')).toEqual([]); + write({ 'loader.py': LOADER }); + await cg!.sync(); + const synced = importEdges('packs.py'); + cg!.close(); + fs.rmSync(path.join(tempDir, '.codegraph'), { recursive: true, force: true }); + cg = await CodeGraph.init(tempDir, { index: true }); + expect(synced).toEqual(importEdges('packs.py')); + expect(synced).toEqual(['class:Registry@loader.py', 'file:loader.py@loader.py']); + }); +}); diff --git a/__tests__/python-module-resolution.test.ts b/__tests__/python-module-resolution.test.ts index 05350950..99a0a1ec 100644 --- a/__tests__/python-module-resolution.test.ts +++ b/__tests__/python-module-resolution.test.ts @@ -338,7 +338,8 @@ def fetch(): cg = await CodeGraph.init(tempDir, { index: true }); expect(targets('analysis.py', 'analysis.py', 'imports')).toEqual([]); - expect(targets('report.py', 'report.py', 'imports')).toEqual(['summary@analysis.py']); + // The from-import's module is analysis.py itself; the name is its `summary`. + expect(targets('report.py', 'report.py', 'imports').sort()).toEqual(['analysis.py@analysis.py', 'summary@analysis.py']); expect(cg.findCircularDependencies()).toEqual([]); }); @@ -651,7 +652,8 @@ def main(): 'main.py': 'from pkg import f\n\n\ndef main():\n f()\n', }); cg = await CodeGraph.init(tempDir, { index: true }); - expect(targets('main.py', 'main.py', 'imports')).toEqual(['f@pkg/__init__.py']); + // The package itself is the from-import's module; `f` is its attribute. + expect(targets('main.py', 'main.py', 'imports').sort()).toEqual(['__init__.py@pkg/__init__.py', 'f@pkg/__init__.py']); }); it('binds it through a relative import too', async () => { @@ -675,7 +677,7 @@ def main(): 'main.py': 'from pkg import thing\n\n\ndef main():\n thing()\n', }); cg = await CodeGraph.init(tempDir, { index: true }); - expect(targets('main.py', 'main.py', 'imports')).toEqual(['thing@pkg/impl.py']); + expect(targets('main.py', 'main.py', 'imports').sort()).toEqual(['__init__.py@pkg/__init__.py', 'thing@pkg/impl.py']); }); it('still binds the submodule when the package does not bind the name', async () => { @@ -685,7 +687,7 @@ def main(): 'main.py': 'from pkg import f\n\n\ndef main():\n f.g()\n', }); cg = await CodeGraph.init(tempDir, { index: true }); - expect(targets('main.py', 'main.py', 'imports')).toEqual(['f.py@pkg/f.py']); + expect(targets('main.py', 'main.py', 'imports').sort()).toEqual(['__init__.py@pkg/__init__.py', 'f.py@pkg/f.py']); expect(targets('main', 'main.py', 'calls')).toEqual(['g@pkg/f.py']); }); }); diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index 01712149..c05f2682 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -30,6 +30,7 @@ import { tsImportedTypeFile, reExportSource, pythonFunctionModuleAlias, + isPythonFromImportModule, isRustAssociatedItem, tsGlobalScope, } from './name-matcher'; @@ -3661,11 +3662,14 @@ function resolvePythonAbsoluteModule( context: ResolutionContext ): ResolvedRef | null { if (ref.referenceKind !== 'imports') return null; - // Only a DOTTED `import a.b.c` ref carries its full module path. A bare leaf - // (`from app.api.routes import authentication`) is ambiguous on its own — three - // `authentication.py` files may exist — so leave it to resolveModuleImportToFile, - // which uses the import's source (`app.api.routes`) to build the full path. - if (!ref.referenceName.includes('.')) return null; + // Only a DOTTED `import a.b.c` ref carries its full module path, and so does + // the module a from-import names, dotted or not: `from loader import X` is + // the top-level `loader`, found from the package roots as `import loader` + // finds it. A bare leaf it imports (`from app.api.routes import + // authentication`) is ambiguous on its own — three `authentication.py` files + // may exist — so leave it to resolveModuleImportToFile, which uses the + // import's source (`app.api.routes`) to build the full path. + if (!ref.referenceName.includes('.') && !isPythonFromImportModule(ref, context)) return null; const hit = pythonModuleFileNode(ref.referenceName, ref.filePath, context); return hit ? { original: ref, targetNodeId: hit.id, confidence: 0.9, resolvedBy: 'import' } : null; }