Nebiroth updated this revision to Diff 123069.
Nebiroth marked 3 inline comments as done.
Nebiroth added a comment.

Added test for didChangeConfiguration notification.
Compilation database and extra file flags are now properly reloaded when 
changing database path.
ClangdConfigurationParams now used instead of map.
changeConfiguration removed from ClangdServer. This class will now handle more 
specific settings.
Added more checks when parsing notification from client..


https://reviews.llvm.org/D39571

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/DraftStore.cpp
  clangd/DraftStore.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  test/clangd/did-change-configuration.test
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===================================================================
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -1,3 +1,4 @@
+
 # RUN: clangd -pretty -run-synchronously < %s | FileCheck -strict-whitespace %s
 # It is absolutely vital that this file has CRLF line endings.
 #
@@ -18,6 +19,7 @@
 # CHECK-NEXT:          ":"
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
+# CHECK-NEXT:	   "configurationChangeProvider": true,
 # CHECK-NEXT:      "definitionProvider": true,
 # CHECK-NEXT:      "documentFormattingProvider": true,
 # CHECK-NEXT:      "documentOnTypeFormattingProvider": {
@@ -48,4 +50,4 @@
 # CHECK-NEXT:  "result": null
 Content-Length: 33
 
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0":"method":"exit"}
\ No newline at end of file
Index: test/clangd/initialize-params-invalid.test
===================================================================
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -1,3 +1,4 @@
+
 # RUN: clangd -pretty -run-synchronously < %s | FileCheck -strict-whitespace %s
 # It is absolutely vital that this file has CRLF line endings.
 #
@@ -18,6 +19,7 @@
 # CHECK-NEXT:          ":"
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
+# CHECK-NEXT:	   "configurationChangeProvider": true,
 # CHECK-NEXT:      "definitionProvider": true,
 # CHECK-NEXT:      "documentFormattingProvider": true,
 # CHECK-NEXT:      "documentOnTypeFormattingProvider": {
@@ -45,4 +47,4 @@
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
 Content-Length: 33
 
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0":"method":"exit"}
\ No newline at end of file
Index: test/clangd/did-change-configuration.test
===================================================================
--- /dev/null
+++ test/clangd/did-change-configuration.test
@@ -0,0 +1,46 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 150
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"voidmain(){}"}}}
+
+Content-Length: 86
+
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":""}}
+#Failed to decode workspace/didChangeConfiguration request.
+#Incorrect mapping node
+
+Content-Length: 114
+
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":{}}}}
+#Failed to decode workspace/didChangeConfiguration request.
+#compilationDatabasePath is not a scalar node
+
+Content-Length: 140
+
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":"/","testBadValue":"foo1234"}}}
+#Ignored unknown field "testBadValue"
+#Failed to find compilation database for / in overriden directory /
+#Bad field, bad compilation database
+
+Content-Length: 722
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///compile_commands.json","languageId":"json","version":1,"text":"[\n{\n"directory":"/",\n"command":"/usr/bin/c++-DGTEST_HAS_RTTI=0-D_GNU_SOURCE-D__STDC_CONSTANT_MACROS-D__STDC_FORMAT_MACROS-D__STDC_LIMIT_MACROS-Ilib/Demangle-I../lib/Demangle-I/usr/include/libxml2-Iinclude-I../include-fPIC-fvisibility-inlines-hidden-Werror=date-time-std=c++11-Wall-W-Wno-unused-parameter-Wwrite-strings-Wcast-qual-Wno-missing-field-initializers-pedantic-Wno-long-long-Wno-maybe-uninitialized-Wdelete-non-virtual-dtor-Wno-comment-O0-g-fno-exceptions-fno-rtti-o/foo.c.o-c/foo.c",\n"file":"/foo.c"\n},"}}}
+
+Content-Length: 115
+
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":"/"}}}
+#CHECK:{"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"diagnostics":[{"message":"type specifier missing, defaults to 'int'","range":{"end":{"character":1,"line":0},"start":{"character":1,"line":0}},"severity":2},{"message":"control reaches end of non-void function","range":{"end":{"character":12,"line":0},"start":{"character":12,"line":0}},"severity":2}],"uri":"file:///foo.c"}}
+
+Content-Length: 48
+
+{"jsonrpc":"2.0","id":10000,"method":"shutdown"}
+
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}
\ No newline at end of file
Index: clangd/ProtocolHandlers.h
===================================================================
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -53,7 +53,9 @@
   virtual void onSwitchSourceHeader(Ctx C, TextDocumentIdentifier &Params) = 0;
   virtual void onFileEvent(Ctx C, DidChangeWatchedFilesParams &Params) = 0;
   virtual void onCommand(Ctx C, ExecuteCommandParams &Params) = 0;
