diff --git a/cr_checker/tests/test_cr_checker.py b/cr_checker/tests/test_cr_checker.py index c9c64bf..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], @@ -481,6 +485,97 @@ def test_exclusion_file_respected_in_nested_directory(tmp_path): 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\ntool/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 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 @@ -508,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 16240f4..8a323a9 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[set[str], bool]: """ Loads the list of files being excluded from the copyright check. @@ -176,23 +177,45 @@ 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. + path (Path): 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(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, + 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 - resolved = Path(workspace_dir / item) + if any(char in item for char in GLOB_CHARS): + 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 + continue + exclusion |= matches + continue + resolved = workspace_dir / item if not resolved.exists(): LOGGER.error("Excluded file %s does not exist.", item) valid = False @@ -201,8 +224,7 @@ 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 @@ -573,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 @@ -585,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. @@ -595,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 @@ -763,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: