mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-11-14 16:52:09 +08:00
fix(python): a base list wrapped over several lines is read whole
Both of the resolver's readers of a Python base list stopped early. pythonBases, behind module-global and self. dispatch, read only the class line. declaredSupertypes, behind class-name calls, read three lines and needed the closing parenthesis on them. black and ruff wrap a long list one base per line, so class Wrapped(Mixin, Base) written that way seemed to have no bases: current = Wrapped() then current.run(), and Wrapped.run(x), reached nothing a base declares. Extraction already read the whole list. A new pythonBaseList reads from the class line to the list's closing parenthesis, counting nested parentheses, within the class's extent, and leaves out comments, so a ) in one doesn't end the list. Both readers use it. declaredSupertypes still drops type arguments first, the composition the author posted on PR #3001, and keeps its metaclass= and factory rewrites. Validation: - The test file: 7 of 9 fail without this change (the two one-line controls pass), 9 pass here. Putting the type-argument drop out of the merged case 'python' fails the new composition case and the two of PR #3083. - Wrapped.run(None) on a wrapped base list reaches Base::run with the kernel and with wasm, as do a list over five lines, a decorated class, and a wrapped Base[Optional[User]]. - A/B on top of PR #2998, 7 Python repos, 459,440 edges: 118 added (102 calls, 16 references), 0 removed, 959 call sites moved, 113 kept their target with higher confidence. requests, flask, litestar, mypy and django do not change. polar: 942 XRepository.from_session(session) calls move from a sibling repository's from_session at 0.65 to RepositoryBase.from_session at 0.85. sqlalchemy: wrapped heads like class Table( and class ColumnElement( now reach what they inherit: t.select() reaches FromClause.select. Its 17 moved sites read right; 15 of them land on the typing-only SQLCoreOperations overload of the same ColumnOperators method. - Controls express and gin: full dumps byte-identical. - No kernel change. Index cost with PR #2998, sqlalchemy, median of 3 interleaved: 23.7 s on main, 25.2 s here. Original PR #3001 by @JosefAschauer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
committed by
Colby Mchenry
co-authored by
Claude Opus 5.5
parent
08b187aaf6
commit
27c904b011
@@ -43,6 +43,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
- 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)
|
||||
- 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 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,212 @@
|
||||
/**
|
||||
* A Python base list wrapped over several lines — as black and ruff write a
|
||||
* long one, one base per line — is read whole by the resolver's own reader of
|
||||
* a class's bases (`pythonBases`, behind module-global and `self.` dispatch).
|
||||
* It read only the `class` line, so a module global holding such a class,
|
||||
* `current = Wrapped()` then `current.run()`, reached no method its bases
|
||||
* declare. Extraction already read the whole list.
|
||||
*/
|
||||
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-multiline-bases-'));
|
||||
fs.mkdirSync(path.join(root, 'app'));
|
||||
fs.writeFileSync(path.join(root, 'app/__init__.py'), '');
|
||||
fs.writeFileSync(path.join(root, 'app/m.py'), `from typing import Generic, TypeVar
|
||||
|
||||
T = TypeVar("T")
|
||||
|
||||
|
||||
class Base(Generic[T]):
|
||||
def run(self):
|
||||
pass
|
||||
|
||||
|
||||
class Other:
|
||||
def run(self):
|
||||
pass
|
||||
|
||||
|
||||
class Mixin:
|
||||
pass
|
||||
|
||||
|
||||
class User:
|
||||
def save(self):
|
||||
pass
|
||||
|
||||
|
||||
class Account:
|
||||
def save(self):
|
||||
pass
|
||||
|
||||
|
||||
class OneLine(Mixin, Base):
|
||||
pass
|
||||
|
||||
|
||||
class Wrapped(
|
||||
Mixin,
|
||||
Base,
|
||||
):
|
||||
pass
|
||||
|
||||
|
||||
class Commented(
|
||||
Mixin, # no run here :)
|
||||
# the base that declares run:
|
||||
Base,
|
||||
):
|
||||
pass
|
||||
|
||||
|
||||
class WrappedGeneric(
|
||||
Mixin,
|
||||
Base[
|
||||
dict[str, int]
|
||||
],
|
||||
):
|
||||
pass
|
||||
|
||||
|
||||
current_a = OneLine()
|
||||
current_b = Wrapped()
|
||||
current_c = Commented()
|
||||
current_d = WrappedGeneric()
|
||||
|
||||
|
||||
def use_one_line():
|
||||
current_a.run()
|
||||
|
||||
|
||||
def use_wrapped():
|
||||
current_b.run()
|
||||
|
||||
|
||||
def use_commented():
|
||||
current_c.run()
|
||||
|
||||
|
||||
def use_wrapped_generic():
|
||||
current_d.run()
|
||||
|
||||
|
||||
def keep(cls):
|
||||
return cls
|
||||
|
||||
|
||||
@keep
|
||||
class DecoratedOneLine(Mixin, Base):
|
||||
pass
|
||||
|
||||
|
||||
@keep
|
||||
class DecoratedWrapped(
|
||||
Mixin,
|
||||
Base,
|
||||
):
|
||||
pass
|
||||
|
||||
|
||||
def class_one_line(x):
|
||||
OneLine.run(x)
|
||||
|
||||
|
||||
def class_wrapped(x):
|
||||
Wrapped.run(x)
|
||||
|
||||
|
||||
def class_commented(x):
|
||||
Commented.run(x)
|
||||
|
||||
|
||||
def class_decorated_one_line(x):
|
||||
DecoratedOneLine.run(x)
|
||||
|
||||
|
||||
def class_decorated_wrapped(x):
|
||||
DecoratedWrapped.run(x)
|
||||
|
||||
|
||||
class WrappedRepo(
|
||||
Mixin,
|
||||
Base[User],
|
||||
):
|
||||
pass
|
||||
|
||||
|
||||
def class_wrapped_base_method(x):
|
||||
WrappedRepo.run(x)
|
||||
|
||||
|
||||
def class_wrapped_type_argument(x):
|
||||
WrappedRepo.save(x)
|
||||
`);
|
||||
cg = await CodeGraph.init(root, { index: true });
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
cg?.close();
|
||||
if (root) fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
/** Qualified names of the methods `fn` calls. */
|
||||
const calls = (fn: string): string[] => {
|
||||
const node = cg.getNodesInFile('app/m.py').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);
|
||||
};
|
||||
|
||||
describe('a Python base list wrapped over several lines', () => {
|
||||
// Base and Other both declare `run`, and the globals are not named after
|
||||
// their classes, so only the class's own bases can pick one.
|
||||
it('resolves a module global through a one-line list (control)', () => {
|
||||
expect(calls('use_one_line')).toEqual(['Base::run']);
|
||||
});
|
||||
|
||||
it('resolves a module global through a wrapped list', () => {
|
||||
expect(calls('use_wrapped')).toEqual(['Base::run']);
|
||||
});
|
||||
|
||||
it('skips comments inside the list, parentheses and all', () => {
|
||||
expect(calls('use_commented')).toEqual(['Base::run']);
|
||||
});
|
||||
|
||||
it('reads a parameterized base wrapped inside the list', () => {
|
||||
expect(calls('use_wrapped_generic')).toEqual(['Base::run']);
|
||||
});
|
||||
});
|
||||
|
||||
// A call through the class name, `Wrapped.run(x)`, finds `run` through the
|
||||
// supertypes the class head declares (`declaredSupertypes`), a second reader.
|
||||
describe('a class-name call through a wrapped base list', () => {
|
||||
it('resolves through a one-line list (control)', () => {
|
||||
expect(calls('class_one_line')).toEqual(['Base::run']);
|
||||
});
|
||||
|
||||
it('resolves through a wrapped list', () => {
|
||||
expect(calls('class_wrapped')).toEqual(['Base::run']);
|
||||
});
|
||||
|
||||
it('skips comments inside the list, parentheses and all', () => {
|
||||
expect(calls('class_commented')).toEqual(['Base::run']);
|
||||
});
|
||||
|
||||
it('reads the head of a decorated class, one line or wrapped', () => {
|
||||
expect(calls('class_decorated_one_line')).toEqual(['Base::run']);
|
||||
expect(calls('class_decorated_wrapped')).toEqual(['Base::run']);
|
||||
});
|
||||
|
||||
it('keeps the base of a wrapped list and drops its type argument', () => {
|
||||
expect(calls('class_wrapped_base_method')).toEqual(['Base::run']);
|
||||
expect(calls('class_wrapped_type_argument')).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -410,10 +410,31 @@ function pythonRefClass(name: string, ref: UnresolvedRef, context: ResolutionCon
|
||||
return classes.length === 1 ? classes[0]! : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
* per line. Comments in it are left out. Undefined for a class without one.
|
||||
*/
|
||||
function pythonBaseList(cls: Node, context: ResolutionContext): string | undefined {
|
||||
const lines = context.getFileLines?.(cls.filePath) ?? context.readFile(cls.filePath)?.split(/\r?\n/) ?? [];
|
||||
const head = /^\s*class\s+\w+\s*\(/.exec(lines[cls.startLine - 1] ?? '');
|
||||
if (!head) return undefined;
|
||||
let list = '';
|
||||
let depth = 1;
|
||||
for (let i = cls.startLine - 1; i < Math.max(cls.endLine, cls.startLine); i++) {
|
||||
const text = (i === cls.startLine - 1 ? lines[i]!.slice(head[0].length) : lines[i] ?? '').replace(/#.*$/, '');
|
||||
for (const ch of text) {
|
||||
if (ch === '(') depth++;
|
||||
else if (ch === ')' && --depth === 0) return list;
|
||||
list += ch;
|
||||
}
|
||||
list += ' ';
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function pythonBases(cls: Node, ref: UnresolvedRef, context: ResolutionContext): Node[] {
|
||||
const line = context.getFileLines?.(cls.filePath)?.[cls.startLine - 1]
|
||||
?? context.readFile(cls.filePath)?.split('\n')[cls.startLine - 1] ?? '';
|
||||
let bases = line.match(/^\s*class\s+\w+\s*\(([^)]*)\)/)?.[1];
|
||||
let bases = pythonBaseList(cls, context);
|
||||
// A parameterized base, `Base[str]` or `Pair[str, Dict[str, int]]`, names its class before the brackets.
|
||||
while (bases && /\[[^[\]]*\]/.test(bases)) bases = bases.replace(/\[[^[\]]*\]/g, '');
|
||||
return (bases?.split(',') ?? []).flatMap(name => {
|
||||
@@ -21517,8 +21538,10 @@ function readDeclaredSupertypes(cls: Node, context: ResolutionContext): string[]
|
||||
// BaseManager.from_queryset(QuerySet))` — subclasses that class: the
|
||||
// qualifier is the supertype, not the method called on it.
|
||||
// A parameterized base, `Base[User]`, is `Base`: its type arguments are no supertypes.
|
||||
// The list is read whole, as `pythonBases` reads it: black and ruff wrap a
|
||||
// long one past the three lines `head` holds.
|
||||
case 'python': {
|
||||
let bases = /\bclass\s+\w+\s*\(([^)]*)\)/.exec(head)?.[1];
|
||||
let bases = pythonBaseList(cls, context);
|
||||
while (bases && /\[[^[\]]*\]/.test(bases)) bases = bases.replace(/\[[^[\]]*\]/g, '');
|
||||
return names(bases?.replace(/\w+\s*=\s*[\w.]+/g, '').replace(/([A-Za-z_][\w.]*)\.\w+\s*\(/g, '$1 ('));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user