From fb2239d1d5ee099f695de90d095bf79fd9eb88ca Mon Sep 17 00:00:00 2001 From: MaximilianSoerenPollak Date: Thu, 3 Sep 2026 11:02:26 +0200 Subject: [PATCH 1/3] Feat: Enable globs & comments in exclude file --- cr_checker/tests/test_cr_checker.py | 65 +++++++++++++++++++++++++++++ cr_checker/tool/cr_checker.py | 41 ++++++++++++------ 2 files changed, 94 insertions(+), 12 deletions(-) diff --git a/cr_checker/tests/test_cr_checker.py b/cr_checker/tests/test_cr_checker.py index c9c64bf..994b70d 100644 --- a/cr_checker/tests/test_cr_checker.py +++ b/cr_checker/tests/test_cr_checker.py @@ -447,6 +447,8 @@ def test_exclusion_file_respected_at_root(tmp_path): assert test_file.read_text(encoding="utf-8") == original_content + + # test that an exclusion.txt file sitting in a deeply nested directory is respected: # the listed file is neither flagged nor altered def test_exclusion_file_respected_in_nested_directory(tmp_path): @@ -480,6 +482,69 @@ def test_exclusion_file_respected_in_nested_directory(tmp_path): assert results["fixed"] == 0 assert test_file.read_text(encoding="utf-8") == original_content +# test that a glob entry in the exclusion file is expanded recursively: +# `.claude/**/*` covers direct children as well as arbitrarily nested files, +# and a literal entry in the same file keeps working alongside it +def test_exclusion_file_expands_glob_pattern(tmp_path, monkeypatch): + cr_checker = load_cr_checker_module() + header_template = load_template("py") + + workspace_dir = tmp_path / "workspace" + nested_dir = workspace_dir / ".claude" / "skills" / "some_skill" / "scripts" + nested_dir.mkdir(parents=True) + (workspace_dir / ".claude" / "agents").mkdir() + (workspace_dir / "tool").mkdir() + + original_content = "some content\n" + nested_file = nested_dir / "fix_titles.py" + direct_child = workspace_dir / ".claude" / "settings.py" + agent_file = workspace_dir / ".claude" / "agents" / "reviewer.py" + literal_file = workspace_dir / "tool" / "generated.py" + outside_file = workspace_dir / "tool" / "checked.py" + for excluded in (nested_file, direct_child, agent_file, literal_file, outside_file): + excluded.write_text(original_content, encoding="utf-8") + + exclusion_file = workspace_dir / "exclusion.txt" + exclusion_file.write_text( + "# AI - Stuff\n" + ".claude/**/*\n" + "\n" + "tool/generated.py\n", + encoding="utf-8", + ) + + execroot = tmp_path / "execroot" + execroot.mkdir() + monkeypatch.chdir(execroot) + monkeypatch.setenv("BUILD_WORKSPACE_DIRECTORY", str(workspace_dir)) + + exclusion, valid = cr_checker.load_exclusion(exclusion_file) + + assert valid is True + excluded_files = sorted(item for item in exclusion if Path(item).is_file()) + assert excluded_files == sorted( + str(path) for path in (nested_file, direct_child, agent_file, literal_file) + ) + assert str(outside_file) not in exclusion + + results = cr_checker.process_files( + files=[nested_file, direct_child, agent_file, literal_file, outside_file], + templates={"py": header_template}, + fix=True, + exclusion=exclusion, + use_mmap=False, + encoding="utf-8", + ) + + assert results["no_copyright"] == 1 + assert results["fixed"] == 1 + for skipped in (nested_file, direct_child, agent_file, literal_file): + assert skipped.read_text(encoding="utf-8") == original_content + skipped_header_inserted = outside_file.read_text(encoding="utf-8") + assert skipped_header_inserted.startswith( + header_template.format(year=datetime.now().year) + ) + # test that a workspace-relative exclusion entry (as produced by e.g. `git ls-files`) # is still resolved correctly when the process's cwd is not the workspace root, which diff --git a/cr_checker/tool/cr_checker.py b/cr_checker/tool/cr_checker.py index 16240f4..a025c29 100755 --- a/cr_checker/tool/cr_checker.py +++ b/cr_checker/tool/cr_checker.py @@ -30,6 +30,7 @@ BORDER_FILL_PATTERN = re.compile(r"([/*#'\-=+])\1{4,}") FILL_CHARS_REGEX = r"[/*#'\-=+]+" +GLOB_CHARS = ("*", "?", "[") LOGGER = logging.getLogger() @@ -165,7 +166,7 @@ def add_template_for_extensions( return templates -def load_exclusion(path): +def load_exclusion(path: Path) -> tuple[list[str], bool]: """ Loads the list of files being excluded from the copyright check. @@ -176,23 +177,40 @@ def load_exclusion(path): exclusion list is normalized the same way so it can be matched against the paths produced by `collect_inputs`. + Lines may be either a literal path or a glob pattern. A line containing + any of ``*``, ``?`` or ``[`` is expanded with `Path.glob` relative to the + same base directory; ``**`` matches zero or more directory levels, so + ``.claude/**/*`` excludes every file and sub-directory below ``.claude`` + at any depth. A literal path must point at an existing file, as before. + Blank lines and lines starting with ``#`` are ignored. + Args: path (str): Path to the exclusion file. Returns: - tuple(list, bool): a list of files that are excluded from the copyright check and a boolean indicating whether - all paths listed in the exclusion file exist and are files. + tuple(list[str], bool): a sorted, de-duplicated list of paths (as str) that are + excluded from the copyright check, and a boolean + indicating whether every line resolved to + something: literal paths must exist and be files, + glob patterns must match at least one path. """ - workspace_dir = Path(os.environ.get("BUILD_WORKSPACE_DIRECTORY", "").strip()) - - exclusion = [] + exclusion: set[str] = set() valid = True with open(path, "r", encoding="utf-8") as file: - for item in file.read().splitlines(): - if not item: + for line in file.read().splitlines(): + item = line.strip() + if not item or item.startswith("#"): + continue + if any(char in item for char in GLOB_CHARS): + matches = {str(match) for match in workspace_dir.glob(item)} + if not matches: + LOGGER.error("Exclusion pattern %s matched nothing.", item) + valid = False + continue + exclusion |= matches continue - resolved = Path(workspace_dir / item) + resolved = workspace_dir / item if not resolved.exists(): LOGGER.error("Excluded file %s does not exist.", item) valid = False @@ -201,10 +219,9 @@ def load_exclusion(path): LOGGER.error("Excluded file %s is not a file.", item) valid = False continue - exclusion.append(str(resolved)) - + exclusion.add(str(resolved)) LOGGER.debug(exclusion) - return exclusion, valid + return sorted(exclusion), valid def configure_logging(log_file_path=None, verbose=False): From 61b9d87e5f5ed80c0ad683e5c24cd1dcad1a74c7 Mon Sep 17 00:00:00 2001 From: MaximilianSoerenPollak Date: Thu, 3 Sep 2026 11:07:36 +0200 Subject: [PATCH 2/3] chore: formatting --- cr_checker/tests/test_cr_checker.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/cr_checker/tests/test_cr_checker.py b/cr_checker/tests/test_cr_checker.py index 994b70d..4e3b5f2 100644 --- a/cr_checker/tests/test_cr_checker.py +++ b/cr_checker/tests/test_cr_checker.py @@ -447,8 +447,6 @@ def test_exclusion_file_respected_at_root(tmp_path): assert test_file.read_text(encoding="utf-8") == original_content - - # test that an exclusion.txt file sitting in a deeply nested directory is respected: # the listed file is neither flagged nor altered def test_exclusion_file_respected_in_nested_directory(tmp_path): @@ -482,6 +480,7 @@ def test_exclusion_file_respected_in_nested_directory(tmp_path): assert results["fixed"] == 0 assert test_file.read_text(encoding="utf-8") == original_content + # test that a glob entry in the exclusion file is expanded recursively: # `.claude/**/*` covers direct children as well as arbitrarily nested files, # and a literal entry in the same file keeps working alongside it @@ -506,10 +505,7 @@ def test_exclusion_file_expands_glob_pattern(tmp_path, monkeypatch): exclusion_file = workspace_dir / "exclusion.txt" exclusion_file.write_text( - "# AI - Stuff\n" - ".claude/**/*\n" - "\n" - "tool/generated.py\n", + "# AI - Stuff\n.claude/**/*\n\ntool/generated.py\n", encoding="utf-8", ) From 1a8317fbac01c1b8ff0e56abaf16b1053e2f96b9 Mon Sep 17 00:00:00 2001 From: MaximilianSoerenPollak Date: Tue, 8 Sep 2026 11:49:50 +0200 Subject: [PATCH 3/3] Fix: add better error handling & one test --- cr_checker/tests/test_cr_checker.py | 42 ++++++++++++++++++++++++++--- cr_checker/tool/cr_checker.py | 25 ++++++++++------- 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/cr_checker/tests/test_cr_checker.py b/cr_checker/tests/test_cr_checker.py index 4e3b5f2..06d3f65 100644 --- a/cr_checker/tests/test_cr_checker.py +++ b/cr_checker/tests/test_cr_checker.py @@ -14,12 +14,16 @@ # unit tests for the shebang handling in the cr_checker module from __future__ import annotations +import logging import importlib.util import pytest from datetime import datetime from pathlib import Path +LOGGER = logging.getLogger(__name__) + + # load the cr_checker module def load_cr_checker_module(): module_path = Path(__file__).resolve().parents[1] / "tool" / "cr_checker.py" @@ -170,7 +174,7 @@ def test_process_files_skips_exclusion_with_missing_header(prepare_test_no_heade files=[test_file], templates={extension: header_template}, fix=False, - exclusion=[str(test_file)], + exclusion={str(test_file)}, use_mmap=False, encoding="utf-8", ) @@ -431,7 +435,7 @@ def test_exclusion_file_respected_at_root(tmp_path): exclusion, valid = cr_checker.load_exclusion(exclusion_file) assert valid is True - assert exclusion == [str(test_file)] + assert exclusion == {str(test_file)} results = cr_checker.process_files( files=[test_file], @@ -465,7 +469,7 @@ def test_exclusion_file_respected_in_nested_directory(tmp_path): exclusion, valid = cr_checker.load_exclusion(exclusion_file) assert valid is True - assert exclusion == [str(test_file)] + assert exclusion == {str(test_file)} results = cr_checker.process_files( files=[test_file], @@ -542,6 +546,36 @@ def test_exclusion_file_expands_glob_pattern(tmp_path, monkeypatch): ) +# test the pathway where a glob pattern does not match anything +def test_exclusion_file_glob_without_match_is_invalid(tmp_path, monkeypatch, caplog): + cr_checker = load_cr_checker_module() + + workspace_dir = tmp_path / "workspace" + nested_dir = workspace_dir / ".claude" / "skills" / "some_skill" / "scripts" + nested_dir.mkdir(parents=True) + + original_content = "some content\n" + nested_file = nested_dir / "fix_titles.py" + nested_file.write_text(original_content, encoding="utf-8") + + exclusion_file = workspace_dir / "exclusion.txt" + exclusion_file.write_text( + "# This doesn't exists \n.not_here/**/*\n\n.claude/**/*\n", + encoding="utf-8", + ) + + execroot = tmp_path / "execroot" + execroot.mkdir() + monkeypatch.chdir(execroot) + monkeypatch.setenv("BUILD_WORKSPACE_DIRECTORY", str(workspace_dir)) + + with caplog.at_level(logging.WARNING): + _, valid = cr_checker.load_exclusion(exclusion_file) + assert "Exclusion pattern .not_here/**/* matched nothing." in caplog.text + + assert valid is False + + # test that a workspace-relative exclusion entry (as produced by e.g. `git ls-files`) # is still resolved correctly when the process's cwd is not the workspace root, which # is what happens under `bazel run`/`bazel test` (BUILD_WORKSPACE_DIRECTORY is set to @@ -569,7 +603,7 @@ def test_exclusion_file_respected_under_bazel_run_cwd(tmp_path, monkeypatch): exclusion, valid = cr_checker.load_exclusion(exclusion_file) assert valid is True - assert exclusion == [str(test_file)] + assert exclusion == {str(test_file)} collected_files = cr_checker.collect_inputs([relative_entry], exts=["py"]) assert collected_files == [test_file] diff --git a/cr_checker/tool/cr_checker.py b/cr_checker/tool/cr_checker.py index a025c29..8a323a9 100755 --- a/cr_checker/tool/cr_checker.py +++ b/cr_checker/tool/cr_checker.py @@ -166,7 +166,7 @@ def add_template_for_extensions( return templates -def load_exclusion(path: Path) -> tuple[list[str], bool]: +def load_exclusion(path: Path) -> tuple[set[str], bool]: """ Loads the list of files being excluded from the copyright check. @@ -185,10 +185,10 @@ def load_exclusion(path: Path) -> tuple[list[str], bool]: Blank lines and lines starting with ``#`` are ignored. Args: - path (str): Path to the exclusion file. + path (Path): Path to the exclusion file. Returns: - tuple(list[str], bool): a sorted, de-duplicated list of paths (as str) that are + tuple(set[str], bool): a de-duplicated set of paths (as str) that are excluded from the copyright check, and a boolean indicating whether every line resolved to something: literal paths must exist and be files, @@ -203,7 +203,12 @@ def load_exclusion(path: Path) -> tuple[list[str], bool]: if not item or item.startswith("#"): continue if any(char in item for char in GLOB_CHARS): - matches = {str(match) for match in workspace_dir.glob(item)} + try: + matches = {str(match) for match in workspace_dir.glob(item)} + except (ValueError, NotImplementedError) as err: + LOGGER.error("Invalid exclusion pattern %s: %s", item, err) + valid = False + continue if not matches: LOGGER.error("Exclusion pattern %s matched nothing.", item) valid = False @@ -221,7 +226,7 @@ def load_exclusion(path: Path) -> tuple[list[str], bool]: continue exclusion.add(str(resolved)) LOGGER.debug(exclusion) - return sorted(exclusion), valid + return exclusion, valid def configure_logging(log_file_path=None, verbose=False): @@ -590,7 +595,7 @@ def process_files( files, templates, fix, - exclusion: list[str] | None = None, + exclusion: set[str] | None = None, use_mmap=False, encoding="utf-8", ): # pylint: disable=too-many-arguments @@ -602,8 +607,8 @@ def process_files( templates (dict): A dictionary where keys are file extensions (e.g., '.py', '.txt') and values are strings or patterns representing the required copyright text. - exclusion (list): A list of paths to files to be excluded from the copyright - check. + exclusion (set): A set of paths (as str) to files to be excluded from the + copyright check. use_mmap (bool): Flag for using mmap function for reading files (instead of standard option). encoding (str): Encoding type to use when reading the file. @@ -612,7 +617,7 @@ def process_files( int: The number of files that do not contain the required copyright text. """ if exclusion is None: - exclusion = [] + exclusion = set() results = {"no_copyright": 0, "fixed": 0, "duplicate_copyright": 0} for item in files: name = Path(item).name @@ -780,7 +785,7 @@ def main(argv=None): LOGGER.error("Failed to load copyright text: %s", err) return err.errno - exclusion = [] + exclusion = set() exclusion_valid = True if args.exclusion_file: try: