[clang-tidy] run-clang-tidy -export-fixes exports only fixes, not all warnings

[AlexanderLanin]

Summary

The generated file is now small enough to not hinder any post processing.

Create a yaml file to store suggested fixes in, which can be applied with clang-apply-replacements.

So it's enough to store the fixes, not all the warnings.
Before this change the resulting file was simply too large as it contained all warnings, least of which had FIX-ITs.

This is trivially achievable with no measureable runtime overhead during run-clang-tidy runs, instead of post processing where even parsing the file once is extremely slow.

[AlexanderLanin]

Important: I'm not a python expert. It works fine as far as I can tell. I would feel better if someone with more than a day python experience would say that it is fine.

[kimgr]

A small Python suggestion.

clang-tools-extra/clang-tidy/tool/run-clang-tidy.py

112

This double lookup is unnecessary, you can do for note in diag.get("Notes", []): to default to empty list if "Notes" is not in diag.

114

Same here (not sure if Replacements holds a list, but I think it might)

[AlexanderLanin]

Review findings applied (no real measurable difference in speed, maybe 100ms) but it's indeed more readable.
Without this fix the export took 12.96 seconds, with this patch 11.07 seconds.
Difference is even bigger when there are less auto fixable findings (70% with FIX-IT in my example).

As there is no test suite available I run this without and with the change and manually compared the output.
All warnings without FIX-IT have disappeared from exported, all warnings with FIX-IT are unmodified.

[AlexanderLanin]

On second thought maybe this should be fixed in clang-tidy and not here?
Maybe besides -export-fixes there should be an -export-warnings or just -yaml-export?

[alexfh]

In D72730#1866043, @AlexanderLanin wrote:

On second thought maybe this should be fixed in clang-tidy and not here?
Maybe besides -export-fixes there should be an -export-warnings or just -yaml-export?

I like the idea of introducing -export-warnings (to export warnings completely - with messages, notes, ranges and fixes) and changing -export-fixes to only export fixes.

clang-tools-extra/clang-tidy/tool/run-clang-tidy.py

def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
   start.append('-p=' + build_path)
   if quiet:
       start.append('-quiet')
   if config:
       start.append('-config=' + config)
   start.append(f)
   return start
 
+# returns whether passed yaml Diagnostic (see mergekey) element contains FIX-IT
+# FIX-IT replacements can be within the warning directly or within the notes.
+def isAutoFixable(diag):
+  if diag["DiagnosticMessage"]["Replacements"]: return True
+
+  for note in diag.get("Notes", []):
+    if note.get("Replacements", []):
+      return True
+
+  return False
 
 def merge_replacement_files(tmpdir, mergefile):
   """Merge all replacement files in a directory into a single file"""
   # The fixes suggested by clang-tidy >= 4.0.0 are given under
   # the top level key 'Diagnostics' in the output yaml files
   mergekey="Diagnostics"
   merged=[]
   for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')):
     content = yaml.safe_load(open(replacefile, 'r'))
     if not content:
       continue # Skip empty files.
-    merged.extend(content.get(mergekey, []))
+    for d in content.get(mergekey, []):
+      if isAutoFixable(d):
+        merged.append(d)
 
   if merged:
     # MainSourceFile: The key is required by the definition inside
     # include/clang/Tooling/ReplacementsYaml.h, but the value
     # is actually never used inside clang-apply-replacements,
     # so we set it to '' here.
     output = { 'MainSourceFile': '', mergekey: merged }
     with open(mergefile, 'w') as out: