[clangd] DidChangeConfiguration Notification

Summary:

Implementation of DidChangeConfiguration notification handling in
clangd.  This currently only supports changing one setting: the path of
the compilation database to be used for the current project.   In other
words, it is no longer necessary to restart clangd with a different
command line argument in order to change the compilation database.

Reviewers: malaperle, krasimir, bkramer, ilya-biryukov

Subscribers: jkorous-apple, ioeric, simark, klimek, ilya-biryukov, arphaman, rwols, cfe-commits

Differential Revision: https://reviews.llvm.org/D39571

Signed-off-by: Simon Marchi <simon.marchi@ericsson.com>
Signed-off-by: William Enright <william.enright@polymtl.ca>
llvm-svn: 325784

clang-tools-extra/clangd/ClangdLSPServer.cpp

@@ -370,6 +370,18 @@ void ClangdLSPServer::onHover(TextDocumentPositionParams &Params) {
                    });
 }
 
+// FIXME: This function needs to be properly tested.
+void ClangdLSPServer::onChangeConfiguration(
+    DidChangeConfigurationParams &Params) {
+  ClangdConfigurationParamsChange &Settings = Params.settings;
+
+  // Compilation database change.
+  if (Settings.compilationDatabasePath.hasValue()) {
+    CDB.setCompileCommandsDir(Settings.compilationDatabasePath.getValue());
+    Server.reparseOpenedFiles();
+  }
+}
+
 ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount,
                                  bool StorePreamblesInMemory,
                                  const clangd::CodeCompleteOptions &CCOpts,

clang-tools-extra/clangd/ClangdLSPServer.h

@@ -76,6 +76,7 @@ private:
   void onCommand(ExecuteCommandParams &Params) override;
   void onRename(RenameParams &Parames) override;
   void onHover(TextDocumentPositionParams &Params) override;
+  void onChangeConfiguration(DidChangeConfigurationParams &Params) override;
 
   std::vector<TextEdit> getFixIts(StringRef File, const clangd::Diagnostic &D);
 

clang-tools-extra/clangd/ClangdServer.cpp

@@ -554,6 +554,11 @@ void ClangdServer::scheduleReparseAndDiags(
                        WantDiags, std::move(Callback));
 }
 
