diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -40,6 +40,7 @@ #include "llvm/ADT/Optional.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Errc.h" #include "llvm/Support/Error.h" @@ -52,6 +53,7 @@ #include #include #include +#include #include namespace clang { @@ -109,6 +111,41 @@ ClangdServer::Callbacks *ServerCallbacks; bool TheiaSemanticHighlighting; }; + +// Set of clang-tidy checks that don't work well in clangd, either due to +// crashes or false positives. +// Returns a clang-tidy filter string: -check1,-check2. +llvm::StringRef getUnusableTidyChecks() { + static const std::string FalsePositives = + llvm::join_items(", ", + // Check relies on seeing ifndef/define/endif directives, + // clangd doesn't replay those when using a preamble. + "-llvm-header-guard"); + static const std::string CrashingChecks = + llvm::join_items(", ", + // Check can choke on invalid (intermediate) c++ code, + // which is often the case when clangd tries to build an + // AST. + "-bugprone-use-after-move"); + static const std::string UnusableTidyChecks = + llvm::join_items(", ", FalsePositives, CrashingChecks); + return UnusableTidyChecks; +} + +// Returns a clang-tidy options string: check1,check2. +llvm::StringRef getDefaultTidyChecks() { + // These default checks are chosen for: + // - low false-positive rate + // - providing a lot of value + // - being reasonably efficient + static const std::string DefaultChecks = llvm::join_items( + ",", "readability-misleading-indentation", "readability-deleted-default", + "bugprone-integer-division", "bugprone-sizeof-expression", + "bugprone-suspicious-missing-comma", "bugprone-unused-raii", + "bugprone-unused-return-value", "misc-unused-using-decls", + "misc-unused-alias-decls", "misc-definitions-in-headers"); + return DefaultChecks; +} } // namespace ClangdServer::Options ClangdServer::optsForTest() { @@ -200,6 +237,15 @@ if (GetClangTidyOptions) Opts.ClangTidyOpts = GetClangTidyOptions(*TFS.view(/*CWD=*/llvm::None), File); + if (Opts.ClangTidyOpts.Checks.hasValue()) { + // If the set of checks was configured, make sure clangd incompatible ones + // are disabled. + Opts.ClangTidyOpts.Checks = llvm::join_items( + ", ", *Opts.ClangTidyOpts.Checks, getUnusableTidyChecks()); + } else { + // Otherwise provide a nice set of defaults. + Opts.ClangTidyOpts.Checks = getDefaultTidyChecks().str(); + } Opts.SuggestMissingIncludes = SuggestMissingIncludes; // Compile command is set asynchronously during update, as it can be slow. diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -808,23 +808,6 @@ // FIXME: use the FS provided to the function. Opts = ClangTidyOptProvider->getOptions(File); } - if (!Opts.Checks) { - // If the user hasn't configured clang-tidy checks at all, including - // via .clang-tidy, give them a nice set of checks. - // (This should be what the "default" options does, but it isn't...) - // - // These default checks are chosen for: - // - low false-positive rate - // - providing a lot of value - // - being reasonably efficient - Opts.Checks = llvm::join_items( - ",", "readability-misleading-indentation", - "readability-deleted-default", "bugprone-integer-division", - "bugprone-sizeof-expression", "bugprone-suspicious-missing-comma", - "bugprone-unused-raii", "bugprone-unused-return-value", - "misc-unused-using-decls", "misc-unused-alias-decls", - "misc-definitions-in-headers"); - } return Opts; }; } diff --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp --- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdTests.cpp @@ -26,9 +26,11 @@ #include "llvm/ADT/Optional.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/Errc.h" #include "llvm/Support/Path.h" #include "llvm/Support/Regex.h" +#include "llvm/Support/VirtualFileSystem.h" #include "llvm/Testing/Support/Error.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -1194,6 +1196,44 @@ } #endif +TEST(ClangdServer, TidyOverrideTest) { + struct DiagsCheckingCallback : public ClangdServer::Callbacks { + public: + void onDiagnosticsReady(PathRef File, llvm::StringRef Version, + std::vector Diagnostics) override { + std::lock_guard Lock(Mutex); + HadDiagsInLastCallback = !Diagnostics.empty(); + } + + std::mutex Mutex; + bool HadDiagsInLastCallback = false; + } DiagConsumer; + + MockFS FS; + MockCompilationDatabase CDB; + CDB.ExtraClangFlags = {"-xc++"}; + auto Opts = ClangdServer::optsForTest(); + Opts.GetClangTidyOptions = [](llvm::vfs::FileSystem &, llvm::StringRef) { + auto Opts = tidy::ClangTidyOptions::getDefaults(); + // These checks don't work well in clangd, even if configured they shouldn't + // run. + Opts.Checks = "bugprone-use-after-move,llvm-header-guard"; + return Opts; + }; + ClangdServer Server(CDB, FS, Opts, &DiagConsumer); + const char *SourceContents = R"cpp( + struct Foo { Foo(); Foo(Foo&); Foo(Foo&&); }; + namespace std { Foo&& move(Foo&); } + void foo() { + Foo x; + Foo y = std::move(x); + Foo z = x; + })cpp"; + Server.addDocument(testPath("foo.h"), SourceContents); + ASSERT_TRUE(Server.blockUntilIdleForTest()); + EXPECT_FALSE(DiagConsumer.HadDiagsInLastCallback); +} + } // namespace } // namespace clangd } // namespace clang