diff --git a/dep_freshness/install.py b/dep_freshness/install.py index eff5500..035eebe 100644 --- a/dep_freshness/install.py +++ b/dep_freshness/install.py @@ -49,10 +49,32 @@ def _needs_hook(repo: Path) -> bool: return f"id: {HOOK_ID}" not in target.read_text(encoding="utf-8") -def _write_hook(repo: Path) -> None: - """Append the hook block to the repo's `local` hooks list. +LOCAL_REPO = " - repo: local" - Appending rather than parsing-and-re-emitting is deliberate: every one of + +def _local_block_end(lines: list[str]) -> int: + """Index just past the LAST `- repo: local` block, or -1 if there is none. + + Appending at end-of-file is wrong whenever a config interleaves local and + remote hook repos, which most of these do -- diet-guard, screen-locker and + wake-alarm each carry nine `- repo:` blocks. A block appended under + `- repo: https://github.com/...` would be handed to that remote repo's + hook definitions and fail with an unhelpful error about an unknown id. + """ + starts = [n for n, line in enumerate(lines) if line.rstrip() == LOCAL_REPO] + if not starts: + return -1 + after = [ + n for n in range(starts[-1] + 1, len(lines)) + if lines[n].startswith(" - repo:") + ] + return after[0] if after else len(lines) + + +def _write_hook(repo: Path) -> None: + """Insert the hook into the repo's LAST `local` hooks list. + + Inserting rather than parsing-and-re-emitting is deliberate: every one of these configs carries comments explaining why a hook exists, and a YAML round-trip through the standard library drops all of them. """ @@ -60,8 +82,19 @@ def _write_hook(repo: Path) -> None: body = target.read_text(encoding="utf-8") if target.is_file() else EMPTY_PRECOMMIT if not body.endswith("\n"): body += "\n" + # `body` was just made to end in a newline, so the split always leaves a + # trailing empty element; dropping it keeps the join below symmetric. + lines = body.split("\n")[:-1] + cut = _local_block_end(lines) + if cut < 0: + raise ValueError( + f"{target} has no `{LOCAL_REPO}` block to add the hook to; " + "add one by hand, then re-run" + ) + block = _template("precommit-hook.yaml").rstrip("\n").split("\n") + lines[cut:cut] = ["", *block] target.parent.mkdir(parents=True, exist_ok=True) - target.write_text(f"{body}\n{_template('precommit-hook.yaml')}", encoding="utf-8") + target.write_text("\n".join(lines) + "\n", encoding="utf-8") def plan(repo: Path) -> list[str]: diff --git a/dep_freshness/tests/test_install.py b/dep_freshness/tests/test_install.py index 9cfff4a..197bf35 100644 --- a/dep_freshness/tests/test_install.py +++ b/dep_freshness/tests/test_install.py @@ -180,3 +180,52 @@ def test_the_module_entrypoint_runs_the_cli(repo, monkeypatch): with pytest.raises(SystemExit) as exit_info: runpy.run_module("dep_freshness.install_main", run_name="__main__") assert exit_info.value.code == 0 + + +def test_the_hook_lands_in_the_last_local_block_not_at_end_of_file(repo): + """Most of these configs interleave local and remote hook repos. + + diet-guard, screen-locker and wake-alarm carry nine `- repo:` blocks each, + and a hook appended under a remote one is handed to that repo's hook + definitions -- which fails with an error naming an unknown id rather than + the real problem. + """ + write( + repo, + ".pre-commit-config.yaml", + """\ +repos: + - repo: local + hooks: + - id: file-length-cap + entry: bash scripts/check_file_length.sh + language: system + + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.16.0 + hooks: + - id: ruff +""", + ) + installer.install(repo) + lines = (repo / ".pre-commit-config.yaml").read_text(encoding="utf-8").split("\n") + hook = lines.index(" - id: dependency-freshness") + remote = lines.index(" - repo: https://github.com/astral-sh/ruff-pre-commit") + assert hook < remote, "the hook must sit inside the local block" + assert lines[-2] == " - id: ruff", "the remote block keeps its own hooks" + + +def test_a_config_with_no_local_block_refuses_rather_than_guessing(repo): + write( + repo, + ".pre-commit-config.yaml", + """\ +repos: + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.16.0 + hooks: + - id: ruff +""", + ) + with pytest.raises(ValueError, match="no ` - repo: local` block"): + installer.install(repo)