Skip to content

fix(python): a parameter or local named like an imported module is not that module #3046

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
colbymchenry merged 1 commit into from
Oct 30, 2026
+629 −4
Merged

fix(python): a parameter or local named like an imported module is not that module #3046

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- In JavaScript and TypeScript, a call on one of the browser's own objects, like `window.open('/help')`, `localStorage.getItem('token')`, `location.reload()`, `history.back()`, `window.localStorage.getItem('token')` or a worker's `self.postMessage(…)`, no longer links to a method of your own that only shares its name, like a `WindowService.open` or a `StorageService.getItem` an app wraps the browser in, so `codegraph callers` and impact on such a wrapper no longer list every direct browser call in the app. A name the file declares, imports or takes as a parameter, like `const history = createRouter()`, still calls what it holds, `this` still means the class or object it is written in, and so does a `self` the file sets to it, like `var self = this`. Re-index JavaScript and TypeScript projects after upgrading. Thanks @Gobakabana for the report and @KissFeng. (#2907)
- In JavaScript and TypeScript, a method called on a freshly constructed object, like `new RegExp(p).exec(s)`, `new URL(u).toString()` or `new Runner().run()`, now links only to that class's own or inherited method, on the class the file imports when several share its name, and no longer to an unrelated project method of the same name; a class from outside your project links nothing. Re-index JavaScript and TypeScript projects after upgrading. Thanks @danusha2345. (#1945)
- 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, a call through a name the file imports as a module, like `other.update(…)` after `from pkg import other`, now links to that module only where nothing closer gives the name another value: a parameter, a local, a `with … as` or `except … as` target, a loop or comprehension variable, a lambda parameter or a `case` capture named `other` is that value, so its calls no longer show up as calls into the module. A `from pkg import third as other` written inside a function now links that function's `other.get()` to `third`, and no longer the rest of the file's. Re-index Python projects after upgrading. Thanks @BGamboa13. (#2632)
- 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)
Expand Down
235 changes: 235 additions & 0 deletions __tests__/python-receiver-rebinding.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,235 @@
/**
* A Python call through a name the file imports as a module reads that module
* only where nothing nearer binds the name:
*
* from pkg import other
*
* def real():
* other.update({}) # pkg/other.py's update
*
* def by_param(other):
* other.update({}) # dict.update, or whatever the caller passed
*
* The import resolver read the file's imports alone, so a parameter, a local,
* a `with … as` / `except … as` / loop target, a walrus, a `case` capture or a
* lambda or comprehension variable named like the module still linked the
* call to the module's function. A function-local `from pkg import third as
* other` linked to `pkg.other` too, and so did, from every other function of
* the file, though it binds the name only where it is written.
*/
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import CodeGraph from '../src/index';

const FILES: Record<string, string> = {
'pyproject.toml': '[project]\nname = "pkg"\n',
'src/pkg/__init__.py': '',
'src/pkg/other.py': [
'def update(x=None):',
' return x',
'',
'',
'def persist(x=None):',
' return x',
'',
'',
'def get(key=None):',
' return key',
'',
].join('\n'),
'src/pkg/third.py': ['def get(key=None):', ' return key', ''].join('\n'),
'tests/shadow.py': [
'from pkg import other',
'',
'',
'def real():',
' other.update({"b": 2})',
' return other.persist(1)',
'',
'',
'def by_param(other):',
' other.update({"a": 1})',
' return other.persist(2)',
'',
'',
'def by_with(path):',
' with open(path, "w") as other:',
' return other.persist(3)',
'',
'',
'def by_assign():',
' other = {}',
' other.update({"c": 3})',
'',
'',
'def by_for(items):',
' for other in items:',
' other.persist(4)',
'',
'',
'def by_except():',
' try:',
' pass',
' except OSError as other:',
' return other.persist(5)',
'',
'',
'def by_lambda():',
' return lambda other: other.persist(6)',
'',
'',
'def by_comprehension(items):',
' return [other.persist(7) for other in items]',
'',
'',
'def by_walrus(load):',
' if (other := load()):',
' return other.persist(8)',
'',
'',
'def by_closure(other):',
' def inner():',
' return other.persist(9)',
' return inner',
'',
'',
'def by_capture(v):',
' match v:',
' case other:',
' return other.persist(10)',
'',
'',
'def by_local_import():',
' from pkg import third as other',
' return other.get(11)',
'',
'',
// Each of these reads the module the file imports.
'def keyword_argument(run):',
' run(',
' other=1,',
' )',
' return other.persist(12)',
'',
'',
'def in_a_string():',
' print("with x as other", "other = 1") # other = 2',
' return other.persist(13)',
'',
'',
'def comprehension_of_others(items):',
' return [x for x in other.persist(items)]',
'',
'',
'def lambda_beside(run):',
' return run(lambda other: 1, other.persist(14))',
'',
'',
'def declared_global():',
' global other',
' return other.persist(15)',
'',
'',
'def keyword_pattern(v):',
' match v:',
' case Point(other=1):',
' return other.persist(16)',
'',
'',
'def same_module_again():',
' from pkg import other',
' return other.get(17)',
'',
].join('\n'),
// The module binds `other` with a plain import, which the file's mappings
// list after every from-import, the function-local one included.
'tests/leak.py': [
'import pkg.other as other',
'',
'',
'def local_third():',
' from pkg import third as other',
' return other.get(1)',
'',
'',
'def untouched():',
' return other.get(2)',
'',
].join('\n'),
};

describe.each([
['default', 'LF'],
['wasm', 'LF'],
['default', 'CRLF'],
['wasm', 'CRLF'],
])('a Python receiver the function binds itself is not the module the file imports (%s, %s)', (backend, eol) => {
let root = '';
let cg: CodeGraph | undefined;
let kernel: string | undefined;

beforeAll(async () => {
kernel = process.env.CODEGRAPH_KERNEL;
if (backend === 'wasm') process.env.CODEGRAPH_KERNEL = '0';
else delete process.env.CODEGRAPH_KERNEL;
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-py-rebind-'));
for (const [rel, content] of Object.entries(FILES)) {
fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true });
fs.writeFileSync(path.join(root, rel), eol === 'CRLF' ? content.replace(/\n/g, '\r\n') : content);
}
cg = await CodeGraph.init(root, { index: true });
}, 60_000);

afterAll(() => {
cg?.destroy();
if (root) fs.rmSync(root, { recursive: true, force: true });
if (kernel === undefined) delete process.env.CODEGRAPH_KERNEL;
else process.env.CODEGRAPH_KERNEL = kernel;
});

/** `file:name` of each project function `name` in `file` calls, sorted. */
const callsFrom = (file: string, name: string): string[] => {
const from = cg!.getNodesInFile(file).filter((n) => n.name === name && (n.kind === 'function' || n.kind === 'method'));
expect(from, `${name} in ${file}`).toHaveLength(1);
return cg!.getOutgoingEdgesFrom([from[0]!.id])
.filter((e) => e.kind === 'calls')
.map((e) => cg!.getNode(e.target)!)
.filter((t) => t.filePath.startsWith('src/'))
.map((t) => `${t.filePath}:${t.name}`)
.sort();
};

it('a call where nothing nearer binds the name reaches the module', () => {
expect(callsFrom('tests/shadow.py', 'real')).toEqual(['src/pkg/other.py:persist', 'src/pkg/other.py:update']);
});

it.each([
'by_param', 'by_with', 'by_assign', 'by_for', 'by_except', 'by_lambda',
'by_comprehension', 'by_walrus', 'inner', 'by_capture',
])('%s: a name the code around the call binds links no module function', (fn) => {
expect(callsFrom('tests/shadow.py', fn)).toEqual([]);
});

it('a function-local `from pkg import third as other` is that module', () => {
expect(callsFrom('tests/shadow.py', 'by_local_import')).toEqual(['src/pkg/third.py:get']);
expect(callsFrom('tests/leak.py', 'local_third')).toEqual(['src/pkg/third.py:get']);
});

it('a function-local import binds the name in that function only', () => {
expect(callsFrom('tests/leak.py', 'untouched')).toEqual(['src/pkg/other.py:get']);
});

it.each([
['keyword_argument', 'src/pkg/other.py:persist'],
['in_a_string', 'src/pkg/other.py:persist'],
['comprehension_of_others', 'src/pkg/other.py:persist'],
['lambda_beside', 'src/pkg/other.py:persist'],
['declared_global', 'src/pkg/other.py:persist'],
['keyword_pattern', 'src/pkg/other.py:persist'],
['same_module_again', 'src/pkg/other.py:get'],
])('%s binds nothing of that name: the call reaches the module', (fn, target) => {
expect(callsFrom('tests/shadow.py', fn)).toEqual([target]);
});
});
54 changes: 53 additions & 1 deletion src/resolution/import-resolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import {
tsImportedTypeFile,
reExportSource,
pythonFunctionModuleAlias,
pythonReceiverBinding,
pythonModuleImportKeys,
isPythonFromImportModule,
tsGlobalScope,
rustPathTarget,
Expand Down Expand Up @@ -3127,7 +3129,7 @@ export function resolveViaImport(
}

// Use cached import mappings (avoids re-reading and re-parsing per ref)
const imports = context.getImportMappings(ref.filePath, ref.language);
let imports = context.getImportMappings(ref.filePath, ref.language);
if (imports.length === 0 && !context.readFile(ref.filePath)) {
return null;
}
Expand Down Expand Up @@ -3157,6 +3159,9 @@ export function resolveViaImport(
// submodule (a file), not a symbol, so the generic symbol lookup below would
// search the *package* for `certs` instead of looking inside the module.
if (ref.language === 'python') {
// Only the imports the receiver means where it is read: none of the file's
// when the function binds the name itself (see pythonImportsInScope).
imports = pythonImportsInScope(ref, imports, context);
const pyResult = resolvePythonModuleMember(ref, imports, context);
if (pyResult) return pyResult;
// Absolute dotted module import: `import conduit.apps.articles.signals`
Expand Down Expand Up @@ -3373,6 +3378,53 @@ export function reachesAmbientModule(ref: UnresolvedRef, specifier: string, decl
return LAST_NAME_KINDS.has(ref.referenceKind) && mappings.some((m) => leadsTo(m.source, () => false));
}

/**
* The file's import mappings as a Python dotted reference reads them. Its
* receiver is the module the file imports under that name only where nothing
* nearer binds the name: a parameter, a local, a `with … as` / `except … as` /
* loop target, a `case` capture, or a lambda or comprehension variable around
* the reference is that value instead, and none of the receiver's mappings
* applies there (`def by_param(other): other.update(…)` linked to the module
* imported as `other`). A function-local import of the name leaves only the
* mappings binding it the same way: the file's mappings read from-imports at
* any depth, so a local `from x import y as name` is listed beside the
* module-level import, and a local `import pkg.sub` binds the `pkg` that a
* module-level `import pkg` does. Other references and receivers keep every
* mapping.
*/
export function pythonImportsInScope(ref: UnresolvedRef, imports: ImportMapping[], context: ResolutionContext): ImportMapping[] {
if (ref.language !== 'python' || ref.referenceKind === 'imports') return imports;
const dot = ref.referenceName.indexOf('.');
if (dot <= 0) return imports;
const receiver = ref.referenceName.slice(0, dot);
// `import pkg.mod` binds `pkg`, under a mapping named after `mod`.
const binds = (m: ImportMapping): boolean => m.localName === receiver ||
(m.isNamespace && m.source.split('.')[0] === receiver && m.localName === m.source.split('.').pop());
if (!imports.some(binds)) return imports;
const binding = pythonReceiverBinding(receiver, ref, context);
if (binding === 'module') {
// Where nothing nearer binds the name, the module's own import of it
// does — not a from-import written inside some other function, which the
// mappings list too (first, ahead of the module's `import pkg.other as
// other`). Asked only when the receiver's mappings disagree; with no
// module-level one found, every mapping stays as it was.
const keyOf = (m: ImportMapping): string => (m.isNamespace ? `${m.source}:*` : `${m.source}:${m.exportedName}`);
if (new Set(imports.filter(binds).map(keyOf)).size < 2) return imports;
const moduleLevel = pythonModuleImportKeys(receiver, ref.filePath, context);
const kept = imports.filter((m) => !binds(m) || m.isNamespace || moduleLevel.has(keyOf(m)));
return kept.some(binds) ? kept : imports;
}
return imports.filter((m) => {
if (!binds(m)) return true;
if (binding === 'local') return false;
// The module a namespace mapping binds the receiver to: `import x as
// receiver` / `import receiver` bind its source, `import receiver.sub`
// the top-level `receiver`.
if ('module' in binding) return m.isNamespace && (m.localName === receiver ? m.source : receiver) === binding.module;
return !m.isNamespace && m.source === binding.source && m.exportedName === binding.exportedName;
});
}

/**
* Resolve a Python qualified reference whose receiver is an imported MODULE:
* `certs.where()` after `from . import certs`, `mod.func()` after `import mod`
Expand Down
Loading