From f08a74c84e1dafb23a7d5d9e70e17bf56929f6c4 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 17:18:07 -0400 Subject: [PATCH] copyright: --fix keeps blank lines (a blank comment line is not a notice line) The licence block's own blank comment lines (`#`, `//`) reduce to "", so _CANON_LINES held the empty string and canonical_notice_line() said yes to every blank or bare-comment line in a file. Two effects: - add_notice() dropped every such line in the 20-line header window. On #2183's 13 two-liner headers --fix gave +143 -68: it removed the separator after the description, the blank lines between #include groups and the one after `namespace {`. - repair_notice()'s doubled-block dedupe dropped every blank line after the first in the whole file (0 of 36 kept in the test case). Only lines with words in them can be a claim, so the set and the predicate now exclude the empty key. With this, --fix on the same 13 files gives +156 -0 (exactly the 12 licence lines each), and --check passes. tests/copyright_check_untracked_test.py: two cases compare the written file with the exact expected text, for the add path and for the repair path (blank lines past the header window). Main's tool fails both (lost 5, lost 7); with the fix all 19 pass. tests/copyright_gate_test.sh still passes. Co-Authored-By: Claude Opus 5.5 --- tests/copyright_check_untracked_test.py | 39 +++++++++++++++++++++++++ tools/copyright.py | 10 +++++-- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/tests/copyright_check_untracked_test.py b/tests/copyright_check_untracked_test.py index cbe0fd5c6..3b1888ce4 100755 --- a/tests/copyright_check_untracked_test.py +++ b/tests/copyright_check_untracked_test.py @@ -165,5 +165,44 @@ def git(repo, *args): check(rc.returncode != 0, "the same generated source TRACKED is still an error: it is repo content now", rc.stdout + rc.stderr) +# --fix must not delete blank lines (#2183 review). The licence block's own blank comment lines reduce to the empty +# string, and canonical_notice_line() treated "" as a notice line. So add_notice() deleted every blank or bare `//` +# line in the header window: on #2183's 13 two-liner headers --fix gave +143 -68, dropping the separator after the +# description, the blank lines between #include groups and the one after `namespace {`. And repair_notice()'s +# doubled-block dedupe dropped every blank line after the first in the WHOLE file. Both cases compare the written +# file with the exact expected text, so a single lost line fails them. +CPP_NOTICE = "".join(("//" + line[1:]) for line in NOTICE.splitlines(keepends=True)) +CPP_BODY = ("//\n// x.cpp -- what this file is for.\n//\n// A second paragraph.\n\n" + "#include \n#include \n\n#include \"x.h\"\n\nnamespace {\n\nint g = 0;\n\n} // namespace\n") +with tempfile.TemporaryDirectory() as repo: + git(repo, "init", "-q") + path = os.path.join(repo, "twoliner.cpp") + with open(path, "w") as f: + f.write("// Copyright 2026 bong-water-water-bong\n// SPDX-License-Identifier: Apache-2.0\n" + CPP_BODY) + run(repo, "--fix") + with open(path) as f: + fixed = f.read() + check(fixed == CPP_NOTICE + CPP_BODY, + "--fix on a two-liner header keeps every blank and bare `//` line below it", + "lost %d blank line(s)" % (CPP_BODY.count("\n\n") - fixed.count("\n\n"))) + rc = run(repo, "--check") + check(rc.returncode == 0, "...and the file it wrote passes --check", rc.stdout + rc.stderr) + +with tempfile.TemporaryDirectory() as repo: + git(repo, "init", "-q") + path = os.path.join(repo, "nearmiss.cpp") + # blank lines well past the 20-line header window: the repair path reads the whole file + long_body = CPP_BODY + "".join("int v%d = %d;\n" % (i, i) for i in range(30)) + "\nint z;\n\nint w;\n" + with open(path, "w") as f: + f.write(CPP_NOTICE + "// Copyright 2026 bong-water-bong\n// SPDX-License-Identifier: Apache-2.0\n" + long_body) + run(repo, "--fix") + with open(path) as f: + fixed = f.read() + check(fixed == CPP_NOTICE + long_body, + "--fix repairing a near-miss holder keeps every blank line in the file, past the header window too", + "lost %d blank line(s)" % (long_body.count("\n\n") - fixed[len(CPP_NOTICE):].count("\n\n"))) + rc = run(repo, "--check") + check(rc.returncode == 0, "...and the repaired file passes --check", rc.stdout + rc.stderr) + print(f"copyright_check_untracked_test: {len(FAIL)} failure(s)") sys.exit(1 if FAIL else 0) diff --git a/tools/copyright.py b/tools/copyright.py index 2e3888f6a..6af956bba 100755 --- a/tools/copyright.py +++ b/tools/copyright.py @@ -229,7 +229,12 @@ def near_miss_holder(line): return n >= HOLDER_NEAR_MISS_PREFIX -_CANON_LINES = {re.sub(r"^\W+", "", n).strip() for n in NOTICE} +# The licence block's own blank comment lines (`#`, `//`) reduce to "", so a set built from every NOTICE line held the +# empty string, and canonical_notice_line() said yes to every blank or bare-comment line in a file. add_notice() then +# deleted each one in the header window (the separator after the description, the blank lines between #include groups +# and after `namespace {`), and repair_notice()'s doubled-block dedupe deleted every blank line after the first in the +# whole file. Only lines with words in them can be a claim, so only those are canonical. +_CANON_LINES = {k for k in (re.sub(r"^\W+", "", n).strip() for n in NOTICE) if k} def canonical_notice_line(line): @@ -241,7 +246,8 @@ def canonical_notice_line(line): cover it by construction: it returns False for an exact holder, because an exact holder next to the canonical block is the *repaired* state. The block already contains both lines, so a copy inside the window is not a second claim. """ - return re.sub(r"^\W+", "", line).strip() in _CANON_LINES + key = re.sub(r"^\W+", "", line).strip() + return bool(key) and key in _CANON_LINES def repair_notice(text, block):