njames93 created this revision.
njames93 added reviewers: sammccall, kadircet.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
njames93 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Expand D92874 <https://reviews.llvm.org/D92874> to also ensure all items in
CheckOptions correspond to actual options in the clang-tidy checks.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126859
Files:
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/TidyProvider.cpp
clang-tools-extra/clangd/TidyProvider.h
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ 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) {
Index: clang-tools-extra/clangd/TidyProvider.h
===================================================================
--- clang-tools-extra/clangd/TidyProvider.h
+++ 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
Index: clang-tools-extra/clangd/TidyProvider.cpp
===================================================================
--- clang-tools-extra/clangd/TidyProvider.cpp
+++ 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 <memory>
@@ -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<llvm::BumpPtrAllocator> AllChecks = [] {
- llvm::StringSet<llvm::BumpPtrAllocator> Result;
+struct AllCheckInfo {
+ using SetType = llvm::StringSet<llvm::BumpPtrAllocator &>;
+ 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<AllCheckInfo> CheckInfo;
+
+} // namespace
- return AllChecks.contains(Check);
+static void getBestGuess(StringRef Str, const AllCheckInfo::SetType &Items,
+ StringRef &Result) {
+ unsigned MaxEditDistance = std::min<unsigned>(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
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ 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<std::pair<std::string, std::string>> 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)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
