[clangd] Add CMake option to (not) link in clang-tidy checks

[sammccall]

Summary

This reduces the size of the dependency graph and makes incremental
development a little more pleasant (less rebuilding).

This introduces a bit of complexity/fragility as some tests verify
clang-tidy behavior. I attempted to isolate these and build/run as much
of the tests as possible in both configs to prevent rot.

Expectation is that (some) developers will use this locally, but
buildbots etc will keep testing clang-tidy.

Fixes https://github.com/clangd/clangd/issues/233

[kadircet]

clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

280

why do we need to disable this test? it doesn't really build an ast or assert on the diagnostics from clang-tidy in any way, seems to be purely about configs.

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

153–154

you probably want to drop this too ?

167

ah, this was actually checking for a particular change in the way we transform clang(-tidy) provided diag ranges to editor-friendly ones.

any diagnostic without any range info and containing a fix which is only insertion should be fine (one that i could find is for_range_dereference).

266

ah, should've kept reading before leaving the comment above. feel free to ignore that (or just drop this test and rely only on clang diagnostics for checking that behaviour?)

294–312

i think we should suppress this with -Wno-unsequenced (and get rid of the Else part). Moreover this is the only test requiring an Else bit (if I didn't miss any). WDYT about just having a new file TidyIntegrationTests or TidyDiagnosticTests and moving all of these there, or having two different sections in this file to only enable these tests when tidy is on. It would imply tests that want to check a mix of tidy and clang diagnostics needs to go into the tidy section and cannot be tested on non-tidy builds, but that sounds like the price we need to pay anyway, whereas introducing the ifTidyChecks complicates things a little more (IMO).

[nridge]

Will building with this option just prevent linking the clang-tidy checks into clangd, or will it also prevent building them in the first place?

In my experience, it's the building that's the primary compile time expense, so if the intention is to address https://github.com/clangd/clangd/issues/233, we would want to avoid building them as well.

[sammccall]

In D105679#2873109, @nridge wrote:

Will building with this option just prevent linking the clang-tidy checks into clangd, or will it also prevent building them in the first place?

If you're running ninja clangd or ninja check-clangd they aren't built anymore even if dirty. (Just tested this)
This change breaks the only build dependency path between clangd/ClangdTests and the clang-tidy checks.

Of course all/check-all/check-clang-tools will still build the clang-tidy checks.

clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

280

We have clever logic that verifies that check names (but not wildcards) in configs actually match real checks.

Put ifdefs around the relevant assertions instead, so it should be now self-explanatory.

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

266

This really belongs in the diagnostic ranges test, so switched to a different diag as you suggested.

294–312

i think we should suppress this with -Wno-unsequenced (and get rid of the Else part). Moreover this is the only test requiring an Else bit (if I didn't miss any).

Done, and ripped out Else support.

WDYT about just having a new file TidyIntegrationTests or TidyDiagnosticTests and moving all of these there, or having two different sections in this file to only enable these tests when tidy is on

I started with this but having substantial amounts of nontrivial code completely ifdef'd out makes me nervous about maintenance, as we'd lose all tooling support on these tests if we use this config.

This solution is significantly safer I think:

• mostly preserves refactoring options even with clang-tidy off (e.g. local xrefs)
• type checks all the matchers, even the clang-tidy ones with clang-tidy off
• verifies that the code snippets in TestTU actually works
• explicitly shows the difference with clang-tidy on vs off

ifTidyChecks complicates things a little more (IMO).

Agree. I think the benefits of the tests being live code outweigh the relatively minor complexity. Maybe there's a better tradeoff to be had though.

[sammccall]

Address review comments

[kadircet]

thanks, lgtm!

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

266

nit: is this testing anything useful now? maybe just drop it?

[thakis]

Nit: I find the name a bit confusing. When I first saw this commit, I thought this would be a list of clang-tidy checks to support, not a binary on/off toggle.

clang-tools-extra/clangd/CMakeLists.txt

if (NOT DEFINED CLANGD_BUILD_XPC)
 
   llvm_canonicalize_cmake_booleans(CLANGD_BUILD_XPC_DEFAULT)
 
   set(CLANGD_BUILD_XPC ${CLANGD_BUILD_XPC_DEFAULT} CACHE BOOL "Build XPC Support For Clangd." FORCE)
   unset(CLANGD_BUILD_XPC_DEFAULT)
 endif ()
 
 option(CLANGD_MALLOC_TRIM "Call malloc_trim(3) periodically in Clangd. (only takes effect when using glibc)" ON)
+# -DCLANG_TIDY_CHECKS=Off avoids a dependency on clang-tidy, reducing rebuilds.
+option(CLANGD_TIDY_CHECKS "Link all clang-tidy checks into clangd" ON)
 
 llvm_canonicalize_cmake_booleans(
   CLANGD_BUILD_XPC
   CLANGD_ENABLE_REMOTE
   ENABLE_GRPC_REFLECTION
   CLANGD_MALLOC_TRIM
+  CLANGD_TIDY_CHECKS
   LLVM_ENABLE_ZLIB
 )
 
 configure_file(
   ${CMAKE_CURRENT_SOURCE_DIR}/Features.inc.in
   ${CMAKE_CURRENT_BINARY_DIR}/Features.inc
 )
 
clang_target_link_libraries(clangDaemon
   clangToolingSyntax
   )
 
 target_link_libraries(clangDaemon
   PRIVATE
   ${LLVM_PTHREAD_LIB}
 
   clangTidy
-  ${ALL_CLANG_TIDY_CHECKS}
 
   clangdSupport
   )
+if(CLANGD_TIDY_CHECKS)
+  target_link_libraries(clangDaemon PRIVATE ${ALL_CLANG_TIDY_CHECKS})
+endif()
 
 add_subdirectory(refactor/tweaks)
 if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
   # FIXME: Make fuzzer not use linux-specific APIs, build it everywhere.
   add_subdirectory(fuzzer)
 endif()
 add_subdirectory(tool)
 add_subdirectory(indexer)

clang-tools-extra/clangd/Features.cpp

 #endif
 
 #if CLANGD_ENABLE_REMOTE
       "+grpc"
 #endif
 #if CLANGD_BUILD_XPC
       "+xpc"
 #endif
+
+#if !CLANGD_TIDY_CHECKS
+      "-tidy"
+#endif
       ;
 }
 
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/Features.inc.in

 // IWYU pragma: private, include "Features.h"
 #define CLANGD_BUILD_XPC @CLANGD_BUILD_XPC@
 #define CLANGD_ENABLE_REMOTE @CLANGD_ENABLE_REMOTE@
 #define ENABLE_GRPC_REFLECTION @ENABLE_GRPC_REFLECTION@
 #define CLANGD_MALLOC_TRIM @CLANGD_MALLOC_TRIM@
+#define CLANGD_TIDY_CHECKS @CLANGD_TIDY_CHECKS@

clang-tools-extra/clangd/ParsedAST.cpp

 #include "../clang-tidy/ClangTidyCheck.h"
 #include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
 #include "../clang-tidy/ClangTidyModuleRegistry.h"
 #include "AST.h"
 #include "Compiler.h"
 #include "Config.h"
 #include "Diagnostics.h"
 #include "FeatureModule.h"
+#include "Features.h"
 #include "Headers.h"
 #include "HeuristicResolver.h"
 #include "IncludeFixer.h"
 #include "Preamble.h"
 #include "SourceCode.h"
 #include "TidyProvider.h"
 #include "index/CanonicalIncludes.h"
 #include "index/Index.h"

 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <memory>
 #include <vector>
 
 // Force the linker to link in Clang-tidy modules.
 // clangd doesn't support the static analyzer.
+#if CLANGD_TIDY_CHECKS
 #define CLANG_TIDY_DISABLE_STATIC_ANALYZER_CHECKS
 #include "../clang-tidy/ClangTidyForceLinker.h"
+#endif
 
 namespace clang {
 namespace clangd {
 namespace {
 
 template <class T> std::size_t getUsedBytes(const std::vector<T> &Vec) {
   return Vec.capacity() * sizeof(T);
 }

clang-tools-extra/clangd/test/diagnostics-tidy.test

+# REQUIRES: clangd-tidy-checks
 # RUN: clangd -lit-test -clang-tidy-checks=bugprone-sizeof-expression < %s | FileCheck -strict-whitespace %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","text":"void main() {\n(void)sizeof(42);\n}"}}}
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","text":"int main() {\n(void)sizeof(42);\n}"}}}
 #      CHECK:  "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "diagnostics": [
 # CHECK-NEXT:      {
-# CHECK-NEXT:        "code": "-Wmain-return-type",
-# CHECK-NEXT:        "message": "Return type of 'main' is not 'int' (fix available)",
-# CHECK-NEXT:        "range": {
-# CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 4,
-# CHECK-NEXT:            "line": 0
-# CHECK-NEXT:          },
-# CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 0,
-# CHECK-NEXT:            "line": 0
-# CHECK-NEXT:          }
-# CHECK-NEXT:        },
-# CHECK-NEXT:        "severity": 2,
-# CHECK-NEXT:        "source": "clang"
-# CHECK-NEXT:      },
-# CHECK-NEXT:      {
 # CHECK-NEXT:        "code": "bugprone-sizeof-expression",
 # CHECK-NEXT:        "message": "Suspicious usage of 'sizeof(K)'; did you mean 'K'?",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
 # CHECK-NEXT:            "character": 12,
 # CHECK-NEXT:            "line": 1
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {

 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "diagnostics": [],
 # CHECK-NEXT:    "uri": "file://{{.*}}/foo.c"
 # CHECK-NEXT:  }
 ---
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}
+

clang-tools-extra/clangd/test/lit.cfg.py

config.substitutions.append(('%clangd-benchmark-dir',
                              config.clangd_binary_dir + "/benchmarks"))
 
 if config.clangd_build_xpc:
   config.available_features.add('clangd-xpc-support')
 
 if config.clangd_enable_remote:
   config.available_features.add('clangd-remote-index')
 
+if config.clangd_tidy_checks:
+  config.available_features.add('clangd-tidy-checks')
+
 if config.have_zlib:
   config.available_features.add('zlib')

clang-tools-extra/clangd/test/lit.site.cfg.py.in

except KeyError:
     e = sys.exc_info()[1]
     key, = e.args
     lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))
 
 config.clangd_source_dir = "@CMAKE_CURRENT_SOURCE_DIR@/.."
 config.clangd_binary_dir = "@CMAKE_CURRENT_BINARY_DIR@/.."
 config.clangd_build_xpc = @CLANGD_BUILD_XPC@
 config.clangd_enable_remote = @CLANGD_ENABLE_REMOTE@
+config.clangd_tidy_checks = @CLANGD_TIDY_CHECKS@
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 
 # Delegate logic to lit.cfg.py.
 lit_config.load_config(config, "@CMAKE_CURRENT_SOURCE_DIR@/lit.cfg.py")

clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::warn_alloca,
                                             Conf.Diagnostics.Suppress));
 
   Frag.Diagnostics.Suppress.emplace_back("*");
   EXPECT_TRUE(compileAndApply());
   EXPECT_TRUE(Conf.Diagnostics.SuppressAll);
   EXPECT_THAT(Conf.Diagnostics.Suppress, IsEmpty());
 }
 
 TEST_F(ConfigCompileTests, Tidy) {
   auto &Tidy = Frag.Diagnostics.ClangTidy;
   Tidy.Add.emplace_back("bugprone-use-after-move");
   Tidy.Add.emplace_back("llvm-*");
   Tidy.Remove.emplace_back("llvm-include-order");
   Tidy.Remove.emplace_back("readability-*");
   Tidy.CheckOptions.emplace_back(
       std::make_pair(std::string("StrictMode"), std::string("true")));
   Tidy.CheckOptions.emplace_back(std::make_pair(
       std::string("example-check.ExampleOption"), std::string("0")));
   EXPECT_TRUE(compileAndApply());
-  EXPECT_EQ(
-      Conf.Diagnostics.ClangTidy.Checks,
-      "bugprone-use-after-move,llvm-*,-llvm-include-order,-readability-*");
   EXPECT_EQ(Conf.Diagnostics.ClangTidy.CheckOptions.size(), 2U);
   EXPECT_EQ(Conf.Diagnostics.ClangTidy.CheckOptions.lookup("StrictMode"),
             "true");
   EXPECT_EQ(Conf.Diagnostics.ClangTidy.CheckOptions.lookup(
                 "example-check.ExampleOption"),
             "0");
+#if CLANGD_TIDY_CHECKS
+  EXPECT_EQ(
+      Conf.Diagnostics.ClangTidy.Checks,
+      "bugprone-use-after-move,llvm-*,-llvm-include-order,-readability-*");
   EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+#else // !CLANGD_TIDY_CHECKS
+  EXPECT_EQ(Conf.Diagnostics.ClangTidy.Checks, "llvm-*,-readability-*");
+  EXPECT_THAT(
+      Diags.Diagnostics,
+      ElementsAre(
+          DiagMessage(
+              "clang-tidy check 'bugprone-use-after-move' was not found"),
+          DiagMessage("clang-tidy check 'llvm-include-order' was not found")));
+#endif
 }
 
 TEST_F(ConfigCompileTests, TidyBadChecks) {
   auto &Tidy = Frag.Diagnostics.ClangTidy;
   Tidy.Add.emplace_back("unknown-check");
   Tidy.Remove.emplace_back("*");
   Tidy.Remove.emplace_back("llvm-includeorder");
   EXPECT_TRUE(compileAndApply());

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

 //===--- DiagnosticsTests.cpp ------------------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Annotations.h"
 #include "Config.h"
 #include "Diagnostics.h"
 #include "FeatureModule.h"
+#include "Features.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "TidyProvider.h"
 #include "index/MemIndex.h"

 // Helper function to make tests shorter.
 Position pos(int line, int character) {
   Position Res;
   Res.line = line;
   Res.character = character;
   return Res;
 }
 
+// Normally returns the provided diagnostics matcher.
+// If clang-tidy checks are not linked in, returns a matcher for no diagnostics!
+// This is intended for tests where the diagnostics come from clang-tidy checks.
+// We don't #ifdef each individual test as it's intrusive and we want to ensure
+// that as much of the test is still compiled an run as possible.
+::testing::Matcher<std::vector<clangd::Diag>>
+ifTidyChecks(::testing::Matcher<std::vector<clangd::Diag>> M) {
+  if (!CLANGD_TIDY_CHECKS)
+    return IsEmpty();
+  return M;
+}
+
 TEST(DiagnosticsTest, DiagnosticRanges) {
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
     // error-ok
     #define ID(X) X
     namespace test{};
     void $decl[[foo]]();
-    class T{$explicit[[]]$constructor[[T]](int a);};
     int main() {
+      struct Container { int* begin(); int* end(); } *container;
+      for (auto i : $insertstar[[]]$range[[container]]) {
+      }
+
       $typo[[go\
 o]]();
       foo()$semicolon[[]]//with comments
       $unk[[unknown]]();
       double $type[[bar]] = "foo";
       struct Foo { int x; }; Foo a;
       a.$nomember[[y]];
       test::$nomembernamespace[[test]];
       $macro[[ID($macroarg[[fod]])]]();
     }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
-  TU.ClangTidyProvider = addTidyChecks("google-explicit-constructor");
   EXPECT_THAT(
       *TU.build().getDiagnostics(),
       ElementsAre(
+          // Make sure the whole token is highlighted.
+          AllOf(Diag(Test.range("range"),
+                     "invalid range expression of type 'struct Container *'; "
+                     "did you mean to dereference it with '*'?"),
+                WithFix(Fix(Test.range("insertstar"), "*", "insert '*'"))),
           // This range spans lines.
           AllOf(Diag(Test.range("typo"),
                      "use of undeclared identifier 'goo'; did you mean 'foo'?"),
                 DiagSource(Diag::Clang), DiagName("undeclared_var_use_suggest"),
                 WithFix(
                     Fix(Test.range("typo"), "foo", "change 'go\\…' to 'foo'")),
                 // This is a pretty normal range.
                 WithNote(Diag(Test.range("decl"), "'foo' declared here"))),
           // This range is zero-width and insertion. Therefore make sure we are
           // not expanding it into other tokens. Since we are not going to
           // replace those.
           AllOf(Diag(Test.range("semicolon"), "expected ';' after expression"),
                 WithFix(Fix(Test.range("semicolon"), ";", "insert ';'"))),
           // This range isn't provided by clang, we expand to the token.
           Diag(Test.range("unk"), "use of undeclared identifier 'unknown'"),
           Diag(Test.range("type"),
                "cannot initialize a variable of type 'double' with an lvalue "
                "of type 'const char [4]'"),
           Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
           Diag(Test.range("nomembernamespace"),
                "no member named 'test' in namespace 'test'"),
           AllOf(Diag(Test.range("macro"),
                      "use of undeclared identifier 'fod'; did you mean 'foo'?"),
                 WithFix(Fix(Test.range("macroarg"), "foo",
-                            "change 'fod' to 'foo'"))),
-          // We make sure here that the entire token is highlighted
-          AllOf(Diag(Test.range("constructor"),
-                     "single-argument constructors must be marked explicit to "
-                     "avoid unintentional implicit conversions"),
-                WithFix(Fix(Test.range("explicit"), "explicit ",
-                            "insert 'explicit '")))));
+                            "change 'fod' to 'foo'")))));
 }
 
 TEST(DiagnosticsTest, FlagsMatter) {
   Annotations Test("[[void]] main() {} // error-ok");
   auto TU = TestTU::withCode(Test.code());
   EXPECT_THAT(*TU.build().getDiagnostics(),
               ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"),
                                 WithFix(Fix(Test.range(), "int",
TEST(DiagnosticsTest, DeduplicatedClangTidyDiagnostics) {
   auto TU = TestTU::withCode(Test.code());
   // Enable alias clang-tidy checks, these check emits the same diagnostics
   // (except the check name).
   TU.ClangTidyProvider = addTidyChecks("readability-uppercase-literal-suffix,"
                                        "hicpp-uppercase-literal-suffix");
   // Verify that we filter out the duplicated diagnostic message.
   EXPECT_THAT(
       *TU.build().getDiagnostics(),
-      UnorderedElementsAre(::testing::AllOf(
+      ifTidyChecks(UnorderedElementsAre(::testing::AllOf(
           Diag(Test.range(),
                "floating point literal has suffix 'f', which is not uppercase"),
-          DiagSource(Diag::ClangTidy))));
+          DiagSource(Diag::ClangTidy)))));
 
   Test = Annotations(R"cpp(
     template<typename T>
     void func(T) {
       float f = [[0.3f]];
     }
     void k() {
       func(123);
       func(2.0);
     }
   )cpp");
   TU.Code = std::string(Test.code());
   // The check doesn't handle template instantiations which ends up emitting
   // duplicated messages, verify that we deduplicate them.
   EXPECT_THAT(
       *TU.build().getDiagnostics(),
-      UnorderedElementsAre(::testing::AllOf(
+      ifTidyChecks(UnorderedElementsAre(::testing::AllOf(
           Diag(Test.range(),
                "floating point literal has suffix 'f', which is not uppercase"),
-          DiagSource(Diag::ClangTidy))));
+          DiagSource(Diag::ClangTidy)))));
 }
 
 TEST(DiagnosticsTest, ClangTidy) {
   Annotations Test(R"cpp(
     #include $deprecated[["assert.h"]]
 
     #define $macrodef[[SQUARE]](X) (X)*(X)
     int $main[[main]]() {
       int y = 4;
       return SQUARE($macroarg[[++]]y);
       return $doubled[[sizeof]](sizeof(int));
     }
 
     // misc-no-recursion uses a custom traversal from the TUDecl
     void foo();
     void $bar[[bar]]() {
       foo();
     }
     void $foo[[foo]]() {
       bar();
     }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
   TU.HeaderFilename = "assert.h"; // Suppress "not found" error.
   TU.ClangTidyProvider = addTidyChecks("bugprone-sizeof-expression,"
                                        "bugprone-macro-repeated-side-effects,"
                                        "modernize-deprecated-headers,"
                                        "modernize-use-trailing-return-type,"
                                        "misc-no-recursion");
+  TU.ExtraArgs.push_back("-Wno-unsequenced");
   EXPECT_THAT(
       *TU.build().getDiagnostics(),
-      UnorderedElementsAre(
+      ifTidyChecks(UnorderedElementsAre(
           AllOf(Diag(Test.range("deprecated"),
                      "inclusion of deprecated C++ header 'assert.h'; consider "
                      "using 'cassert' instead"),
                 DiagSource(Diag::ClangTidy),
                 DiagName("modernize-deprecated-headers"),
                 WithFix(Fix(Test.range("deprecated"), "<cassert>",
                             "change '\"assert.h\"' to '<cassert>'"))),
           Diag(Test.range("doubled"),
                "suspicious usage of 'sizeof(sizeof(...))'"),
-          AllOf(
-              Diag(Test.range("macroarg"),
-                   "side effects in the 1st macro argument 'X' are repeated in "
+          AllOf(Diag(Test.range("macroarg"),
+                     "side effects in the 1st macro argument 'X' are "
+                     "repeated in "
                      "macro expansion"),
                 DiagSource(Diag::ClangTidy),
                 DiagName("bugprone-macro-repeated-side-effects"),
-              WithNote(
-                  Diag(Test.range("macrodef"), "macro 'SQUARE' defined here"))),
-          Diag(Test.range("macroarg"),
-               "multiple unsequenced modifications to 'y'"),
-          AllOf(
-              Diag(Test.range("main"),
+                WithNote(Diag(Test.range("macrodef"),
+                              "macro 'SQUARE' defined here"))),
+          AllOf(Diag(Test.range("main"),
                      "use a trailing return type for this function"),
                 DiagSource(Diag::ClangTidy),
                 DiagName("modernize-use-trailing-return-type"),
-              // Verify that we don't have "[check-name]" suffix in the message.
-              WithFix(
-                  FixMessage("use a trailing return type for this function"))),
+                // Verify there's no "[check-name]" suffix in the message.
+                WithFix(FixMessage(
+                    "use a trailing return type for this function"))),
           Diag(Test.range("foo"),
                "function 'foo' is within a recursive call chain"),
           Diag(Test.range("bar"),
-               "function 'bar' is within a recursive call chain")));
+               "function 'bar' is within a recursive call chain"))));
 }
 
 TEST(DiagnosticsTest, ClangTidyEOF) {
   // clang-format off
   Annotations Test(R"cpp(
   [[#]]include <b.h>
   #include "a.h")cpp");
   // clang-format on
   auto TU = TestTU::withCode(Test.code());
   TU.ExtraArgs = {"-isystem."};
   TU.AdditionalFiles["a.h"] = TU.AdditionalFiles["b.h"] = "";
   TU.ClangTidyProvider = addTidyChecks("llvm-include-order");
   EXPECT_THAT(
       *TU.build().getDiagnostics(),
-      Contains(AllOf(Diag(Test.range(), "#includes are not sorted properly"),
-                     DiagSource(Diag::ClangTidy),
-                     DiagName("llvm-include-order"))));
+      ifTidyChecks(Contains(
+          AllOf(Diag(Test.range(), "#includes are not sorted properly"),
+                DiagSource(Diag::ClangTidy), DiagName("llvm-include-order")))));
 }
 
 TEST(DiagnosticTest, TemplatesInHeaders) {
   // Diagnostics from templates defined in headers are placed at the expansion.
   Annotations Main(R"cpp(
     Derived<int> [[y]]; // error-ok
   )cpp");
   Annotations Header(R"cpp(
void f() {
         auto S = *it;
       }
     }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.ClangTidyProvider = addTidyChecks("modernize-loop-convert");
   EXPECT_THAT(
       *TU.build().getDiagnostics(),
-      UnorderedElementsAre(::testing::AllOf(
+      ifTidyChecks(UnorderedElementsAre(::testing::AllOf(
           Diag(Main.range(), "use range-based for loop instead"),
-          DiagSource(Diag::ClangTidy), DiagName("modernize-loop-convert"))));
+          DiagSource(Diag::ClangTidy), DiagName("modernize-loop-convert")))));
 }
 
 TEST(DiagnosticTest, RespectsDiagnosticConfig) {
   Annotations Main(R"cpp(
     // error-ok
     void x() {
       [[unknown]]();
       $ret[[return]] 42;
int main() {
       #define BAD2 BAD
       double h = BAD2;  // NOLINT
     }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.ClangTidyProvider = addTidyChecks("bugprone-integer-division");
   EXPECT_THAT(
       *TU.build().getDiagnostics(),
-      UnorderedElementsAre(::testing::AllOf(
+      ifTidyChecks(UnorderedElementsAre(::testing::AllOf(
           Diag(Main.range(), "result of integer division used in a floating "
                              "point context; possible loss of precision"),
-          DiagSource(Diag::ClangTidy), DiagName("bugprone-integer-division"))));
+          DiagSource(Diag::ClangTidy),
+          DiagName("bugprone-integer-division")))));
 }
 
 TEST(DiagnosticTest, ClangTidyWarningAsError) {
   Annotations Main(R"cpp(
     int main() {
       int i = 3;
       double f = [[8]] / i; // error-ok
     }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.ClangTidyProvider =
       addTidyChecks("bugprone-integer-division", "bugprone-integer-division");
   EXPECT_THAT(
       *TU.build().getDiagnostics(),
-      UnorderedElementsAre(::testing::AllOf(
+      ifTidyChecks(UnorderedElementsAre(::testing::AllOf(
           Diag(Main.range(), "result of integer division used in a floating "
                              "point context; possible loss of precision"),
           DiagSource(Diag::ClangTidy), DiagName("bugprone-integer-division"),
-          DiagSeverity(DiagnosticsEngine::Error))));
+          DiagSeverity(DiagnosticsEngine::Error)))));
 }
 
 TEST(DiagnosticTest, LongFixMessages) {
   // We limit the size of printed code.
   Annotations Source(R"cpp(
     int main() {
       // error-ok
       int somereallyreallyreallyreallyreallyreallyreallyreallylongidentifier;
Annotations Main(R"cpp(
     } [[else]] if (cond == 2) {
       return 43;
     }
     return 44;
     }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.ClangTidyProvider = addTidyChecks("llvm-else-after-return");
-  EXPECT_THAT(
-      *TU.build().getDiagnostics(),
-      ElementsAre(Diag(Main.range(), "do not use 'else' after 'return'")));
+  EXPECT_THAT(*TU.build().getDiagnostics(),
+              ifTidyChecks(ElementsAre(
+                  Diag(Main.range(), "do not use 'else' after 'return'"))));
 }
 
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
   //  - the #else doesn't generate diagnostics (we had this bug)
   //  - we get diagnostics from the taken branch
   //  - we get no diagnostics from the not taken branch