[clang-tidy] Add -path option to clang-tidy-diff.py

[• ehsan]

Summary

This flag allows specifying a custom path for the compilation
database. Unfortunately we can't use the -p flag like other
clang-tidy tools because it's already taken.

[alexfh]

What's your use case? Can it be addressed by just forwarding the -p flag to clang-tidy?

The script shouldn't know anything about implementation details of the compilation database being used (since it can be something other than JSON compilation database).

[• ehsan]

In D29806#673329, @alexfh wrote:

What's your use case? Can it be addressed by just forwarding the -p flag to clang-tidy?

I just need to pass the full path to the compilation DB to clang-tidy. The problem is that invoking clang-tidy-diff.py -- -p PATH will run clang-tidy -- -p PATH, in order words adding -p to the compiler command line, not clang-tidy's.

The script shouldn't know anything about implementation details of the compilation database being used (since it can be something other than JSON compilation database).

I copied the code to look for the DB verbatim from run-clang-tidy.py. I personally don't need the search logic, and just tried to keep this consistent with run-clang-tidy.py. I'd be happy to remove the search logic if you prefer that.

[• ehsan]

Gentle ping. :-)

[alexfh]

In D29806#673489, @ehsan wrote:

In D29806#673329, @alexfh wrote:

What's your use case? Can it be addressed by just forwarding the -p flag to clang-tidy?

I just need to pass the full path to the compilation DB to clang-tidy. The problem is that invoking clang-tidy-diff.py -- -p PATH will run clang-tidy -- -p PATH, in order words adding -p to the compiler command line, not clang-tidy's.

I meant adding -p flag to the script and forwarding it to clang-tidy the same way as -quiet, -checks, etc. But there's already -p flag with a different meaning, so calling the new flag -path makes sense. I would still just forward it, if it's present.

The script shouldn't know anything about implementation details of the compilation database being used (since it can be something other than JSON compilation database).

I copied the code to look for the DB verbatim from run-clang-tidy.py. I personally don't need the search logic, and just tried to keep this consistent with run-clang-tidy.py. I'd be happy to remove the search logic if you prefer that.

I see. I guess, this is needed as an optimization or something like that. Not useful for clang-tidy-diff.py.

clang-tidy/tool/clang-tidy-diff.py

34–44

Let's remove this.

78–85

Let's remove this.

129

Let's only add this flag when args.build_path is not None.

[• ehsan]

Addressed the review comments.

[alexfh]

LG

clang-tidy/tool/clang-tidy-diff.py

 
 import argparse
 import json
 import re
 import subprocess
 import sys
 
 
 def main():
   parser = argparse.ArgumentParser(description=
                                    'Run clang-tidy against changed files, and '
                                    'output diagnostics only for modified '
                                    'lines.')
   parser.add_argument('-clang-tidy-binary', metavar='PATH',
                       default='clang-tidy',
                       help='path to clang-tidy binary')
   parser.add_argument('-p', metavar='NUM', default=0,
                       help='strip the smallest prefix containing P slashes')
   parser.add_argument('-regex', metavar='PATTERN', default=None,
                       help='custom pattern selecting file paths to check '
                       '(case sensitive, overrides -iregex)')
   parser.add_argument('-iregex', metavar='PATTERN', default=
                       r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hpp|m|mm|inc)',
                       help='custom pattern selecting file paths to check '
                       '(case insensitive, overridden by -regex)')
 
   parser.add_argument('-fix', action='store_true', default=False,
                       help='apply suggested fixes')
   parser.add_argument('-checks',
                       help='checks filter, when not specified, use clang-tidy '
                       'default',
                       default='')
+  parser.add_argument('-path', dest='build_path',
+                      help='Path used to read a compile command database.')
   parser.add_argument('-extra-arg', dest='extra_arg',
                       action='append', default=[],
                       help='Additional argument to append to the compiler '
                       'command line.')
   parser.add_argument('-extra-arg-before', dest='extra_arg_before',
                       action='append', default=[],
                       help='Additional argument to prepend to the compiler '
                       'command line.')
   parser.add_argument('-quiet', action='store_true', default=False,
                       help='Run clang-tidy in quiet mode')
   clang_tidy_args = []
   argv = sys.argv[1:]
   if '--' in argv:
     clang_tidy_args.extend(argv[argv.index('--'):])
     argv = argv[:argv.index('--')]
 
   args = parser.parse_args(argv)
 
   # Extract changed lines for each file.
   filename = None
   lines_by_file = {}
   for line in sys.stdin:
     match = re.search('^\+\+\+\ \"?(.*?/){%s}([^ \t\n\"]*)' % args.p, line)
     if match:
       filename = match.group(2)
     if filename == None:
       continue
 
     if args.regex is not None:
       if not re.match('^%s$' % args.regex, filename):
         continue
     else:
       if not re.match('^%s$' % args.iregex, filename, re.IGNORECASE):
         continue
def main():
   command = [args.clang_tidy_binary]
   command.append('-line-filter=' + quote + line_filter_json + quote)
   if args.fix:
     command.append('-fix')
   if args.checks != '':
     command.append('-checks=' + quote + args.checks + quote)
   if args.quiet:
     command.append('-quiet')
+  if args.build_path is not None:
+    command.append('-p=%s' % args.build_path)
   command.extend(lines_by_file.keys())
   for arg in args.extra_arg:
       command.append('-extra-arg=%s' % arg)
   for arg in args.extra_arg_before:
       command.append('-extra-arg-before=%s' % arg)
   command.extend(clang_tidy_args)
 
   sys.exit(subprocess.call(' '.join(command), shell=True))
 
 if __name__ == '__main__':
   main()

test/clang-tidy/clang-tidy-diff.cpp

 // RUN: sed 's/placeholder_for_f/f/' %s > %t.cpp
 // RUN: clang-tidy -checks=-*,modernize-use-override %t.cpp -- -std=c++11 | FileCheck -check-prefix=CHECK-SANITY %s
 // RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -- -std=c++11 2>&1 | FileCheck %s
 // RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -quiet -- -std=c++11 2>&1 | FileCheck -check-prefix=CHECK-QUIET %s
+// RUN: mkdir -p %T/compilation-database-test/
+// RUN: echo '[{"directory": "%T", "command": "clang++ -o test.o -std=c++11 %t.cpp", "file": "%t.cpp"}]' > %T/compilation-database-test/compile_commands.json
+// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -path %T/compilation-database-test 2>&1 | FileCheck -check-prefix=CHECK %s
 struct A {
   virtual void f() {}
   virtual void g() {}
 };
 // CHECK-NOT: warning:
 // CHECK-QUIET-NOT: warning:
 struct B : public A {
   void placeholder_for_f() {}