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 @@ -39,6 +39,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" @@ -50,6 +51,7 @@ #include #include #include +#include #include namespace clang { @@ -107,6 +109,25 @@ ClangdServer::Callbacks *ServerCallbacks; bool TheiaSemanticHighlighting; }; + +// Set of clang-tidy checks that are not suitable to be run through clangd, +// either due to crashes or false positives. +const char *getClangTidyBlacklist() { + 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 BlackList = + llvm::join_items(", ", FalsePositives, CrashingChecks); + return BlackList.c_str(); +} } // namespace ClangdServer::Options ClangdServer::optsForTest() { @@ -186,6 +207,28 @@ if (GetClangTidyOptions) Opts.ClangTidyOpts = GetClangTidyOptions(*TFS.view(/*CWD=*/llvm::None), File); + if (!Opts.ClangTidyOpts.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.ClangTidyOpts.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"); + } else { + // If user has enabled some checks, make sure clangd incompatible ones are + // disabled. + Opts.ClangTidyOpts.Checks = llvm::join_items( + ", ", *Opts.ClangTidyOpts.Checks, getClangTidyBlacklist()); + } 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 @@ -729,23 +729,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 @@ -23,9 +23,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 "gmock/gmock.h" #include "gtest/gtest.h" #include @@ -1121,6 +1123,42 @@ } #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(); + 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