diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt --- a/clang-tools-extra/clangd/CMakeLists.txt +++ b/clang-tools-extra/clangd/CMakeLists.txt @@ -81,6 +81,7 @@ SemanticSelection.cpp SourceCode.cpp QueryDriverDatabase.cpp + TidyProvider.cpp TUScheduler.cpp URI.cpp XRefs.cpp diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -41,13 +41,6 @@ namespace clang { namespace clangd { - -/// When set, used by ClangdServer to get clang-tidy options for each particular -/// file. Must be thread-safe. We use this instead of ClangTidyOptionsProvider -/// to allow reading tidy configs from the VFS used for parsing. -using ClangTidyOptionsBuilder = std::function; - /// Manages a collection of source files and derived data (ASTs, indexes), /// and provides language-aware features such as code completion. /// @@ -121,12 +114,9 @@ /// If set, queried to obtain the configuration to handle each request. config::Provider *ConfigProvider = nullptr; - /// If set, enable clang-tidy in clangd and use to it get clang-tidy - /// configurations for a particular file. - /// Clangd supports only a small subset of ClangTidyOptions, these options - /// (Checks, CheckOptions) are about which clang-tidy checks will be - /// enabled. - ClangTidyOptionsBuilder GetClangTidyOptions; + /// The Options provider to use when running clang-tidy. If null, clang-tidy + /// checks will be disabled. + TidyProviderRef ClangTidyProvider; /// If true, force -frecovery-ast flag. /// If false, respect the value in clang. @@ -381,7 +371,7 @@ std::vector> MergedIdx; // When set, provides clang-tidy options for a specific file. - ClangTidyOptionsBuilder GetClangTidyOptions; + TidyProviderRef ClangTidyProvider; // If this is true, suggest include insertion fixes for diagnostic errors that // can be caused by missing includes (e.g. member access in incomplete type). 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 @@ -114,40 +114,6 @@ 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() { @@ -178,7 +144,7 @@ ? new FileIndex(Opts.HeavyweightDynamicSymbolIndex, Opts.CollectMainFileRefs) : nullptr), - GetClangTidyOptions(Opts.GetClangTidyOptions), + ClangTidyProvider(Opts.ClangTidyProvider), SuggestMissingIncludes(Opts.SuggestMissingIncludes), BuildRecoveryAST(Opts.BuildRecoveryAST), PreserveRecoveryASTType(Opts.PreserveRecoveryASTType), @@ -236,20 +202,6 @@ llvm::StringRef Version, WantDiagnostics WantDiags, bool ForceRebuild) { ParseOptions Opts; - Opts.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults(); - // FIXME: call tidy options builder on the worker thread, it can do IO. - 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. @@ -260,6 +212,7 @@ Inputs.ForceRebuild = ForceRebuild; Inputs.Opts = std::move(Opts); Inputs.Index = Index; + Inputs.ClangTidyProvider = ClangTidyProvider; Inputs.Opts.BuildRecoveryAST = BuildRecoveryAST; Inputs.Opts.PreserveRecoveryASTType = PreserveRecoveryASTType; bool NewFile = WorkScheduler.update(File, Inputs, WantDiags); diff --git a/clang-tools-extra/clangd/Compiler.h b/clang-tools-extra/clangd/Compiler.h --- a/clang-tools-extra/clangd/Compiler.h +++ b/clang-tools-extra/clangd/Compiler.h @@ -15,8 +15,8 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILER_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILER_H -#include "../clang-tidy/ClangTidyOptions.h" #include "GlobalCompilationDatabase.h" +#include "TidyProvider.h" #include "index/Index.h" #include "support/ThreadsafeFS.h" #include "clang/Frontend/CompilerInstance.h" @@ -37,7 +37,6 @@ // Options to run clang e.g. when parsing AST. struct ParseOptions { - tidy::ClangTidyOptions ClangTidyOpts; bool SuggestMissingIncludes = false; bool BuildRecoveryAST = false; bool PreserveRecoveryASTType = false; @@ -56,6 +55,7 @@ // Used to recover from diagnostics (e.g. find missing includes for symbol). const SymbolIndex *Index = nullptr; ParseOptions Opts = ParseOptions(); + TidyProviderRef ClangTidyProvider = {}; }; /// Builds compiler invocation that could be used to build AST or preamble. diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -17,6 +17,7 @@ #include "IncludeFixer.h" #include "Preamble.h" #include "SourceCode.h" +#include "TidyProvider.h" #include "index/CanonicalIncludes.h" #include "index/Index.h" #include "support/Logger.h" @@ -236,6 +237,10 @@ } // namespace +/// Empty clang tidy provider, using this as a provider will disable clang-tidy. +static void emptyTidyProvider(tidy::ClangTidyOptions &, llvm::StringRef, + unsigned &) {} + llvm::Optional ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, std::unique_ptr CI, @@ -292,16 +297,17 @@ llvm::Optional CTContext; { trace::Span Tracer("ClangTidyInit"); - dlog("ClangTidy configuration for file {0}: {1}", Filename, - tidy::configurationAsText(Inputs.Opts.ClangTidyOpts)); tidy::ClangTidyCheckFactories CTFactories; for (const auto &E : tidy::ClangTidyModuleRegistry::entries()) E.instantiate()->addCheckFactories(CTFactories); - CTContext.emplace(std::make_unique( - tidy::ClangTidyGlobalOptions(), Inputs.Opts.ClangTidyOpts)); + CTContext.emplace(asClangTidyOptionsProvider(Inputs.ClangTidyProvider + ? Inputs.ClangTidyProvider + : emptyTidyProvider)); CTContext->setDiagnosticsEngine(&Clang->getDiagnostics()); CTContext->setASTContext(&Clang->getASTContext()); CTContext->setCurrentFile(Filename); + dlog("ClangTidy configuration for file {0}: {1}", Filename, + tidy::configurationAsText(CTContext->getOptions())); CTChecks = CTFactories.createChecks(CTContext.getPointer()); llvm::erase_if(CTChecks, [&](const auto &Check) { return !Check->isLanguageVersionSupported(CTContext->getLangOpts()); diff --git a/clang-tools-extra/clangd/TidyProvider.h b/clang-tools-extra/clangd/TidyProvider.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/TidyProvider.h @@ -0,0 +1,59 @@ +//===--- TidyProvider.h - create options for running clang-tidy------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TIDYPROVIDER_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TIDYPROVIDER_H + +#include "../clang-tidy/ClangTidyOptions.h" +#include "support/ThreadsafeFS.h" +#include "llvm/ADT/FunctionExtras.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringRef.h" + +namespace clang { +namespace clangd { + +namespace detail { +using TidyProviderInterface = void(tidy::ClangTidyOptions &, + /*Filename=*/llvm::StringRef, + /*Priority=*/unsigned &); +} // namespace detail + +using TidyProvider = llvm::unique_function; +using TidyProviderRef = llvm::function_ref; + +TidyProvider combine(std::vector Providers); +std::unique_ptr + asClangTidyOptionsProvider(TidyProviderRef); + +/// Provider that just sets the defaults. +TidyProvider provideEnvironment(); + +/// Provider that will enable a nice set of default checks if none are +/// specified. +TidyProvider provideDefaultChecks(); + +/// Provider the enables a specific set of checks and warnings as errors. +TidyProvider fixedTidyProvider(llvm::StringRef Checks, + llvm::StringRef WarningsAsErrors = {}); + +/// Provider that will disable checks known to not work with clangd. +TidyProvider +disableUnusableChecks(std::vector ChecksToDisable = {}); + +/// Provider that searches for .clang-tidy configuration files in the directory +/// tree. +TidyProvider provideClangTidyFiles(ThreadsafeFS &); + +// Provider that uses clangd configuration files. +TidyProvider provideClangdConfig(); + +} // namespace clangd +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_TIDYPROVIDER_H diff --git a/clang-tools-extra/clangd/TidyProvider.cpp b/clang-tools-extra/clangd/TidyProvider.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/TidyProvider.cpp @@ -0,0 +1,222 @@ +//===--- TidyProvider.cpp - create options for running clang-tidy----------===// +// +// 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 "TidyProvider.h" +#include "Config.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/StringExtras.h" +#include "llvm/Support/Process.h" +#include "llvm/Support/VirtualFileSystem.h" +#include + +namespace clang { +namespace clangd { + +static void mergeCheckList(llvm::Optional &Checks, + llvm::StringRef List) { + if (List.empty()) + return; + if (!Checks || Checks->empty()) { + Checks.emplace(List); + return; + } + *Checks = llvm::join_items(",", *Checks, List); +} + +static llvm::Optional +tryReadConfigFile(llvm::vfs::FileSystem *FS, llvm::StringRef Directory) { + assert(!Directory.empty()); + + llvm::ErrorOr DirectoryStatus = FS->status(Directory); + + if (!DirectoryStatus || !DirectoryStatus->isDirectory()) { + llvm::errs() << "Error reading configuration from " << Directory + << ": directory doesn't exist.\n"; + return llvm::None; + } + + llvm::SmallString<128> ConfigFile(Directory); + llvm::sys::path::append(ConfigFile, ".clang-tidy"); + + llvm::ErrorOr FileStatus = FS->status(ConfigFile); + + if (!FileStatus || !FileStatus->isRegularFile()) + return llvm::None; + + llvm::ErrorOr> Text = + FS->getBufferForFile(ConfigFile); + if (std::error_code EC = Text.getError()) { + llvm::errs() << "Can't read " << ConfigFile << ": " << EC.message() << "\n"; + return llvm::None; + } + + // Skip empty files, e.g. files opened for writing via shell output + // redirection. + if ((*Text)->getBuffer().empty()) + return llvm::None; + llvm::ErrorOr ParsedOptions = + tidy::parseConfiguration((*Text)->getBuffer()); + if (!ParsedOptions) { + if (ParsedOptions.getError()) + llvm::errs() << "Error parsing " << ConfigFile << ": " + << ParsedOptions.getError().message() << "\n"; + return llvm::None; + } + return std::move(*ParsedOptions); +} + +TidyProvider provideEnvironment() { + return [](tidy::ClangTidyOptions &Opts, llvm::StringRef, unsigned &Order) { + auto EmptyDefaults = tidy::ClangTidyOptions::getDefaults(); + EmptyDefaults.Checks.reset(); // So we can tell if checks were ever set. + EmptyDefaults.User = llvm::sys::Process::GetEnv("USER"); +#ifdef _WIN32 + if (!EmptyDefaults.User) + EmptyDefaults.User = llvm::sys::Process::GetEnv("USERNAME"); +#endif + Opts.mergeWith(EmptyDefaults, Order); + }; +} + +TidyProvider provideDefaultChecks() { + return [](tidy::ClangTidyOptions &Opts, llvm::StringRef, unsigned &Order) { + if (!Opts.Checks || Opts.Checks->empty()) { + // 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"); + Opts.Checks.emplace(DefaultChecks); + } + }; +} + +TidyProvider fixedTidyProvider(llvm::StringRef Checks, + llvm::StringRef WarningsAsErrors) { + return [Checks = std::string(Checks), + WarningsAsErrors = std::string(WarningsAsErrors)]( + tidy::ClangTidyOptions &Opts, llvm::StringRef, unsigned &Order) { + mergeCheckList(Opts.Checks, Checks); + mergeCheckList(Opts.WarningsAsErrors, WarningsAsErrors); + }; +} + +TidyProvider disableUnusableChecks(std::vector ChecksToDisable) { + llvm::erase_if(ChecksToDisable, + [](const std::string &Str) { return Str.empty(); }); + for (std::string &Str : ChecksToDisable) { + if (Str.front() != '-') + Str.insert(Str.begin(), '-'); + } + return [ChecksToDisable = std::move(ChecksToDisable)]( + tidy::ClangTidyOptions &Opts, llvm::StringRef, unsigned &Order) { + if (Opts.Checks && !Opts.Checks->empty()) { + 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"); + Opts.Checks = + llvm::join_items(",", *Opts.Checks, FalsePositives, CrashingChecks, + llvm::join(ChecksToDisable, ",")); + } + }; +} + +TidyProvider provideClangdConfig() { + return [](tidy::ClangTidyOptions &Opts, llvm::StringRef, unsigned &Order) { + const auto &CurTidyConfig = Config::current().ClangTidy; + if (!CurTidyConfig.Checks.empty()) + mergeCheckList(Opts.Checks, CurTidyConfig.Checks); + + for (const auto &CheckOption : CurTidyConfig.CheckOptions) + Opts.CheckOptions.insert_or_assign(CheckOption.getKey(), + tidy::ClangTidyOptions::ClangTidyValue( + CheckOption.getValue(), Order)); + }; +} + +TidyProvider provideClangTidyFiles(ThreadsafeFS &TFS) { + return [&](tidy::ClangTidyOptions &Opts, llvm::StringRef Filename, + unsigned &Order) { + llvm::SmallVector OptionStack; + auto FS(TFS.view(llvm::None)); + llvm::SmallString<128> AbsolutePath(Filename); + + if (FS->makeAbsolute(AbsolutePath)) + return; + + llvm::StringRef Path = llvm::sys::path::parent_path(AbsolutePath); + for (llvm::StringRef CurrentPath = Path; !CurrentPath.empty(); + CurrentPath = llvm::sys::path::parent_path(CurrentPath)) { + auto ConfigFile = tryReadConfigFile(FS.get(), CurrentPath); + if (!ConfigFile) + continue; + OptionStack.push_back(std::move(*ConfigFile)); + if (!OptionStack.back().InheritParentConfig.getValueOr(false)) + break; + } + for (auto &Option : llvm::reverse(OptionStack)) + Opts.mergeWith(Option, ++Order); + }; +} + +TidyProvider combine(std::vector Providers) { + return [Providers = std::move(Providers)](tidy::ClangTidyOptions &Opts, + llvm::StringRef Filename, + unsigned &Order) mutable { + for (auto &Provider : Providers) { + Provider(Opts, Filename, Order); + ++Order; + } + }; +} + +namespace { + +class TidyProviderInstance : public tidy::ClangTidyOptionsProvider { +public: + TidyProviderInstance(TidyProviderRef Provider) : Provider(Provider) {} + + const tidy::ClangTidyGlobalOptions &getGlobalOptions() override { + return Defaults; + } + std::vector getRawOptions(llvm::StringRef FileName) override { + std::vector Opts; + Opts.emplace_back(); + unsigned Priority = 0; + Provider(Opts.back().first, FileName, Priority); + return Opts; + } + +private: + TidyProviderRef Provider; + static const tidy::ClangTidyGlobalOptions Defaults; +}; +const tidy::ClangTidyGlobalOptions TidyProviderInstance::Defaults; +} // namespace + +std::unique_ptr +asClangTidyOptionsProvider(TidyProviderRef Provider) { + return std::make_unique(Provider); +} +} // namespace clangd +} // namespace clang diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -133,8 +133,6 @@ return false; } } - Inputs.Opts.ClangTidyOpts = - Opts.GetClangTidyOptions(*TFS.view(llvm::None), File); log("Parsing command..."); Invocation = buildCompilerInvocation(Inputs, CaptureInvocationDiags, &CC1Args); 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 @@ -11,6 +11,7 @@ #include "Features.inc" #include "PathMapping.h" #include "Protocol.h" +#include "TidyProvider.h" #include "Transport.h" #include "index/Background.h" #include "index/Index.h" @@ -162,6 +163,10 @@ init(""), }; +list DisabledClangTidyChecks{ + "disable-clang-tidy-checks", cat(Features), + desc("List of clang-tidy checks to disable"), Hidden, CommaSeparated}; + opt CodeCompletionParse{ "completion-parse", cat(Features), @@ -837,35 +842,21 @@ } // Create an empty clang-tidy option. - std::mutex ClangTidyOptMu; - std::unique_ptr - ClangTidyOptProvider; /*GUARDED_BY(ClangTidyOptMu)*/ + TidyProvider ClangTidyOptProvider; if (EnableClangTidy) { - auto EmptyDefaults = tidy::ClangTidyOptions::getDefaults(); - EmptyDefaults.Checks.reset(); // So we can tell if checks were ever set. - EmptyDefaults.User = llvm::sys::Process::GetEnv("USER"); -#ifdef _WIN32 - if (!EmptyDefaults.User) - EmptyDefaults.User = llvm::sys::Process::GetEnv("USERNAME"); -#endif - tidy::ClangTidyOptions OverrideClangTidyOptions; - if (!ClangTidyChecks.empty()) - OverrideClangTidyOptions.Checks = ClangTidyChecks; - ClangTidyOptProvider = std::make_unique( - tidy::ClangTidyGlobalOptions(), - /* Default */ EmptyDefaults, - /* Override */ OverrideClangTidyOptions, TFS.view(/*CWD=*/llvm::None)); - Opts.GetClangTidyOptions = [&](llvm::vfs::FileSystem &, - llvm::StringRef File) { - // This function must be thread-safe and tidy option providers are not. - tidy::ClangTidyOptions Opts; - { - std::lock_guard Lock(ClangTidyOptMu); - // FIXME: use the FS provided to the function. - Opts = ClangTidyOptProvider->getOptions(File); - } - return Opts; - }; + std::vector Providers; + Providers.reserve(4 + EnableConfig); + Providers.push_back(provideEnvironment()); + Providers.push_back(provideClangTidyFiles(TFS)); + if (EnableConfig) + Providers.push_back(provideClangdConfig()); + if (ClangTidyChecks.empty()) + Providers.push_back(fixedTidyProvider(ClangTidyChecks)); + else + Providers.push_back(provideDefaultChecks()); + Providers.push_back(disableUnusableChecks(DisabledClangTidyChecks)); + ClangTidyOptProvider = combine(std::move(Providers)); + Opts.ClangTidyProvider = ClangTidyOptProvider; } Opts.AsyncPreambleBuilds = AsyncPreamble; Opts.SuggestMissingIncludes = SuggestMissingIncludes; 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 @@ -16,6 +16,7 @@ #include "SyncAPI.h" #include "TestFS.h" #include "TestTU.h" +#include "TidyProvider.h" #include "URI.h" #include "support/MemoryTree.h" #include "support/Path.h" @@ -1214,17 +1215,25 @@ bool HadDiagsInLastCallback = false; } DiagConsumer; + // We need to use the actual options provider here as we're interested in + // making sure those providers correctly disable problematic checks. + // As an added bonus it demonstrates the provider is capable of handling the + // ThreadsafeFS. + MockFS FS; + FS.Files[testPath(".clang-tidy")] = R"( + Checks: -*,bugprone-use-after-move,llvm-header-guard + )"; MockCompilationDatabase CDB; + std::vector Stack; + Stack.push_back(provideEnvironment()); + Stack.push_back(provideClangTidyFiles(FS)); + Stack.push_back(provideDefaultChecks()); + Stack.push_back(disableUnusableChecks()); + TidyProvider Provider = combine(std::move(Stack)); 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; - }; + Opts.ClangTidyProvider = Provider; ClangdServer Server(CDB, FS, Opts, &DiagConsumer); const char *SourceContents = R"cpp( struct Foo { Foo(); Foo(Foo&); Foo(Foo&&); }; diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -14,6 +14,7 @@ #include "TestFS.h" #include "TestIndex.h" #include "TestTU.h" +#include "TidyProvider.h" #include "index/MemIndex.h" #include "support/Path.h" #include "clang/Basic/Diagnostic.h" @@ -129,7 +130,7 @@ } )cpp"); auto TU = TestTU::withCode(Test.code()); - TU.ClangTidyChecks = "-*,google-explicit-constructor"; + TU.ClangTidyProvider = fixedTidyProvider("-*,google-explicit-constructor"); EXPECT_THAT( TU.build().getDiagnostics(), ElementsAre( @@ -201,8 +202,9 @@ auto TU = TestTU::withCode(Test.code()); // Enable alias clang-tidy checks, these check emits the same diagnostics // (except the check name). - TU.ClangTidyChecks = "-*, readability-uppercase-literal-suffix, " - "hicpp-uppercase-literal-suffix"; + TU.ClangTidyProvider = + fixedTidyProvider("-*, readability-uppercase-literal-suffix, " + "hicpp-uppercase-literal-suffix"); // Verify that we filter out the duplicated diagnostic message. EXPECT_THAT( TU.build().getDiagnostics(), @@ -245,9 +247,9 @@ )cpp"); auto TU = TestTU::withCode(Test.code()); TU.HeaderFilename = "assert.h"; // Suppress "not found" error. - TU.ClangTidyChecks = + TU.ClangTidyProvider = fixedTidyProvider( "-*, bugprone-sizeof-expression, bugprone-macro-repeated-side-effects, " - "modernize-deprecated-headers, modernize-use-trailing-return-type"; + "modernize-deprecated-headers, modernize-use-trailing-return-type"); EXPECT_THAT( TU.build().getDiagnostics(), UnorderedElementsAre( @@ -289,7 +291,7 @@ auto TU = TestTU::withCode(Test.code()); TU.ExtraArgs = {"-isystem."}; TU.AdditionalFiles["a.h"] = TU.AdditionalFiles["b.h"] = ""; - TU.ClangTidyChecks = "-*, llvm-include-order"; + TU.ClangTidyProvider = fixedTidyProvider("-*, llvm-include-order"); EXPECT_THAT( TU.build().getDiagnostics(), Contains(AllOf(Diag(Test.range(), "#includes are not sorted properly"), @@ -361,7 +363,7 @@ } )cpp"); TestTU TU = TestTU::withCode(Main.code()); - TU.ClangTidyChecks = "modernize-loop-convert"; + TU.ClangTidyProvider = fixedTidyProvider("modernize-loop-convert"); EXPECT_THAT( TU.build().getDiagnostics(), UnorderedElementsAre(::testing::AllOf( @@ -384,7 +386,7 @@ } )cpp"); TestTU TU = TestTU::withCode(Main.code()); - TU.ClangTidyChecks = "bugprone-integer-division"; + TU.ClangTidyProvider = fixedTidyProvider("bugprone-integer-division"); EXPECT_THAT( TU.build().getDiagnostics(), UnorderedElementsAre(::testing::AllOf( @@ -401,8 +403,8 @@ } )cpp"); TestTU TU = TestTU::withCode(Main.code()); - TU.ClangTidyChecks = "bugprone-integer-division"; - TU.ClangTidyWarningsAsErrors = "bugprone-integer-division"; + TU.ClangTidyProvider = fixedTidyProvider("bugprone-integer-division", + "bugprone-integer-division"); EXPECT_THAT( TU.build().getDiagnostics(), UnorderedElementsAre(::testing::AllOf( @@ -452,8 +454,8 @@ } )cpp"); TestTU TU = TestTU::withCode(Main.code()); - TU.ClangTidyChecks = "bugprone-integer-division"; - TU.ClangTidyWarningsAsErrors = "bugprone-integer-division"; + TU.ClangTidyProvider = fixedTidyProvider("bugprone-integer-division", + "bugprone-integer-division"); EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); } @@ -468,7 +470,8 @@ } )cpp"); TestTU TU = TestTU::withCode(Main.code()); - TU.ClangTidyChecks = "-*,bugprone-bad-signal-to-kill-thread"; + TU.ClangTidyProvider = + fixedTidyProvider("-*,bugprone-bad-signal-to-kill-thread"); EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash } diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -24,6 +24,7 @@ #include "SourceCode.h" #include "TestFS.h" #include "TestTU.h" +#include "TidyProvider.h" #include "clang/AST/DeclTemplate.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" @@ -250,7 +251,8 @@ TestTU TU; // this check runs the preprocessor, we need to make sure it does not break // our recording logic. - TU.ClangTidyChecks = "modernize-use-trailing-return-type"; + TU.ClangTidyProvider = + fixedTidyProvider("modernize-use-trailing-return-type"); TU.Code = "inline int foo() {}"; auto AST = TU.build(); @@ -406,7 +408,7 @@ "replay-preamble-module", ""); TestTU TU; // This check records inclusion directives replayed by clangd. - TU.ClangTidyChecks = "replay-preamble-check"; + TU.ClangTidyProvider = fixedTidyProvider("replay-preamble-check"); llvm::Annotations Test(R"cpp( $hash^#$include[[import]] $filebegin^"$filerange[[bar.h]]" $hash^#$include[[include_next]] $filebegin^"$filerange[[baz.h]]" diff --git a/clang-tools-extra/clangd/unittests/TestTU.h b/clang-tools-extra/clangd/unittests/TestTU.h --- a/clang-tools-extra/clangd/unittests/TestTU.h +++ b/clang-tools-extra/clangd/unittests/TestTU.h @@ -17,6 +17,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H +#include "../TidyProvider.h" #include "Compiler.h" #include "ParsedAST.h" #include "TestFS.h" @@ -58,8 +59,7 @@ // Extra arguments for the compiler invocation. std::vector ExtraArgs; - llvm::Optional ClangTidyChecks; - llvm::Optional ClangTidyWarningsAsErrors; + mutable TidyProvider ClangTidyProvider = {}; // Index to use when building AST. const SymbolIndex *ExternalIndex = nullptr; diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp --- a/clang-tools-extra/clangd/unittests/TestTU.cpp +++ b/clang-tools-extra/clangd/unittests/TestTU.cpp @@ -59,8 +59,8 @@ FS.OverlayRealFileSystemForModules = true; Inputs.TFS = &FS; Inputs.Opts = ParseOptions(); - Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks; - Inputs.Opts.ClangTidyOpts.WarningsAsErrors = ClangTidyWarningsAsErrors; + if (ClangTidyProvider) + Inputs.ClangTidyProvider = ClangTidyProvider; Inputs.Index = ExternalIndex; if (Inputs.Index) Inputs.Opts.SuggestMissingIncludes = true;