mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-11-14 16:52:09 +08:00
fix(python): a base imported from outside the project is not a project class of that name (#3109)
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
173953824d
commit
274b43c44b
+1
-3
@@ -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 <name>` 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)
|
||||
|
||||
@@ -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<string, string> = {
|
||||
'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']);
|
||||
});
|
||||
});
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user