[core] clang-tidy fixes

2 files changed, 217 insertions, 49 deletions

diff --git a/scripts/clang-tidy-diff.py b/scripts/clang-tidy-diff.py
new file mode 100755
index 0000000000..e5e0ff3911
--- /dev/null
+++ b/scripts/clang-tidy-diff.py
@@ -0,0 +1,201 @@
+#!/usr/bin/env 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=-*,modernize-use-override
+
+"""
+
+import argparse
+import json
+import re
+import subprocess
+import sys
+import multiprocessing
+import os
+import threading
+
+is_py2 = sys.version[0] == '2'
+
+if is_py2:
+    import Queue as queue
+else:
+    import queue as queue
+
+def run_tidy(command, lines_by_file, queue, failed_files):
+  """Takes filenames out of queue and runs clang-tidy on them."""
+  while True:
+    name = queue.get()
+
+    line_filter_json = json.dumps([{"name" : name, "lines" : lines_by_file[name]}], separators = (',', ':'))
+    if sys.platform == 'win32':
+      line_filter_json = re.sub(r'"', r'"""', line_filter_json)
+    else:
+      line_filter_json = "'" + line_filter_json + "'";
+
+    invocation = list(command)
+    invocation.append('-line-filter=' + line_filter_json)
+    invocation.append(name)
+
+    sys.stdout.write('Checking differences in {}...\n'.format(name))
+    return_code = subprocess.call(' '.join(invocation), shell=True)
+    if return_code != 0:
+      failed_files.append(name)
+    queue.task_done()
+
+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('-j', type=int, default=0,
+                      help='number of tidy instances to be run in parallel.')
+  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
+
+    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)
+
+  # Run clang-tidy on files containing changes.
+  command = [args.clang_tidy_binary]
+  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)
+  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)
+
+  try:
+    invocation = list(command)
+    invocation.append('-')
+    subprocess.check_call(invocation)
+  except:
+    print("Unable to run clang-tidy: {}".format(' '.join(command)))
+    sys.exit(1)
+
+  max_task = args.j
+  if max_task == 0:
+    max_task = multiprocessing.cpu_count()
+
+  return_code = 0
+  try:
+    # Spin up a bunch of tidy-launching threads.
+    task_queue = queue.Queue(max_task)
+    # List of files with a non-zero return code.
+    failed_files = []
+    for _ in range(max_task):
+      t = threading.Thread(target=run_tidy,
+                           args=(command, lines_by_file, task_queue, failed_files))
+      t.daemon = True
+      t.start()
+
+    # Fill the queue with files.
+    for name in lines_by_file:
+      task_queue.put(name)
+
+    # Wait for all threads to be done.
+    task_queue.join()
+    if len(failed_files):
+      return_code = 1
+
+  except KeyboardInterrupt:
+    # This is a sad hack. Unfortunately subprocess goes
+    # bonkers with ctrl-c and we start forking merrily.
+    print('\nCtrl-C detected, goodbye.')
+    os.kill(0, 9)
+    return_code = 1
+
+  sys.exit(return_code)
+
+if __name__ == '__main__':
+  main()
diff --git a/scripts/clang-tools.sh b/scripts/clang-tools.sh
index 1e73951176..54a3958488 100755
--- a/scripts/clang-tools.sh
+++ b/scripts/clang-tools.sh
@@ -1,64 +1,31 @@
 #!/usr/bin/env bash
 
-set -e
-set -o pipefail
+set -euo pipefail
 
-CLANG_TIDY_PREFIX=${CLANG_TIDY_PREFIX:-$(scripts/mason.sh PREFIX clang-tidy VERSION 4.0.1)}
-CLANG_TIDY=${CLANG_TIDY:-${CLANG_TIDY_PREFIX}/bin/clang-tidy}
-CLANG_APPLY=${CLANG_APPLY:-${CLANG_TIDY_PREFIX}/bin/clang-apply-replacements}
+CLANG_TIDY_PREFIX=${CLANG_TIDY_PREFIX:-$(scripts/mason.sh PREFIX clang-tidy VERSION 7.0.0)}
+CLANG_TIDY=${CLANG_TIDY_PREFIX}/bin/clang-tidy
+CLANG_APPLY=${CLANG_TIDY_PREFIX}/bin/clang-apply-replacements
 
