diff --git a/graphify/extract.py b/graphify/extract.py index 5cc0e3661..93e9b5304 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -8357,6 +8357,16 @@ def extract( _member_nids = { e.get("target") for e in all_edges if e.get("relation") in ("case_of", "defines") } + # Top-level Elixir module names per file, for scoping unqualified Elixir + # calls below. Read from `resolution_nodes` so an unchanged file's modules + # stay visible on an incremental rebuild (`_elixir_module` is carried by the + # resolution-context allow-list). + _elixir_modules_by_file: dict[str, set[str]] = {} + for n in resolution_nodes: + if n.get("_elixir_module") and n.get("source_file"): + _elixir_modules_by_file.setdefault(str(n["source_file"]), set()).add( + str(n.get("label", "")) + ) _csharp_stub_resolver: CsharpNameResolver | None = None # built on first use for rc in all_raw_calls: if rc.get("_ambiguous_python_import"): @@ -8459,6 +8469,21 @@ def extract( if not candidates: continue go_exact_import = True + # An unqualified Elixir call can only reach the caller's own module + # (resolved in-file by the extractor), Kernel, or a module the file + # `import`s or `use`s. A same-named def in any other module is out of + # scope: binding to it by name landed every migration's `table(:users)` + # (Ecto.Migration, pulled in by `use`) on an unrelated Phoenix + # component's `table/1` and made it the top god node (#4001). + elixir_call_scope = rc.get("elixir_call_scope") + if elixir_call_scope is not None: + scope = set(elixir_call_scope) + candidates = [ + candidate for candidate in candidates + if _elixir_modules_by_file.get(nid_to_source_file.get(candidate, ""), set()) & scope + ] + if not candidates: + continue caller = rc["caller_nid"] # Resolve the caller's file via the raw_call's own source_file string, # which is stable regardless of any caller_nid remap. An indirect diff --git a/graphify/extractors/elixir.py b/graphify/extractors/elixir.py index 5d2745db2..13f508158 100644 --- a/graphify/extractors/elixir.py +++ b/graphify/extractors/elixir.py @@ -51,6 +51,7 @@ def extract_elixir(path: Path) -> dict: file_nid = _make_id(str(path)) add_node(file_nid, path.name, 1) + call_scope_modules: list[str] = [] _IMPORT_KEYWORDS = frozenset({"alias", "import", "require", "use"}) @@ -234,6 +235,10 @@ def extract_elixir(path: Path) -> dict: for module_name in _get_alias_modules(arguments_node): tgt_nid = _make_id(module_name) add_edge(file_nid, tgt_nid, "imports", line, context="import") + # Only import/use bring functions into scope for unqualified + # calls; alias/require do not. + if keyword in ("import", "use"): + call_scope_modules.append(module_name) return for child in node.children: @@ -297,6 +302,7 @@ def extract_elixir(path: Path) -> dict: "is_member_call": is_member_call, "source_file": str_path, "source_location": f"L{node.start_point[0] + 1}", + "elixir_call_scope": call_scope_modules, }) for child in node.children: walk_calls(child, caller_nid) diff --git a/tests/test_elixir_unqualified_call_scope.py b/tests/test_elixir_unqualified_call_scope.py new file mode 100644 index 000000000..adec49a3d --- /dev/null +++ b/tests/test_elixir_unqualified_call_scope.py @@ -0,0 +1,150 @@ +"""Unqualified Elixir calls resolve only into imported/used modules (#4001). + +The shared cross-file call pass bound a bare call to any same-named def in the +corpus. In Elixir an unqualified call can only reach the caller's own module, +Kernel, or a module the file `import`s or `use`s, so every migration's +`table(:users)` (Ecto.Migration, pulled in by `use`) landed on an unrelated +Phoenix component's `table/1` and turned it into the top god node. +""" +from __future__ import annotations + +import json +from pathlib import Path + +from graphify.extract import extract + +_COMPONENTS = ( + "defmodule MyAppWeb.CoreComponents do\n" + " def table(assigns), do: assigns\n" + "end\n" +) +_HELPERS = ( + "defmodule MyApp.Helpers do\n" + " def fmt(x), do: x\n" + "end\n" +) + + +def _cross_file_calls(tmp_path: Path, files: dict[str, str]) -> set[tuple[str, str]]: + paths = [] + for name, body in files.items(): + path = tmp_path / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body, encoding="utf-8") + paths.append(path) + result = extract(paths, cache_root=tmp_path / "graphify-out") + label = {n["id"]: n["label"] for n in result["nodes"]} + return { + (label[e["source"]], label[e["target"]]) + for e in result["edges"] + if e["relation"] == "calls" and e["source"] in label and e["target"] in label + } + + +def test_dsl_call_does_not_bind_to_an_unrelated_same_named_def(tmp_path: Path): + calls = _cross_file_calls(tmp_path, { + "lib/my_app_web/components/core_components.ex": _COMPONENTS, + "priv/repo/migrations/20240101000000_create_users.exs": ( + "defmodule MyApp.Repo.Migrations.CreateUsers do\n" + " use Ecto.Migration\n" + "\n" + " def change do\n" + " create table(:users) do\n" + " add :name, :string\n" + " end\n" + " end\n" + "end\n" + ), + }) + assert ("change()", "table()") not in calls + + +def test_call_without_import_or_use_does_not_bind(tmp_path: Path): + calls = _cross_file_calls(tmp_path, { + "lib/my_app/helpers.ex": _HELPERS, + # `alias` only shortens the module name; `fmt` is still not in scope. + "lib/my_app/other.ex": ( + "defmodule MyApp.Other do\n" + " alias MyApp.Helpers\n" + "\n" + " def go(x) do\n" + " fmt(x)\n" + " end\n" + "end\n" + ), + }) + assert ("go()", "fmt()") not in calls + + +def test_imported_and_used_module_calls_still_resolve(tmp_path: Path): + calls = _cross_file_calls(tmp_path, { + "lib/my_app/helpers.ex": _HELPERS, + "lib/my_app/user.ex": ( + "defmodule MyApp.User do\n" + " import MyApp.Helpers\n" + "\n" + " def show(x) do\n" + " fmt(x)\n" + " end\n" + "end\n" + ), + "lib/my_app/schema.ex": ( + "defmodule MyApp.Schema do\n" + " def timestamps_opts, do: []\n" + "end\n" + ), + "lib/my_app/post.ex": ( + "defmodule MyApp.Post do\n" + " use MyApp.Schema\n" + "\n" + " def opts do\n" + " timestamps_opts()\n" + " end\n" + "end\n" + ), + }) + assert ("show()", "fmt()") in calls + assert ("opts()", "timestamps_opts()") in calls + + +def test_imported_call_survives_incremental_rebuild(tmp_path: Path): + """On `graphify update` the unchanged helpers file arrives only as + resolution-context nodes; its module must still count as in scope.""" + from graphify.watch import _rebuild_code + + corpus = tmp_path / "corpus" + corpus.mkdir() + (corpus / "helpers.ex").write_text(_HELPERS, encoding="utf-8") + caller = corpus / "user.ex" + + def _caller(extra: str = "") -> str: + return ( + "defmodule MyApp.User do\n" + " import MyApp.Helpers\n" + "\n" + " def show(x) do\n" + " fmt(x)\n" + " end\n" + f"{extra}" + "end\n" + ) + + caller.write_text(_caller(), encoding="utf-8") + graph_path = corpus / "graphify-out" / "graph.json" + + def calls() -> set[tuple[str, str]]: + data = json.loads(graph_path.read_text(encoding="utf-8")) + label = {n["id"]: n["label"] for n in data["nodes"]} + return { + (label[e["source"]], label[e["target"]]) + for e in data["links"] + if e.get("relation") == "calls" and e["source"] in label and e["target"] in label + } + + assert _rebuild_code(corpus, no_cluster=True, acquire_lock=False) is True + assert ("show()", "fmt()") in calls() + + caller.write_text(_caller("\n def other, do: :ok\n"), encoding="utf-8") + assert _rebuild_code(corpus, changed_paths=[caller], no_cluster=True, + acquire_lock=False) is True + assert ("show()", "fmt()") in calls()