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 @@ -474,10 +474,19 @@ diag(Error, "Invalid clang-tidy check name", Arg.Range); return; } - if (!Str.contains('*') && !isRegisteredTidyCheck(Str)) { - diag(Warning, - llvm::formatv("clang-tidy check '{0}' was not found", Str).str(), - Arg.Range); + StringRef Typo; + if (!Str.contains('*') && !isRegisteredTidyCheck(Str, &Typo)) { + if (Typo.empty()) + diag(Warning, + llvm::formatv("clang-tidy check '{0}' was not found", Str).str(), + Arg.Range); + else + diag(Warning, + llvm::formatv( + "clang-tidy check '{0}' was not found; did you mean '{1}'", + Str, Typo) + .str(), + Arg.Range); return; } CurSpec += ','; @@ -504,9 +513,26 @@ }); if (!F.CheckOptions.empty()) { std::vector> CheckOptions; - for (auto &Opt : F.CheckOptions) + CheckOptions.reserve(F.CheckOptions.size()); + for (auto &Opt : F.CheckOptions) { + StringRef Typo; + if (!isRecognisedTidyOption(*Opt.first, &Typo)) { + if (Typo.empty()) + diag(Warning, + llvm::formatv("unknown clang-tidy option '{0}'", *Opt.first) + .str(), + Opt.first.Range); + else + diag(Warning, + llvm::formatv( + "unknown clang-tidy option '{0}'; did you mean '{1}'", + *Opt.first, Typo) + .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 @@ -58,7 +58,11 @@ /// Returns if \p Check is a registered clang-tidy check /// \pre \p must not be empty, must not contain '*' or ',' or start with '-'. -bool isRegisteredTidyCheck(llvm::StringRef Check); +bool isRegisteredTidyCheck(llvm::StringRef Check, + llvm::StringRef *Typo = nullptr); + +bool isRecognisedTidyOption(llvm::StringRef CheckOption, + llvm::StringRef *Typo = nullptr); } // 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,57 @@ return Opts; } -bool isRegisteredTidyCheck(llvm::StringRef Check) { +namespace { +struct Creator { + static void *call() { + return new tidy::NamesAndOptions(tidy::getAllChecksAndOptions(false)); + } +}; +} // namespace + +static llvm::ManagedStatic CheckInfo; + +static void getBestGuess(StringRef Str, const llvm::StringSet<> &Items, + StringRef &Result) { + unsigned MaxEditDistance = std::min(Str.size() / 3, 5); + if (MaxEditDistance == 0) + return; + for (auto Item : Items.keys()) { + unsigned int CurEdit = Str.edit_distance(Item, true, MaxEditDistance); + if (CurEdit <= 1) { + Result = Item; + return; + } + if (CurEdit < MaxEditDistance) { + Result = Item; + MaxEditDistance = CurEdit; + } + } +} + +bool isRegisteredTidyCheck(llvm::StringRef Check, llvm::StringRef *Typo) { 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; - }(); + if (CheckInfo->Names.contains(Check)) + return true; + if (Typo) + getBestGuess(Check, CheckInfo->Names, *Typo); + return false; +} - return AllChecks.contains(Check); +bool isRecognisedTidyOption(llvm::StringRef CheckOption, + llvm::StringRef *Typo) { + assert(!CheckOption.empty()); + + if (CheckInfo->Options.contains(CheckOption)) + return true; + if (Typo) + getBestGuess(CheckOption, CheckInfo->Options, *Typo); + return false; } + } // 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,15 @@ Tidy.Add.emplace_back("unknown-check"); Tidy.Remove.emplace_back("*"); Tidy.Remove.emplace_back("llvm-includeorder"); +#if CLANGD_TIDY_CHECKS + StringRef IncludeOrderMessage = + "clang-tidy check 'llvm-includeorder' was not found; did you mean " + "'llvm-include-order'"; +#else // !CLANGD_TIDY_CHECKS + StringRef IncludeOrderMessage = + "clang-tidy check 'llvm-includeorder' was not found"; +#endif + EXPECT_TRUE(compileAndApply()); // Ensure bad checks are stripped from the glob. EXPECT_EQ(Conf.Diagnostics.ClangTidy.Checks, "-*"); @@ -363,9 +376,49 @@ ElementsAre( AllOf(diagMessage("clang-tidy check 'unknown-check' was not found"), diagKind(llvm::SourceMgr::DK_Warning)), - AllOf( - diagMessage("clang-tidy check 'llvm-includeorder' was not found"), - diagKind(llvm::SourceMgr::DK_Warning)))); + AllOf(diagMessage(IncludeOrderMessage), + 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'; did you mean " + "'StrictMode'"), + diagMessage( + "unknown clang-tidy option " + "'readability-braces-around-statements.ShortStatementsLines'; " + "did you mean " + "'readability-braces-around-statements.ShortStatementLines'"))); +#else // !CLANGD_TIDY_CHECKS + EXPECT_THAT( + Diags.Diagnostics, + ElementsAre( + diagMessage("unknown clang-tidy option 'BadGlobal'"), + diagMessage("unknown clang-tidy option 'StrictModes'; did you mean " + "'StrictMode'"), + 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) {