mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-15 00:19:33 +00:00
fix(elixir): scope unqualified calls to imported and used modules
The shared cross-file call pass bound an unqualified 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 imports or uses, so DSL calls landed on unrelated project functions: every migration's `table(:users)` (Ecto.Migration, brought in by `use`) became a calls edge to a Phoenix component's `table/1`, making it the top god node. The Elixir extractor now records, on each raw call, the modules its file names in `import` or `use` (`alias` and `require` do not bring functions into scope). The shared pass keeps only candidates whose file defines one of those modules, the same way Go selectors are already filtered by import path, and leaves the call unresolved otherwise. The module index is read from the resolution nodes, so an unchanged file's modules still count on an incremental rebuild. Calls into a module that a `use` macro imports indirectly (for example `use MyAppWeb, :live_view` importing CoreComponents) are no longer resolved, since that import is not visible in the source. Refs #4001
This commit is contained in:
1 parent
e10df08877
commit
98d4ae9878
3 files changed
+181
No files matched your search
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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()
|
||||
Reference in new issue
Block a user