diff --git a/src/graph/extract.ts b/src/graph/extract.ts index d0f9dff19..bd1283421 100644 --- a/src/graph/extract.ts +++ b/src/graph/extract.ts @@ -309,6 +309,7 @@ const FUNCTION_VALUE_TYPES = new Set([ ]); const EMPTY_SET: ReadonlySet = new Set(); +const EMPTY_MODULE_MAP: ReadonlyMap = new Map(); const parser = new Parser(); const GRAMMARS: Record = { @@ -335,6 +336,12 @@ export interface WalkCtx { enclosingClass: string | null; // nearest enclosing class (py/ts `self`/`this`) goReceiverVar: string | null; // Go receiver var, e.g. `w` in `func (w *Worker)` importedSymbols: ReadonlyMap; + /** Python only: local name → the dotted module path it may bind, from + * `import a.b as x` / `from a.b import c [as x]`. A call through such a name + * (`x.f()`) is a module-member call: the module names the target file and the + * attribute names the symbol, so resolve.ts can resolve it exactly instead of + * dropping it for want of a receiver type. Empty for every other language. */ + importedModules: ReadonlyMap; // R6 (Phase 2): which list we're inside while walking an `R6Class(...)` call's // arguments — set only for the direct span of a `public =`/`private =`/ // `active =` `list(...)`'s own entries (see walk()'s special-cased `argument` @@ -389,6 +396,7 @@ export function extractFile(rel: string, source: string, lang: Language): Extrac const root = parseSource(source); const bindings = collectBindings(root, lang); const importedSymbols = collectImportedSymbols(root, lang); + const importedModules = lang === "python" ? collectPythonImportedModules(root) : EMPTY_MODULE_MAP; const rGenerics = lang === "r" ? collectRGenerics(root) : EMPTY_SET; const nodes: NodeV1[] = [ @@ -422,6 +430,7 @@ export function extractFile(rel: string, source: string, lang: Language): Extrac enclosingClass: null, goReceiverVar: null, importedSymbols, + importedModules, rR6Access: null, rGenerics, rSuperClass: null, @@ -642,6 +651,10 @@ function walk(node: Parser.SyntaxNode, ctx: WalkCtx, out: NodeV1[], edges: RawEd desc.kind === "function" || desc.kind === "method" ? withoutShadowedImports(ctx.importedSymbols, node) : ctx.importedSymbols, + importedModules: + ctx.lang === "python" && (desc.kind === "function" || desc.kind === "method") + ? withoutPythonShadowedModules(ctx.importedModules, node) + : ctx.importedModules, // Reset on every new definition — this is a purely local marker for "we're // still inside THIS class-defining call's own public=/private=/active= // argument chain," not something that should leak into a nested definition @@ -765,8 +778,18 @@ function walk(node: Parser.SyntaxNode, ctx: WalkCtx, out: NodeV1[], edges: RawEd implicitSelf: true, }); } else { - const recvType = resolveRecvType(callee.receiver, ctx); - edges.push(recvType ? { ...callEdge, recvType } : callEdge); + // Python `mod.fn()` where `mod` is an imported module alias: the import + // states the file and the attribute states the symbol, which is the same + // two-halves evidence a named import gives a `references` edge. Carry the + // module as the specifier so resolve.ts can look the name up in that file + // alone — no bare-name guessing, and no receiver type to infer. + const moduleSpec = + ctx.lang === "python" && callee.viaMember && callee.receiver + ? ctx.importedModules.get(callee.receiver) + : undefined; + const recvType = moduleSpec ? undefined : resolveRecvType(callee.receiver, ctx); + if (moduleSpec) edges.push({ ...callEdge, specifier: moduleSpec }); + else edges.push(recvType ? { ...callEdge, recvType } : callEdge); } } } else if (ctx.lang === "php" && node.type === "use_declaration") { @@ -848,6 +871,97 @@ function walk(node: Parser.SyntaxNode, ctx: WalkCtx, out: NodeV1[], edges: RawEd for (const child of node.namedChildren) walk(child, ctx, out, edges, minted); } +/** + * Python import bindings, as MODULE paths: local name → the dotted path that + * name may denote. `import a.b as x` and `from a.b import c as x` both bind one + * name to one dotted path (`a.b`, `a.b.c`); a relative import keeps its leading + * dots (`from . import c` → `.c`) for resolve.ts to anchor against the importing + * file's directory. + * + * `from a.b import c` is ambiguous in Python's own grammar — `c` is a submodule + * or an exported symbol, and only the file tree can say which. That is resolve.ts's + * job: the path is offered as a candidate here, and a call through it resolves only + * if the module file actually exists. A plain `import a.b` (no alias) binds the + * TOP package name, so the call site reads `a.b.f()` — a two-hop receiver Python's + * callee reader does not report — and is therefore left out rather than guessed at. + */ +function collectPythonImportedModules(root: Parser.SyntaxNode): Map { + const out = new Map(); + const bind = (aliasNode: Parser.SyntaxNode, path: string): void => { + const names = aliasNode.namedChildren; + if (aliasNode.type === "aliased_import") { + const target = aliasNode.childForFieldName("name") ?? names[0]; + const alias = aliasNode.childForFieldName("alias") ?? names[1]; + if (target && alias) out.set(alias.text, joinModulePath(path, target.text)); + return; + } + // `from a.b import c` binds `c`; a plain `import a.b` binds the top package, + // which the header explains is deliberately left alone. + if (aliasNode.type === "dotted_name" && path) { + out.set(aliasNode.text, joinModulePath(path, aliasNode.text)); + } + }; + const visit = (node: Parser.SyntaxNode): void => { + if (node.type === "import_statement") { + for (const child of node.namedChildren) if (child.type === "aliased_import") bind(child, ""); + return; + } + if (node.type === "import_from_statement") { + const moduleNode = + node.childForFieldName("module_name") ?? + node.namedChildren.find((c) => c.type === "dotted_name" || c.type === "relative_import"); + if (!moduleNode) return; + const modulePath = moduleNode.text; + for (const child of node.namedChildren) { + if (child === moduleNode) continue; + if (child.type === "aliased_import" || child.type === "dotted_name") bind(child, modulePath); + } + return; + } + for (const child of node.namedChildren) visit(child); + }; + visit(root); + return out; +} + +/** `a.b` + `c` → `a.b.c`; a relative prefix keeps its dots (`.` + `c` → `.c`). */ +function joinModulePath(prefix: string, tail: string): string { + if (!prefix) return tail; + return prefix.endsWith(".") ? `${prefix}${tail}` : `${prefix}.${tail}`; +} + +/** + * Python module aliases minus the names this definition rebinds — a parameter or + * an assignment named like an imported module shadows it for the whole body, and + * a call on the local value is not a call into that module. + */ +function withoutPythonShadowedModules( + modules: ReadonlyMap, + definition: Parser.SyntaxNode, +): ReadonlyMap { + if (modules.size === 0) return modules; + const shadowed = new Set(); + const params = definition.childForFieldName("parameters"); + const collectParam = (node: Parser.SyntaxNode): void => { + if (node.type === "identifier") shadowed.add(node.text); + else for (const child of node.namedChildren) collectParam(child); + }; + if (params) for (const p of params.namedChildren) collectParam(p); + const body = definition.childForFieldName("body"); + const visitBody = (node: Parser.SyntaxNode): void => { + if (node.type === "assignment") { + const left = node.childForFieldName("left"); + if (left?.type === "identifier") shadowed.add(left.text); + } + for (const child of node.namedChildren) visitBody(child); + }; + if (body) visitBody(body); + if (![...shadowed].some((name) => modules.has(name))) return modules; + const out = new Map(modules); + for (const name of shadowed) out.delete(name); + return out; +} + /** * Named imports whose local binding can be recognized later as a symbol use. * Namespace/default imports are intentionally excluded: they do not tell us @@ -2496,7 +2610,12 @@ function importSpecifier(node: Parser.SyntaxNode, lang: Language): string | null if (lang === "python") { const m = node.childForFieldName("module_name") ?? - node.namedChildren.find((c) => c.type === "dotted_name" || c.type === "relative_import"); + node.namedChildren.find((c) => c.type === "dotted_name" || c.type === "relative_import") ?? + // `import a.b as x` wraps the path in an `aliased_import`, so a child-level + // search finds nothing and the import went unrecorded entirely. + node.namedChildren + .find((c) => c.type === "aliased_import") + ?.childForFieldName("name"); return m?.text ?? null; } if (lang === "go") { diff --git a/src/graph/resolve.ts b/src/graph/resolve.ts index c78a61e6c..86f341676 100644 --- a/src/graph/resolve.ts +++ b/src/graph/resolve.ts @@ -128,6 +128,11 @@ export function resolveEdges( // node ids. A `use App\Models\User` names a PSR-4 class whose file mirrors the namespace // tail under some (unknown) source root, so the suffix is the portable key. const phpFilesBySuffix = new Map(); + // Python module resolution: a file's importable path-suffix (`app/db/session.py` + // → `app/db/session`, `app/db/__init__.py` → `app/db`) → its file node ids. An + // absolute import names a module by dotted path relative to a sys.path root the + // repo never states, so — as for Java and PHP — the suffix is the portable key. + const pythonFilesByModule = new Map(); const hasGoModules = !!opts.goModules?.length; for (const n of nodes) { if (n.kind === "file") { @@ -150,6 +155,13 @@ export function resolveEdges( const parts = toPosixPath(n.path).split("/"); for (let i = 0; i < parts.length; i++) push(phpFilesBySuffix, parts.slice(i).join("/"), n.id); } + if (n.path.endsWith(".py")) { + // `pkg/__init__.py` IS the module `pkg`, so the marker is dropped before + // indexing — an import names the package, never the init file. + const module = toPosixPath(n.path).replace(/\.py$/, "").replace(/\/__init__$/, ""); + const parts = module.split("/"); + for (let i = 0; i < parts.length; i++) push(pythonFilesByModule, parts.slice(i).join("/"), n.id); + } { const p = toPosixPath(n.path); if (p === "lib.rs" || p === "main.rs") rustCrateRoots.push(""); @@ -216,7 +228,9 @@ export function resolveEdges( ? resolveRustUse(e.specifier, e.file, byId, rustCrateRoots) : e.file.endsWith(".php") ? resolvePhpUse(e.specifier, phpFilesBySuffix) - : resolveImport(e.specifier, e.file, byId); + : e.file.endsWith(".py") + ? resolvePythonImport(e.specifier, e.file, byId, pythonFilesByModule) + : resolveImport(e.specifier, e.file, byId); add(e.source, target, "imports", "extracted"); } else if (e.relation === "extends" || e.relation === "implements") { // `implements` also resolves to a `trait` — PHP models trait composition @@ -268,6 +282,21 @@ export function resolveEdges( if (hit && hit.id !== e.source) add(e.source, hit.id, "references", hit.confidence); } } else if (e.relation === "calls") { + // A Python call through an imported module alias (`credits.reserve()`) + // carries the module as its specifier. That is the same two-halves evidence + // a named import gives — the file to look in, and the name to look for — so + // it resolves inside that file alone. `from a.b import c` is ambiguous in + // Python's grammar (submodule or symbol?); an unresolved module means it + // was the symbol reading, and the call falls through to the paths below + // rather than guessing. + if (e.specifier && e.name && e.file.endsWith(".py")) { + const moduleFile = resolvePythonImport(e.specifier, e.file, byId, pythonFilesByModule); + const candidates = byId.has(moduleFile) ? (perFileName.get(moduleFile)?.get(e.name) ?? []) : []; + if (candidates.length === 1) { + add(e.source, candidates[0].id, "calls", "extracted"); + continue; + } + } if (e.viaMember) { if (!e.recvType) continue; const hit = resolveTypedMember(e.recvType, e.name!, e.file, ownerMethod, classParents, classTraits, e.argCount); @@ -487,6 +516,41 @@ function resolveTraitMember( return "ambiguous"; } +/** + * Resolve a Python import's dotted module path to an in-repo file node id; + * otherwise return the raw specifier (stdlib or third-party package). + * + * Two shapes, one answer. A RELATIVE import (`.service`, `..core.config`) states + * its anchor: one leading dot is the importing file's own package, each further + * dot climbs one level, so the path is computed, not searched. An ABSOLUTE import + * (`app.db.session`) names a module relative to a sys.path root the repo never + * writes down — it may be the repo root, a `src/` dir, or a package dir — so the + * dotted tail is matched against each file's importable path SUFFIX, exactly the + * root-agnostic key Java and PHP imports use here. + * + * A suffix shared by two files (`a/util.py` and `b/util.py` for `import util`) is + * left unresolved: picking one would invent an edge the source does not state. + */ +function resolvePythonImport( + spec: string, + file: string, + byId: Map, + filesByModule: Map, +): string { + if (spec.startsWith(".")) { + const dots = /^\.+/.exec(spec)![0].length; + const tail = spec.slice(dots).replace(/\./g, "/"); + // One dot = the importing file's own package, so the first dot costs no climb. + const base = posix.dirname(toPosixPath(file)); + const anchor = posix.normalize(posix.join(base, ...Array(dots - 1).fill(".."))); + const target = tail ? posix.join(anchor, tail) : anchor; + for (const c of [`${target}.py`, `${target}/__init__.py`]) if (byId.has(c)) return c; + return spec; + } + const hits = filesByModule.get(spec.replace(/\./g, "/")); + return hits?.length === 1 ? hits[0] : spec; +} + /** * Resolve a module specifier to a file node id when it points inside the repo; * otherwise return the raw specifier (external package or unresolved path). diff --git a/test/graph-python-module-calls.test.ts b/test/graph-python-module-calls.test.ts new file mode 100644 index 000000000..3185b79ce --- /dev/null +++ b/test/graph-python-module-calls.test.ts @@ -0,0 +1,221 @@ +/** + * Tests for Python calls made through an imported MODULE alias — the shape + * `from app.services import billing` / `billing.charge()`, which a layered + * codebase uses for most of its cross-module traffic (importing the module, not + * each function, is what keeps `billing.charge` readable at the call site). + * + * Before this, such a call reached the resolver as a member call with no + * inferable receiver type and was dropped — correctly, by the drop-rather-than- + * guess rule in resolve.ts, since a bare `charge` says nothing about which file + * it lives in. The missing half was the import binding: Python had no + * module-alias table at all, so the module the alias names was never carried to + * the call site. With it, the call states both halves — the file (the import) + * and the symbol (the attribute) — which is the same evidence a named import + * already gives a `references` edge, and resolves exactly. + * + * The negative tests matter more than the positive ones: a module-qualified call + * must not become a licence to guess. A rebound local name, an unresolvable + * module and a name absent from the module it was looked up in must each leave + * the edge unresolved rather than reach for a same-named symbol elsewhere. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, writeFileSync, mkdirSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { buildGraph } from "../src/graph/build.js"; +import { readGraph, wiringPath } from "../src/graph/write.js"; +import type { GraphV1 } from "../src/graph/types.js"; + +const BILLING = `def charge(user, amount): + return amount + + +def refund(user, amount): + return -amount +`; + +/** A second `charge`, in a module nobody imports here: a bare-name fallback + * would have two candidates, so any edge to THIS one is a guess. */ +const LEGACY = `def charge(user, amount): + return 0 +`; + +const CONFIG = `TIMEOUT = 30 + + +def load(): + return TIMEOUT +`; + +/** Every supported binding shape, plus the two that must NOT resolve. */ +const API = `from app.services import billing +from app.services import billing as money +from app import config as settings +import app.services.billing as direct +from .local import helper + + +def pay(user): + return billing.charge(user, 10) + + +def pay_aliased(user): + return money.refund(user, 10) + + +def read_timeout(): + return settings.load() + + +def pay_direct(user): + return direct.charge(user, 1) + + +def use_relative(): + return helper.run() + + +def shadowed(billing): + # The parameter rebinds the name: this is a call on the argument, not into + # the module, and must not produce an edge into app/services/billing.py. + return billing.charge(1, 2) + + +def rebound(): + billing = _Stub() + return billing.charge(3, 4) + + +def absent(user): + # The module resolves, but it has no "void" — no edge, and no reaching + # for a same-named symbol in another file. + return billing.void(user) + + +class _Stub: + def charge(self, a, b): + return 0 +`; + +const LOCAL_HELPER = `def run(): + return 1 +`; + +/** An import of a module that is not in the repo — the specifier must stay + * unresolved and the call must not fall back to the in-repo `charge`. */ +const EXTERNAL = `import thirdparty.billing as vendor + + +def pay(user): + return vendor.charge(user, 5) +`; + +function makeFixture(): string { + const dir = mkdtempSync(join(tmpdir(), "graft-py-modcalls-")); + mkdirSync(join(dir, "app", "services"), { recursive: true }); + mkdirSync(join(dir, "app", "local"), { recursive: true }); + writeFileSync(join(dir, "app", "__init__.py"), ""); + writeFileSync(join(dir, "app", "config.py"), CONFIG); + writeFileSync(join(dir, "app", "services", "__init__.py"), ""); + writeFileSync(join(dir, "app", "services", "billing.py"), BILLING); + writeFileSync(join(dir, "app", "legacy.py"), LEGACY); + writeFileSync(join(dir, "app", "local", "__init__.py"), ""); + writeFileSync(join(dir, "app", "api.py"), API); + writeFileSync(join(dir, "app", "local", "helper.py"), LOCAL_HELPER); + writeFileSync(join(dir, "app", "external.py"), EXTERNAL); + return dir; +} + +async function buildFixture(dir: string): Promise { + await buildGraph(dir); // $0, Tier-1 only + const graph = readGraph(wiringPath(join(dir, "graft"))); + assert.ok(graph, "wiring graph should be written"); + return graph!; +} + +const callsOf = (graph: GraphV1, source: string): string[] => + graph.edges.filter((e) => e.relation === "calls" && e.source === source).map((e) => e.target); + +test("Python: a call through an imported module alias resolves to that module's function", async () => { + const graph = await buildFixture(makeFixture()); + + assert.deepEqual( + callsOf(graph, "app/api.py#pay"), + ["app/services/billing.py#charge"], + "billing.charge() should resolve into the imported module", + ); + assert.deepEqual( + callsOf(graph, "app/api.py#pay_aliased"), + ["app/services/billing.py#refund"], + "an `as` alias binds the same module", + ); + assert.deepEqual( + callsOf(graph, "app/api.py#read_timeout"), + ["app/config.py#load"], + "`from app import config as settings` binds a module one level up", + ); + assert.deepEqual( + callsOf(graph, "app/api.py#pay_direct"), + ["app/services/billing.py#charge"], + "`import a.b.c as x` binds the dotted module too", + ); + assert.deepEqual( + callsOf(graph, "app/api.py#use_relative"), + ["app/local/helper.py#run"], + "a relative import anchors on the importing file's own package", + ); +}); + +test("Python: a module-qualified call never guesses past its own module", async () => { + const graph = await buildFixture(makeFixture()); + + assert.deepEqual( + callsOf(graph, "app/api.py#shadowed"), + [], + "a parameter of the same name shadows the module for the whole body", + ); + // The local `billing = _Stub()` keeps its own edges (constructor, and the + // stub's method via the receiver-binding table) — what must not appear is an + // edge into the module the name no longer denotes. + assert.ok( + !callsOf(graph, "app/api.py#rebound").some((t) => t.startsWith("app/services/billing.py")), + "a local assignment rebinds the name away from the module", + ); + assert.deepEqual( + callsOf(graph, "app/api.py#absent"), + [], + "a name the module does not define resolves to nothing, not to a namesake elsewhere", + ); + assert.deepEqual( + callsOf(graph, "app/external.py#pay"), + [], + "an out-of-repo module leaves the call unresolved", + ); +}); + +test("Python: absolute and relative imports resolve to in-repo files", async () => { + const graph = await buildFixture(makeFixture()); + const imports = graph.edges + .filter((e) => e.relation === "imports" && e.source === "app/api.py") + .map((e) => e.target); + + // `from app.services import billing` names the PACKAGE, so the import edge + // lands on its `__init__.py` — the module itself is named by the call edge. + // `import app.services.billing as direct` names the module outright. + assert.ok( + imports.includes("app/services/__init__.py"), + `a dotted import should name an in-repo file, got ${JSON.stringify(imports)}`, + ); + assert.ok( + imports.includes("app/services/billing.py"), + `an aliased dotted import should name the module file, got ${JSON.stringify(imports)}`, + ); + assert.ok(imports.includes("app/local/__init__.py"), "a relative import should name the file"); + assert.ok( + graph.edges.some( + (e) => e.relation === "imports" && e.source === "app/external.py" && e.target === "thirdparty.billing", + ), + "an external package stays a truthful unresolved specifier", + ); +});