[clang-tools-extra] r211698 - Add clang-tidy-diff.py script to run clang-tidy and display warnings on changed lines only.

Alexander Kornienko alexfh at google.com
Thu Jun 26 07:46:40 PDT 2014


It seems like we can't require Python 2.7 for running tests, but I'm also
against rewriting the script using deprecated APIs. I'd better make this
test conditional on the availability of Python 2.7. What do you think?

Here's the patch which implements this:

Index: test/clang-tidy/clang-tidy-diff.cpp
===================================================================
--- test/clang-tidy/clang-tidy-diff.cpp (revision 211769)
+++ test/clang-tidy/clang-tidy-diff.cpp (working copy)
@@ -1,6 +1,7 @@
+// REQUIRES: python27
 // RUN: sed 's/placeholder_for_f/f/' %s > %t.cpp
 // RUN: clang-tidy -checks=-*,misc-use-override %t.cpp -- -std=c++11
| FileCheck -check-prefix=CHECK-SANITY %s
-// R_U_N: not diff -U0 %s %t.cpp | python
%S/../../clang-tidy/tool/clang-tidy-diff.py
-checks=-*,misc-use-override -- -std=c++11 2>&1 | FileCheck %s
+// RUN: not diff -U0 %s %t.cpp | python
%S/../../clang-tidy/tool/clang-tidy-diff.py
-checks=-*,misc-use-override -- -std=c++11 2>&1 | FileCheck %s
 struct A {
   virtual void f() {}
   virtual void g() {}
Index: test/lit.cfg
===================================================================
--- test/lit.cfg        (revision 211769)
+++ test/lit.cfg        (working copy)
@@ -174,3 +174,8 @@
 # ANSI escape sequences in non-dumb terminal
 if platform.system() not in ['Windows']:
     config.available_features.add('ansi-escape-sequences')
+
+import sys
+# Scripts using argparse need Python 2.7.
+if sys.version_info >= (2, 7):
+  config.available_features.add('python27')



On Thu, Jun 26, 2014 at 1:45 AM, Alexander Kornienko <alexfh at google.com>
wrote:

> On Thu, Jun 26, 2014 at 1:24 AM, NAKAMURA Takumi <geek4civic at gmail.com>
> wrote:
>
>> 2014-06-25 23:09 GMT+09:00 Alexander Kornienko <alexfh at google.com>:
>> > Author: alexfh
>> > Date: Wed Jun 25 09:09:52 2014
>> > New Revision: 211698
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=211698&view=rev
>> > Log:
>> > Add clang-tidy-diff.py script to run clang-tidy and display warnings on
>> changed lines only.
>> >
>> > Reviewers: djasper
>> >
>> > Reviewed By: djasper
>> >
>> > Subscribers: cfe-commits
>> >
>> > Differential Revision: http://reviews.llvm.org/D4288
>> >
>> > Added:
>> >     clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py   (with
>> props)
>> >     clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp
>> >
>> > Added: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py?rev=211698&view=auto
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py (added)
>> > +++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py Wed Jun
>> 25 09:09:52 2014
>> > @@ -0,0 +1,115 @@
>> > +#!/usr/bin/python
>> > +#
>> > +#===- clang-tidy-diff.py - ClangTidy Diff Checker ------------*-
>> python -*--===#
>> > +#
>> > +#                     The LLVM Compiler Infrastructure
>> > +#
>> > +# This file is distributed under the University of Illinois Open Source
>> > +# License. See LICENSE.TXT for details.
>> > +#
>> >
>> +#===------------------------------------------------------------------------===#
>> > +
>> > +r"""
>> > +ClangTidy Diff Checker
>> > +======================
>> > +
>> > +This script reads input from a unified diff, runs clang-tidy on all
>> changed
>> > +files and outputs clang-tidy warnings in changed lines only. This is
>> useful to
>> > +detect clang-tidy regressions in the lines touched by a specific patch.
>> > +Example usage for git/svn users:
>> > +
>> > +  git diff -U0 HEAD^ | clang-tidy-diff.py -p1
>> > +  svn diff --diff-cmd=diff -x-U0 | \
>> > +      clang-tidy-diff.py -fix -checks=-*,misc-use-override
>> > +
>> > +"""
>> > +
>> > +import argparse
>>
>> argparse has been introduced since 2.7. I suppose we can still use 2.6.
>>
>
> Python 2.7 has been available since 2010, and it should be available
> virtually everywhere. I've not seen any python-related buildbot failures
> after this patch, so I suppose they all use up-to-date python. Can you
> provide more details: where is python 2.6 still used and why do we need to
> support it?
>
>
>> Suppressed in r211741. Could you rewrite to avoid argparse?
>>
>
> Python 2.6 had optparse, which has been deprecated since 2.7. I don't
> think it's a good idea to rewrite the script using deprecated api.
>
>
>> > +import json
>> > +import re
>> > +import subprocess
>> > +import sys
>> > +
>> > +
>> > +def main():
>> > +  parser = argparse.ArgumentParser(description=
>> > +                                   'Reformat changed lines in diff.
>> Without -i '
>> > +                                   'option just output the diff that
>> would be '
>> > +                                   'introduced.')
>> > +  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
>> reformat '
>> > +                      '(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
>> reformat '
>> > +                      '(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='')
>> > +  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}(\S*)' % 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
>> > +
>> > +    match = re.search('^@@.*\+(\d+)(,(\d+))?', line)
>> > +    if match:
>> > +      start_line = int(match.group(1))
>> > +      line_count = 1
>> > +      if match.group(3):
>> > +        line_count = int(match.group(3))
>> > +      if line_count == 0:
>> > +        continue
>> > +      end_line = start_line + line_count - 1;
>> > +      lines_by_file.setdefault(filename, []).append([start_line,
>> end_line])
>> > +
>> > +  if len(lines_by_file) == 0:
>> > +    print "No relevant changes found."
>> > +    sys.exit(0)
>> > +
>> > +  line_filter_json = json.dumps(
>> > +    [{"name" : name, "lines" : lines_by_file[name]} for name in
>> lines_by_file],
>> > +    separators = (',', ':'))
>> > +
>> > +  # Run clang-tidy on files containing changes.
>> > +  command = [args.clang_tidy_binary]
>> > +  command.append('-line-filter=\'' + line_filter_json + '\'')
>> > +  if args.fix:
>> > +    command.append('-fix')
>> > +  if args.checks != '':
>> > +    command.append('-checks=\'' + args.checks + '\'')
>> > +  command.extend(lines_by_file.keys())
>> > +  command.extend(clang_tidy_args)
>> > +
>> > +  sys.exit(subprocess.call(' '.join(command), shell=True))
>> > +
>> > +if __name__ == '__main__':
>> > +  main()
>> >
>> > Propchange: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
>> >
>> ------------------------------------------------------------------------------
>> >     svn:executable = *
>> >
>> > Added: clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp?rev=211698&view=auto
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp (added)
>> > +++ clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp Wed Jun
>> 25 09:09:52 2014
>> > @@ -0,0 +1,18 @@
>> > +// RUN: sed 's/placeholder_for_f/f/' %s > %t.cpp
>> > +// RUN: clang-tidy -checks=-*,misc-use-override %t.cpp -- -std=c++11 |
>> FileCheck -check-prefix=CHECK-SANITY %s
>> > +// RUN: not diff -u0 %s %t.cpp | python
>> %S/../../clang-tidy/tool/clang-tidy-diff.py -checks=-*,misc-use-override --
>> -std=c++11 2>&1 | FileCheck %s
>> > +struct A {
>> > +  virtual void f() {}
>> > +  virtual void g() {}
>> > +};
>> > +// CHECK-NOT: warning
>> > +struct B : public A {
>> > +  void placeholder_for_f() {}
>> > +// CHECK-SANITY: [[@LINE-1]]:8: warning: Use exactly
>> > +// CHECK: [[@LINE-2]]:8: warning: Use exactly
>> > +  void g() {}
>> > +// CHECK-SANITY: [[@LINE-1]]:8: warning: Use exactly
>> > +// CHECK-NOT: warning:
>> > +};
>> > +// CHECK-SANITY-NOT: Suppressed
>> > +// CHECK: Suppressed 1 warnings (1 due to line filter).
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140626/30fe4f2e/attachment.html>


More information about the cfe-commits mailing list