rwols updated this revision to Diff 121605.
rwols added a comment.

- Rebase to latest SVN revision.
- Use `void reply(llvm:Expected<Twine> Result)` for transparent error handling.
- Use `FSProvider` from `ClangdServer` to provide a filesystem for 
`format::getStyle`.
- Organize things around `llvm::Expected`, I've become a fan of that class :-)


https://reviews.llvm.org/D39430

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/JSONRPCDispatcher.cpp
  clangd/JSONRPCDispatcher.h
  test/clangd/formatting.test

Index: test/clangd/formatting.test
===================================================================
--- test/clangd/formatting.test
+++ test/clangd/formatting.test
@@ -1,4 +1,4 @@
-# RUN: clangd < %s | FileCheck %s
+# RUN: clangd -run-synchronously < %s | FileCheck %s
 # It is absolutely vital that this file has CRLF line endings.
 #
 Content-Length: 125
Index: clangd/JSONRPCDispatcher.h
===================================================================
--- clangd/JSONRPCDispatcher.h
+++ clangd/JSONRPCDispatcher.h
@@ -15,6 +15,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/YAMLParser.h"
+#include "llvm/Support/Error.h"
 #include <iosfwd>
 #include <mutex>
 
@@ -54,10 +55,14 @@
 public:
   RequestContext(JSONOutput &Out, StringRef ID) : Out(Out), ID(ID) {}
 
-  /// Sends a successful reply. Result should be well-formed JSON.
-  void reply(const Twine &Result);
+  /// Send a response to a request from the client, or an error.
+  /// 
+  /// \param Result Well-formed JSON, or an llvm::Error.
+  void reply(llvm::Expected<Twine> Result);
   /// Sends an error response to the client, and logs it.
   void replyError(int code, const llvm::StringRef &Message);
+  /// Send all error messages contained in Err to the client, and log them.
+  void replyError(llvm::Error Err);
   /// Sends a request to the client.
   void call(llvm::StringRef Method, StringRef Params);
 
Index: clangd/JSONRPCDispatcher.cpp
===================================================================
--- clangd/JSONRPCDispatcher.cpp
+++ clangd/JSONRPCDispatcher.cpp
@@ -47,13 +47,18 @@
   InputMirror->flush();
 }
 
-void RequestContext::reply(const llvm::Twine &Result) {
+void RequestContext::reply(llvm::Expected<llvm::Twine> Result) {
   if (ID.empty()) {
     Out.log("Attempted to reply to a notification!\n");
     return;
   }
-  Out.writeMessage(llvm::Twine(R"({"jsonrpc":"2.0","id":)") + ID +
-                   R"(,"result":)" + Result + "}");
+  if (Result) {
+    Out.writeMessage(llvm::Twine(R"({"jsonrpc":"2.0","id":)") + ID +
+                     R"(,"result":)" + Result.get() + "}");
+  }
+  else {
+    replyError(Result.takeError());
+  }
 }
 
 void RequestContext::replyError(int code, const llvm::StringRef &Message) {
@@ -66,6 +71,11 @@
   }
 }
 
