Add support for '--config=clang-tidy' (#3559)

This sets things up to use `bazel` to run `clang-tidy` using
https://github.com/erenon/bazel_clang_tidy.

I'm fixing issues outside of explorer, and disabling clang-tidy for
targets in explorer that have legacy issues. I was going to disable
clang-tidy for targets in explorer such as interpreter anyways, because
they're slow to parse, and just extended that to the currently failing
targets.

.bazelrc

@@ -2,6 +2,13 @@
 # Exceptions. See /LICENSE for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
+# Support running clang-tidy with:
+#   bazel build --config=clang-tidy -k //...
+# See: https://github.com/erenon/bazel_clang_tidy
+build:clang-tidy --aspects @bazel_clang_tidy//clang_tidy:clang_tidy.bzl%clang_tidy_aspect
+build:clang-tidy --output_groups=report
+build:clang-tidy --@bazel_clang_tidy//:clang_tidy_config=//:clang_tidy_config
+
 # Default to using a disk cache to minimize re-building LLVM and Clang which we
 # try to avoid updating too frequently to minimize rebuild cost. The location
 # here can be overridden in the user configuration where needed.

.clang-tidy

@@ -3,6 +3,11 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 ---
+# Get colors when outputting through `bazel build --config=clang-tidy`.
+UseColor: true
+# This is necessary for `--config=clang-tidy` to catch errors.
+WarningsAsErrors: '*'
+
 # - bugprone-exception-escape finds issues like out-of-memory in main(). We
 #   don't use exceptions, so it's unlikely to find real issues.
 # - bugprone-unchecked-optional-access in clang-tidy 16 has false positives on
@@ -33,7 +38,6 @@ Checks:
   -readability-magic-numbers, -readability-make-member-function-const,
   -readability-static-definition-in-anonymous-namespace,
   -readability-suspicious-call-argument, -readability-use-anyofallof
-WarningsAsErrors: true
 CheckOptions:
   - { key: readability-identifier-naming.ClassCase, value: CamelCase }
   - { key: readability-identifier-naming.ClassConstantCase, value: CamelCase }

BUILD

@@ -2,4 +2,8 @@
 # Exceptions. See /LICENSE for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-# Empty stub.
+filegroup(
+    name = "clang_tidy_config",
+    srcs = [".clang-tidy"],
+    visibility = ["//visibility:public"],
+)

MODULE.bazel

@@ -63,6 +63,15 @@ http_archive(
     urls = ["https://github.com/google/libprotobuf-mutator/archive/v{0}.tar.gz".format(libprotobuf_mutator_version)],
 )
 
+clang_tidy_version = "d2aecc583d14c9554febeab185833c1e8cce5384"
+
+http_archive(
+    name = "bazel_clang_tidy",
+    sha256 = "89c198a9f544beac119bb41904d16d8870686ccb5fe946442c1576934c9e6869",
+    strip_prefix = "bazel_clang_tidy-{0}".format(clang_tidy_version),
+    urls = ["https://github.com/erenon/bazel_clang_tidy/archive/{0}.tar.gz".format(clang_tidy_version)],
+)
+
 bazel_cc_toolchain = use_extension(
     "//bazel/cc_toolchains:clang_configuration.bzl",
     "clang_toolchain_extension",

MODULE.bazel.lock

@@ -1,6 +1,6 @@
 {
   "lockFileVersion": 3,
-  "moduleFileHash": "bd9d40db15930ff642c7b8a3449b04e7cb32b8ff7038b0d6c91cc44e9ad68cf5",
+  "moduleFileHash": "5f71efedda279a9c13889001d008f8d4e66e61a9a386c9f4b5b13d35275a0da3",
   "flags": {
     "cmdRegistries": [
       "https://bcr.bazel.build/"
@@ -37,6 +37,7 @@
           },
           "imports": {
             "com_google_libprotobuf_mutator": "com_google_libprotobuf_mutator",
+            "bazel_clang_tidy": "bazel_clang_tidy",
             "llvm-raw": "llvm-raw"
           },
           "devImports": [],
@@ -59,6 +60,23 @@
                 "column": 13
               }
             },
+            {
+              "tagName": "@bazel_tools//tools/build_defs/repo:http.bzl%http_archive",
+              "attributeValues": {
+                "sha256": "89c198a9f544beac119bb41904d16d8870686ccb5fe946442c1576934c9e6869",
+                "strip_prefix": "bazel_clang_tidy-d2aecc583d14c9554febeab185833c1e8cce5384",
+                "urls": [
+                  "https://github.com/erenon/bazel_clang_tidy/archive/d2aecc583d14c9554febeab185833c1e8cce5384.tar.gz"
+                ],
+                "name": "bazel_clang_tidy"
+              },
+              "devDependency": false,
+              "location": {
+                "file": "@@//:MODULE.bazel",
+                "line": 68,
+                "column": 13
+              }
+            },
             {
               "tagName": "@bazel_tools//tools/build_defs/repo:http.bzl%http_archive",
               "attributeValues": {
@@ -81,7 +99,7 @@
               "devDependency": false,
               "location": {
                 "file": "@@//:MODULE.bazel",
-                "line": 98,
+                "line": 107,
                 "column": 13
               }
             }
@@ -95,7 +113,7 @@
           "usingModule": "<root>",
           "location": {
             "file": "@@//:MODULE.bazel",
-            "line": 66,
+            "line": 75,
             "column": 35
           },
           "imports": {
@@ -112,7 +130,7 @@
           "usingModule": "<root>",
           "location": {
             "file": "@@//:MODULE.bazel",
-            "line": 113,
+            "line": 122,
             "column": 29
           },
           "imports": {
@@ -129,7 +147,7 @@
           "usingModule": "<root>",
           "location": {
             "file": "@@//:MODULE.bazel",
-            "line": 125,
+            "line": 134,
             "column": 23
           },
           "imports": {
@@ -145,7 +163,7 @@
               "devDependency": false,
               "location": {
                 "file": "@@//:MODULE.bazel",
-                "line": 126,
+                "line": 135,
                 "column": 17
               }
             }
@@ -159,7 +177,7 @@
           "usingModule": "<root>",
           "location": {
             "file": "@@//:MODULE.bazel",
-            "line": 132,
+            "line": 141,
             "column": 20
           },
           "imports": {
@@ -177,7 +195,7 @@
               "devDependency": false,
               "location": {
                 "file": "@@//:MODULE.bazel",
-                "line": 133,
+                "line": 142,
                 "column": 10
               }
             }
@@ -1651,6 +1669,18 @@
               "name": "_main~_repo_rules~com_google_libprotobuf_mutator"
             }
           },
+          "bazel_clang_tidy": {
+            "bzlFile": "@@bazel_tools//tools/build_defs/repo:http.bzl",
+            "ruleClassName": "http_archive",
+            "attributes": {
+              "sha256": "89c198a9f544beac119bb41904d16d8870686ccb5fe946442c1576934c9e6869",
+              "strip_prefix": "bazel_clang_tidy-d2aecc583d14c9554febeab185833c1e8cce5384",
+              "urls": [
+                "https://github.com/erenon/bazel_clang_tidy/archive/d2aecc583d14c9554febeab185833c1e8cce5384.tar.gz"
+              ],
+              "name": "_main~_repo_rules~bazel_clang_tidy"
+            }
+          },
           "llvm-raw": {
             "bzlFile": "@@bazel_tools//tools/build_defs/repo:http.bzl",
             "ruleClassName": "http_archive",

common/init_llvm.cpp

@@ -21,7 +21,7 @@ InitLLVM::InitLLVM(int& argc, char**& argv)
   argv = args_.data();
 
   // `argv[argc]` is expected to be a null pointer.
-  args_.push_back(0);
+  args_.push_back(nullptr);
 
   llvm::setBugReportMsg(
       "Please report issues to "

explorer/BUILD

@@ -46,6 +46,9 @@ cc_binary(
     name = "file_test",
     testonly = 1,
     srcs = ["file_test.cpp"],
+    # Running clang-tidy is slow, and explorer is currently feature frozen, so
+    # don't spend time linting it.
+    tags = ["no-clang-tidy"],
     deps = [
         ":main",
         "//common:check",

explorer/ast/BUILD

@@ -41,6 +41,9 @@ cc_library(
         "value_node.h",
         "value_transform.h",
     ],
+    # Running clang-tidy is slow, and explorer is currently feature frozen, so
+    # don't spend time linting it.
+    tags = ["no-clang-tidy"],
     textual_hdrs = [
         "value_kinds.def",
     ],
@@ -95,6 +98,9 @@ cc_test(
     name = "expression_test",
     size = "small",
     srcs = ["expression_test.cpp"],
+    # Running clang-tidy is slow, and explorer is currently feature frozen, so
+    # don't spend time linting it.
+    tags = ["no-clang-tidy"],
     deps = [
         ":ast",
         ":paren_contents",
@@ -137,6 +143,9 @@ cc_test(
     name = "pattern_test",
     size = "small",
     srcs = ["pattern_test.cpp"],
+    # Running clang-tidy is slow, and explorer is currently feature frozen, so
+    # don't spend time linting it.
+    tags = ["no-clang-tidy"],
     deps = [
         ":ast",
         ":paren_contents",

explorer/base/BUILD

@@ -19,6 +19,9 @@ cc_test(
     name = "arena_test",
     size = "small",
     srcs = ["arena_test.cpp"],
+    # Running clang-tidy is slow, and explorer is currently feature frozen, so
+    # don't spend time linting it.
+    tags = ["no-clang-tidy"],
     deps = [
         ":arena",
         "//testing/base:gtest_main",

explorer/fuzzing/BUILD

@@ -97,7 +97,12 @@ cc_fuzz_test(
     srcs = ["explorer_fuzzer.cpp"],
     corpus = glob(["fuzzer_corpus/*"]),
     shard_count = 8,
-    tags = ["proto-fuzzer"],
+    tags = [
+        "proto-fuzzer",
+        # Running clang-tidy is slow, and explorer is currently feature frozen,
+        # so don't spend time linting it.
+        "no-clang-tidy",
+    ],
     deps = [
         ":fuzzer_util",
         "//common:error",

explorer/interpreter/BUILD

@@ -14,6 +14,9 @@ cc_library(
     hdrs = [
         "action.h",
     ],
+    # Running clang-tidy is slow, and explorer is currently feature frozen, so
+    # don't spend time linting it.
+    tags = ["no-clang-tidy"],
     deps = [
         ":dictionary",
         ":heap_allocation_interface",
@@ -116,6 +119,9 @@ cc_library(
     hdrs = [
         "interpreter.h",
     ],
+    # Running clang-tidy is slow, and explorer is currently feature frozen, so
+    # don't spend time linting it.
+    tags = ["no-clang-tidy"],
     deps = [
         ":action",
         ":action_stack",
@@ -205,6 +211,9 @@ cc_library(
         "matching_impl_set.h",
         "type_checker.h",
     ],
+    # Running clang-tidy is slow, and explorer is currently feature frozen, so
+    # don't spend time linting it.
+    tags = ["no-clang-tidy"],
     textual_hdrs = [
         "builtins.def",
     ],
@@ -272,6 +281,9 @@ cc_library(
     hdrs = [
         "type_structure.h",
     ],
+    # Running clang-tidy is slow, and explorer is currently feature frozen, so
+    # don't spend time linting it.
+    tags = ["no-clang-tidy"],
     deps = [
         "//common:ostream",
         "//explorer/ast",

language_server/language_server.cpp

@@ -28,7 +28,7 @@ void LanguageServer::OnDidChangeTextDocument(
 }
 
 void LanguageServer::OnInitialize(
-    clang::clangd::NoParams const& client_capabilities,
+    clang::clangd::NoParams const& /*client_capabilities*/,
     clang::clangd::Callback<llvm::json::Object> cb) {
   llvm::json::Object capabilities{{"documentSymbolProvider", true},
                                   {"textDocumentSync", /*Full=*/1}};

language_server/language_server.h

@@ -38,8 +38,8 @@ class LanguageServer : public clang::clangd::Transport::MessageHandler,
   // LSPBinder::RawOutgoing
 
   // Send method call to client
-  void callMethod(llvm::StringRef Method, llvm::json::Value Params,
-                  clang::clangd::Callback<llvm::json::Value> Reply) override {
+  void callMethod(llvm::StringRef method, llvm::json::Value params,
+                  clang::clangd::Callback<llvm::json::Value> reply) override {
     // TODO: implement when needed
   }
 

toolchain/check/check.cpp

@@ -185,6 +185,7 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info)
 
 // Loops over all nodes in the tree. On some errors, this may return early,
 // for example if an unrecoverable state is encountered.
+// NOLINTNEXTLINE(readability-function-size)
 static auto ProcessParseNodes(Context& context,
                               ErrorTrackingDiagnosticConsumer& err_tracker)
     -> bool {

toolchain/check/handle_name.cpp

@@ -76,7 +76,7 @@ static auto GetClassElementIndex(Context& context, SemIR::InstId element_id)
 // an implicit `self` parameter.
 static auto IsInstanceMethod(const SemIR::File& sem_ir,
                              SemIR::FunctionId function_id) -> bool {
-  auto& function = sem_ir.functions().Get(function_id);
+  const auto& function = sem_ir.functions().Get(function_id);
   for (auto param_id :
        sem_ir.inst_blocks().Get(function.implicit_param_refs_id)) {
     auto param =

toolchain/lex/lex.cpp

@@ -1251,7 +1251,7 @@ auto Lexer::LexFileEnd(llvm::StringRef source_text, ssize_t position) -> void {
 // recovery. These are buffered so that we can perform them in linear time.
 class Lexer::ErrorRecoveryBuffer {
  public:
-  ErrorRecoveryBuffer(TokenizedBuffer& buffer) : buffer_(buffer) {}
+  explicit ErrorRecoveryBuffer(TokenizedBuffer& buffer) : buffer_(buffer) {}
 
   auto empty() const -> bool {
     return new_tokens_.empty() && !any_error_tokens_;

toolchain/lex/tokenized_buffer_benchmark.cpp

@@ -854,6 +854,7 @@ constexpr DispatchTableT DispatchTable = []() {
   for (int i = 0; i < 256; ++i) {
     tmp_table[i] = &BasicDispatch<DispatchTable<NumDispatchTargets>>;
   }
+  // NOLINTNEXTLINE(readability-braces-around-statements): False positive.
   if constexpr (NumDispatchTargets > 1) {
     // Add additional dispatch targets from our specializable array.
     SpecializeDispatchTable<DispatchTable<NumDispatchTargets>>(

toolchain/parse/extract.cpp

@@ -51,7 +51,7 @@ struct Extractable<NodeId> {
     if (trace) {
       *trace << "NodeId: " << tree->node_kind(*it) << " consumed\n";
     }
-    return NodeId(*it++);
+    return *it++;
   }
 };
 
@@ -288,7 +288,7 @@ struct Extractable {
     return ExtractTupleLikeType<T>(
         tree, it, end, trace,
         std::make_index_sequence<std::tuple_size_v<TupleType>>(),
-        (TupleType*)nullptr);
+        static_cast<TupleType*>(nullptr));
   }
 
   static auto Extract(const Tree* tree, Tree::SiblingIterator& it,

utils/treesitter/test_runner.cpp

@@ -14,7 +14,7 @@
 #include <vector>
 
 extern "C" {
-TSLanguage* tree_sitter_carbon();
+auto tree_sitter_carbon() -> TSLanguage*;
 }
 
 // Reads a file to string.