-  virtual void onRename(Ctx C, RenameParams &Parames) = 0;
+  virtual void onRename(Ctx C, RenameParams &Params) = 0;
+  virtual void onChangeConfiguration(Ctx C,
+                                     DidChangeConfigurationParams &Params) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, JSONOutput &Out,
Index: clangd/ProtocolHandlers.cpp
===================================================================
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -74,4 +74,6 @@
   Register("textDocument/rename", &ProtocolCallbacks::onRename);
   Register("workspace/didChangeWatchedFiles", &ProtocolCallbacks::onFileEvent);
   Register("workspace/executeCommand", &ProtocolCallbacks::onCommand);
+  Register("workspace/didChangeConfiguration",
+           &ProtocolCallbacks::onChangeConfiguration);
 }
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -289,6 +289,31 @@
   parse(llvm::yaml::MappingNode *Params, clangd::Logger &Logger);
 };
 
+/// Clangd extension to manage a workspace/didChangeConfiguration notification
+/// since the data received is described as 'any' type in LSP.
+
+struct ClangdConfigurationParams {
+
+  llvm::Optional<std::string> compilationDatabasePath;
+  static llvm::Optional<ClangdConfigurationParams>
+  parse(llvm::yaml::MappingNode *Params, clangd::Logger &Logger);
+  bool operator!() { return compilationDatabasePath.hasValue(); };
+};
+
+struct DidChangeConfigurationParams {
+
+  DidChangeConfigurationParams() {}
+  DidChangeConfigurationParams(ClangdConfigurationParams settings) {}
+
+  // We use this predefined struct because it is easier to use
+  // than the protocol specified type of 'any'.
+  ClangdConfigurationParams settings;
+
+  static llvm::Optional<DidChangeConfigurationParams>
+  parse(llvm::yaml::MappingNode *Params, clangd::Logger &Logger);
+  static std::string unparse(const DidChangeConfigurationParams &P);
+};
+
 struct FormattingOptions {
   /// Size of a tab in spaces.
   int tabSize;
Index: clangd/Protocol.cpp
===================================================================
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -528,6 +528,63 @@
   return Result;
 }
 
+llvm::Optional<DidChangeConfigurationParams>
+DidChangeConfigurationParams::parse(llvm::yaml::MappingNode *Params,
+                                    clangd::Logger &Logger) {
+  DidChangeConfigurationParams Result;
+  for (auto &NextKeyValue : *Params) {
+    auto *KeyString = dyn_cast<llvm::yaml::ScalarNode>(NextKeyValue.getKey());
+    if (!KeyString)
+      return llvm::None;
+
+    llvm::SmallString<10> KeyStorage;
+    StringRef KeyValue = KeyString->getValue(KeyStorage);
+    auto *Value =
+        dyn_cast_or_null<llvm::yaml::MappingNode>(NextKeyValue.getValue());
+    if (!Value)
+      return llvm::None;
+
+    llvm::SmallString<10> Storage;
+    if (KeyValue == "settings") {
+      auto Parsed = ClangdConfigurationParams::parse(Value, Logger);
+      if (!Parsed)
+        return llvm::None;
+      Result.settings = Parsed.getValue();
+    } else {
+      logIgnoredField(KeyValue, Logger);
+    }
+  }
+  if (!Result.settings.compilationDatabasePath.hasValue())
+    return llvm::None;
+
+  return Result;
+}
+
+llvm::Optional<ClangdConfigurationParams>
+ClangdConfigurationParams::parse(llvm::yaml::MappingNode *Params,
+                                 clangd::Logger &Logger) {
+  ClangdConfigurationParams Result;
+  for (auto &NextKeyValue : *Params) {
+    auto *KeyString = dyn_cast<llvm::yaml::ScalarNode>(NextKeyValue.getKey());
+    if (!KeyString)
+      return llvm::None;
+
+    llvm::SmallString<10> KeyStorage;
+    StringRef KeyValue = KeyString->getValue(KeyStorage);
+    auto *Value =
+        dyn_cast_or_null<llvm::yaml::ScalarNode>(NextKeyValue.getValue());
+    if (!Value)
+      return llvm::None;
+
+    if (KeyValue == "compilationDatabasePath") {
+      Result.compilationDatabasePath = Value->getValue(KeyStorage);
+    } else {
+      logIgnoredField(KeyValue, Logger);
+    }
+  }
+  return Result;
+}
+
 llvm::Optional<TextDocumentContentChangeEvent>
 TextDocumentContentChangeEvent::parse(llvm::yaml::MappingNode *Params,
                                       clangd::Logger &Logger) {
Index: clangd/GlobalCompilationDatabase.h
===================================================================
--- clangd/GlobalCompilationDatabase.h
+++ clangd/GlobalCompilationDatabase.h
@@ -52,12 +52,16 @@
 
   std::vector<tooling::CompileCommand>
   getCompileCommands(PathRef File) override;
-
+  void setCompileCommandsDir(Path P) {
+    CompileCommandsDir = P;
+    getCompilationDatabase(llvm::StringRef(P));
+    CompilationDatabases.clear();
+  }
   void setExtraFlagsForFile(PathRef File, std::vector<std::string> ExtraFlags);
 
 private:
-  tooling::CompilationDatabase *getCompilationDatabase(PathRef File);
   tooling::CompilationDatabase *tryLoadDatabaseFromPath(PathRef File);
+  tooling::CompilationDatabase *getCompilationDatabase(PathRef File);
 
   std::mutex Mutex;
   /// Caches compilation databases loaded from directories(keys are
Index: clangd/GlobalCompilationDatabase.cpp
===================================================================
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -10,6 +10,7 @@
 #include "GlobalCompilationDatabase.h"
 #include "Logger.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 
@@ -104,7 +105,7 @@
         tryLoadDatabaseFromPath(CompileCommandsDir.getValue());
     if (ReturnValue == nullptr)
       Logger.log("Failed to find compilation database for " + Twine(File) +
-                 "in overriden directory " + CompileCommandsDir.getValue() +
+                 " in overriden directory " + CompileCommandsDir.getValue() +
                  "\n");
     return ReturnValue;
   }