+void RequestContext::replyError(llvm::Error Err) {
+  const auto Message = llvm::toString(std::move(Err));
+  replyError(/*UnknownErrorCode*/ -32001, Message);
+}
+
 void RequestContext::call(StringRef Method, StringRef Params) {
   // FIXME: Generate/Increment IDs for every request so that we can get proper
   // replies once we need to.
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -284,12 +284,18 @@
   /// given a header file and vice versa.
   llvm::Optional<Path> switchSourceHeader(PathRef Path);
 
-  /// Run formatting for \p Rng inside \p File.
-  std::vector<tooling::Replacement> formatRange(PathRef File, Range Rng);
-  /// Run formatting for the whole \p File.
-  std::vector<tooling::Replacement> formatFile(PathRef File);
-  /// Run formatting after a character was typed at \p Pos in \p File.
-  std::vector<tooling::Replacement> formatOnType(PathRef File, Position Pos);
+  /// Run formatting for \p Rng inside \p File with content \p Code.
+  llvm::Expected<Tagged<std::vector<tooling::Replacement>>>
+  formatRange(llvm::StringRef Code, PathRef File, Range Rng);
+
+  /// Run formatting for the whole \p File with content \p Code.
+  llvm::Expected<Tagged<std::vector<tooling::Replacement>>>
+  formatFile(llvm::StringRef Code, PathRef File);
+
+  /// Run formatting after a character was typed at \p Pos in \p File with
+  /// content \p Code.
+  llvm::Expected<Tagged<std::vector<tooling::Replacement>>>
+  formatOnType(llvm::StringRef Code, PathRef File, Position Pos);
 
   /// Gets current document contents for \p File. \p File must point to a
   /// currently tracked file.
@@ -305,6 +311,12 @@
   void onFileEvent(const DidChangeWatchedFilesParams &Params);
 
 private:
+  // FIXME: We don't need the Code argument, but that data just so happens to
+  // be present when invoking this method.
+  llvm::Expected<Tagged<std::vector<tooling::Replacement>>>
+  formatCode(llvm::StringRef Code, PathRef File,
+             ArrayRef<tooling::Range> Ranges);
+
   std::future<void>
   scheduleReparseAndDiags(PathRef File, VersionedDraft Contents,
                           std::shared_ptr<CppFile> Resources,
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -36,16 +36,6 @@
   std::promise<void> &Promise;
 };
 
-std::vector<tooling::Replacement> formatCode(StringRef Code, StringRef Filename,
-                                             ArrayRef<tooling::Range> Ranges) {
-  // Call clang-format.
-  // FIXME: Don't ignore style.
-  format::FormatStyle Style = format::getLLVMStyle();
-  auto Result = format::reformat(Style, Code, Ranges, Filename);
-
-  return std::vector<tooling::Replacement>(Result.begin(), Result.end());
-}
-
 std::string getStandardResourceDir() {
   static int Dummy; // Just an address in this process.
   return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
@@ -304,28 +294,25 @@
   return make_tagged(std::move(Result), TaggedFS.Tag);
 }
 
-std::vector<tooling::Replacement> ClangdServer::formatRange(PathRef File,
-                                                            Range Rng) {
-  std::string Code = getDocument(File);
-
+llvm::Expected<Tagged<std::vector<tooling::Replacement>>>
+ClangdServer::formatRange(llvm::StringRef Code, PathRef File, Range Rng) {
   size_t Begin = positionToOffset(Code, Rng.start);
   size_t Len = positionToOffset(Code, Rng.end) - Begin;
   return formatCode(Code, File, {tooling::Range(Begin, Len)});
 }
 
-std::vector<tooling::Replacement> ClangdServer::formatFile(PathRef File) {
+llvm::Expected<Tagged<std::vector<tooling::Replacement>>>
+ClangdServer::formatFile(llvm::StringRef Code, PathRef File) {
   // Format everything.
-  std::string Code = getDocument(File);
   return formatCode(Code, File, {tooling::Range(0, Code.size())});
 }
 
-std::vector<tooling::Replacement> ClangdServer::formatOnType(PathRef File,
-                                                             Position Pos) {
+llvm::Expected<Tagged<std::vector<tooling::Replacement>>>
+ClangdServer::formatOnType(llvm::StringRef Code, PathRef File, Position Pos) {
   // Look for the previous opening brace from the character position and
   // format starting from there.
-  std::string Code = getDocument(File);
   size_t CursorPos = positionToOffset(Code, Pos);
-  size_t PreviousLBracePos = StringRef(Code).find_last_of('{', CursorPos);
+  size_t PreviousLBracePos = Code.find_last_of('{', CursorPos);
   if (PreviousLBracePos == StringRef::npos)
     PreviousLBracePos = CursorPos;
   size_t Len = 1 + CursorPos - PreviousLBracePos;
@@ -434,6 +421,23 @@
   return llvm::None;
 }
 
+llvm::Expected<Tagged<std::vector<tooling::Replacement>>>
+ClangdServer::formatCode(llvm::StringRef Code, PathRef File,
+                         ArrayRef<tooling::Range> Ranges) {
+  // Call clang-format.
+  auto TaggedFS = FSProvider.getTaggedFileSystem(File);
+  auto StyleOrError =
+      format::getStyle("file", File, "LLVM", Code, TaggedFS.Value.get());
+  if (!StyleOrError) {
+    return StyleOrError.takeError();
+  } else {
+    auto Result = format::reformat(StyleOrError.get(), Code, Ranges, File);
+    return make_tagged(
+        std::vector<tooling::Replacement>(Result.begin(), Result.end()),
+        TaggedFS.Tag);
+  }
+}
+
 std::future<void> ClangdServer::scheduleReparseAndDiags(
     PathRef File, VersionedDraft Contents, std::shared_ptr<CppFile> Resources,
     Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS) {
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -9,6 +9,7 @@
 
 #include "ClangdLSPServer.h"
 #include "JSONRPCDispatcher.h"
+#include <llvm/Support/Error.h>
 
 #include "llvm/Support/FormatVariadic.h"
 
@@ -119,29 +120,35 @@
 
 void ClangdLSPServer::onDocumentOnTypeFormatting(
     Ctx C, DocumentOnTypeFormattingParams &Params) {
-  auto File = Params.textDocument.uri.file;
-  std::string Code = Server.getDocument(File);
-  std::string Edits = TextEdit::unparse(
-      replacementsToEdits(Code, Server.formatOnType(File, Params.position)));
-  C.reply(Edits);
+  const auto File = Params.textDocument.uri.file;
+  const auto Code = Server.getDocument(File);
+  auto Edits = Server.formatOnType(Code, File, Params.position);
+  if (Edits)
+    C.reply(TextEdit::unparse(replacementsToEdits(Code, Edits->Value)));
+  else
+    C.reply(Edits.takeError());
 }
 
 void ClangdLSPServer::onDocumentRangeFormatting(
     Ctx C, DocumentRangeFormattingParams &Params) {
-  auto File = Params.textDocument.uri.file;
-  std::string Code = Server.getDocument(File);
-  std::string Edits = TextEdit::unparse(
-      replacementsToEdits(Code, Server.formatRange(File, Params.range)));
-  C.reply(Edits);
+  const auto File = Params.textDocument.uri.file;
+  const auto Code = Server.getDocument(File);
+  auto Edits = Server.formatRange(Code, File, Params.range);
+  if (Edits)
+    C.reply(TextEdit::unparse(replacementsToEdits(Code, Edits->Value)));
+  else
+    C.reply(Edits.takeError());
 }
 
 void ClangdLSPServer::onDocumentFormatting(Ctx C,
                                            DocumentFormattingParams &Params) {
-  auto File = Params.textDocument.uri.file;
-  std::string Code = Server.getDocument(File);
-  std::string Edits =
-      TextEdit::unparse(replacementsToEdits(Code, Server.formatFile(File)));
-  C.reply(Edits);
+  const auto File = Params.textDocument.uri.file;
+  const auto Code = Server.getDocument(File);
+  auto Edits = Server.formatFile(Code, File);
+  if (Edits)
+    C.reply(TextEdit::unparse(replacementsToEdits(Code, Edits->Value)));
+  else
+    C.reply(Edits.takeError());
 }
 
 void ClangdLSPServer::onCodeAction(Ctx C, CodeActionParams &Params) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to