[PATCH] D92874: [clangd] Validate clang-tidy Checks in clangd config.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks! Just some organization nits.



================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:26
 
+#include "../clang-tidy/ClangTidyModuleRegistry.h"
 #include "CompileCommands.h"
----------------
This is a pretty weird place to depend on clang-tidy.
Can we move the "is this a clang-tidy check name" function to somewhere more clang-tidy related, like `TidyProvider.h` or even `clang-tidy/ClangTidyModuleRegistry.h`? ("isRegisteredCheck")


================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:355
+  // tidy check, most likely due to a typo.
+  static bool isInvalidCheckGlob(StringRef CheckGlob) {
+    // Any wildcards just assume they aren't invalid
----------------
naming nits:
 - prefer positive over negative names
 - "glob" is confusing here as we don't actually handle globs
 - outside of the clang-tidy namespace, "check" needs a "tidy" qualifier

What about `isTidyCheckName`, and move the `*` test into the caller? Then the semantics of the function are really clear without having to read the body or comment.


================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:386
+           llvm::formatv(
+               "Check glob '{0}' doesn't specify any known clang-tidy check",
+               Str)
----------------
message could be clearer - "clang-tidy check '{0}' was not found"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92874/new/

https://reviews.llvm.org/D92874