Index: clangd/DraftStore.h
===================================================================
--- clangd/DraftStore.h
+++ clangd/DraftStore.h
@@ -40,6 +40,9 @@
   /// \return version and contents of the stored document.
   /// For untracked files, a (0, None) pair is returned.
   VersionedDraft getDraft(PathRef File) const;
+
+  const llvm::StringMap<VersionedDraft> &getDrafts() const { return Drafts; };
+
   /// \return version of the tracked document.
   /// For untracked files, 0 is returned.
   DocVersion getVersion(PathRef File) const;
Index: clangd/DraftStore.cpp
===================================================================
--- clangd/DraftStore.cpp
+++ clangd/DraftStore.cpp
@@ -32,6 +32,7 @@
 
 DocVersion DraftStore::updateDraft(PathRef File, StringRef Contents) {
   std::lock_guard<std::mutex> Lock(Mutex);
+  assert(!File.empty());
 
   auto &Entry = Drafts[File];
   DocVersion NewVersion = ++Entry.Version;
@@ -41,6 +42,7 @@
 
 DocVersion DraftStore::removeDraft(PathRef File) {
   std::lock_guard<std::mutex> Lock(Mutex);
+  assert(!File.empty());
 
   auto &Entry = Drafts[File];
   DocVersion NewVersion = ++Entry.Version;
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -307,6 +307,10 @@
   /// Called when an event occurs for a watched file in the workspace.
   void onFileEvent(const DidChangeWatchedFilesParams &Params);
 
+  /// Called when the compilation database is changed. Calls forceReparse() on
+  /// every file currently managed by DraftMgr.
+  void reparseOpenedFiles();
+
 private:
   std::future<void>
   scheduleReparseAndDiags(PathRef File, VersionedDraft Contents,
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -576,6 +576,12 @@
   return DoneFuture;
 }
 
+void ClangdServer::reparseOpenedFiles() {
+  for (auto Draft : DraftMgr.getDrafts().keys()) {
+    forceReparse(Draft);
+  }
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -71,7 +71,8 @@
   void onFileEvent(Ctx C, DidChangeWatchedFilesParams &Params) override;
   void onCommand(Ctx C, ExecuteCommandParams &Params) override;
   void onRename(Ctx C, RenameParams &Parames) override;
-
+  void onChangeConfiguration(Ctx C,
+                             DidChangeConfigurationParams &Params) override;
   std::vector<clang::tooling::Replacement>
   getFixIts(StringRef File, const clangd::Diagnostic &D);
 
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -9,6 +9,11 @@
 
 #include "ClangdLSPServer.h"
 #include "JSONRPCDispatcher.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "llvm/Support/raw_ostream.h"
 
 #include "llvm/Support/FormatVariadic.h"
 
@@ -57,6 +62,7 @@
                  {"triggerCharacters", {"(", ","}},
              }},
             {"definitionProvider", true},
+            {"configurationChangeProvider", true},
             {"renameProvider", true},
             {"executeCommandProvider",
              json::obj{
@@ -234,6 +240,20 @@
   C.reply(Result ? URI::fromFile(*Result).uri : "");
 }
 
+void ClangdLSPServer::onChangeConfiguration(
+    Ctx C, DidChangeConfigurationParams &Params) {
+
+  // Parse whatever settings might be found inside Params.
+  ClangdConfigurationParams Settings = Params.settings;
+
+  // Verify if path has value and is a valid path
+  if (Params.settings.compilationDatabasePath.hasValue()) {
+    CDB.setCompileCommandsDir(
+        Params.settings.compilationDatabasePath.getValue());
+    Server.reparseOpenedFiles();
+  }
+}
+
 ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount,
                                  bool SnippetCompletions,
                                  llvm::Optional<StringRef> ResourceDir,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to