Using comp_db for clang-tidy

.clang_complete

@@ -1,20 +0,0 @@
--Wall
--Wc++-compat
--I.
--Igens
--Iinclude
--Isrc/core/ext/upb-generated
--Ithird_party/abseil-cpp
--Ithird_party/address_sorting/include
--Ithird_party/benchmark/include
--Ithird_party/boringssl-with-bazel/src/include
--Ithird_party/cares
--Ithird_party/cares/cares
--Ithird_party/googletest
--Ithird_party/googletest/googlemock/include
--Ithird_party/googletest/googletest/include
--Ithird_party/googletest/include
--Ithird_party/protobuf/src
--Ithird_party/re2
--Ithird_party/upb
--Ithird_party/zlib

.gitignore

@@ -151,3 +151,6 @@ BenchmarkDotNet.Artifacts/
 
 # pyenv config
 .python-version
+
+# clang JSON compilation database file
+compile_commands.json

bazel/grpc_deps.bzl

@@ -269,6 +269,17 @@ def grpc_deps():
             sha256 = "97e70364e9249702246c0e9444bccdc4b847bed1eb03c5a3ece4f83dfe6abc44",
         )
 
+    if "bazel_compdb" not in native.existing_rules():
+        http_archive(
+            name = "bazel_compdb",
+            sha256 = "bcecfd622c4ef272fd4ba42726a52e140b961c4eac23025f18b346c968a8cfb4",
+            strip_prefix = "bazel-compilation-database-0.4.5",
+            urls = [
+                "https://storage.googleapis.com/grpc-bazel-mirror/github.com/grailbio/bazel-compilation-database/archive/0.4.5.tar.gz",
+                "https://github.com/grailbio/bazel-compilation-database/archive/0.4.5.tar.gz",
+            ],
+        )
+
     if "io_opencensus_cpp" not in native.existing_rules():
         http_archive(
             name = "io_opencensus_cpp",

templates/tools/dockerfile/grpc_clang_tidy/Dockerfile.template

@@ -20,7 +20,7 @@
   # This is because clang-tidy 7.0 started treating compiler errors as tidy errors
   # and there are a couple of files which are not properly compiled via tidy so it
   # should be using 6.0 version until all compilation errors are addressed.
-  RUN apt-get update && apt-get install -y clang-tidy-6.0
+  RUN apt-get update && apt-get install -y clang-tidy-6.0 jq
   ENV CLANG_TIDY=clang-tidy-6.0
 
   ADD clang_tidy_all_the_things.sh /

templates/tools/dockerfile/test/sanity/Dockerfile.template

@@ -40,7 +40,7 @@
   # This is because clang-tidy 7.0 started treating compiler errors as tidy errors
   # and there are a couple of files which are not properly compiled via tidy so it
   # should be using 6.0 version until all compilation errors are addressed.
-  RUN apt-get install -y clang-tidy-6.0
+  RUN apt-get install -y clang-tidy-6.0 jq
   ENV CLANG_TIDY=clang-tidy-6.0
 
 

tools/bazel.rc

@@ -103,3 +103,6 @@ build:mutrace --copt=-O3
 build:mutrace --copt=-fno-omit-frame-pointer
 build:mutrace --copt=-DNDEBUG
 build:mutrace --linkopt=-rdynamic
+
+# Compile database generation config
+build:compdb --build_tag_filters=-nocompdb

tools/distrib/clang_tidy_code.sh

@@ -21,6 +21,22 @@ set -ex
 cd $(dirname $0)/../..
 REPO_ROOT=$(pwd)
 
+# grep targets with manual tag, which is not included in a result of bazel build using ...
+# let's get a list of them using query command and pass it to gen_compilation_database.py
+export MANUAL_TARGETS=$(bazel query 'attr("tags", "manual", tests(//test/cpp/...))' | grep -v _on_ios)
+
+# generate a clang compilation database
+tools/distrib/gen_compilation_database.py \
+  --include_headers \
+  --ignore_system_headers \
+  --dedup_targets \
+  "//:*" \
+  "//src/core/..." \
+  "//src/compiler/..." \
+  "//test/core/..." \
+  "//test/cpp/..." \
+  $MANUAL_TARGETS
+
 if [ "$CLANG_TIDY_SKIP_DOCKER" == "" ]
 then
   # build clang-tidy docker image
@@ -29,7 +45,15 @@ then
   # run clang-tidy against the checked out codebase
   # when modifying the checked-out files, the current user will be impersonated
   # so that the updated files don't end up being owned by "root".
-  docker run -e TEST="$TEST" -e CHANGED_FILES="$CHANGED_FILES" -e CLANG_TIDY_ROOT="/local-code" --rm=true -v "${REPO_ROOT}":/local-code --user "$(id -u):$(id -g)" -t grpc_clang_tidy /clang_tidy_all_the_things.sh "$@"
+  docker run \
+    -e TEST="$TEST" \
+    -e CHANGED_FILES="$CHANGED_FILES" \
+    -e CLANG_TIDY_ROOT="/local-code" \
+    --rm=true \
+    -v "${REPO_ROOT}":/local-code \
+    -v "${HOME/.cache/bazel}":"${HOME/.cache/bazel}" \
+    --user "$(id -u):$(id -g)" \
+    -t grpc_clang_tidy /clang_tidy_all_the_things.sh "$@"
 else
   CLANG_TIDY_ROOT="${REPO_ROOT}" tools/dockerfile/grpc_clang_tidy/clang_tidy_all_the_things.sh "$@"
 fi

tools/distrib/gen_compilation_database.py

@@ -0,0 +1,135 @@
+#!/usr/bin/env python3
+
+# Copyright 2020 gRPC authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# This is based on the script on the Envoy project
+# https://github.com/envoyproxy/envoy/blob/master/tools/gen_compilation_database.py
+
+import argparse
+import glob
+import json
+import logging
+import os
+import re
+import shlex
+import subprocess
+from pathlib import Path
+
+RE_INCLUDE_SYSTEM = re.compile("\s*-I\s+/usr/[^ ]+")
+
+
+# This method is equivalent to https://github.com/grailbio/bazel-compilation-database/blob/master/generate.sh
+def generateCompilationDatabase(args):
+    # We need to download all remote outputs for generated source code.
+    # This option lives here to override those specified in bazelrc.
+    bazel_options = shlex.split(os.environ.get("BAZEL_BUILD_OPTIONS", "")) + [
+        "--config=compdb",
+        "--remote_download_outputs=all",
+    ]
+
+    subprocess.check_call(["bazel", "build"] + bazel_options + [
+        "--aspects=@bazel_compdb//:aspects.bzl%compilation_database_aspect",
+        "--output_groups=compdb_files,header_files"
+    ] + args.bazel_targets)
+
+    execroot = subprocess.check_output(["bazel", "info", "execution_root"] +
+                                       bazel_options).decode().strip()
+
+    compdb = []
+    for compdb_file in Path(execroot).glob("**/*.compile_commands.json"):
+        compdb.extend(
+            json.loads(
+                "[" +
+                compdb_file.read_text().replace("__EXEC_ROOT__", execroot) +
+                "]"))
+
+    if args.dedup_targets:
+        compdb_map = {target["file"]: target for target in compdb}
+        compdb = list(compdb_map.values())
+
+    return compdb
+
+
+def isHeader(filename):
+    for ext in (".h", ".hh", ".hpp", ".hxx"):
+        if filename.endswith(ext):
+            return True
+    return False
+
+
+def isCompileTarget(target, args):
+    filename = target["file"]
+    if not args.include_headers and isHeader(filename):
+        return False
+    if not args.include_genfiles:
+        if filename.startswith("bazel-out/"):
+            return False
+    if not args.include_external:
+        if filename.startswith("external/"):
+            return False
+    return True
+
+
+def modifyCompileCommand(target, args):
+    cc, options = target["command"].split(" ", 1)
+
+    # Workaround for bazel added C++11 options, those doesn't affect build itself but
+    # clang-tidy will misinterpret them.
+    options = options.replace("-std=c++0x ", "")
+    options = options.replace("-std=c++11 ", "")
+
+    if args.vscode:
+        # Visual Studio Code doesn't seem to like "-iquote". Replace it with
+        # old-style "-I".
+        options = options.replace("-iquote ", "-I ")
+
+    if args.ignore_system_headers:
+        # Remove all include options for /usr/* directories
+        options = RE_INCLUDE_SYSTEM.sub("", options)
+
+    if isHeader(target["file"]):
+        options += " -Wno-pragma-once-outside-header -Wno-unused-const-variable"
+        options += " -Wno-unused-function"
+        if not target["file"].startswith("external/"):
+            # *.h file is treated as C header by default while our headers files are all C++11.
+            options = "-x c++ -std=c++11 -fexceptions " + options
+
+    target["command"] = " ".join([cc, options])
+    return target
+
+
+def fixCompilationDatabase(args, db):
+    db = [
+        modifyCompileCommand(target, args)
+        for target in db
+        if isCompileTarget(target, args)
+    ]
+
+    with open("compile_commands.json", "w") as db_file:
+        json.dump(db, db_file, indent=2)
+
+
+if __name__ == "__main__":
+    parser = argparse.ArgumentParser(
+        description='Generate JSON compilation database')
+    parser.add_argument('--include_external', action='store_true')
+    parser.add_argument('--include_genfiles', action='store_true')
+    parser.add_argument('--include_headers', action='store_true')
+    parser.add_argument('--vscode', action='store_true')
+    parser.add_argument('--ignore_system_headers', action='store_true')
+    parser.add_argument('--dedup_targets', action='store_true')
+    parser.add_argument('bazel_targets', nargs='*', default=["//..."])
+    args = parser.parse_args()
+    fixCompilationDatabase(args, generateCompilationDatabase(args))

tools/distrib/run_clang_tidy.py

@@ -24,17 +24,6 @@ sys.path.append(
                  'python_utils'))
 import jobset
 
