mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-13 15:40:05 +00:00
fix(elixir): scope import/use to the declaring module, not the file
The unqualified-call scope added for #4001 collected every `import` and `use` in a file into one list, so an import in one module leaked into a sibling module defined in the same file: `MyApp.Sibling.go/1` calling `fmt(x)` still bound to `MyApp.Helpers.fmt/1` because `MyApp.Importer` imported it. Elixir scopes `import`/`use` to the module body it appears in and to the modules nested in it. Record targets per enclosing module, with the file's top level as its own scope, and build each call's scope from its own module, the modules enclosing it, and the top level. Each raw call now gets its own list rather than sharing one mutable list with every other call in the file. A module nested in a same-named module shares its node id, so the walk up the enclosing chain stops at a module it has already visited. Follow-up to #4015. Refs #4001
This commit is contained in:
1 parent
48d7c0e832
commit
e508142dac
3 files changed
+59
-5
No files matched your search
+3
-2
@@ -8544,8 +8544,9 @@ def extract(
|
||||
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
|
||||
# (resolved in-file by the extractor), Kernel, or a module imported or
|
||||
# used in the caller's lexical scope (the extractor records those as
|
||||
# `elixir_call_scope`). 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).
|
||||
|
||||
@@ -51,7 +51,11 @@ def extract_elixir(path: Path) -> dict:
|
||||
|
||||
file_nid = _make_id(str(path))
|
||||
add_node(file_nid, path.name, 1)
|
||||
call_scope_modules: list[str] = []
|
||||
# Elixir scopes `import`/`use` to the module body it appears in and to the
|
||||
# modules nested in it, so targets are recorded per enclosing module (None
|
||||
# is the file's top level), along with each module's and function's parent.
|
||||
call_scope: dict[str | None, list[str]] = {}
|
||||
enclosing: dict[str, str | None] = {}
|
||||
|
||||
_IMPORT_KEYWORDS = frozenset({"alias", "import", "require", "use"})
|
||||
|
||||
@@ -150,6 +154,7 @@ def extract_elixir(path: Path) -> dict:
|
||||
add_node(module_nid, module_name, line,
|
||||
**({"_elixir_module": True} if parent_module_nid is None else {}))
|
||||
add_edge(file_nid, module_nid, "contains", line)
|
||||
enclosing[module_nid] = parent_module_nid
|
||||
if do_block_node:
|
||||
for child in do_block_node.children:
|
||||
walk(child, parent_module_nid=module_nid)
|
||||
@@ -168,6 +173,7 @@ def extract_elixir(path: Path) -> dict:
|
||||
add_node(proto_nid, proto_name, line,
|
||||
**({"_elixir_module": True} if parent_module_nid is None else {}))
|
||||
add_edge(parent_module_nid or file_nid, proto_nid, "contains", line)
|
||||
enclosing[proto_nid] = parent_module_nid
|
||||
if do_block_node:
|
||||
for child in do_block_node.children:
|
||||
walk(child, parent_module_nid=proto_nid)
|
||||
@@ -184,6 +190,7 @@ def extract_elixir(path: Path) -> dict:
|
||||
label = f"{proto_name} (for {target})" if target else proto_name
|
||||
add_node(impl_nid, label, line)
|
||||
add_edge(parent_module_nid or file_nid, impl_nid, "contains", line)
|
||||
enclosing[impl_nid] = parent_module_nid
|
||||
# Link the implementation to the protocol it satisfies. A same-file
|
||||
# protocol resolves directly; a cross-file target is filtered out by
|
||||
# the dangling-edge guard below rather than left hanging.
|
||||
@@ -232,6 +239,7 @@ def extract_elixir(path: Path) -> dict:
|
||||
container = parent_module_nid or file_nid
|
||||
func_nid = _make_id(container, func_name)
|
||||
add_node(func_nid, f"{func_name}()", line)
|
||||
enclosing[func_nid] = parent_module_nid
|
||||
if parent_module_nid:
|
||||
add_edge(parent_module_nid, func_nid, "method", line)
|
||||
else:
|
||||
@@ -247,7 +255,7 @@ def extract_elixir(path: Path) -> dict:
|
||||
# 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)
|
||||
call_scope.setdefault(parent_module_nid, []).append(module_name)
|
||||
return
|
||||
|
||||
for child in node.children:
|
||||
@@ -269,6 +277,19 @@ def extract_elixir(path: Path) -> dict:
|
||||
"if", "unless", "case", "cond", "with", "for",
|
||||
})
|
||||
|
||||
def _call_scope_for(caller_nid: str) -> list[str]:
|
||||
"""import/use targets visible to a call inside ``caller_nid``: its own
|
||||
module's, each enclosing module's, then the file's top-level ones."""
|
||||
scope: list[str] = []
|
||||
seen: set[str] = set() # a module nested in a same-named one shares its nid
|
||||
container = enclosing.get(caller_nid)
|
||||
while container is not None and container not in seen:
|
||||
seen.add(container)
|
||||
scope.extend(call_scope.get(container, ()))
|
||||
container = enclosing.get(container)
|
||||
scope.extend(call_scope.get(None, ()))
|
||||
return scope
|
||||
|
||||
def walk_calls(node, caller_nid: str) -> None:
|
||||
if node.type != "call":
|
||||
for child in node.children:
|
||||
@@ -311,7 +332,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,
|
||||
"elixir_call_scope": _call_scope_for(caller_nid),
|
||||
})
|
||||
for child in node.children:
|
||||
walk_calls(child, caller_nid)
|
||||
|
||||
@@ -107,6 +107,38 @@ def test_imported_and_used_module_calls_still_resolve(tmp_path: Path):
|
||||
assert ("opts()", "timestamps_opts()") in calls
|
||||
|
||||
|
||||
def test_import_is_scoped_to_the_module_that_declares_it(tmp_path: Path):
|
||||
"""An `import` applies to the module body it appears in and to modules
|
||||
nested in it, not to a sibling module that happens to share the file."""
|
||||
calls = _cross_file_calls(tmp_path, {
|
||||
"lib/my_app/helpers.ex": _HELPERS,
|
||||
"lib/my_app/pair.ex": (
|
||||
"defmodule MyApp.Importer do\n"
|
||||
" import MyApp.Helpers\n"
|
||||
"\n"
|
||||
" def show(x) do\n"
|
||||
" fmt(x)\n"
|
||||
" end\n"
|
||||
"\n"
|
||||
" defmodule Nested do\n"
|
||||
" def inner(x) do\n"
|
||||
" fmt(x)\n"
|
||||
" end\n"
|
||||
" end\n"
|
||||
"end\n"
|
||||
"\n"
|
||||
"defmodule MyApp.Sibling do\n"
|
||||
" def go(x) do\n"
|
||||
" fmt(x)\n"
|
||||
" end\n"
|
||||
"end\n"
|
||||
),
|
||||
})
|
||||
assert ("show()", "fmt()") in calls
|
||||
assert ("inner()", "fmt()") in calls
|
||||
assert ("go()", "fmt()") not 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."""
|
||||
|
||||
Reference in new issue
Block a user