From 06b27a756bf1ec3c738a41c63d6e9fbbca236fb5 Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Fri, 30 Oct 2026 16:52:59 -0500 Subject: [PATCH] fix(python): a class-name call doesn't follow a base imported from outside the project PR #3109 stopped a base imported from a package linking to a project class of its name, but only for the extends edge. The reader behind class-name calls follows the base list as written: declaredSupertypes gives the names and findInheritedClassMethod looks each up by name. So with class Color(Enum) after from enum import Enum, Color.name_of() still reached a project Enum.name_of, as did color.name_of() on a receiver named after the class. On sqlalchemy, Query(..., Generic[_T]) with Generic from typing walked to a test's local class Generic, then to that test's Base and on to FunctionElement, so query.select() linked to FunctionElement.select. The walk now skips a project class whose name the head binds outside the project, with the check PR #3109 added, moved to name-matcher.ts so both can call it (the import resolver already imports from there, not the reverse). A dotted base is bound through its first word: enum.Enum through import enum, e.Enum through import enum as e, collections.abc.Mapping through import collections.abc, which the check now counts as binding collections. Validation: - python-outside-bases.test.ts gains the calls: named import, receiver, dotted, aliased and deep bases reach nothing, and a project base and a class in a file the import path names still reach their methods. Without the gate 2 of the 4 new tests fail, and taking out the dotted word or the import a.b binding fails the dotted one. 11 pass here. - A/B on 11 Python repos plus express and gin, 496,746 edges: 10 calls removed, 0 added, 0 moved, 0 extends touched. All 10 are in sqlalchemy and all were wrong: 6 query.select(), query.alias() and query.compile() and 4 value.self_group(), each through typing's Generic into a test's own class Generic and its Base. - Controls express and gin unchanged. - Index cost on sqlalchemy, median of 3 interleaved: 27.8 s on main, 26.4 s here, which is noise. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/python-outside-bases.test.ts | 104 ++++++++++++++++++++++++- src/resolution/import-resolver.ts | 27 +------ src/resolution/name-matcher.ts | 37 +++++++++ 4 files changed, 142 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad0c603b5..2594f6cf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - 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 on a class whose base comes from outside the project, like `Color.name_of()` with `class Color(Enum)` after `from enum import Enum`, or with `class Color(enum.Enum)`, no longer links to a method of a project class that only shares the base's name. Re-index Python projects after upgrading. - In Python, a class written with PEP 695 type parameters, like `class Repo[T](Base[T]):` or `class Pair[K, V: (int, str)](Base[V]):`, now reaches the methods its bases declare, through a module global and through the class name, as a class without type parameters already did. Before, the type parameter list stood where the base list was expected, so such a class seemed to have no bases there, although its `extends` edge was already right. Re-index Python projects after upgrading. Thanks @JosefAschauer. - 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) diff --git a/__tests__/python-outside-bases.test.ts b/__tests__/python-outside-bases.test.ts index e5b6cad1f..457fc3977 100644 --- a/__tests__/python-outside-bases.test.ts +++ b/__tests__/python-outside-bases.test.ts @@ -13,7 +13,8 @@ import { CodeGraph } from '../src'; const FILES: Record = { 'app/__init__.py': '', 'app/schema.py': `class Enum: - pass + def name_of(self): + pass class Sequence: @@ -34,6 +35,16 @@ class BaseModel: class Thing: pass + + +class Mapping: + def name_of(self): + pass + + +class Other: + def name_of(self): + pass `, 'app/outside.py': `from enum import Enum from typing import Generic, Protocol, Sequence, TypeVar @@ -114,13 +125,70 @@ class Keep(Thing): pass `, 'lib/support/helpers/base.py': `class Base: - pass + def helper(self): + pass + + +class Unrelated: + def helper(self): + pass `, 'tests/test_base.py': `from helpers.base import Base class TestBase(Base): pass + + +def call_helper(): + TestBase.helper() +`, + 'app/calls.py': `import collections.abc +import enum +import enum as e +from enum import Enum + +from .inside import Mine + + +class Color(Enum): + pass + + +class Dotted(enum.Enum): + pass + + +class Aliased(e.Enum): + pass + + +class Deep(collections.abc.Mapping): + pass + + +def named_import(): + Color.name_of() + + +def on_a_receiver(color): + color.name_of() + + +def dotted(): + Dotted.name_of() + + +def aliased(): + Aliased.name_of() + + +def deep(): + Deep.name_of() + + +def from_the_project(): + Mine.name_of() `, }; @@ -141,6 +209,14 @@ afterAll(() => { if (root) fs.rmSync(root, { recursive: true, force: true }); }); +/** Qualified names of the methods `fn` in `file` calls. */ +const calls = (file: string, fn: string): string[] => { + const node = cg.getNodesInFile(file).find((n) => n.name === fn && n.kind === 'function')!; + expect(node, fn).toBeDefined(); + return cg.getOutgoingEdges(node.id).filter((e) => e.kind === 'calls') + .map((e) => cg.getNode(e.target)!).filter((n) => n.kind === 'method').map((n) => n.qualifiedName); +}; + /** `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')!; @@ -187,3 +263,27 @@ describe('a Python base imported from outside the project', () => { expect(bases('tests/test_base.py', 'TestBase')).toEqual(['lib/support/helpers/base.py:Base']); }); }); + +// The reader behind `Color.name_of()` follows the base list as written, not the +// `extends` edge. `Enum` and `Other` both declare `name_of`, so no lone +// candidate picks one. +describe('a call through a Python base imported from outside the project', () => { + it('reaches no method of a project class of that name', () => { + expect(calls('app/calls.py', 'named_import')).toEqual([]); + expect(calls('app/calls.py', 'on_a_receiver')).toEqual([]); + }); + + it('holds for a dotted base, through what its first word binds', () => { + expect(calls('app/calls.py', 'dotted')).toEqual([]); + expect(calls('app/calls.py', 'aliased')).toEqual([]); + expect(calls('app/calls.py', 'deep')).toEqual([]); + }); + + it('still reaches a method of a base the project declares', () => { + expect(calls('app/calls.py', 'from_the_project')).toEqual(['Enum::name_of']); + }); + + it('still reaches a method in a file the import path names', () => { + expect(calls('tests/test_base.py', 'call_helper')).toEqual(['Base::helper']); + }); +}); diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index a20e2949d..aca107fe7 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -33,6 +33,7 @@ import { pythonReceiverBinding, pythonModuleImportKeys, isPythonFromImportModule, + isPythonNameFromOutside, tsGlobalScope, rustPathTarget, isRustNameFromStd, @@ -4345,7 +4346,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 (ref.language === 'python') return isPythonNameFromOutside(name, ref.filePath, target, context); if (!ESM_IMPORT_LANGUAGES.has(ref.language)) return false; for (const imp of context.getImportMappings(ref.filePath, ref.language)) { @@ -4359,30 +4360,6 @@ 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/name-matcher.ts b/src/resolution/name-matcher.ts index 523f09e7a..120c20a4f 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -410,6 +410,32 @@ function pythonRefClass(name: string, ref: UnresolvedRef, context: ResolutionCon return classes.length === 1 ? classes[0]! : null; } +/** + * Whether every import binding a Python name in `filePath` 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 `filePath`, + * which may rebind the name (`class Enum(Enum)`). A relative import is the + * project's. `import a.b` binds `a`. + */ +export function isPythonNameFromOutside(name: string, filePath: string, target: Node | null | undefined, context: ResolutionContext): boolean { + if (target?.filePath === filePath) return false; + const bindings = context.getImportMappings(filePath, 'python') + .filter((imp) => imp.localName === name || (imp.isNamespace && imp.source.split('.')[0] === name)); + return bindings.length > 0 && bindings.every((imp) => + !imp.source.startsWith('.') && + context.resolveModuleFile?.(imp.source, filePath, 'python') === 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`); +} + /** * A Python class's base list as written, up to its closing parenthesis, which * may be lines below the `class` line: black and ruff wrap a long list one base @@ -445,6 +471,16 @@ function pythonBaseList(cls: Node, context: ResolutionContext): string | undefin return undefined; } +/** + * Whether the base `sup` of Python class `cls` comes from outside the project, + * so that `decl`, the project class of that name, isn't it. A dotted base is + * bound through its first word: `enum.Enum` through `import enum`. + */ +function isPythonBaseFromOutside(cls: Node, sup: string, decl: Node, context: ResolutionContext): boolean { + const written = new RegExp(`(? n.kind === 'method' && n.name === name && n.qualifiedName.slice(0, Math.max(0, n.qualifiedName.lastIndexOf('::'))).split(/::|\./).pop() === decl.name), kind)[0];