From 7551ae91058a07941bd188717023f1ebbec47055 Mon Sep 17 00:00:00 2001 From: Jochen Hoenle <173445474+hoe-jo@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:35:12 +0100 Subject: [PATCH] feat(cr_checker): accept // and /* */ headers C, C++, Rust and TRLC headers may use either comment form. The other form is derived from the template, so no extra template is needed. --fix keeps the form a file already uses. --- cr_checker/README.md | 15 ++ cr_checker/tests/test_cr_checker.py | 224 +++++++++++++++++- .../tests/test_cr_checker_integration.py | 44 ++++ cr_checker/tool/cr_checker.py | 96 +++++++- 4 files changed, 376 insertions(+), 3 deletions(-) diff --git a/cr_checker/README.md b/cr_checker/README.md index d6d336ee..93ffd42b 100644 --- a/cr_checker/README.md +++ b/cr_checker/README.md @@ -167,6 +167,21 @@ Example templates.ini: // Copyright (c) {year} {author} ``` +#### Accepted comment forms + +Border lines may use any run of 5 or more of `/*#'-=+`. Beyond that, the comment form of the +template's own section is canonical and, for the languages below, the checker also accepts the +same header in the other form the language allows. No second template is needed. + +| Extensions | Accepted forms | +| --- | --- | +| `c`, `cpp`, `h`, `hpp`, `rs`, `trlc`, `rsl` | `/* */` block and `//` line | +| all other extensions | only the form used by the template | + +The header text, the copyright line and the SPDX identifier must still match the template. +`--fix` leaves a compliant header in either form untouched, repairs a broken header in the +form it is already written in, and writes a missing header in the template's form. + ## Exit Codes - 0: All files contain the required copyright text. diff --git a/cr_checker/tests/test_cr_checker.py b/cr_checker/tests/test_cr_checker.py index b689ff43..9b1b1c5d 100644 --- a/cr_checker/tests/test_cr_checker.py +++ b/cr_checker/tests/test_cr_checker.py @@ -100,7 +100,6 @@ def prepare_test_no_header(request, tmp_path): extension = request.param test_file = tmp_path / ("file." + extension) header_template = load_template(extension) - current_year = datetime.now().year test_file.write_text( "some content\n", encoding="utf-8", @@ -1412,3 +1411,226 @@ def test_atomic_write_succeeds_and_leaves_no_temp_file(tmp_path): assert target.read_text(encoding="utf-8") == "new content\n" leftover = [p for p in tmp_path.iterdir() if p != target] assert leftover == [] + + +# --- comment form per language (block / line comments) ----------------------- + + +def _rendered(extension): + return load_template(extension).format(year=datetime.now().year, author="Author") + + +def _to_slash(block_header): + """Rewrites a ``/* */`` header as a ``//`` header with a shorter border.""" + lines = block_header.splitlines() + out = ["// " + "*" * 77] + out += [("// " + line[3:]) if line.startswith(" * ") else "//" for line in lines[1:-1]] + out.append("// " + "*" * 77) + return "\n".join(out) + "\n" + + +def _to_block(slash_header): + """Rewrites a ``//`` header as a ``/* */`` header with a shorter border.""" + lines = slash_header.splitlines() + out = ["/" + "*" * 77] + out += [(" * " + line[3:]) if line.startswith("// ") else " *" for line in lines[1:-1]] + out.append(" " + "*" * 77 + "/") + return "\n".join(out) + "\n" + + +def _alternative_form_header(extension): + """Header for `extension` in the comment form its template doesn't use.""" + rendered = _rendered(extension) + return _to_block(rendered) if extension == "rs" else _to_slash(rendered) + + +def _check(cr_checker, path, extension, fix=False, config=None, force=False): + return cr_checker.process_files( + [path], + {extension: load_template(extension)}, + fix, + config=config, + use_mmap=False, + encoding="utf-8", + offset=0, + remove_offset=0, + force=force, + ) + + +@pytest.mark.parametrize("extension", ["c", "cpp", "h", "hpp", "trlc", "rsl"]) +def test_style_variants_adds_slash_form_to_block_template(extension): + cr_checker = load_cr_checker_module() + template = load_template(extension) + + variants = cr_checker.style_variants(template, extension) + + assert len(variants) == 2 + assert variants[0] == template + lines = variants[1].splitlines() + assert lines[0].startswith("// *****") + assert lines[-1].startswith("// *****") + assert "// Copyright (c) {year} {author}" in lines + assert "//" in lines + assert not any(line.startswith(" *") for line in lines) + + +def test_style_variants_adds_block_form_to_slash_template(): + cr_checker = load_cr_checker_module() + template = load_template("rs") + + variants = cr_checker.style_variants(template, "rs") + + assert len(variants) == 2 + assert variants[0] == template + lines = variants[1].splitlines() + assert lines[0].startswith("/*****") + assert lines[-1].startswith(" *****") and lines[-1].endswith("*/") + assert " * Copyright (c) {year} {author}" in lines + assert " *" in lines + + +@pytest.mark.parametrize("extension", ["py", "sh", "bzl", "BUILD", "bazel", "yml", "yaml", "ini", "rst", "md", "puml"]) +def test_style_variants_keeps_single_form_for_other_languages(extension): + cr_checker = load_cr_checker_module() + template = load_template(extension) + + assert cr_checker.style_variants(template, extension) == (template,) + + +def test_style_variants_ignores_extensions_without_allowed_forms(): + cr_checker = load_cr_checker_module() + template = load_template("cpp") + + assert cr_checker.style_variants(template, "txt") == (template,) + + +def test_style_variants_ignores_template_without_borders(): + cr_checker = load_cr_checker_module() + template = "// Copyright (c) {year} {author}\n//\n// SPDX-License-Identifier: Apache-2.0\n" + + assert cr_checker.style_variants(template, "h") == (template,) + + +@pytest.mark.parametrize("extension", ["c", "cpp", "h", "hpp", "trlc", "rsl", "rs"]) +def test_process_files_accepts_header_in_alternative_comment_form(tmp_path, extension): + cr_checker = load_cr_checker_module() + test_file = tmp_path / f"file.{extension}" + content = _alternative_form_header(extension) + "\nbody\n" + test_file.write_text(content, encoding="utf-8") + + check_results = _check(cr_checker, test_file, extension) + fix_results = _check(cr_checker, test_file, extension, fix=True, config=write_config(tmp_path, "Author")) + + zero = {"missing": 0, "misplaced": 0, "wrong_format": 0, "duplicate": 0, "license_mismatch": 0, "fixed": 0} + assert check_results == zero + assert fix_results == zero + assert test_file.read_text(encoding="utf-8") == content + + +@pytest.mark.parametrize( + "extension,header", + [ + ("py", _to_slash(_rendered("cpp"))), + ("sh", _to_slash(_rendered("cpp"))), + ("md", _to_slash(_rendered("cpp"))), + ("puml", _to_slash(_rendered("cpp"))), + ("h", _rendered("py")), + ("trlc", _rendered("py")), + ("rs", _rendered("py")), + ], +) +def test_process_files_rejects_comment_form_not_allowed_for_language(tmp_path, extension, header): + cr_checker = load_cr_checker_module() + test_file = tmp_path / f"file.{extension}" + test_file.write_text(header + "\nbody\n", encoding="utf-8") + + results = _check(cr_checker, test_file, extension) + + assert results["wrong_format"] == 1 + + +def test_process_files_reports_license_mismatch_for_alternative_comment_form(tmp_path): + cr_checker = load_cr_checker_module() + test_file = tmp_path / "file.h" + content = _to_slash(_rendered("h")).replace("Apache-2.0", "MIT") + "\nbody\n" + test_file.write_text(content, encoding="utf-8") + config = write_config(tmp_path, "Author") + + check_results = _check(cr_checker, test_file, "h") + fix_results = _check(cr_checker, test_file, "h", fix=True, config=config, force=True) + + assert check_results["license_mismatch"] == 1 + assert fix_results["fixed"] == 0 + assert test_file.read_text(encoding="utf-8") == content + + +def test_process_files_detects_duplicate_header_in_alternative_comment_form(tmp_path): + cr_checker = load_cr_checker_module() + test_file = tmp_path / "file.h" + header = _to_slash(_rendered("h")) + test_file.write_text(header + header + "\nbody\n", encoding="utf-8") + + results = _check(cr_checker, test_file, "h") + + assert results["duplicate"] == 1 + + +def test_process_files_fix_repairs_broken_header_in_its_own_comment_form(tmp_path): + cr_checker = load_cr_checker_module() + test_file = tmp_path / "file.h" + broken = _to_slash(_rendered("h")).replace("Apache License", "Apache Licence") + test_file.write_text(broken + "\nbody\n", encoding="utf-8") + config = write_config(tmp_path, "Author") + + fix_results = _check(cr_checker, test_file, "h", fix=True, config=config) + + fixed = test_file.read_text(encoding="utf-8") + assert fix_results["fixed"] == 1 + assert fixed.startswith("// *****") + assert "Licence" not in fixed + assert "/*" not in fixed + assert not any(line.startswith(" *") for line in fixed.splitlines()) + assert fixed.endswith("\n\nbody\n") + assert _check(cr_checker, test_file, "h")["wrong_format"] == 0 + + +def test_process_files_fix_repairs_broken_block_header_in_block_form(tmp_path): + cr_checker = load_cr_checker_module() + test_file = tmp_path / "file.rs" + broken = _to_block(_rendered("rs")).replace("Apache License", "Apache Licence") + test_file.write_text(broken + "\nfn main() {}\n", encoding="utf-8") + config = write_config(tmp_path, "Author") + + fix_results = _check(cr_checker, test_file, "rs", fix=True, config=config) + + fixed = test_file.read_text(encoding="utf-8") + assert fix_results["fixed"] == 1 + assert fixed.startswith("/*****") + assert "Licence" not in fixed + assert not any(line.startswith("//") for line in fixed.splitlines()) + assert _check(cr_checker, test_file, "rs")["wrong_format"] == 0 + + +def test_process_files_fix_uses_template_form_for_missing_header(tmp_path): + cr_checker = load_cr_checker_module() + test_file = tmp_path / "file.h" + test_file.write_text("int x;\n", encoding="utf-8") + config = write_config(tmp_path, "Author") + + _check(cr_checker, test_file, "h", fix=True, config=config) + + assert test_file.read_text(encoding="utf-8").startswith("/*****") + + +def test_select_variant_follows_the_files_comment_form(): + cr_checker = load_cr_checker_module() + variants = cr_checker.style_variants(load_template("h"), "h") + + def pick(header): + return cr_checker.select_variant(cr_checker.locate_header(header + "int x;\n"), variants, None) + + assert pick(_rendered("h")) == variants[0] + assert pick(_to_slash(_rendered("h"))) == variants[1] + assert pick(_to_slash(_rendered("h")).replace("Apache License", "Apache Licence")) == variants[1] + assert cr_checker.select_variant(cr_checker.locate_header("int x;\n"), variants, None) == variants[0] diff --git a/cr_checker/tests/test_cr_checker_integration.py b/cr_checker/tests/test_cr_checker_integration.py index 87802834..7dba54e8 100644 --- a/cr_checker/tests/test_cr_checker_integration.py +++ b/cr_checker/tests/test_cr_checker_integration.py @@ -121,6 +121,50 @@ def test_fix_then_check_across_comment_styles(tmp_path, extension): assert ("\n\n\n" + body) not in fixed_content +# --- Block and line comment forms accepted side by side --------------------- + + +def _slash_form(block_header): + lines = block_header.splitlines() + body = [("// " + line[3:]) if line.startswith(" * ") else "//" for line in lines[1:-1]] + return "\n".join(["// " + "*" * 77, *body, "// " + "*" * 77]) + "\n" + + +def test_main_accepts_mixed_block_and_line_comment_headers(tmp_path): + cr_checker = load_cr_checker_module() + config_file = write_config(tmp_path, "Author") + block_header = load_template("cpp").format(year=datetime.now().year, author="Author") + files = {"block.h": block_header, "line.h": _slash_form(block_header), "line.cpp": _slash_form(block_header)} + for name, header in files.items(): + (tmp_path / name).write_text(header + "\nint x;\n", encoding="utf-8") + paths = [str(tmp_path / name) for name in files] + + exit_code = cr_checker.main(["-t", str(REAL_TEMPLATES_FILE), "-c", str(config_file), *paths]) + + assert exit_code == 0 + + +def test_main_fix_keeps_each_files_comment_form(tmp_path): + cr_checker = load_cr_checker_module() + config_file = write_config(tmp_path, "Author") + block_header = load_template("cpp").format(year=datetime.now().year, author="Author") + block_file = tmp_path / "block.h" + line_file = tmp_path / "line.h" + block_file.write_text(block_header.replace("Apache License", "Apache Licence") + "\nint x;\n", encoding="utf-8") + line_file.write_text( + _slash_form(block_header).replace("Apache License", "Apache Licence") + "\nint x;\n", encoding="utf-8" + ) + args = ["-t", str(REAL_TEMPLATES_FILE), "-c", str(config_file)] + + fix_exit_code = cr_checker.main([*args, "--fix", str(block_file), str(line_file)]) + check_exit_code = cr_checker.main([*args, str(block_file), str(line_file)]) + + assert fix_exit_code == 1 + assert check_exit_code == 0 + assert block_file.read_text(encoding="utf-8").startswith("/*") + assert line_file.read_text(encoding="utf-8").startswith("// ") + + # --- Duplicate header detection, exercised through the CLI ----------------- diff --git a/cr_checker/tool/cr_checker.py b/cr_checker/tool/cr_checker.py index 9b099469..6f74fe54 100755 --- a/cr_checker/tool/cr_checker.py +++ b/cr_checker/tool/cr_checker.py @@ -666,6 +666,94 @@ def compile_template_regex(template): return re.compile("".join(regex_parts) + "\n?") +@dataclass(frozen=True) +class CommentStyle: + """Comment syntax a header can be written in. + + A header is an opening border line, text lines and a closing border line: + ``first + run``, ``middle + text`` and ``last + run + suffix``. + """ + + first: str + middle: str + last: str + suffix: str + + def split(self, template): + """Returns ``(first_run, texts, last_run)`` if `template` is written in this + style, else None. The runs are the border fill strings.""" + lines = template.rstrip("\n").split("\n") + if len(lines) < 3: + return None + first, *middle, last = lines + if not (first.startswith(self.first) and last.startswith(self.last) and last.endswith(self.suffix)): + return None + first_run = first[len(self.first) :] + last_run = last[len(self.last) : len(last) - len(self.suffix)] + if not (BORDER_FILL_PATTERN.fullmatch(first_run) and BORDER_FILL_PATTERN.fullmatch(last_run)): + return None + texts = [] + for line in middle: + if line == self.middle.rstrip(): + texts.append("") + elif line.startswith(self.middle): + texts.append(line[len(self.middle) :]) + else: + return None + return first_run, texts, last_run + + def render(self, parts): + """Builds a template from `split` output.""" + first_run, texts, last_run = parts + lines = [self.first + first_run] + lines += [self.middle + text if text else self.middle.rstrip() for text in texts] + lines.append(self.last + last_run + self.suffix) + return "\n".join(lines) + "\n" + + +C_BLOCK_COMMENT = CommentStyle(first="/", middle=" * ", last=" ", suffix="/") +SLASH_LINE_COMMENT = CommentStyle(first="// ", middle="// ", last="// ", suffix="") + +# Comment forms the language accepts for a file header, by extension key. +# Extensions not listed accept only the template's own form. +ALLOWED_COMMENT_STYLES = { + extension: (C_BLOCK_COMMENT, SLASH_LINE_COMMENT) for extension in ("c", "cpp", "h", "hpp", "rs", "trlc", "rsl") +} + + +@functools.lru_cache(maxsize=None) +def style_variants(template, extension): + """Returns `template` followed by the same header in every other comment + form `extension` allows, or just `(template,)` when the template isn't + written in one of those forms.""" + styles = ALLOWED_COMMENT_STYLES.get(extension, ()) + for canonical in styles: + parts = canonical.split(template) + if parts: + return (template, *(style.render(parts) for style in styles if style is not canonical)) + return (template,) + + +def select_variant(layout, variants, config): + """Picks the variant to judge and rewrite the file's header against: the + first one the header matches, else the most similar one (so a broken + header is repaired in its own comment form), else the first one when + there is no header.""" + if len(variants) == 1 or not layout.blocks: + return variants[0] + block_text = layout.blocks[0].text + extended_text = layout.leading_junk + block_text + best, best_similarity = variants[0], -1.0 + for variant in variants: + regex = compile_template_regex(variant) + if regex.match(block_text) or regex.match(extended_text): + return variant + similarity = fuzz.ratio(block_text, render_template(variant, config)) + if similarity > best_similarity: + best, best_similarity = variant, similarity + return best + + def _blocks_in_duplicate_window(layout, template): """Blocks close enough to `layout.prefix_end` to count as (part of) the file's real header, for both DUPLICATE detection and rewriting. @@ -1186,7 +1274,9 @@ def _process_file_check(item, template, encoding, offset, use_mmap, config, resu text, _ = _strip_bom(raw) text = _normalize_line_endings(text) - layout = locate_header(text, manual_prefix_offset=offset, extension=extension_key(item)) + extension = extension_key(item) + layout = locate_header(text, manual_prefix_offset=offset, extension=extension) + template = select_variant(layout, style_variants(template, extension), config) status, similarity = classify(layout, template, config) _log_status(item, status, layout, similarity, template) _tally(status, results) @@ -1207,7 +1297,9 @@ def _process_file_fix(item, template, encoding, offset, remove_offset, config, r line_ending = _detect_line_ending(text) text = _normalize_line_endings(text) - layout = locate_header(text, manual_prefix_offset=offset, extension=extension_key(item)) + extension = extension_key(item) + layout = locate_header(text, manual_prefix_offset=offset, extension=extension) + template = select_variant(layout, style_variants(template, extension), config) status, similarity = classify(layout, template, config) _log_status(item, status, layout, similarity, template) _tally(status, results)