diff --git a/tools/check_conventions.py b/tools/check_conventions.py index c9eefd7222..de1af96502 100644 --- a/tools/check_conventions.py +++ b/tools/check_conventions.py @@ -76,17 +76,23 @@ class Check: self.blobs = {} self.tree = set(self.modes) self.dirs = {str(parent) for path in self.tree for parent in PurePosixPath(path).parents} - statuses = git("diff", "--name-status", "--no-renames", "-z", self.base, head).split("\0") - for status, path in zip(statuses[::2], statuses[1::2]): - self.status[path] = status - for line in git("diff", "--numstat", "--no-renames", "-z", self.base, head).split("\0"): - if not line: - continue + diff = ("--find-renames", "-l0", self.base, head) + statuses = iter(git("diff", "--name-status", "-z", *diff).split("\0")[:-1]) + for status in statuses: + path = next(statuses) + if status.startswith("R"): + path = next(statuses) + self.status[path] = status[0] + statistics = iter(git("diff", "--numstat", "-z", *diff).split("\0")[:-1]) + for line in statistics: added, deleted, path = line.split("\t", 2) + if not path: + next(statistics) + path = next(statistics) self.numstat[path] = (added, deleted) paths = iter([path for path, status in self.status.items() for _ in range(2 if status == "T" else 1)]) path, number = None, None - for line in git("diff", "-U0", "--no-color", "--no-renames", self.base, head).split("\n"): + for line in git("diff", "-U0", "--no-color", *diff).split("\n"): if line.startswith("diff --git "): path, number = next(paths), None elif line.startswith("@@"): @@ -206,11 +212,15 @@ class Check: suffix = PurePosixPath(path).suffix.lower() if self.status.get(path) != "D" and (added == "-" or suffix in IMAGES or not utf8(self.blob(path))): self.report("binary", path, 0) - if self.status.get(path) == "A" and allowed_notes(path) is False: + if self.status.get(path) in ("A", "R") and allowed_notes(path) is False: self.report("notes-file", path, 0) def docs(self): - for path, lines in self.files(lambda p: p.suffix == ".md"): + files = dict(self.files(lambda p: p.suffix == ".md")) + for path, status in self.status.items(): + if status == "R" and not path.startswith("3rdparty/") and PurePosixPath(path).suffix == ".md": + files[path] = list(enumerate(self.show(path).split("\n"), 1)) + for path, lines in files.items(): fenced = fenced_lines(self.show(path)) for number, text in lines: name = debt_name(text) if is_debt(path) and text.startswith("- ") else None diff --git a/tools/tests/test_check_conventions_comments.py b/tools/tests/test_check_conventions_comments.py index 539cbc1b74..867c3d1a0d 100644 --- a/tools/tests/test_check_conventions_comments.py +++ b/tools/tests/test_check_conventions_comments.py @@ -498,6 +498,80 @@ class RepositoryChecksTests(unittest.TestCase): check.code() self.assertIn(("comment", "core/example.cpp", 3, ""), check.findings) + def test_unchanged_move_preserves_existing_comments_and_extensions(self): + source = self.root / "core/tests/Existing.cpp" + source.parent.mkdir(parents=True) + source.write_text("// existing explanation\n__attribute__((noinline)) int Existing() { return 1; }\n", encoding="utf-8") + self.commit() + destination = self.root / "core/tests/group/Café example.cpp" + destination.parent.mkdir() + source.rename(destination) + (self.root / "a.bin").write_bytes(b"binary\x00data") + (self.root / "README.md").unlink() + self.commit() + check = Check("HEAD~1", "HEAD") + check.code() + check.repository() + name = destination.relative_to(self.root).as_posix() + self.assertEqual(check.status[name], "R") + self.assertEqual(check.numstat[name], ("0", "0")) + self.assertNotIn(name, check.added) + self.assertEqual(check.findings, [("binary", "a.bin", 0, "")]) + + def test_edited_move_checks_new_lines_at_the_destination(self): + source = self.root / "core/tests/Existing.cpp" + source.parent.mkdir(parents=True) + original = "// existing explanation\n__attribute__((noinline)) int Existing() { return 1; }\n" + original += "".join(f"int Existing{index}() {{ return {index}; }}\n" for index in range(12)) + source.write_text(original, encoding="utf-8") + self.commit() + destination = self.root / "core/tests/group/Edited.cpp" + destination.parent.mkdir() + source.rename(destination) + destination.write_text(original + "// new explanation\n__declspec(noinline) int Added() { return 2; }\n", encoding="utf-8") + self.commit() + check = Check("HEAD~1", "HEAD") + check.code() + name = destination.relative_to(self.root).as_posix() + self.assertEqual(check.status[name], "R") + self.assertEqual(check.numstat[name], ("2", "0")) + self.assertEqual(check.findings, [("comment", name, 15, ""), ("extension", name, 16, "__declspec")]) + + def test_copied_code_is_still_checked_as_an_addition(self): + source = self.root / "core/tests/Existing.cpp" + source.parent.mkdir(parents=True) + source.write_text("// existing explanation\n__attribute__((noinline)) int Existing() { return 1; }\n", encoding="utf-8") + self.commit() + destination = self.root / "core/tests/Copy.cpp" + destination.write_bytes(source.read_bytes()) + self.commit() + check = Check("HEAD~1", "HEAD") + check.code() + name = destination.relative_to(self.root).as_posix() + self.assertEqual(check.status[name], "A") + self.assertEqual(check.findings, [("comment", name, 1, ""), ("extension", name, 2, "__attribute__")]) + + def test_moving_a_document_to_a_forbidden_path_is_rejected(self): + (self.root / "README.md").rename(self.root / "investigation.md") + self.commit() + check = Check("HEAD~1", "HEAD") + check.repository() + self.assertEqual(check.findings, [("notes-file", "investigation.md", 0, "")]) + + def test_moving_a_document_rechecks_relative_links(self): + source = self.root / "docs/dev/guide.md" + source.parent.mkdir(parents=True) + source.write_text("[reference](reference.md)\n", encoding="utf-8") + (source.parent / "reference.md").write_text("Reference\n", encoding="utf-8") + self.commit() + destination = self.root / "docs/dev/group/guide.md" + destination.parent.mkdir() + source.rename(destination) + self.commit() + check = Check("HEAD~1", "HEAD") + check.docs() + self.assertEqual(check.findings, [("doc-link", "docs/dev/group/guide.md", 1, "reference.md")]) + def test_unicode_raw_literals_keep_git_patch_and_grep_records_intact(self): literals = 'R"text(foo\u2028+tail\u2029+tail)text"' fixtures = (