From 6b8a794079baba24efd2793b1d45fd24dfe64feb Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Fri, 30 Oct 2026 15:46:09 -0500 Subject: [PATCH] fix(python): a base imported from outside the project is not a project class of that name A Python class whose base the file imports from a package, like class Color(Enum) after from enum import Enum, linked to any project class named Enum: the import found no project file, and the name match or a framework resolver then took the first class of that name. sqlalchemy's enums all extended its own sql/sqltypes.py Enum, and litestar's pydantic models extended a BaseModel in a docs example. PR #2998 extended this to parameterized bases, like Sequence[_T] from typing. isBoundToOutOfRepoImport is the gate that drops such an edge for JS, TS and Rust. It now answers for Python too: the name is outside when every import binding it is absolute and resolveImportPath, which searches for a module the way Python does, finds none. A class in a file the import path names stays, because that search can miss a module on a path the program sets up itself, and so does a class of the referring file, which may rebind the name. A name no named import binds, like one from a star import, is left as it was. Validation: - New python-outside-bases.test.ts: the 7 outside bases fail on main, plain and parameterized, and every case that must keep its edge passes both ways. Green here (7 passed). Taking out each of the 4 guards fails exactly its own test. - A/B on 7 Python repos, 469,927 edges with the controls: 67 extends removed, 0 added, 0 moved. All 67 read against their imports and all were wrong: sqlalchemy 31 (28 from enum, 3 from typing), litestar 21 (pydantic BaseModel), polar 14 (11 fastapi APIRoute, 2 enum IntEnum, 1 opentelemetry Meter), django 1 (ctypes Union). - Controls express and gin unchanged. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 +- __tests__/python-outside-bases.test.ts | 189 +++++++++++++++++++++++++ src/resolution/import-resolver.ts | 40 +++++- src/resolution/index.ts | 2 +- 4 files changed, 226 insertions(+), 9 deletions(-) create mode 100644 __tests__/python-outside-bases.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 997e2deed..a264d1759 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,9 +25,6 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `codegraph serve --mcp --no-telemetry` turns telemetry off for one MCP server entry, so a shared MCP config can opt out without an environment variable; a background daemon that's already running for the project keeps its own setting until it exits. Thanks @danusha2345. (#1908) - `codegraph implementations ` lists the types that implement or extend a trait, interface or class, with each one's file and line, how it's tied to the type, and whether it's test code, a Rust mock in a `#[cfg(test)]` module included; `--depth` follows subclasses of subclasses. Thanks @danusha2345. (#2833) - Set `CODEGRAPH_SYNTH_MIN_INTERVAL_MS` to limit how often live syncs rebuild the callback and event links across the whole project, which on a large TypeScript or Vue repo otherwise happens on almost every edit; a rebuild that's put off runs when the interval is up even if no edit follows, and until then those links can be that far behind your code. A full `codegraph index` is never held. Thanks @danusha2345. (#2911) - - - - New `codegraph cycles` command lists the groups of files that import each other in a circle, as text or `--json`, and exits 1 when it finds any, so it works as a pre-commit or CI check (#888). Thanks @jiangyq9 and @goutham80808. - `codegraph init --local-only` keeps `.codegraph/` out of git entirely, so nothing of CodeGraph's shows in `git status`, and it works on a project that's already set up, leaving a `.codegraph/.gitignore` you've edited or committed alone (#910). Thanks @jadchene, @jiangyq9, @Wood-Q and @walterf3. @@ -47,6 +44,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Python, a call on a class or on a receiver named after it, like `Repo.save()` or `repo.save()` with `class Repo(Base[User])`, no longer links to a method of the base's type argument, like `User.save`. Before, every name in the base list counted as a supertype, so `User` was searched as if `Repo` inherited from it. The base itself, `Base`, is still searched. Re-index Python projects after upgrading. Thanks @JosefAschauer. - In Python, a class with a parameterized base, like `class Leaf(Base[str])`, `class Repo(models.Base[int])` or `class Both(Mixin, Base[str])`, is now linked to that base, as `class Leaf(Base)` already was, and a call through a module global holding such a class, like `current = Leaf()` then `current.run()`, reaches the method the base declares. Before, such a base was skipped, so the class had no `extends` edge and anything that follows the class hierarchy missed it. Re-index Python projects after upgrading. Thanks @JosefAschauer. - In Python, a class whose base list is wrapped over several lines, as black and ruff write a long one, now reaches the methods its bases declare, through a module global like `current = Wrapped()` then `current.run()` and through the class name like `Wrapped.run(x)`, as a one-line base list already did. Before, only the first line or three were read for this, so such a class seemed to have no bases. Re-index Python projects after upgrading. Thanks @JosefAschauer. +- In Python, a base class your file imports from outside the project, like `class Color(Enum)` after `from enum import Enum` or `class Item(BaseModel)` after `from pydantic import BaseModel`, no longer links to a project class that only shares its name. Re-index Python projects after upgrading. - 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, Scala and Java, a standard-library method called on a value whose type CodeGraph can't see, like `options.setdefault(…)` on a dictionary, `m.getOrElse(…)` on an `Option` or `data.put(…)` on a map held in a `var`, no longer links to an unrelated project method that happens to share its name; the call still links when the variable is named after your class, like `registry.get(…)` for a `Registry`. Re-index Python, Scala and Java projects after upgrading. Thanks @danusha2345. (#1947) - 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-outside-bases.test.ts b/__tests__/python-outside-bases.test.ts new file mode 100644 index 000000000..e5b6cad1f --- /dev/null +++ b/__tests__/python-outside-bases.test.ts @@ -0,0 +1,189 @@ +/** + * A Python base the file imports from outside the project is that package's + * class, plain or parameterized: `class Color(Enum)` after `from enum import + * Enum` extends no project class that happens to be named `Enum`. Names the + * file binds to the project, or doesn't bind at all, keep linking. + */ +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'; + +const FILES: Record = { + 'app/__init__.py': '', + 'app/schema.py': `class Enum: + pass + + +class Sequence: + pass + + +class Generic: + pass + + +class Protocol: + pass + + +class BaseModel: + pass + + +class Thing: + pass +`, + 'app/outside.py': `from enum import Enum +from typing import Generic, Protocol, Sequence, TypeVar + +from pydantic import BaseModel + +T = TypeVar("T") + + +class Color(Enum): + pass + + +class Plain(Sequence): + pass + + +class Items(Sequence[int]): + pass + + +class Box(Generic[T]): + pass + + +class Greeter(Protocol[T]): + pass + + +class Item(BaseModel): + pass + + +class Row(BaseModel, Enum): + pass +`, + 'app/inside.py': `from app.schema import Sequence + +from .schema import Enum + + +class Mine(Enum): + pass + + +class Seq(Sequence[int]): + pass +`, + 'app/shadow.py': `from enum import Enum + + +class Enum(Enum): + pass + + +class Own(Enum): + pass +`, + 'app/fallback.py': `try: + from pydantic import BaseModel +except ImportError: + from .schema import BaseModel + + +class Either(BaseModel): + pass +`, + 'app/starred.py': `from .schema import * + + +class Starred(Enum): + pass +`, + 'app/relative.py': `from .moved import Thing + + +class Keep(Thing): + pass +`, + 'lib/support/helpers/base.py': `class Base: + pass +`, + 'tests/test_base.py': `from helpers.base import Base + + +class TestBase(Base): + pass +`, +}; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-py-outside-bases-')); + 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 }); +}); + +/** `file:class` of what the class `name` in `file` extends. */ +const bases = (file: string, name: string): string[] => { + const node = cg.getNodesInFile(file).find((n) => n.name === name && n.kind === 'class')!; + expect(node, name).toBeDefined(); + return cg.getOutgoingEdges(node.id).filter((e) => e.kind === 'extends').map((e) => { + const target = cg.getNode(e.target)!; + return `${target.filePath}:${target.name}`; + }); +}; + +describe('a Python base imported from outside the project', () => { + it('links no project class of that name, plain or parameterized', () => { + expect(bases('app/outside.py', 'Color')).toEqual([]); + expect(bases('app/outside.py', 'Plain')).toEqual([]); + expect(bases('app/outside.py', 'Items')).toEqual([]); + expect(bases('app/outside.py', 'Box')).toEqual([]); + expect(bases('app/outside.py', 'Greeter')).toEqual([]); + expect(bases('app/outside.py', 'Item')).toEqual([]); + expect(bases('app/outside.py', 'Row')).toEqual([]); + }); + + it('keeps a base the file imports from the project', () => { + expect(bases('app/inside.py', 'Mine')).toEqual(['app/schema.py:Enum']); + expect(bases('app/inside.py', 'Seq')).toEqual(['app/schema.py:Sequence']); + }); + + it('keeps a class the file declares itself under the imported name', () => { + expect(bases('app/shadow.py', 'Own')).toEqual(['app/shadow.py:Enum']); + }); + + it('keeps a base one of two imports of the name finds in the project', () => { + expect(bases('app/fallback.py', 'Either')).toEqual(['app/schema.py:BaseModel']); + }); + + it('keeps a base no named import binds', () => { + expect(bases('app/starred.py', 'Starred')).toEqual(['app/schema.py:Enum']); + }); + + it('keeps a base a relative import names, though that module is gone', () => { + expect(bases('app/relative.py', 'Keep')).toEqual(['app/schema.py:Thing']); + }); + + it('keeps a class in a file the import path names, though no search root finds it', () => { + expect(bases('tests/test_base.py', 'TestBase')).toEqual(['lib/support/helpers/base.py:Base']); + }); +}); diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index d1492238b..a20e2949d 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -4312,7 +4312,7 @@ function resolveStaticMember( * instead of removing it, so locality has to be checked too. * * Answers only when it can be CERTAIN, because a false "yes" deletes a real - * edge. Two languages qualify, each with an oracle that cannot be wrong: + * edge. Three languages qualify, each with an oracle of its own: * * - **Rust** — the `use` path is rooted at a standard-library crate * (`std`/`core`/`alloc`/`proc_macro`), which by definition ships outside @@ -4325,14 +4325,19 @@ function resolveStaticMember( * path aliases and monorepo workspace packages, unless the specifier still * names a project file through the importing app's `baseUrl` or catch-all * `paths` (`import Base from 'components/Base'`). + * - **Python** — `resolveImportPath`, which searches for a module the way + * Python does (see {@link isPythonNameFromOutside}). * - * Everything else returns false and resolves exactly as before. JVM and Python - * imports notably do NOT go through `resolveImportPath` (they have dedicated - * FQN/module matchers), so there is no trustworthy oracle to consult here. + * Everything else returns false and resolves exactly as before. JVM imports + * notably do NOT go through `resolveImportPath` (they have dedicated FQN + * matchers), so there is no trustworthy oracle to consult here. + * + * `target` is what a strategy found, when the caller has it. */ export function isBoundToOutOfRepoImport( ref: UnresolvedRef, - context: ResolutionContext + context: ResolutionContext, + target?: Node | null ): boolean { const name = ref.referenceName; if (name.includes('::') || name.includes('.')) return false; // qualified refs resolve by path @@ -4340,6 +4345,7 @@ export function isBoundToOutOfRepoImport( // (A 2015-edition crate-relative path can shadow a stdlib root with a local // module of the same name: a `use core::…` with a `mod core;` is the crate's.) if (ref.language === 'rust') return isRustNameFromStd(ref, context); + if (ref.language === 'python') return isPythonNameFromOutside(ref, target, context); if (!ESM_IMPORT_LANGUAGES.has(ref.language)) return false; for (const imp of context.getImportMappings(ref.filePath, ref.language)) { @@ -4353,6 +4359,30 @@ export function isBoundToOutOfRepoImport( return false; } +/** + * Whether every import binding a Python name is of a module Python's search + * doesn't find in the project: `class Color(Enum)` after `from enum import + * Enum` is not the project's own `Enum`. That search can miss a project module + * only on a path the program sets up itself (`sys.path`), so a `target` in a + * file the import path names stays, as does one in the referring file, which + * may rebind the name (`class Enum(Enum)`). A relative import is the project's. + */ +function isPythonNameFromOutside(ref: UnresolvedRef, target: Node | null | undefined, context: ResolutionContext): boolean { + if (target?.filePath === ref.filePath) return false; + const bindings = context.getImportMappings(ref.filePath, 'python').filter((imp) => imp.localName === ref.referenceName); + return bindings.length > 0 && bindings.every((imp) => + !imp.source.startsWith('.') && + resolveImportPath(imp.source, ref.filePath, 'python', context) === null && + !(target && isInPythonModulePath(target.filePath, imp.source))); +} + +/** Whether `file` is the module `dotted` or inside it, under any root: `lib/x/helpers/base.py` for `helpers.base`. */ +function isInPythonModulePath(file: string, dotted: string): boolean { + const tail = `/${dotted.replace(/\./g, '/')}`; + const at = `/${file}`; + return at.includes(`${tail}/`) || at.endsWith(`${tail}.py`) || at.endsWith(`${tail}.pyi`); +} + /** * The tail a reference that failed to resolve is parked under when it goes * through an import binding the module's key finds and its own tail does not: diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 4deedf88b..01da3d483 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -4168,7 +4168,7 @@ export class ReferenceResolver { if (!type) return null; result = { ...result, targetNodeId: type.id }; } - if (isBoundToOutOfRepoImport(ref, this.context)) return null; + if (isBoundToOutOfRepoImport(ref, this.context, target)) return null; // A Go type embedded through a package (`clientv3.KV`) is that package's. // When the package is none of the file's imports as indexed (clientv3 // imported unaliased as `go.etcd.io/etcd/client/v3`), a type found by its