diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -504,9 +504,17 @@ }); if (!F.CheckOptions.empty()) { std::vector> CheckOptions; - for (auto &Opt : F.CheckOptions) + CheckOptions.reserve(F.CheckOptions.size()); + for (auto &Opt : F.CheckOptions) { + if (!isRecognisedTidyOption(*Opt.first)) { + diag(Warning, + llvm::formatv("unknown clang-tidy option '{0}'", *Opt.first) + .str(), + Opt.first.Range); + } CheckOptions.emplace_back(std::move(*Opt.first), std::move(*Opt.second)); + } Out.Apply.push_back( [CheckOptions = std::move(CheckOptions)](const Params &, Config &C) { for (auto &StringPair : CheckOptions) diff --git a/clang-tools-extra/clangd/TidyProvider.h b/clang-tools-extra/clangd/TidyProvider.h --- a/clang-tools-extra/clangd/TidyProvider.h +++ b/clang-tools-extra/clangd/TidyProvider.h @@ -60,6 +60,8 @@ /// \pre \p must not be empty, must not contain '*' or ',' or start with '-'. bool isRegisteredTidyCheck(llvm::StringRef Check); +bool isRecognisedTidyOption(llvm::StringRef CheckOption); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/TidyProvider.cpp b/clang-tools-extra/clangd/TidyProvider.cpp --- a/clang-tools-extra/clangd/TidyProvider.cpp +++ b/clang-tools-extra/clangd/TidyProvider.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "TidyProvider.h" +#include "../clang-tidy/ClangTidy.h" #include "../clang-tidy/ClangTidyModuleRegistry.h" #include "Config.h" #include "support/FileCache.h" @@ -18,6 +19,7 @@ #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringSet.h" #include "llvm/Support/Allocator.h" +#include "llvm/Support/ManagedStatic.h" #include "llvm/Support/Process.h" #include "llvm/Support/SourceMgr.h" #include @@ -283,24 +285,30 @@ return Opts; } +namespace { +struct Creator { + static void *call() { + return new tidy::NamesAndOptions(tidy::getAllChecksAndOptions(false)); + } +}; +} // namespace + +static llvm::ManagedStatic CheckInfo; + bool isRegisteredTidyCheck(llvm::StringRef Check) { assert(!Check.empty()); assert(!Check.contains('*') && !Check.contains(',') && "isRegisteredCheck doesn't support globs"); assert(Check.ltrim().front() != '-'); - static const llvm::StringSet AllChecks = [] { - llvm::StringSet Result; - tidy::ClangTidyCheckFactories Factories; - for (tidy::ClangTidyModuleRegistry::entry E : - tidy::ClangTidyModuleRegistry::entries()) - E.instantiate()->addCheckFactories(Factories); - for (const auto &Factory : Factories) - Result.insert(Factory.getKey()); - return Result; - }(); + return CheckInfo->Names.contains(Check); +} - return AllChecks.contains(Check); +bool isRecognisedTidyOption(llvm::StringRef CheckOption) { + assert(!CheckOption.empty()); + + return CheckInfo->Options.contains(CheckOption); } + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp --- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -338,7 +338,9 @@ EXPECT_EQ( Conf.Diagnostics.ClangTidy.Checks, "bugprone-use-after-move,llvm-*,-llvm-include-order,-readability-*"); - EXPECT_THAT(Diags.Diagnostics, IsEmpty()); + EXPECT_THAT(Diags.Diagnostics, + ElementsAre(diagMessage( + "unknown clang-tidy option 'example-check.ExampleOption'"))); #else // !CLANGD_TIDY_CHECKS EXPECT_EQ(Conf.Diagnostics.ClangTidy.Checks, "llvm-*,-readability-*"); EXPECT_THAT( @@ -346,7 +348,9 @@ ElementsAre( diagMessage( "clang-tidy check 'bugprone-use-after-move' was not found"), - diagMessage("clang-tidy check 'llvm-include-order' was not found"))); + diagMessage("clang-tidy check 'llvm-include-order' was not found"), + diagMessage( + "unknown clang-tidy option 'example-check.ExampleOption'"))); #endif } @@ -355,6 +359,7 @@ Tidy.Add.emplace_back("unknown-check"); Tidy.Remove.emplace_back("*"); Tidy.Remove.emplace_back("llvm-includeorder"); + EXPECT_TRUE(compileAndApply()); // Ensure bad checks are stripped from the glob. EXPECT_EQ(Conf.Diagnostics.ClangTidy.Checks, "-*"); @@ -368,6 +373,43 @@ diagKind(llvm::SourceMgr::DK_Warning)))); } +TEST_F(ConfigCompileTests, TidyBadOptions) { + auto &Tidy = Frag.Diagnostics.ClangTidy; + Tidy.CheckOptions.emplace_back( + std::make_pair(std::string("BadGlobal"), std::string("true"))); + Tidy.CheckOptions.emplace_back( + std::make_pair(std::string("StrictModes"), std::string("true"))); + Tidy.CheckOptions.emplace_back(std::make_pair( + std::string("readability-braces-around-statements.ShortStatementsLines"), + std::string("1"))); + Tidy.CheckOptions.emplace_back(std::make_pair( + std::string("readability-braces-around-statements.ShortStatementLines"), + std::string("1"))); + EXPECT_TRUE(compileAndApply()); +#if CLANGD_TIDY_CHECKS + EXPECT_THAT( + Diags.Diagnostics, + ElementsAre( + diagMessage("unknown clang-tidy option 'BadGlobal'"), + diagMessage("unknown clang-tidy option 'StrictModes'"), + diagMessage( + "unknown clang-tidy option " + "'readability-braces-around-statements.ShortStatementsLines'"))); +#else // !CLANGD_TIDY_CHECKS + EXPECT_THAT( + Diags.Diagnostics, + ElementsAre( + diagMessage("unknown clang-tidy option 'BadGlobal'"), + diagMessage("unknown clang-tidy option 'StrictModes';"), + diagMessage( + "unknown clang-tidy option " + "'readability-braces-around-statements.ShortStatementsLines'"), + diagMessage( + "unknown clang-tidy option " + "'readability-braces-around-statements.ShortStatementLines'"))); +#endif +} + TEST_F(ConfigCompileTests, ExternalServerNeedsTrusted) { Fragment::IndexBlock::ExternalBlock External; External.Server.emplace("xxx");