diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ebebeb31..5a0a69f6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- **Python calls on a local built through a module, or through one of its fields, now resolve.** `bp = pkg.Blueprint()`, `c.Blueprint()` and `core.Blueprint()` type the local like a class imported by name, and `app.config.from_mapping()` resolves on the class the field holds, as `self.config.from_mapping()` already did. On Flask and requests this adds 98 and 103 call edges (almost all from tests, none removed); a 30-edge sample was all correct. - **A derived TypeScript class no longer treats an unforwarded constructor parameter as a field.** In `class B extends A { constructor(req: Request) { super(); } }`, a call through `this.req` was resolved as if the parameter declared `req`, even when nothing passed it on. A plain constructor parameter now names an inherited field only when the constructor hands it to `super(...)` as an argument; `super(options.req)` does not count. - **Explore's Relationships list marks a call matched by name alone.** `codegraph_node`'s trail and explore's Flow already label a hop the resolver could bind only by the callee's name as `[name match, unverified]`; the flat Relationships list (shown for repositories of 500 files or more) now does too, so every place explore draws a call edge shows the same warning. - **A TypeScript property written as a getter now types the calls made through it.** `c.req.header()` reached nothing when `req` was `get req(): HonoRequest`, because only `req: T` fields were read. A plain constructor parameter named like the field (`constructor(req: Request)`) was also taken for the field's declaration and hid the real one; it now counts only with a modifier (`private req: T`), or in a class that extends another and may pass it to `super`. On hono this adds 74 call edges and removes none, and no added edge contradicts the compiler where it reached the caller; on zod the edges are unchanged. diff --git a/__tests__/python-module-qualified-receiver.test.ts b/__tests__/python-module-qualified-receiver.test.ts new file mode 100644 index 000000000..feddc32de --- /dev/null +++ b/__tests__/python-module-qualified-receiver.test.ts @@ -0,0 +1,142 @@ +/** + * A Python local typed by a constructor named through a module + * (`bp = pkg.Blueprint()`, `c.Blueprint()`, `core.Blueprint()`) is typed like + * one built from a class imported by name, and `local.field.method()` resolves + * on the class the field holds, as `self.field.method()` does. + */ +import { afterEach, describe, expect, it } from 'vitest'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { CodeGraph } from '../src'; + +let dir: string | undefined; +let graph: CodeGraph | undefined; +afterEach(() => { + graph?.close(); + graph = undefined; + if (dir) fs.rmSync(dir, { recursive: true, force: true }); + dir = undefined; +}); + +async function project(files: Record) { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-py-module-receiver-')); + for (const [name, source] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(dir, name)), { recursive: true }); + fs.writeFileSync(path.join(dir, name), source); + } + graph = await CodeGraph.init(dir, { index: true }); + graph.resolveReferences(); +} + +/** `Qualified::name` of every call target of the function `caller`. */ +function calls(caller: string): string[] { + const from = graph!.getNodesByKind('function').find(n => n.name === caller); + expect(from, caller).toBeDefined(); + return graph! + .getOutgoingEdges(from!.id) + .filter(e => e.kind === 'calls') + .map(e => graph!.getNode(e.target)!.qualifiedName) + .sort(); +} + +const core = `class Config(dict): + def from_mapping(self, m): + return True + + +class Base: + def add_x(self): + return 1 + + +class Blueprint(Base): + def own(self): + return 2 + + +class Flask: + def __init__(self): + self.config = Config() + + def test_client(self): + return 3 +`; +const files = (test: string, extra: Record = {}) => ({ + 'pkg/__init__.py': 'from .core import Blueprint as Blueprint\nfrom .core import Flask as Flask\n', + 'pkg/core.py': core, + 'tests/test_a.py': test, + ...extra, +}); + +describe('a local typed by a module-qualified constructor', () => { + it.each([ + ['a re-exported class', 'import pkg\n\n\ndef t():\n bp = pkg.Blueprint()\n bp.own()\n'], + ['a dotted module path', 'import pkg.core\n\n\ndef t():\n bp = pkg.core.Blueprint()\n bp.own()\n'], + ['an aliased module', 'import pkg.core as c\n\n\ndef t():\n bp = c.Blueprint()\n bp.own()\n'], + ['a module imported by name', 'from pkg import core\n\n\ndef t():\n bp = core.Blueprint()\n bp.own()\n'], + ])('reaches the class method through %s', async (_label, test) => { + await project(files(test)); + expect(calls('t')).toEqual(['Blueprint::own']); + }); + + it('reaches an inherited method', async () => { + await project(files('import pkg\n\n\ndef t():\n bp = pkg.Blueprint()\n bp.add_x()\n')); + expect(calls('t')).toEqual(['Base::add_x']); + }); + + it('does not type a local from a call on another local object', async () => { + await project( + files( + 'import pkg\n\n\ndef make(factory):\n bp = factory.Blueprint()\n bp.own()\n', + { 'other.py': 'class Blueprint:\n def own(self):\n return 9\n' }, + ), + ); + expect(calls('make')).toEqual([]); + }); +}); + +describe('a parameter that shadows the imported module', () => { + it('does not type a local from the shadowed module', async () => { + await project( + files('import pkg\n\n\ndef t(pkg):\n bp = pkg.Blueprint()\n bp.own()\n'), + ); + expect(calls('t')).toEqual([]); + }); + + it('does not follow a field of a local built from the shadowed module', async () => { + await project( + files('import pkg\n\n\ndef t(pkg):\n app = pkg.Flask()\n app.config.from_mapping({})\n'), + ); + expect(calls('t')).toEqual([]); + }); +}); + +describe('a call through a field of a typed local', () => { + it.each([ + ['a class imported by name', 'from pkg import Flask\n\n\ndef t():\n app = Flask()\n app.config.from_mapping({})\n'], + ['a module-qualified class', 'import pkg\n\n\ndef t():\n app = pkg.Flask()\n app.config.from_mapping({})\n'], + ])('resolves on the class the field holds for %s', async (_label, test) => { + await project(files(test)); + expect(calls('t')).toEqual(['Config::from_mapping']); + }); + + it('does not borrow a same-named method of an unrelated class', async () => { + await project( + files('from pkg import Flask\n\n\ndef t():\n app = Flask()\n app.config.save({})\n', { + 'other.py': 'class Unrelated:\n def save(self, m):\n return 1\n', + }), + ); + expect(calls('t')).toEqual([]); + }); + + it('does not resolve when the local has no known type', async () => { + await project(files('def t(app):\n app.config.from_mapping({})\n')); + expect(calls('t')).toEqual([]); + }); + + it('does not resolve a field the class never assigns', async () => { + await project(files('from pkg import Flask\n\n\ndef t():\n app = Flask()\n app.missing.from_mapping({})\n')); + expect(calls('t')).toEqual([]); + }); +}); diff --git a/codegraph-kernel/src/resolve/affix.rs b/codegraph-kernel/src/resolve/affix.rs index 0ded5a48c..8a23ab93b 100644 --- a/codegraph-kernel/src/resolve/affix.rs +++ b/codegraph-kernel/src/resolve/affix.rs @@ -246,7 +246,11 @@ pub(super) static RECEIVER_TYPE_PATTERNS: LazyLock..method()` in Python: the local's class from its + /// declaration (`app = Flask()`, `app = pkg.Flask()`, an annotation), then + /// the call settles on the field's type as `self..method()` does. + /// `None`: the local or field type is not known here, so the name + /// strategies decide as before. + pub(super) fn python_local_field_call(&mut self, receiver: &str, member: &str, r: &ResolveRefIn) -> Res>> { + let Some((root, field)) = receiver.split_once('.').filter(|(a, b)| is_word(a) && is_word(b) && !matches!(*a, "self" | "cls")) else { + return Ok(None); + }; + let Some(binding) = self.receiver_binding(root, r)?.filter(|b| b.kind != "import" && b.line <= r.line) else { + return Ok(None); + }; + let mut site = r.clone(); + site.line = binding.line; + if let Some(nid) = &binding.node_id { + site.from_node_id = nid.clone(); + } + let Some(ty) = self.infer_local_receiver_type(root, &site, true)? else { return Ok(None) }; + // `pkg.Flask()` names the import only while no parameter or local + // called `pkg` hides it. + if ty.contains('.') && self.is_shadowed_import(ty.split('.').next().unwrap_or(&ty), &site)? { + return Ok(None); + } + let Some(owner) = self.python_ref_class(&ty, r)? else { return Ok(None) }; + self.python_field_member_call(&owner, field, member, r) + } + + /// The call `..()`. `Some(None)`: the + /// field's type is known and its member is data or a property. + fn python_field_member_call(&mut self, owner: &Arc, field: &str, member: &str, r: &ResolveRefIn) -> Res>> { // The field's evidence in the class and every base along its MRO, // each read in its own file, must name one class: a subclass's // `self.cache = Other()` conflicts with the base's `Store()`, and @@ -108,7 +141,7 @@ impl KernelResolver { // (`cache: Base`) may be narrowed to a subclass of it. let mut classes: Vec> = vec![owner.clone()]; if self.supertypes_complete { - if let Some(mro) = self.python_mro(&owner, 0)? { + if let Some(mro) = self.python_mro(owner, 0)? { classes.extend(mro.iter().skip(1).cloned()); } } diff --git a/codegraph-kernel/src/resolve/method_call.rs b/codegraph-kernel/src/resolve/method_call.rs index 01c927c8e..10d237465 100644 --- a/codegraph-kernel/src/resolve/method_call.rs +++ b/codegraph-kernel/src/resolve/method_call.rs @@ -671,6 +671,14 @@ impl KernelResolver { } } + // Python `..method()`: the local's class settles the + // field's type, as it does for `self.`. + if r.language == "python" && dotted { + if let Some(verdict) = self.python_local_field_call(&object_or_class, &method_name, r)? { + return Ok(verdict); + } + } + if inferable { if r.language == "go" { if let Some(verdict) = self.go_asserted_member(&object_or_class, &method_name, r)? { return Ok(verdict); }