Use a System clang-tidy When Available and Make --fix Converge in One Pass (#16199)

* Use a System clang-tidy When Available and Make --fix Converge in One Pass

scripts/run_clang_tidy.sh only looked at CLANG_TIDY and the venv it creates,
so a clang-tidy already on the system was never used. It is now the first
choice: the pinned version outright, another version after a prompt that
says results may differ slightly from CI, which -y and an existing pinned
venv skip.

Two problems in clang_tidy_diff.py made --fix need several runs and still
leave the plain check failing:

- A deleted #include orphans uses on unchanged lines. The plain check runs
  such a file whole and reports them, but --fix kept the line filter to the
  changed lines, so they were never fixed. Fix mode now runs the file whole
  first and then fixes exactly the changed lines plus the lines that run
  found wanting, so unrelated lines are still never rewritten.

- clang-tidy exits non-zero for the findings it just fixed, so every fixed
  file was reported as failed and the user ran --fix again to see what was
  left. A file --fix changed is now checked again and the fixed files are
  listed separately from what --fix could not add.

CI runs the script without --fix and is unchanged.

* Keep the a/ b/ Diff Prefixes Whatever the User's Git Config Says

parse_diff recognises a changed file by its +++ b/ header. With
diff.noprefix or diff.mnemonicPrefix set, git prints +++ src/x.cpp or
+++ w/src/x.cpp instead, every file was dropped, and the local check
reported no changed C++ lines. The diff is now asked for the a/ and b/
prefixes outright, which overrides both settings.

* Warn When No Remote Points at OrcaSlicer/OrcaSlicer

Without one, run_clang_tidy.sh compares against origin/main. When origin
is a fork whose main already holds the commits, the check finds nothing
and says so, without hinting at why. The script now names the base it
fell back to and how to point it at the upstream repository.
This commit is contained in:
HanifKoh
2026-10-06 17:16:09 +08:00
committed by GitHub
parent f8dd56053c
commit d1afb1fed6
3 changed files with 142 additions and 24 deletions
+57
View File
@@ -155,6 +155,8 @@ class TestSubprocessCalls(unittest.TestCase):
diff = "+++ b/src/libslic3r/Über.cpp\n@@ -1,0 +2 @@\n+// 打印\n"
files, call, _ = self.run_patched(clang_tidy_diff.changed_files, "base", stdout=diff)
self.assertIn("core.quotePath=false", call.args[0])
# Whatever diff.noprefix or diff.mnemonicPrefix a user has set.
self.assertIn("--dst-prefix=b/", call.args[0])
self.assertEqual(call.kwargs["encoding"], "utf-8")
self.assertEqual(files["src/libslic3r/Über.cpp"].lines, [[2, 2]])
@@ -167,5 +169,60 @@ class TestSubprocessCalls(unittest.TestCase):
self.assertEqual(call.kwargs["encoding"], "utf-8")
class TestCheckFileFix(unittest.TestCase):
"""check_file with -- --fix: what clang-tidy is run on, and what is reported afterwards."""
def setUp(self):
self.dir = tempfile.TemporaryDirectory()
self.addCleanup(self.dir.cleanup)
self.path = os.path.join(self.dir.name, "Color.cpp")
with open(self.path, "w") as f:
f.write("int x;\n")
def finding(self, line, include=""):
return clang_tidy_diff.Diagnostic(self.path, line, 1, "error",
'no header providing "x" is directly included [misc-include-cleaner]', include)
def check(self, change, results, fix_writes=None):
"""Run check_file with run_clang_tidy answering from `results` in turn; the --fix run
rewrites the file with `fix_writes` when given. Returns (result, calls)."""
calls = []
def run(clang_tidy, build_dir, path, lines, extra_args):
calls.append((lines, extra_args))
if "--fix" in extra_args and fix_writes is not None:
with open(path, "w") as f:
f.write(fix_writes)
return results[len(calls) - 1]
with mock.patch.object(clang_tidy_diff, "run_clang_tidy", side_effect=run):
result = clang_tidy_diff.check_file("clang-tidy", "build", "base", self.path, change, ["--fix"])
return result, calls
def test_a_deleted_include_is_fixed_on_the_lines_it_orphaned_only(self):
change = clang_tidy_diff.FileChange(lines=[[4, 4]], removed_includes={"libslic3r/Point.hpp"})
orphaned = self.finding(50, "<libslic3r/Point.hpp>")
unrelated = self.finding(60, "<vector>")
(failed, _, failing, fixed), calls = self.check(
change, [(1, "", [orphaned, unrelated]), (1, "", [orphaned]), (0, "", [])], fix_writes="#include <libslic3r/Point.hpp>\n")
self.assertEqual(calls, [(None, []), ([[4, 4], [50, 50]], ["--fix"]), (None, [])])
self.assertEqual((failed, failing, fixed), (False, [], True))
def test_a_file_the_fix_did_not_change_keeps_its_findings(self):
change = clang_tidy_diff.FileChange(lines=[[4, 4]])
error = clang_tidy_diff.Diagnostic(self.path, 4, 1, "error", "unknown type name 'Foo' [clang-diagnostic-error]")
(failed, _, failing, fixed), calls = self.check(change, [(1, "", [error])])
self.assertEqual(calls, [([[4, 4]], ["--fix"])])
self.assertEqual((failed, failing, fixed), (True, [error], False))
def test_a_changed_file_is_checked_again_and_reports_what_is_left(self):
change = clang_tidy_diff.FileChange(lines=[[4, 4]])
error = clang_tidy_diff.Diagnostic(self.path, 4, 1, "error", "unknown type name 'Foo' [clang-diagnostic-error]")
(failed, _, failing, fixed), calls = self.check(
change, [(1, "", [self.finding(4, "<vector>"), error]), (1, "", [error])], fix_writes="#include <vector>\n")
self.assertEqual(calls, [([[4, 4]], ["--fix"]), ([[4, 4]], [])])
self.assertEqual((failed, failing, fixed), (True, [error], True))
if __name__ == "__main__":
unittest.main()