-extra_args = [
-    '-x',
-    'c++',
-    '-std=c++11',
-]
-with open('.clang_complete') as f:
-    for line in f:
-        line = line.strip()
-        if line.startswith('-I'):
-            extra_args.append(line)
-
 clang_tidy = os.environ.get('CLANG_TIDY', 'clang-tidy')
 
 argp = argparse.ArgumentParser(description='Run clang-tidy against core')
@@ -50,17 +39,19 @@ args = argp.parse_args()
 
 cmdline = [
     clang_tidy,
-] + ['--extra-arg-before=%s' % arg for arg in extra_args]
+]
 
 if args.fix:
     cmdline.append('--fix')
 
 jobs = []
 for filename in args.files:
-    jobs.append(jobset.JobSpec(
+    jobs.append(
-        cmdline + [filename],
+        jobset.JobSpec(
-        shortname=filename,
+            cmdline + [filename],
-    ))  #verbose_success=True))
+            shortname=filename,
+            timeout_seconds=15 * 60,
+        ))
 
 num_fails, res_set = jobset.run(jobs, maxjobs=args.jobs)
 sys.exit(num_fails)

tools/dockerfile/grpc_clang_tidy/Dockerfile

@@ -65,7 +65,7 @@ RUN mkdir /var/local/jenkins
 # This is because clang-tidy 7.0 started treating compiler errors as tidy errors
 # and there are a couple of files which are not properly compiled via tidy so it
 # should be using 6.0 version until all compilation errors are addressed.
