fix: resolve ${VAR} bash sources against the variable's real base (#2172)
#2079 resolved `source "${VAR}/lib/x.sh"` by stripping the leading expansion and resolving the literal suffix against the sourcing script's own directory. That is correct for the canonical `DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"` idiom, but wrong whenever the variable points somewhere else. With ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" source "${ROOT}/lib/utils.sh" the real target is <root>/lib/utils.sh, yet a same-named decoy under the script dir (<root>/scripts/lib/utils.sh) won: a wrong imports_from edge to a real node. Track top-level variable assignments and use the assigned base for the leading variable: - the script-dir idiom, including any number of trailing `/..` hops (and the `$0` spelling), resolves to the script dir walked up that many times - a literal value resolves as-is when absolute, or against the script dir when relative - anything else -- a value built from other variables, or command substitution we do not model -- stays untracked, so the previous script-dir guess is kept Only the base changes. The suffix guards are untouched: an expansion left in the suffix, an empty suffix, and a `..` component in the suffix are all still rejected, the edge is still gated on is_file() and still emitted as INFERRED.
This commit is contained in:
Souptik Chakraborty
committed by
safishamsi
safishamsi
parent
4710b864ea
commit
44241dd10c
@@ -29,6 +29,45 @@ def _bash_source_suffix(raw: str) -> str | None:
|
||||
return suffix
|
||||
|
||||
|
||||
# Name of the leading variable of a `source` argument: `${ROOT}/lib/x.sh` -> ROOT.
|
||||
_BASH_LEADING_VAR = re.compile(
|
||||
r"^\$\{([A-Za-z_][A-Za-z0-9_]*)[^}]*\}|^\$([A-Za-z_][A-Za-z0-9_]*)"
|
||||
)
|
||||
|
||||
# The `$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)` idiom, capturing the
|
||||
# trailing `/..` hops applied to the dirname result.
|
||||
_BASH_DIRNAME_IDIOM = re.compile(r"dirname[^)]*\)((?:/\.\.)*)")
|
||||
|
||||
|
||||
def _bash_assignment_base(value: str, script_dir: Path) -> Path | None:
|
||||
"""Resolve a top-level assignment's value to a directory, or None if untracked.
|
||||
|
||||
Covers the two forms that make a ``source "${VAR}/lib/x.sh"`` target knowable
|
||||
statically (#2172):
|
||||
|
||||
* the script-dir idiom ``"$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"``,
|
||||
including any number of trailing ``/..`` hops (and the ``$0`` spelling)
|
||||
* a literal path, absolute or relative to the script's own directory
|
||||
|
||||
Anything else -- a value built from other variables, or command substitution
|
||||
we do not model -- is left untracked so the caller keeps its previous
|
||||
script-dir guess rather than binding somewhere invented.
|
||||
"""
|
||||
val = value.strip().strip("'\"")
|
||||
if not val:
|
||||
return None
|
||||
if "dirname" in val and ("BASH_SOURCE" in val or "$0" in val):
|
||||
m = _BASH_DIRNAME_IDIOM.search(val)
|
||||
base = script_dir
|
||||
for _ in range((m.group(1).count("..") if m else 0)):
|
||||
base = base.parent
|
||||
return base
|
||||
if "$" in val or "`" in val:
|
||||
return None
|
||||
candidate = Path(val)
|
||||
return candidate if candidate.is_absolute() else script_dir / candidate
|
||||
|
||||
|
||||
def extract_bash(path: Path) -> dict:
|
||||
"""Extract functions, source imports, and cross-function calls from a .sh file."""
|
||||
try:
|
||||
@@ -92,8 +131,7 @@ def extract_bash(path: Path) -> dict:
|
||||
add_edge(file_nid, entry_nid, "contains", 1)
|
||||
|
||||
_BASH_SOURCE_COMMANDS = frozenset({"source", "."})
|
||||
_BASH_SCRIPT_RUNNERS = frozenset({"bash", "sh", "zsh", "ksh", "dash"})
|
||||
# Parent node types that mean a contained command is part of a substitution
|
||||
_BASH_SCRIPT_RUNNERS = frozenset({"bash", "sh", "zsh", "ksh", "dash"}) # Parent node types that mean a contained command is part of a substitution
|
||||
# or expansion, not a real function call. Token-level filtering misses
|
||||
# these because `$(build)` exposes `build` as a child command whose name
|
||||
# token has no metacharacters — only the parent does.
|
||||
@@ -249,7 +287,16 @@ def extract_bash(path: Path) -> dict:
|
||||
# never a dead id.
|
||||
suffix = _bash_source_suffix(raw)
|
||||
if suffix:
|
||||
resolved = (path.parent / suffix).resolve()
|
||||
# Resolve against the variable's tracked base when
|
||||
# we know it, else fall back to the script's own
|
||||
# directory as before (#2172).
|
||||
base = path.parent
|
||||
var_match = _BASH_LEADING_VAR.match(raw)
|
||||
if var_match:
|
||||
var_name = var_match.group(1) or var_match.group(2)
|
||||
if var_name in var_bases:
|
||||
base = var_bases[var_name]
|
||||
resolved = (base / suffix).resolve()
|
||||
if resolved.is_file():
|
||||
add_edge(file_nid, _make_id(str(resolved)),
|
||||
"imports_from", line,
|
||||
@@ -346,6 +393,27 @@ def extract_bash(path: Path) -> dict:
|
||||
_prescan_functions(child)
|
||||
|
||||
_prescan_functions(root)
|
||||
# Bases for `source "${VAR}/lib/x.sh"` resolution (#2172). #2079 always resolved
|
||||
# the literal suffix against the script's own directory, which is right for the
|
||||
# `DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"` idiom but wrong whenever
|
||||
# the variable points elsewhere -- e.g. ROOT=".../scripts/.." with a same-named
|
||||
# decoy under the script dir bound to the decoy. Track top-level assignments so
|
||||
# the real base is used; untracked variables keep the script-dir guess.
|
||||
var_bases: dict[str, Path] = {}
|
||||
for _assign in root.children:
|
||||
if _assign.type != "variable_assignment":
|
||||
continue
|
||||
_name_node = _assign.child_by_field_name("name")
|
||||
_value_node = _assign.child_by_field_name("value")
|
||||
if _name_node is None or _value_node is None:
|
||||
continue
|
||||
_name = _read_text(_name_node, source).strip()
|
||||
if not _name:
|
||||
continue
|
||||
_base = _bash_assignment_base(_read_text(_value_node, source), path.parent)
|
||||
if _base is not None:
|
||||
var_bases[_name] = _base
|
||||
|
||||
walk(root, file_nid)
|
||||
|
||||
# Second pass: cross-function calls
|
||||
|
||||
@@ -2172,6 +2172,75 @@ def test_extract_bash_source_suffix_guard_rejects_traversal(tmp_path):
|
||||
assert result["bash_sources"] == [], result["bash_sources"]
|
||||
|
||||
|
||||
def test_extract_bash_var_source_uses_tracked_assignment_base(tmp_path):
|
||||
"""#2172: `${VAR}` must resolve against the variable's tracked base.
|
||||
|
||||
#2079 always resolved the literal suffix against the script's own directory.
|
||||
That is right for `DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"`, but
|
||||
when the variable points elsewhere -- here ROOT is the script dir's parent --
|
||||
and a same-named decoy exists under the script dir, the edge bound to the
|
||||
decoy: a wrong edge to a real node.
|
||||
"""
|
||||
(tmp_path / "lib").mkdir()
|
||||
(tmp_path / "lib" / "utils.sh").write_text(
|
||||
"#!/usr/bin/env bash\nreal_util() { echo real; }\n", encoding="utf-8"
|
||||
)
|
||||
scripts = tmp_path / "scripts"
|
||||
(scripts / "lib").mkdir(parents=True)
|
||||
decoy = scripts / "lib" / "utils.sh"
|
||||
decoy.write_text(
|
||||
"#!/usr/bin/env bash\ndecoy_util() { echo decoy; }\n", encoding="utf-8"
|
||||
)
|
||||
script = scripts / "deploy.sh"
|
||||
script.write_text(
|
||||
'#!/usr/bin/env bash\n'
|
||||
'ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"\n'
|
||||
'source "${ROOT}/lib/utils.sh"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
result = extract_bash(script)
|
||||
targets = [str(s["target_path"]) for s in result["bash_sources"]]
|
||||
assert targets, "the ${VAR} source must still resolve"
|
||||
for t in targets:
|
||||
assert Path(t).resolve() == (tmp_path / "lib" / "utils.sh").resolve(), t
|
||||
assert Path(t).resolve() != decoy.resolve(), f"bound to the decoy: {t}"
|
||||
|
||||
|
||||
def test_extract_bash_var_source_script_dir_idiom_still_resolves(tmp_path):
|
||||
"""The canonical script-dir idiom must keep working (#2079 regression guard)."""
|
||||
(tmp_path / "lib").mkdir()
|
||||
(tmp_path / "lib" / "x.sh").write_text(
|
||||
"#!/usr/bin/env bash\nx_fn() { :; }\n", encoding="utf-8"
|
||||
)
|
||||
script = tmp_path / "bench.sh"
|
||||
script.write_text(
|
||||
'#!/usr/bin/env bash\n'
|
||||
'BENCH_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"\n'
|
||||
'source "${BENCH_DIR}/lib/x.sh"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
result = extract_bash(script)
|
||||
targets = [Path(s["target_path"]).resolve() for s in result["bash_sources"]]
|
||||
assert (tmp_path / "lib" / "x.sh").resolve() in targets, targets
|
||||
|
||||
|
||||
def test_extract_bash_var_source_untracked_var_keeps_script_dir_guess(tmp_path):
|
||||
"""An untracked variable (assigned from the environment, or not assigned in
|
||||
this file at all) keeps the #2079 script-dir guess rather than binding
|
||||
nowhere -- the fallback must survive the #2172 change."""
|
||||
lib = tmp_path / "lib"
|
||||
lib.mkdir()
|
||||
(lib / "y.sh").write_text("#!/usr/bin/env bash\ny_fn() { :; }\n", encoding="utf-8")
|
||||
script = tmp_path / "run.sh"
|
||||
script.write_text(
|
||||
'#!/usr/bin/env bash\nsource "${SOME_EXTERNAL_DIR}/lib/y.sh"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
result = extract_bash(script)
|
||||
targets = [Path(s["target_path"]).resolve() for s in result["bash_sources"]]
|
||||
assert (lib / "y.sh").resolve() in targets, targets
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# JSON extractor tests (#866)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user