[PATCH] D55256: [clangd] Support clang-tidy configuration in clangd.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.


================
Comment at: clangd/tool/ClangdMain.cpp:204
 
+static llvm::cl::opt<std::string> ClangTidyChecks(
+    "clang-tidy-checks",
----------------
hokein wrote:
> sammccall wrote:
> > Maybe add a TODO or FIXME to respect .clang-tidy files?
> didn't get the point of the comment -- in this patch, clangd will read configurations from `.clang-tidy` files (`FileOptionsProvider` provides this functionality). This command-line flag is used to overwrite the `.clang-tidy` configurations,.
Ah, I missed that FileOptionsProvider was actually used. Can you update the desc to something like "List of clang-tidy checks to run (overrides .clang-tidy files)"?


================
Comment at: clangd/tool/ClangdMain.cpp:435
 
+  RealFileSystemProvider FSProvider;
+  // Create an empty option.
----------------
please move this up to a section near the other feature configuration stuff. e.g. below where CCOpts is initialized.

The Transport/ClangdLSPServer initialization is more closely related.


================
Comment at: clangd/tool/ClangdMain.cpp:438
+  auto OverrideClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
+  if (!ClangTidyChecks.empty())
+    OverrideClangTidyOptions.Checks = ClangTidyChecks;
----------------
This prevents `-clang-tidy-checks=` from disabling all checks.
use ClangTidyChecks.getNumOccurrences() instead?


================
Comment at: unittests/clangd/TestTU.cpp:39
+  Inputs.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults();
+  Inputs.ClangTidyOpts.Checks =
+      "-*, bugprone-sizeof-expression, bugprone-macro-repeated-side-effects, "
----------------
Make this list an optional attribute to TestTU instead of hard-coding?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55256