mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-27 04:49:43 +08:00
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 maind5fc1fbb. Both arms were built from their own sources with the kernel. Rebased onto628f69e2(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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
628f69e275
commit
23a80a847a
@@ -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']);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user