-CLANG_FORMAT=${CLANG_FORMAT:-$(scripts/mason.sh PREFIX clang-format VERSION 4.0.1)/bin/clang-format}
-
-for CLANG_FILE in "${CLANG_TIDY} ${CLANG_APPLY} ${CLANG_FORMAT}"; do
-    command -v ${CLANG_TIDY} > /dev/null 2>&1 || {
+for CLANG_FILE in "${CLANG_TIDY}" "${CLANG_APPLY}"; do
+    command -v "${CLANG_FILE}" > /dev/null 2>&1 || {
         echo "Can't find ${CLANG_FILE} in PATH."
-        if [ -z ${CLANG_FILE} ]; then
+        if [ -z "${CLANG_FILE}" ]; then
             echo "Alternatively, you can manually set ${!CLANG_FILE@}."
         fi
         exit 1
     }
 done
 
-cd $1
-
-export CDUP=$(git rev-parse --show-cdup)
-export CLANG_TIDY CLANG_APPLY CLANG_FORMAT
-
-function run_clang_tidy() {
-    FILES=$(git ls-files "src/mbgl/*.cpp" "platform/*.cpp" "test/*.cpp")
-    ${CLANG_TIDY_PREFIX}/share/run-clang-tidy.py -j ${JOBS} \
-        -clang-tidy-binary ${CLANG_TIDY} \
-        -clang-apply-replacements-binary ${CLANG_APPLY} \
-        -fix ${FILES} 2>/dev/null || exit 1
-}
-
-function run_clang_tidy_diff() {
-    OUTPUT=$(git diff origin/$2 --src-prefix=${CDUP} --dst-prefix=${CDUP} | \
-                ${CLANG_TIDY_PREFIX}/share/clang-tidy-diff.py \
-                    -clang-tidy-binary ${CLANG_TIDY} \
-                    2>/dev/null)
-    if [[ -n $OUTPUT ]] && [[ $OUTPUT != "No relevant changes found." ]]; then
-        echo -e "${OUTPUT}"
-        exit 1
-    fi
-}
-
-function run_clang_format() {
-    echo "Running clang-format on $0..."
-    DIFF_FILES=$(git diff origin/$2 --name-only *cpp)
-    echo "${DIFF_FILES}" | xargs -I{} -P ${JOBS} bash -c 'run_clang_format' {}
-    ${CLANG_FORMAT} -i ${CDUP}/$0 || exit 1
-}
-
-export -f run_clang_tidy run_clang_tidy_diff run_clang_format
-
 echo "Running Clang checks... (this might take a while)"
 
-if [[ -n $3 ]] && [[ $3 == "--diff" ]]; then
-    run_clang_tidy_diff $@
-    # XXX disabled until we run clang-format over the entire codebase.
-    #run_clang_format $@
-    echo "All checks pass!"
+if [[ -n ${3:-} ]] && [[ ${3:-} == "--diff" ]]; then
+    git diff "origin/$2" | "scripts/clang-tidy-diff.py" \
+        -clang-tidy-binary "${CLANG_TIDY}" \
+        -p 1 -quiet -path "$1"
 else
-    run_clang_tidy $@
+    git ls-files "src/*.cpp" "test/*.cpp" "platform/*.cpp" "bin/*.cpp" | \
+        xargs "${CLANG_TIDY_PREFIX}/share/run-clang-tidy.py" \
+        -clang-tidy-binary "${CLANG_TIDY}" \
+        -clang-apply-replacements-binary "${CLANG_APPLY}" \
+        -quiet -p "$1" -fix
 fi