fix(python): a bare call to a locally bound name is that value (#2198)

A bare Python call whose name the enclosing function binds - an assignment
(`view = SomeView.as_view()`, tuple and annotated forms), a parameter, a
`for` / `with ... as` / `except ... as` target - or the module binds at top
level, no longer resolves to a same-named function in another file. DRF's
tests write `view = X.as_view()` then `view(request)`: 118 such calls went
to one test file's `def view`. Read from code only (docstrings and comments
blanked), and an imported name is left to the import. A pytest fixture
(`@pytest.fixture`, or anything in a conftest.py) is still what a test's
parameter of that name calls.

django-rest-framework -122, pytest -25 (`func` / `xfail` params and
aliases), allauth -12 (`serialize_instance = get_adapter().serialize_instance`),
netbox -6 (migrations' `apps.get_model(...)`), mealie -3, flask -2;
healthchecks, flaskbb, httpx unchanged.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
2026-09-30 20:07:30 +00:00
committed by GitHub
parent 85aad42f6f
commit 71202c38df
3 changed files with 150 additions and 1 deletions
+1
View File
@@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
### Fixes
- A Python call to a name the function or module binds itself now means that local value, not a same-named function in another file. In Django REST Framework's tests, `view = SomeView.as_view()` followed by `view(request)` had pointed at one test file's `def view`. The same goes for a parameter or a `for`/`with … as` target. A pytest fixture a test takes as a parameter is still the fixture.
- A method call on a variable is no longer guessed to be a test double's method just because the double has the only method of that name. That covered classes named `Mocked…`, `Fake…`, `Stub…` or `Dummy…`: django-allauth's `resp.json()` calls had all gone to its `MockedResponse`. A test that names the double still calls it.
- Kotlin imports now decide what a name means, as Java imports already do. `import java.net.Proxy` or `import org.mockito.Mockito.mock` no longer links to a same-named project symbol, and a member the file declares itself still comes first. An `import x.Foo` picks the right one of several same-named classes: on okhttp, `MockResponse.Builder()` and `Request.Builder()` now reach their classes. A renaming import like `import …internal.unsafeFlow as flow` means the function it names.
- In Java, a class you import from outside your project now owns its name. After `import java.lang.reflect.Field;` the file's `Field` no longer links to a project class that happens to share the name. In gson that was a test's nested `Field` class; in jsoup's W3C converter it was jsoup's own `Document` and `Element`. A static import like `import static org.mockito.Mockito.verify;` is covered the same way.
+87
View File
@@ -0,0 +1,87 @@
/**
* A bare Python call to a name its function (or module) binds itself —
* DRF's `view = SomeView.as_view()` then `view(request)` — is that value,
* not another file's `def view` (118 such calls went to one test file's
* `view`). A pytest fixture the test takes as a parameter is still the
* fixture, and an imported name is the import's, whatever a docstring says.
*/
import { describe, it, expect, afterAll, beforeAll } from 'vitest';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { CodeGraph } from '../src';
let root = '';
let cg: CodeGraph;
beforeAll(async () => {
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-py-local-'));
const files: Record<string, string> = {
'tests/test_testing.py': `def view(request):
return request
`,
'tests/test_versioning.py': `class TestVersion:
def test_version(self, request):
view = MyView.as_view()
response = view(request)
return response
`,
'tests/conftest.py': `import pytest
@pytest.fixture
def user_factory():
def make():
return 1
return make
`,
'tests/test_users.py': `def test_user(user_factory):
user = user_factory()
return user
`,
'app/utils.py': `def user_display(user):
return str(user)
`,
'app/tags.py': `from app.utils import user_display
def user_display_tag(user):
"""
{% user_display user as user_display %}
"""
return user_display(user)
`,
};
for (const [rel, content] of Object.entries(files)) {
fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true });
fs.writeFileSync(path.join(root, rel), content);
}
cg = await CodeGraph.init(root, { index: true });
});
afterAll(() => {
cg?.close();
if (root) fs.rmSync(root, { recursive: true, force: true });
});
const callsFrom = (file: string) => {
const ids = cg.getNodesInFile(file).map((n) => n.id);
return cg.getOutgoingEdgesFrom(ids).filter((e) => e.kind === 'calls').map((e) => {
const t = cg.getNode(e.target)!;
return `${t.filePath}: ${t.qualifiedName}`;
}).sort();
};
describe('bare Python calls to locally bound names', () => {
it('are the local value, not another file’s function of that name', () => {
expect(callsFrom('tests/test_versioning.py')).not.toContain('tests/test_testing.py: view');
});
it('still reach a pytest fixture taken as a parameter', () => {
expect(callsFrom('tests/test_users.py')).toContain('tests/conftest.py: user_factory');
});
it('still reach an imported function a docstring happens to mention', () => {
expect(callsFrom('app/tags.py')).toContain('app/utils.py: user_display');
});
});
+62 -1
View File
@@ -1477,8 +1477,12 @@ function pythonCallShape(ref: UnresolvedRef, context: ResolutionContext): Python
function fitsPythonCallShape(n: Node, shape: PythonCallShape, ref: UnresolvedRef, context: ResolutionContext): boolean {
if (shape.kind === 'bare') {
if (n.kind === 'method') return false;
if (n.filePath === ref.filePath) return true;
// `from django.shortcuts import render`: the call is the package's.
return !(n.filePath !== ref.filePath && isPythonNameImportedFromOutside(ref.referenceName, ref, context));
if (isPythonNameImportedFromOutside(ref.referenceName, ref, context)) return false;
// `view = UserView.as_view()` … `view(request)`: the file's own value —
// unless it is a pytest fixture, which a test takes as a parameter of that name.
return isPytestFixture(n) || !isPythonLocallyBound(ref.referenceName, ref, context);
}
// A member of what the chain names: a method of a class of that name, or a
// function / class in a module of that name (`helpers.slugify()`).
@@ -1494,6 +1498,62 @@ function fitsPythonCallShape(n: Node, shape: PythonCallShape, ref: UnresolvedRef
return stem === shape.owner || (stem === '__init__' && parts[parts.length - 2] === shape.owner);
}
/** A pytest fixture: `@pytest.fixture` / `@fixture`, or anything a `conftest.py` defines. */
function isPytestFixture(n: Node): boolean {
return /(?:^|\/)conftest\.py$/.test(n.filePath) || (n.decorators ?? []).some((d) => /(?:^|\.)fixture\b/.test(d));
}
const PY_LOCAL_BINDS = new WeakMap<ResolutionContext, Map<string, boolean>>();
/**
* Whether the function around a Python call — or its module, at top level —
* binds `name` itself: an assignment (`view = X.as_view()`, `a, view = …`,
* `view: T = …`), a parameter, a `for` / `with … as` / `except … as` target.
* DRF's tests write `view = SomeView.as_view()` then `view(request)`, and
* every such call went to one test file's `def view`.
*/
function isPythonLocallyBound(name: string, ref: UnresolvedRef, context: ResolutionContext): boolean {
const fn = context.getNodesInFile(ref.filePath)
.filter((f) => (f.kind === 'function' || f.kind === 'method') && f.startLine <= ref.line && f.endLine >= ref.line)
.sort((a, b) => (a.endLine - a.startLine) - (b.endLine - b.startLine))[0];
let memo = PY_LOCAL_BINDS.get(context);
if (!memo) PY_LOCAL_BINDS.set(context, (memo = new Map()));
const key = `${ref.filePath}\0${fn?.id ?? ''}\0${name}\0${fn ? '' : ref.line}`;
const hit = memo.get(key);
if (hit !== undefined) return hit;
// An imported name is the import's (the import resolver's to follow).
if (pythonFromImports(ref.filePath, context).has(name)) {
memo.set(key, false);
return false;
}
// Code only: `{% user_display user as user_display %}` in a docstring binds nothing.
const lines = stripCommentsForRegex(context.readFile(ref.filePath) ?? '', 'python').split(/\r?\n/);
const n = name;
const assigns = new RegExp(`^\\s*(?:[\\w\\s,*()\\[\\]]*,\\s*)?\\(?\\*?${n}\\)?\\s*(?:,[\\w\\s,*()\\[\\]]*)?(?::[^=]+)?=(?!=)`);
const targets = new RegExp(`\\bfor\\s+[\\w\\s,()]*\\b${n}\\b[\\w\\s,()]*\\s+in\\b|\\bas\\s+${n}\\b`);
const params = new RegExp(`[(,]\\s*\\*{0,2}${n}\\s*(?:[:=,)]|$)`);
let bound = false;
if (fn) {
// The signature up to its `:` (it may span lines), then the body above the call.
let i = fn.startLine - 1;
let signature = '';
for (; i < Math.min(lines.length, fn.startLine + 20); i++) {
signature += lines[i] ?? '';
if (/\)\s*(?:->[^:]*)?:\s*(?:#.*)?$/.test(lines[i] ?? '')) break;
}
bound = params.test(signature.replace(/^[^(]*/, ''));
for (let line = i + 1; !bound && line < ref.line - 1; line++) {
const text = lines[line] ?? '';
bound = assigns.test(text) || targets.test(text);
}
}
// A module-level binding (`view = api_view(['GET'])(handler)`).
const top = new RegExp(`^(?:[\\w,\\s]*,\\s*)?${n}\\s*(?:,[\\w\\s,]*)?(?::[^=]+)?=(?!=)`);
for (let line = 0; !bound && line < lines.length; line++) bound = top.test(lines[line] ?? '');
memo.set(key, bound);
return bound;
}
const PY_IMPORTS = new WeakMap<ResolutionContext, Map<string, Map<string, string>>>();
const PY_MODULE_LOCAL = new WeakMap<ResolutionContext, Map<string, boolean>>();
@@ -4974,6 +5034,7 @@ export function clearNameMatcherMemos(context: ResolutionContext): void {
LUA_LOCALS.delete(context);
LUA_MEMBERS.delete(context);
JVM_PACKAGES.delete(context);
PY_LOCAL_BINDS.delete(context);
PHP_FILE_SCOPES.delete(context);
JAVA_STATIC_IMPORTS.delete(context);
PY_IMPORTS.delete(context);