Skip to content

fix(python): resolve calls through an imported module alias, without guessing past it #437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
JeglovDev wants to merge 1 commit into from
Closed

fix(python): resolve calls through an imported module alias, without guessing past it #437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 122 additions & 3 deletions src/graph/extract.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ const FUNCTION_VALUE_TYPES = new Set([
]);

const EMPTY_SET: ReadonlySet<string> = new Set();
const EMPTY_MODULE_MAP: ReadonlyMap<string, string> = new Map();

const parser = new Parser();
const GRAMMARS: Record<Language, unknown> = {
Expand All @@ -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<string, { name: string; specifier: string }>;
/** 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<string, string>;
// 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`
Expand Down Expand Up @@ -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[] = [
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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") {
Expand Down Expand Up @@ -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<string, string> {
const out = new Map<string, string>();
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<string, string>,
definition: Parser.SyntaxNode,
): ReadonlyMap<string, string> {
if (modules.size === 0) return modules;
const shadowed = new Set<string>();
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
Expand Down Expand Up @@ -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") {
Expand Down
66 changes: 65 additions & 1 deletion src/graph/resolve.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string[]>();
// 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<string, string[]>();
const hasGoModules = !!opts.goModules?.length;
for (const n of nodes) {
if (n.kind === "file") {
Expand All @@ -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("");
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<string, NodeV1>,
filesByModule: Map<string, string[]>,
): 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).
Expand Down
Loading
Loading