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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Nice! I think we should get to `.clang-tidy` files subsequently, but this is a great start and allows us to hook up the right set of checks in other environments.



================
Comment at: clangd/ClangdLSPServer.h:132
 
-  RealFileSystemProvider FSProvider;
   /// Options used for code completion
----------------
ilya-biryukov wrote:
> Could we instead call `getRealFS()` when we try to initialize a clang-tidy options provider in `main()` and avoid changing this?
> To avoid adding extra non-real-fs "modes of operation" to `ClangdLSPServer`. Unless you see other uses for this.
We already have out-of-tree modifications to ClangdLSPServer to use non-real FSes.
Given that, I think this change is OK... though better still might be to move it into `ClangdServer::Options`


================
Comment at: clangd/ClangdUnit.cpp:136
     auto *ExistingCallbacks = PP.getPPCallbacks();
+    // Don't reply preamble if we don't run any clang-tidy checks without
+    // PPCallbacks.
----------------
I have a hard time understanding this comment.
`No need to replay events if nobody is listening`?


================
Comment at: clangd/ClangdUnit.cpp:274
     CTContext.emplace(llvm::make_unique<tidy::DefaultOptionsProvider>(
-        tidy::ClangTidyGlobalOptions(), CTOpts));
+        tidy::ClangTidyGlobalOptions(), ClangTidyOpts));
     CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
----------------
this is a copy


================
Comment at: clangd/ClangdUnit.cpp:540
                           llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents),
-                          PCHs, std::move(VFS));
+                          PCHs, std::move(VFS), Inputs.ClangTidyOpts);
 }
----------------
this is a copy


================
Comment at: clangd/ClangdUnit.h:83
+        IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
+        tidy::ClangTidyOptions ClangTidyOpts);
 
----------------
Passing by value is OK here if deliberate, but let's try to avoid too many random copies below.


================
Comment at: clangd/tool/ClangdMain.cpp:204
 
+static llvm::cl::opt<std::string> ClangTidyChecks(
+    "clang-tidy-checks",
----------------
Maybe add a TODO or FIXME to respect .clang-tidy files?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55256