[clangd] Move dependencies of ClangdLSPServer out of its implementation

[krasimir]

Summary

This patch moves the GlobalCompilationDatabase and FileSystemProvider
dependencies of ClangdLSPServer to ClangdMain and makes ClangdLSPServer itself a DiagnosticsConsumer.

[ilya-biryukov]

clangd/ClangdLSPServer.h

26

I would not expect ClangdLSPServer to be passed as a DiagnosticsConsumer.
It's not the purpose of this class at all, therefore I don't think it's a good idea to inherit publicly from DiagnosticsConsumer.
Private inheritance would be fine (to reduce code repetition), but is generally considered to be hard-to-grasp.

29

What are the use-cases for getting GlobalCompilationDatabase and FileSystemProvider as an outside parameter?
It actually seems to me they are implementation details of ClangdLSPServer.

I.e. the purpose of this class is to provide an easy interface to just run LSP server without worrying about its configuration.

[krasimir]

On a second thought, meh :D

clangd/ClangdLSPServer.h

 
 namespace clang {
 namespace clangd {
 
 class JSONOutput;
 
 /// This class provides implementation of an LSP server, glueing the JSON
 /// dispatch and ClangdServer together.
-class ClangdLSPServer {
+class ClangdLSPServer : public DiagnosticsConsumer {
 public:
-  ClangdLSPServer(JSONOutput &Out, bool RunSynchronously);
+  ClangdLSPServer(JSONOutput &Out,
+                  GlobalCompilationDatabase &CDB,
+                  FileSystemProvider &FSProvider,
+                  bool RunSynchronously);
 
   /// Run LSP server loop, receiving input for it from \p In. \p In must be
   /// opened in binary mode. Output will be written using Out variable passed to
   /// class constructor. This method must not be executed more than once for
   /// each instance of ClangdLSPServer.
   void run(std::istream &In);
 
-private:
-  class LSPProtocolCallbacks;
-  class LSPDiagnosticsConsumer : public DiagnosticsConsumer {
-  public:
-    LSPDiagnosticsConsumer(ClangdLSPServer &Server);
-
-    virtual void
-    onDiagnosticsReady(PathRef File,
-                       Tagged<std::vector<DiagWithFixIts>> Diagnostics);
+  void
+                     Tagged<std::vector<DiagWithFixIts>> Diagnostics) override;
 
-  private:
-    ClangdLSPServer &Server;
-  };
+  class LSPProtocolCallbacks;
 
   std::vector<clang::tooling::Replacement>
   getFixIts(StringRef File, const clangd::Diagnostic &D);
 
   /// Function that will be called on a separate thread when diagnostics are
   /// ready. Sends the Dianostics to LSP client via Out.writeMessage and caches
   /// corresponding fixits in the FixItsMap.
   void consumeDiagnostics(PathRef File,
                           std::vector<DiagWithFixIts> Diagnostics);
 
   JSONOutput &Out;
   /// Used to indicate that the 'shutdown' request was received from the
   /// Language Server client.
   /// It's used to break out of the LSP parsing loop.
   bool IsDone = false;
 
   std::mutex FixItsMutex;
   typedef std::map<clangd::Diagnostic, std::vector<clang::tooling::Replacement>>
       DiagnosticToReplacementMap;
   /// Caches FixIts per file and diagnostics
   llvm::StringMap<DiagnosticToReplacementMap> FixItsMap;
 
   // Various ClangdServer parameters go here. It's important they're created
   // before ClangdServer.
-  DirectoryBasedGlobalCompilationDatabase CDB;
-  LSPDiagnosticsConsumer DiagConsumer;
-  RealFileSystemProvider FSProvider;
+  GlobalCompilationDatabase &CDB;
+  FileSystemProvider &FSProvider;
 
   // Server must be the last member of the class to allow its destructor to exit
   // the worker thread that may otherwise run an async callback on partially
   // destructed instance of ClangdLSPServer.
   ClangdServer Server;
 };
 
 } // namespace clangd
 } // namespace clang
 
 #endif

clangd/ClangdLSPServer.cpp

replacementsToEdits(StringRef Code,
   if (!Edits.empty())
     Edits.pop_back();
 
   return Edits;
 }
 
 } // namespace
 
-ClangdLSPServer::LSPDiagnosticsConsumer::LSPDiagnosticsConsumer(
-    ClangdLSPServer &Server)
-    : Server(Server) {}
-
-void ClangdLSPServer::LSPDiagnosticsConsumer::onDiagnosticsReady(
+void ClangdLSPServer::onDiagnosticsReady(
     PathRef File, Tagged<std::vector<DiagWithFixIts>> Diagnostics) {
-  Server.consumeDiagnostics(File, Diagnostics.Value);
+  consumeDiagnostics(File, Diagnostics.Value);
 }
 
 class ClangdLSPServer::LSPProtocolCallbacks : public ProtocolCallbacks {
 public:
   LSPProtocolCallbacks(ClangdLSPServer &LangServer) : LangServer(LangServer) {}
 
   void onInitialize(StringRef ID, JSONOutput &Out) override;
   void onShutdown(JSONOutput &Out) override;
void ClangdLSPServer::LSPProtocolCallbacks::onCompletion(
   }
   if (!Completions.empty())
     Completions.pop_back();
   Out.writeMessage(
       R"({"jsonrpc":"2.0","id":)" + ID.str() +
       R"(,"result":[)" + Completions + R"(]})");
 }
 
-ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, bool RunSynchronously)
-    : Out(Out), DiagConsumer(*this),
-      Server(CDB, DiagConsumer, FSProvider, RunSynchronously) {}
+ClangdLSPServer::ClangdLSPServer(JSONOutput &Out,
+                                 GlobalCompilationDatabase &CDB,
+                                 FileSystemProvider &FSProvider,
+                                 bool RunSynchronously)
+    : Out(Out), CDB(CDB), FSProvider(FSProvider),
+      Server(CDB, /*DiagnosticsConsumer=*/*this, FSProvider, RunSynchronously) {
+}
 
 void ClangdLSPServer::run(std::istream &In) {
   assert(!IsDone && "Run was called before");
 
   // Set up JSONRPCDispatcher.
   LSPProtocolCallbacks Callbacks(*this);
   JSONRPCDispatcher Dispatcher(llvm::make_unique<Handler>(Out));
   regiterCallbackHandlers(Dispatcher, Out, Callbacks);

clangd/tool/ClangdMain.cpp

int main(int argc, char *argv[]) {
 
   llvm::raw_ostream &Outs = llvm::outs();
   llvm::raw_ostream &Logs = llvm::errs();
   JSONOutput Out(Outs, Logs);
 
   // Change stdin to binary to not lose \r\n on windows.
   llvm::sys::ChangeStdinToBinary();
 
-  ClangdLSPServer LSPServer(Out, RunSynchronously);
+  DirectoryBasedGlobalCompilationDatabase CDB;
+  RealFileSystemProvider FSProvider;
+
+  ClangdLSPServer LSPServer(Out, CDB, FSProvider, RunSynchronously);
   LSPServer.run(std::cin);
 }