-RUN apt-get update && apt-get install -y clang-tidy-6.0
+RUN apt-get update && apt-get install -y clang-tidy-6.0 jq
 ENV CLANG_TIDY=clang-tidy-6.0
 
 ADD clang_tidy_all_the_things.sh /

tools/dockerfile/grpc_clang_tidy/clang_tidy_all_the_things.sh

@@ -20,5 +20,9 @@ CLANG_TIDY=${CLANG_TIDY:-clang-tidy}
 
 cd ${CLANG_TIDY_ROOT}
 
-find src/core src/cpp test/core test/cpp ! -path 'src/core/ext/upb-generated/*' ! -path 'src/core/ext/upbdefs-generated/*' -name '*.h' -or -name '*.cc' -print0 \
+# run clang tidy for all source files
-  | xargs -0 tools/distrib/run_clang_tidy.py "$@"
+cat compile_commands.json | jq -r '.[].file' \
+  | grep -E "(^src/core/|^src/cpp/|^test/core/|^test/cpp/)" \
+  | grep -v -E "/upb-generated/|/upbdefs-generated/" \
+  | sort \
+  | xargs tools/distrib/run_clang_tidy.py "$@"

tools/dockerfile/test/sanity/Dockerfile

@@ -88,7 +88,7 @@ RUN apt-get install -y clang clang-format
 # This is because clang-tidy 7.0 started treating compiler errors as tidy errors
 # and there are a couple of files which are not properly compiled via tidy so it
 # should be using 6.0 version until all compilation errors are addressed.
