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 @@ -473,10 +473,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 += ','; @@ -503,9 +512,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,90 @@ return Opts; } -bool isRegisteredTidyCheck(llvm::StringRef Check) { - assert(!Check.empty()); - assert(!Check.contains('*') && !Check.contains(',') && - "isRegisteredCheck doesn't support globs"); - assert(Check.ltrim().front() != '-'); +namespace { - static const llvm::StringSet AllChecks = [] { - llvm::StringSet Result; +struct AllCheckInfo { + using SetType = llvm::StringSet; + llvm::BumpPtrAllocator Alloc; + SetType Names; + SetType Options; + AllCheckInfo() : Names(Alloc), Options(Alloc) { + // Get Check Names 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; - }(); + Names.insert(Factory.getKey()); + + // GetCheckOptions + tidy::ClangTidyOptions TidyOptions; + TidyOptions.Checks = "*"; + auto CheckOptions = tidy::getCheckOptions(TidyOptions, false); + for (llvm::StringRef OptionName : CheckOptions.keys()) { + Options.insert(OptionName); + } + // No easy way to extract the global options so just hard code them. + static constexpr llvm::StringLiteral GlobalOptions[] = { + "AggressiveDependentMemberLookup", + "CheckFirstDeclaration", + "EnableProto", + "IgnoreMacros", + "IncludeStyle", + "Strict", + "StrictMode", + "UseAssignment"}; + for (auto GlobalOption : GlobalOptions) { + Options.insert(GlobalOption); + } + } +}; + +llvm::ManagedStatic CheckInfo; + +} // namespace - return AllChecks.contains(Check); +static void getBestGuess(StringRef Str, const AllCheckInfo::SetType &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() != '-'); + + if (CheckInfo->Names.contains(Check)) + return true; + if (Typo) + getBestGuess(Check, CheckInfo->Names, *Typo); + return false; +} + +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) {