diff --git a/CHANGELOG.md b/CHANGELOG.md index 85b559613..9f67efde6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,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 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. - 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-pep695-bases.test.ts b/__tests__/python-pep695-bases.test.ts new file mode 100644 index 000000000..cb1c15878 --- /dev/null +++ b/__tests__/python-pep695-bases.test.ts @@ -0,0 +1,171 @@ +/** + * A Python class with PEP 695 type parameters, `class Repo[T](Base[T]):`, + * names its bases after the parameter list. The resolver's base-list reader + * (`pythonBaseList`, behind module-global, `self.` and class-name dispatch) + * expected `(` right after the name, so such a class had no bases there. + */ +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-pep695-bases-')); + fs.mkdirSync(path.join(root, 'app')); + fs.writeFileSync(path.join(root, 'app/__init__.py'), ''); + fs.writeFileSync(path.join(root, 'app/m.py'), `class Base[T]: + def run(self): + pass + + +class Other: + def run(self): + pass + + +class Mixin: + pass + + +class Classic(Mixin, Base): + pass + + +class Repo[T](Base[T]): + pass + + +class Bounded[T: dict[str, int], *Ts, **P](Mixin, Base[T]): + pass + + +class Constrained[T: (int, str)](Mixin, Base[T]): + pass + + +class WrappedParams[ + K, + V: (int, str), +]( + Mixin, + Base[V], +): + pass + + +class NoBases[T]: + pass + + +current_a = Classic() +current_b = Repo() +current_c = Bounded() +current_d = Constrained() +current_e = WrappedParams() + + +def global_classic(): + current_a.run() + + +def global_repo(): + current_b.run() + + +def global_bounded(): + current_c.run() + + +def global_constrained(): + current_d.run() + + +def global_wrapped_params(): + current_e.run() + + +def class_classic(x): + Classic.run(x) + + +def class_repo(x): + Repo.run(x) + + +def class_bounded(x): + Bounded.run(x) + + +def class_constrained(x): + Constrained.run(x) + + +def class_wrapped_params(x): + WrappedParams.run(x) + + +def class_no_bases(x): + NoBases.run(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); +}; + +// 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. +describe('a Python class with PEP 695 type parameters', () => { + it('resolves through a classic base list (control)', () => { + expect(calls('global_classic')).toEqual(['Base::run']); + expect(calls('class_classic')).toEqual(['Base::run']); + }); + + it('reads the bases after a type parameter list', () => { + expect(calls('global_repo')).toEqual(['Base::run']); + expect(calls('class_repo')).toEqual(['Base::run']); + }); + + it('skips bounds with brackets, *Ts and **P in the parameter list', () => { + expect(calls('global_bounded')).toEqual(['Base::run']); + expect(calls('class_bounded')).toEqual(['Base::run']); + }); + + it('does not take a constraint tuple for the base list', () => { + expect(calls('global_constrained')).toEqual(['Base::run']); + expect(calls('class_constrained')).toEqual(['Base::run']); + }); + + it('reads a parameter list and a base list both wrapped over several lines', () => { + expect(calls('global_wrapped_params')).toEqual(['Base::run']); + expect(calls('class_wrapped_params')).toEqual(['Base::run']); + }); + + it('links each such class to its base with an extends edge', () => { + const base = cg.getNodesInFile('app/m.py').find((n) => n.name === 'Base' && n.kind === 'class')!; + for (const name of ['Repo', 'Bounded', 'Constrained', 'WrappedParams']) { + const cls = cg.getNodesInFile('app/m.py').find((n) => n.name === name && n.kind === 'class')!; + const targets = cg.getOutgoingEdges(cls.id).filter((e) => e.kind === 'extends').map((e) => e.target); + expect(targets, name).toContain(base.id); + } + }); + + it('gives a parameterized class without bases none', () => { + expect(calls('class_no_bases')).not.toContain('Base::run'); + expect(calls('class_no_bases')).not.toContain('Other::run'); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 8004d61e4..523f09e7a 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -414,21 +414,33 @@ function pythonRefClass(name: string, ref: UnresolvedRef, context: ResolutionCon * 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. + * A PEP 695 type parameter list between the name and the bases, `class + * Repo[T: (int, str)](Base[T]):`, is skipped whole, wrapped or not, so a + * constraint tuple in it is not taken for the base list. */ 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] ?? ''); + const head = /^\s*class\s+\w+/.exec(lines[cls.startLine - 1] ?? ''); if (!head) return undefined; let list = ''; - let depth = 1; + let depth = 0; + let params = 0; 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 (depth === 0) { + if (ch === '[') params++; + else if (ch === ']') params--; + else if (params === 0 && ch === '(') depth = 1; + // Anything else after the name and its parameters, such as `:`, means no base list. + else if (params === 0 && !/\s/.test(ch)) return undefined; + continue; + } if (ch === '(') depth++; else if (ch === ')' && --depth === 0) return list; list += ch; } - list += ' '; + if (depth > 0) list += ' '; } return undefined; }