Remove unnecessary excludes from pre-commit-config

packages/pre_commit_excludes/README.md

@@ -4,15 +4,12 @@
 [![PyPI - Python Version](https://img.shields.io/pypi/pyversions/pre-commit-excludes)](https://pypi.org/project/pre-commit-excludes/)
 [![PyPI - License](https://img.shields.io/pypi/l/pre-commit-excludes)](https://pypi.org/project/pre-commit-excludes/)
 
-`remove-unnecessary-excludes` should help you to find lines in your exclude list that are no longer required.
-Running this tool will try to remove excludes from your config by removing a line, running the hook, and restore the old config if it is still required.
-
-Right now this is in early development, so we don't automatically update the `.pre-commit-config.yaml` with all unnecessary excludes removed.
-Instead, we only print which excludes can be removed for which hook.
-More automation will come in future releases.
+`remove-unnecessary-excludes` finds lines in your exclude list that are no longer required and removes them from the supplied `.pre-commit-config.yaml`.
+The tool checks each exclude by running the affected hook without excludes and restores changes made by a failing hook when the exclude is still required.
 
 > [!NOTE]
 > This hook deliberately only supports simple `|`-separated lists of file paths in `exclude` fields — complex regular expressions are not supported.
+> Each path must be on its own line in a multiline YAML block scalar; compact inline exclude patterns are not rewritten.
 > Keeping exclusions as plain `|`-separated paths also makes it easier for humans to maintain an overview of what is excluded.
 
 ## Usage

packages/pre_commit_excludes/pre_commit_excludes/hook_utils.py

@@ -3,10 +3,13 @@
 import itertools
 from collections import Counter
 from pathlib import Path
-from typing import Any
+from typing import TYPE_CHECKING, Any
 
 from ruamel.yaml import YAML
 
+if TYPE_CHECKING:
+    from collections.abc import Iterator, Mapping
+
 
 class Hook:
     """Represent a pre-commit hook with its excluded paths."""
@@ -95,8 +98,12 @@ def write_config(config_file: Path, config: dict[str, Any]) -> None:
         yaml.dump(config, output)
 
 
+def get_hook_configs_from_all_repos(config: Mapping[str, Any]) -> Iterator[dict[str, Any]]:
+    return itertools.chain.from_iterable(repo["hooks"] for repo in config["repos"])
+
+
 def load_hooks(root_directory: Path, config_file: Path) -> list[Hook]:
     config = load_config(config_file)
-    hook_configs = itertools.chain(*[repo["hooks"] for repo in config["repos"]])
+    hook_configs = get_hook_configs_from_all_repos(config)
 
     return [Hook.from_hook_config(root_directory, hook) for hook in hook_configs if has_excludes(hook)]

packages/pre_commit_excludes/pre_commit_excludes/remove_unnecessary_excludes.py

@@ -1,13 +1,19 @@
 """Remove unnecessary excludes from a .pre-commit-config.yaml."""
 
 import argparse
+import re
 import subprocess
 import sys
 from collections import defaultdict
 from dataclasses import dataclass
 from pathlib import Path
 
-from pre_commit_excludes.hook_utils import Hook, load_config, load_hooks, write_config
+from ruamel.yaml import YAML
+from ruamel.yaml.comments import CommentedMap
+from ruamel.yaml.scalarstring import LiteralScalarString
+from ruamel.yaml.util import load_yaml_guess_indent
+
+from pre_commit_excludes.hook_utils import Hook, get_hook_configs_from_all_repos, load_config, load_hooks, write_config
 
 
 @dataclass(frozen=True)
@@ -104,6 +110,75 @@ def find_unnecessary_excludes(
     return excludes_to_remove
 
 
+def _exclude_line_value(line: str) -> str | None:
+    value = line.split("#", maxsplit=1)[0].strip()
+    if value.endswith("|"):
+        value = value[:-1].rstrip()
+    if not value or value in {"(?x)^(", ")"}:
+        return None
+    return Path(value.replace(r"\.", ".")).as_posix()
+
+
+def _remove_trailing_separator(line: str) -> str:
+    match = re.search(r"\|(?P<space>\s*)(?P<comment>#.*)?$", line)
+    if match is None:
+        return line
+    comment = match.group("comment") or ""
+    return f"{line[: match.start()]}{match.group('space')}{comment}"
+
+
+def _remove_excludes_from_block(block: str, excludes: set[str]) -> str:
+    lines = block.splitlines()
+    retained_lines = [line for line in lines if _exclude_line_value(line) not in excludes]
+    if len(retained_lines) == len(lines):
+        return block
+
+    alternative_indexes = [index for index, line in enumerate(retained_lines) if _exclude_line_value(line) is not None]
+    if alternative_indexes:
+        final_alternative = alternative_indexes[-1]
+        retained_lines[final_alternative] = _remove_trailing_separator(retained_lines[final_alternative])
+    trailing_newline = "\n" if block.endswith("\n") else ""
+    return "\n".join(retained_lines) + trailing_newline
+
+
+def _load_round_trip_config(content: str) -> tuple[CommentedMap, YAML]:
+    yaml = YAML()
+    yaml.preserve_quotes = True
+    config, indent, block_sequence_indent = load_yaml_guess_indent(content, yaml=yaml)
+    if indent is not None:
+        yaml.indent(sequence=indent, offset=block_sequence_indent)
+    return (config if isinstance(config, CommentedMap) else CommentedMap()), yaml
+
+
+def _remove_excludes_from_hooks(config: CommentedMap, excludes_by_hook: dict[str, set[str]]) -> bool:
+    changed = False
+    hooks_to_update = [
+        hook
+        for hook in get_hook_configs_from_all_repos(config)
+        if hook.get("id") in excludes_by_hook and isinstance(hook.get("exclude"), LiteralScalarString)
+    ]
+    for hook in hooks_to_update:
+        hook_id = hook["id"]
+        exclude = hook["exclude"]
+        updated_exclude = _remove_excludes_from_block(exclude, excludes_by_hook[hook_id])
+        if updated_exclude != exclude:
+            hook["exclude"] = LiteralScalarString(updated_exclude)
+            changed = True
+    return changed
+
+
+def remove_excludes_from_config(config_file: Path, excludes_to_remove: dict[str, list[Path]]) -> None:
+    """Remove matching exclude lines from hooks in a pre-commit config."""
+    relative_excludes = {
+        hook_id: {exclude.relative_to(config_file.parent).as_posix() for exclude in excludes}
+        for hook_id, excludes in excludes_to_remove.items()
+    }
+    original_content = config_file.read_text(encoding="utf-8")
+    config, yaml = _load_round_trip_config(original_content)
+    if _remove_excludes_from_hooks(config, relative_excludes):
+        yaml.dump(config, config_file)
+
+
 def parse_arguments() -> argparse.Namespace:
     parser = argparse.ArgumentParser(description=__doc__)
     parser.add_argument(
@@ -174,6 +249,7 @@ def main() -> int:
     print()
     print("Excludes to remove:")
     print(excludes_to_remove)
+    remove_excludes_from_config(args.config, excludes_to_remove)
 
     return 0
 

tests/pre_commit_excludes/test_remove_unnecessary_excludes.py

@@ -16,6 +16,7 @@
     get_hooks_to_cleanup,
     is_exclude_unnecessary,
     parse_skipped_exclude,
+    remove_excludes_from_config,
     run_pre_commit,
     write_tmp_pre_commit_config_without_excludes,
 )
@@ -221,6 +222,200 @@ def test_find_unnecessary_excludes_when_all_excludes_are_skipped_should_return_e
     is_exclude_unnecessary_mock.assert_not_called()
 
 
+def test_remove_excludes_from_config_should_remove_matching_lines_from_each_hook(fs: FakeFilesystem) -> None:
+    config_file = Path("Repo/.pre-commit-config.yaml")
+    fs.create_file(
+        config_file,
+        contents="""repos:
+  - repo: local
+    hooks:
+      - id: ruff
+        exclude: |
+          (?x)^(
+            generated/foo\\.py|
+            generated/keep.py|
+            generated/bar.py
+          )
+      - id: black
+        exclude: |
+          (?x)^(
+            generated/foo.py|
+            generated/keep.py
+          )
+""",
+    )
+
+    remove_excludes_from_config(
+        config_file,
+        {
+            "ruff": [Path("Repo/generated/foo.py"), Path("Repo/generated/bar.py")],
+            "black": [Path("Repo/generated/foo.py")],
+        },
+    )
+
+    assert (
+        config_file.read_text(encoding="utf-8")
+        == """repos:
+  - repo: local
+    hooks:
+      - id: ruff
+        exclude: |
+          (?x)^(
+            generated/keep.py
+          )
+      - id: black
+        exclude: |
+          (?x)^(
+            generated/keep.py
+          )
+"""
+    )
+
+
+def test_remove_excludes_from_config_should_repair_separator_when_removing_final_alternative(
+    fs: FakeFilesystem,
+) -> None:
+    config_file = Path("Repo/.pre-commit-config.yaml")
+    fs.create_file(
+        config_file,
+        contents="""repos:
+  - repo: local
+    hooks:
+      - id: ruff
+        exclude: |
+          (?x)^(
+            generated/keep.py|  # Still required.
+            generated/remove.py
+          )
+""",
+    )
+
+    remove_excludes_from_config(config_file, {"ruff": [Path("Repo/generated/remove.py")]})
+
+    assert "            generated/keep.py  # Still required.\n" in config_file.read_text(encoding="utf-8")
+
+
+def test_remove_excludes_from_config_should_match_directory_without_trailing_slash(fs: FakeFilesystem) -> None:
+    config_file = Path("Repo/.pre-commit-config.yaml")
+    fs.create_file(
+        config_file,
+        contents="""repos:
+  - repo: local
+    hooks:
+      - id: ruff
+        exclude: |
+          (?x)^(
+            generated/remove/|
+            generated/keep/
+          )
+""",
+    )
+
+    remove_excludes_from_config(config_file, {"ruff": [Path("Repo/generated/remove")]})
+
+    assert "generated/remove/" not in config_file.read_text(encoding="utf-8")
+
+
+def test_remove_excludes_from_config_should_only_change_exclude_block_for_matching_hook(
+    fs: FakeFilesystem,
+) -> None:
+    config_file = Path("Repo/.pre-commit-config.yaml")
+    original_config = """repos:
+  - repo: local
+    hooks:
+      - id: ruff
+        args:
+          - generated/remove.py
+        exclude: generated/remove.py
+      - id: black
+        exclude: |
+          (?x)^(
+            generated/remove.py
+          )
+"""
+    fs.create_file(config_file, contents=original_config)
+
+    remove_excludes_from_config(config_file, {"ruff": [Path("Repo/generated/remove.py")]})
+
+    assert config_file.read_text(encoding="utf-8") == original_config
+
+
+def test_remove_excludes_from_config_should_preserve_detected_yaml_formatting(fs: FakeFilesystem) -> None:
+    config_file = Path("Repo/.pre-commit-config.yaml")
+    fs.create_file(
+        config_file,
+        contents="""# Project hooks
+repos:
+- repo: 'local' # Keep this comment.
+  hooks:
+  - id: ruff
+    exclude: |
+      (?x)^(
+        generated/remove.py|
+        generated/keep.py
+      )
+""",
+    )
+
+    remove_excludes_from_config(config_file, {"ruff": [Path("Repo/generated/remove.py")]})
+
+    assert (
+        config_file.read_text(encoding="utf-8")
+        == """# Project hooks
+repos:
+- repo: 'local' # Keep this comment.
+  hooks:
+  - id: ruff
+    exclude: |
+      (?x)^(
+        generated/keep.py
+      )
+"""
+    )
+
+
+def test_remove_excludes_from_config_when_every_entry_is_removed_should_leave_empty_wrapper(
+    fs: FakeFilesystem,
+) -> None:
+    config_file = Path("Repo/.pre-commit-config.yaml")
+    fs.create_file(
+        config_file,
+        contents="""repos:
+  - repo: local
+    hooks:
+      - id: ruff
+        exclude: |
+          (?x)^(
+            generated/remove.py
+          )
+""",
+    )
+
+    remove_excludes_from_config(config_file, {"ruff": [Path("Repo/generated/remove.py")]})
+
+    assert (
+        config_file.read_text(encoding="utf-8")
+        == """repos:
+  - repo: local
+    hooks:
+      - id: ruff
+        exclude: |
+          (?x)^(
+          )
+"""
+    )
+
+
+def test_remove_excludes_from_config_for_empty_removals_should_leave_config_unchanged(fs: FakeFilesystem) -> None:
+    config_file = Path("Repo/.pre-commit-config.yaml")
+    original_config = "repos: []\n"
+    fs.create_file(config_file, contents=original_config)
+
+    remove_excludes_from_config(config_file, {})
+
+    assert config_file.read_text(encoding="utf-8") == original_config
+
+
 def test_write_tmp_pre_commit_config_without_excludes_should_remove_all_excludes(fs: FakeFilesystem) -> None:
     config_file = Path("Test_directory/.pre-commit-config.yaml")
     fs.create_dir(config_file.parent)