+void ClangdServer::reparseOpenedFiles() {
+  for (const Path &FilePath : DraftMgr.getActiveFiles())
+    forceReparse(FilePath);
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.

clang-tools-extra/clangd/ClangdServer.h

@@ -163,6 +163,11 @@ public:
   /// and AST and rebuild them from scratch.
   void forceReparse(PathRef File);
 
+  /// Calls forceReparse() on all currently opened files.
+  /// As a result, this method may be very expensive.
+  /// This method is normally called when the compilation database is changed.
+  void reparseOpenedFiles();
+
   /// Run code completion for \p File at \p Pos.
   /// Request is processed asynchronously.
   ///

clang-tools-extra/clangd/DraftStore.cpp

@@ -21,6 +21,17 @@ VersionedDraft DraftStore::getDraft(PathRef File) const {
   return It->second;
 }
 
+std::vector<Path> DraftStore::getActiveFiles() const {
+  std::lock_guard<std::mutex> Lock(Mutex);
+  std::vector<Path> ResultVector;
+
+  for (auto DraftIt = Drafts.begin(); DraftIt != Drafts.end(); DraftIt++)
+    if (DraftIt->second.Draft)
+      ResultVector.push_back(DraftIt->getKey());
+
+  return ResultVector;
+}
+
 DocVersion DraftStore::getVersion(PathRef File) const {
   std::lock_guard<std::mutex> Lock(Mutex);
 

clang-tools-extra/clangd/DraftStore.h

@@ -40,6 +40,12 @@ public:
   /// \return version and contents of the stored document.
   /// For untracked files, a (0, None) pair is returned.
   VersionedDraft getDraft(PathRef File) const;
+
+  /// \return List of names of active drafts in this store.  Drafts that were
+  /// removed (which still have an entry in the Drafts map) are not returned by
+  /// this function.
+  std::vector<Path> getActiveFiles() const;
+
   /// \return version of the tracked document.
   /// For untracked files, 0 is returned.
   DocVersion getVersion(PathRef File) const;

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

@@ -51,6 +51,12 @@ DirectoryBasedGlobalCompilationDatabase::getFallbackCommand(
   return C;
 }
 
+void DirectoryBasedGlobalCompilationDatabase::setCompileCommandsDir(Path P) {
+  std::lock_guard<std::mutex> Lock(Mutex);
+  CompileCommandsDir = P;
+  CompilationDatabases.clear();
+}
+
 void DirectoryBasedGlobalCompilationDatabase::setExtraFlagsForFile(
     PathRef File, std::vector<std::string> ExtraFlags) {
   std::lock_guard<std::mutex> Lock(Mutex);

clang-tools-extra/clangd/GlobalCompilationDatabase.h

@@ -61,6 +61,9 @@ public:
   /// Uses the default fallback command, adding any extra flags.
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
 
+  /// Set the compile commands directory to \p P.
+  void setCompileCommandsDir(Path P);
+
   /// Sets the extra flags that should be added to a file.
   void setExtraFlagsForFile(PathRef File, std::vector<std::string> ExtraFlags);
 

clang-tools-extra/clangd/Protocol.cpp

@@ -497,5 +497,15 @@ json::Expr toJSON(const DocumentHighlight &DH) {
   };
 }
 
+bool fromJSON(const json::Expr &Params, DidChangeConfigurationParams &CCP) {
+  json::ObjectMapper O(Params);
+  return O && O.map("settings", CCP.settings);
+}
+
+bool fromJSON(const json::Expr &Params, ClangdConfigurationParamsChange &CCPC) {
+  json::ObjectMapper O(Params);
+  return O && O.map("compilationDatabasePath", CCPC.compilationDatabasePath);
+}
+
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/Protocol.h

@@ -324,6 +324,20 @@ struct DidChangeWatchedFilesParams {
 };
 bool fromJSON(const json::Expr &, DidChangeWatchedFilesParams &);
 
+/// Clangd extension to manage a workspace/didChangeConfiguration notification
+/// since the data received is described as 'any' type in LSP.
+struct ClangdConfigurationParamsChange {
+  llvm::Optional<std::string> compilationDatabasePath;
+};
+bool fromJSON(const json::Expr &, ClangdConfigurationParamsChange &);
+
+struct DidChangeConfigurationParams {
+  // We use this predefined struct because it is easier to use
+  // than the protocol specified type of 'any'.
+  ClangdConfigurationParamsChange settings;
+};
+bool fromJSON(const json::Expr &, DidChangeConfigurationParams &);
+
 struct FormattingOptions {
   /// Size of a tab in spaces.
   int tabSize = 0;

clang-tools-extra/clangd/ProtocolHandlers.cpp

@@ -72,4 +72,6 @@ void clangd::registerCallbackHandlers(JSONRPCDispatcher &Dispatcher,
   Register("workspace/executeCommand", &ProtocolCallbacks::onCommand);
   Register("textDocument/documentHighlight",
            &ProtocolCallbacks::onDocumentHighlight);
+  Register("workspace/didChangeConfiguration",
+           &ProtocolCallbacks::onChangeConfiguration);
 }

clang-tools-extra/clangd/ProtocolHandlers.h

@@ -52,6 +52,7 @@ public:
   virtual void onRename(RenameParams &Parames) = 0;
   virtual void onDocumentHighlight(TextDocumentPositionParams &Params) = 0;
   virtual void onHover(TextDocumentPositionParams &Params) = 0;
+  virtual void onChangeConfiguration(DidChangeConfigurationParams &Params) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, JSONOutput &Out,

clang-tools-extra/unittests/clangd/ClangdTests.cpp

@@ -7,6 +7,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "Annotations.h"
 #include "ClangdLSPServer.h"
 #include "ClangdServer.h"
 #include "Matchers.h"
@@ -77,6 +78,43 @@ private:
   VFSTag LastVFSTag = VFSTag();
 };
 
+/// For each file, record whether the last published diagnostics contained at
+/// least one error.
+class MultipleErrorCheckingDiagConsumer : public DiagnosticsConsumer {
+public:
+  void
+  onDiagnosticsReady(PathRef File,
+                     Tagged<std::vector<DiagWithFixIts>> Diagnostics) override {
+    bool HadError = diagsContainErrors(Diagnostics.Value);
+
+    std::lock_guard<std::mutex> Lock(Mutex);
+    LastDiagsHadError[File] = HadError;
+  }
+
+  /// Exposes all files consumed by onDiagnosticsReady in an unspecified order.
+  /// For each file, a bool value indicates whether the last diagnostics
+  /// contained an error.
+  std::vector<std::pair<Path, bool>> filesWithDiags() const {
+    std::vector<std::pair<Path, bool>> Result;
+    std::lock_guard<std::mutex> Lock(Mutex);
+
+    for (const auto &it : LastDiagsHadError) {
+      Result.emplace_back(it.first(), it.second);
+    }
+
+    return Result;
+  }
+
+  void clear() {
+    std::lock_guard<std::mutex> Lock(Mutex);
+    LastDiagsHadError.clear();
+  }
+
+private:
+  mutable std::mutex Mutex;
+  llvm::StringMap<bool> LastDiagsHadError;
+};
+
 /// Replaces all patterns of the form 0x123abc with spaces
 std::string replacePtrsInDump(std::string const &Dump) {
   llvm::Regex RE("0x[0-9a-fA-F]+");
@@ -413,6 +451,72 @@ int main() { return 0; }
   EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
 }
 
+// Test ClangdServer.reparseOpenedFiles.
+TEST_F(ClangdVFSTest, ReparseOpenedFiles) {
+  Annotations FooSource(R"cpp(
+#ifdef MACRO
+$one[[static void bob() {}]]
+#else
+$two[[static void bob() {}]]
+#endif
+
+int main () { bo^b (); return 0; }
+)cpp");
+
+  Annotations BarSource(R"cpp(
+#ifdef MACRO
+this is an error
+#endif
+)cpp");
+
+  Annotations BazSource(R"cpp(
+int hello;
+)cpp");
+
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  MultipleErrorCheckingDiagConsumer DiagConsumer;
+  ClangdServer Server(CDB, DiagConsumer, FS,
+                      /*AsyncThreadsCount=*/0,
+                      /*StorePreamblesInMemory=*/true);
+
+  auto FooCpp = testPath("foo.cpp");
+  auto BarCpp = testPath("bar.cpp");
+  auto BazCpp = testPath("baz.cpp");
+
+  FS.Files[FooCpp] = "";
+  FS.Files[BarCpp] = "";
+  FS.Files[BazCpp] = "";
+
+  CDB.ExtraClangFlags = {"-DMACRO=1"};
+  Server.addDocument(FooCpp, FooSource.code());
+  Server.addDocument(BarCpp, BarSource.code());
+  Server.addDocument(BazCpp, BazSource.code());
+
+  EXPECT_THAT(DiagConsumer.filesWithDiags(),
+              UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, true),
+                                   Pair(BazCpp, false)));
+
+  auto Locations = runFindDefinitions(Server, FooCpp, FooSource.point());
+  EXPECT_TRUE(bool(Locations));
+  EXPECT_THAT(Locations->Value, ElementsAre(Location{URIForFile{FooCpp},
+                                                     FooSource.range("one")}));
+
+  // Undefine MACRO, close baz.cpp.
+  CDB.ExtraClangFlags.clear();
+  DiagConsumer.clear();
+  Server.removeDocument(BazCpp);
+  Server.reparseOpenedFiles();
+
+  EXPECT_THAT(DiagConsumer.filesWithDiags(),
+              UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, false)));
+
+  Locations = runFindDefinitions(Server, FooCpp, FooSource.point());
+  EXPECT_TRUE(bool(Locations));
+  EXPECT_THAT(Locations->Value, ElementsAre(Location{URIForFile{FooCpp},
+                                                     FooSource.range("two")}));
+}
+
 TEST_F(ClangdVFSTest, MemoryUsage) {
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;