njames93 updated this revision to Diff 311948.
njames93 marked 2 inline comments as done.
njames93 added a comment.
Address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92874/new/
https://reviews.llvm.org/D92874
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
@@ -200,6 +200,24 @@
EXPECT_EQ(Conf.ClangTidy.CheckOptions.lookup("StrictMode"), "true");
EXPECT_EQ(Conf.ClangTidy.CheckOptions.lookup("example-check.ExampleOption"),
"0");
+ EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+}
+
+TEST_F(ConfigCompileTests, TidyBadChecks) {
+ Frag.ClangTidy.Add.emplace_back("unknown-check");
+ Frag.ClangTidy.Remove.emplace_back("*");
+ Frag.ClangTidy.Remove.emplace_back("llvm-includeorder");
+ EXPECT_TRUE(compileAndApply());
+ // Ensure bad checks are stripped from the glob.
+ EXPECT_EQ(Conf.ClangTidy.Checks, "-*");
+ EXPECT_THAT(
+ Diags.Diagnostics,
+ 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))));
}
TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) {
Index: clang-tools-extra/clangd/TidyProvider.h
===================================================================
--- clang-tools-extra/clangd/TidyProvider.h
+++ clang-tools-extra/clangd/TidyProvider.h
@@ -13,6 +13,7 @@
#include "support/ThreadsafeFS.h"
#include "llvm/ADT/FunctionExtras.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
namespace clang {
namespace clangd {
@@ -56,6 +57,10 @@
tidy::ClangTidyOptions getTidyOptionsForFile(TidyProviderRef Provider,
llvm::StringRef Filename);
+/// 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);
+
} // 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/ClangTidyModuleRegistry.h"
#include "Config.h"
#include "support/FileCache.h"
#include "support/Logger.h"
@@ -14,6 +15,8 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Allocator.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/Process.h"
#include "llvm/Support/VirtualFileSystem.h"
@@ -266,5 +269,25 @@
Provider(Opts, Filename);
return Opts;
}
+
+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<llvm::BumpPtrAllocator> AllChecks = [] {
+ llvm::StringSet<llvm::BumpPtrAllocator> 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 AllChecks.contains(Check);
+}
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -28,6 +28,7 @@
#include "ConfigFragment.h"
#include "ConfigProvider.h"
#include "Features.inc"
+#include "TidyProvider.h"
#include "support/Logger.h"
#include "support/Trace.h"
#include "llvm/ADT/None.h"
@@ -349,13 +350,19 @@
void appendTidyCheckSpec(std::string &CurSpec,
const Located<std::string> &Arg, bool IsPositive) {
- StringRef Str = *Arg;
+ StringRef Str = StringRef(*Arg).trim();
// Don't support negating here, its handled if the item is in the Add or
// Remove list.
if (Str.startswith("-") || Str.contains(',')) {
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);
+ return;
+ }
CurSpec += ',';
if (!IsPositive)
CurSpec += '-';
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