-RUN apt-get install -y clang-tidy-6.0
+RUN apt-get install -y clang-tidy-6.0 jq
 ENV CLANG_TIDY=clang-tidy-6.0
 
 

tools/run_tests/sanity/check_bazel_workspace.py

@@ -35,6 +35,7 @@ git_submodule_hashes = {
 
 _BAZEL_SKYLIB_DEP_NAME = 'bazel_skylib'
 _BAZEL_TOOLCHAINS_DEP_NAME = 'bazel_toolchains'
+_BAZEL_COMPDB_DEP_NAME = 'bazel_compdb'
 _TWISTED_TWISTED_DEP_NAME = 'com_github_twisted_twisted'
 _YAML_PYYAML_DEP_NAME = 'com_github_yaml_pyyaml'
 _TWISTED_INCREMENTAL_DEP_NAME = 'com_github_twisted_incremental'
@@ -56,6 +57,7 @@ _GRPC_DEP_NAMES = [
     'envoy_api',
     _BAZEL_SKYLIB_DEP_NAME,
     _BAZEL_TOOLCHAINS_DEP_NAME,
+    _BAZEL_COMPDB_DEP_NAME,
     _TWISTED_TWISTED_DEP_NAME,
     _YAML_PYYAML_DEP_NAME,
     _TWISTED_INCREMENTAL_DEP_NAME,
@@ -75,6 +77,7 @@ _GRPC_BAZEL_ONLY_DEPS = [
     'io_opencensus_cpp',
     _BAZEL_SKYLIB_DEP_NAME,
     _BAZEL_TOOLCHAINS_DEP_NAME,
+    _BAZEL_COMPDB_DEP_NAME,
     _TWISTED_TWISTED_DEP_NAME,
     _YAML_PYYAML_DEP_NAME,
     _TWISTED_INCREMENTAL_DEP_NAME,
@@ -149,7 +152,10 @@ build_rules = {
 exec(bazel_file) in build_rules
 for name in _GRPC_DEP_NAMES:
     assert name in names_and_urls.keys()
-assert len(_GRPC_DEP_NAMES) == len(names_and_urls.keys())
+if len(_GRPC_DEP_NAMES) != len(names_and_urls.keys()):
+    assert False, "Diff: " + (str(set(_GRPC_DEP_NAMES) - set(names_and_urls)) +
+                              "," +
+                              str(set(names_and_urls) - set(_GRPC_DEP_NAMES)))
 
 # There are some "bazel-only" deps that are exceptions to this sanity check,
 # we don't require that there is a corresponding git module for these.

tools/run_tests/sanity/sanity_tests.yaml

@@ -24,6 +24,9 @@
 - script: tools/distrib/check_pytype.sh
 - script: tools/distrib/clang_format_code.sh
 - script: tools/distrib/clang_tidy_code.sh
+  # ClangTidy needs to run exclusively because it runs bazel making changes
+  # and it needs many CPUs to parse all source files to be diagnosed.
+  cpu_cost: 1000
 - script: tools/distrib/pylint_code.sh
 - script: tools/distrib/python/check_grpcio_tools.py
 - script: tools/distrib/yapf_code.sh --diff