[clang-tools-extra] r369100 - [clangd] Simplify code of ClangdLSPServer::onCommand

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Aug 16 05:46:41 2019
New Revision: 369100

URL: http://llvm.org/viewvc/llvm-project?rev=369100&view=rev
Log:
[clangd] Simplify code of ClangdLSPServer::onCommand

Summary:
By inlining a complicated lambda into its single call-site.

Also ensure we call Reply() exactly once even if tweaks return both
ShowMessage and ApplyEdit effects.

Reviewers: hokein

Reviewed By: hokein

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
    clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=369100&r1=369099&r2=369100&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Fri Aug 16 05:46:41 2019
@@ -570,28 +570,28 @@ void ClangdLSPServer::onFileEvent(const
 
 void ClangdLSPServer::onCommand(const ExecuteCommandParams &Params,
                                 Callback<llvm::json::Value> Reply) {
-  auto ApplyEdit = [this](WorkspaceEdit WE,
-                          Callback<ApplyWorkspaceEditResponse> Reply) {
+  auto ApplyEdit = [this](WorkspaceEdit WE, std::string SuccessMessage,
+                          decltype(Reply) Reply) {
     ApplyWorkspaceEditParams Edit;
     Edit.edit = std::move(WE);
-    call("workspace/applyEdit", std::move(Edit), std::move(Reply));
+    call<ApplyWorkspaceEditResponse>(
+        "workspace/applyEdit", std::move(Edit),
+        [Reply = std::move(Reply), SuccessMessage = std::move(SuccessMessage)](
+            llvm::Expected<ApplyWorkspaceEditResponse> Response) mutable {
+          if (!Response)
+            return Reply(Response.takeError());
+          if (!Response->applied) {
+            std::string Reason = Response->failureReason
+                                     ? *Response->failureReason
+                                     : "unknown reason";
+            return Reply(llvm::createStringError(
+                llvm::inconvertibleErrorCode(),
+                ("edits were not applied: " + Reason).c_str()));
+          }
+          return Reply(SuccessMessage);
+        });
   };
-  // FIXME: this lambda is tangled and confusing, refactor it.
-  auto ReplyAfterApplyingEdit =
-      [](decltype(Reply) Reply, std::string SuccessMessage,
-         llvm::Expected<ApplyWorkspaceEditResponse> Response) {
-        if (!Response)
-          return Reply(Response.takeError());
-        if (!Response->applied) {
-          std::string Reason = Response->failureReason
-                                   ? *Response->failureReason
-                                   : "unknown reason";
-          return Reply(llvm::createStringError(
-              llvm::inconvertibleErrorCode(),
-              ("edits were not applied: " + Reason).c_str()));
-        }
-        return Reply(SuccessMessage);
-      };
+
   if (Params.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND &&
       Params.workspaceEdit) {
     // The flow for "apply-fix" :
@@ -602,12 +602,7 @@ void ClangdLSPServer::onCommand(const Ex
     // 5. We unwrap the changes and send them back to the editor
     // 6. The editor applies the changes (applyEdit), and sends us a reply
     // 7. We unwrap the reply and send a reply to the editor.
-    ApplyEdit(*Params.workspaceEdit,
-              [Reply = std::move(Reply), ReplyAfterApplyingEdit](
-                  llvm::Expected<ApplyWorkspaceEditResponse> Response) mutable {
-                ReplyAfterApplyingEdit(std::move(Reply), "Fix applied.",
-                                       std::move(Response));
-              });
+    ApplyEdit(*Params.workspaceEdit, "Fix applied.", std::move(Reply));
   } else if (Params.command == ExecuteCommandParams::CLANGD_APPLY_TWEAK &&
              Params.tweakArgs) {
     auto Code = DraftMgr.getDraft(Params.tweakArgs->file.file());
@@ -616,32 +611,29 @@ void ClangdLSPServer::onCommand(const Ex
           llvm::inconvertibleErrorCode(),
           "trying to apply a code action for a non-added file"));
 
-    auto Action = [this, ApplyEdit, ReplyAfterApplyingEdit,
-                   Reply = std::move(Reply), File = Params.tweakArgs->file,
-                   Code = std::move(*Code)](
+    auto Action = [this, ApplyEdit, Reply = std::move(Reply),
+                   File = Params.tweakArgs->file, Code = std::move(*Code)](
                       llvm::Expected<Tweak::Effect> R) mutable {
       if (!R)
         return Reply(R.takeError());
 
-      if (R->ApplyEdit) {
-        WorkspaceEdit WE;
-        WE.changes.emplace();
-        (*WE.changes)[File.uri()] = replacementsToEdits(Code, *R->ApplyEdit);
-        ApplyEdit(
-            std::move(WE),
-            [Reply = std::move(Reply), ReplyAfterApplyingEdit](
-                llvm::Expected<ApplyWorkspaceEditResponse> Response) mutable {
-              ReplyAfterApplyingEdit(std::move(Reply), "Tweak applied.",
-                                     std::move(Response));
-            });
-      }
+      assert(R->ShowMessage || R->ApplyEdit && "tweak has no effect");
+
       if (R->ShowMessage) {
         ShowMessageParams Msg;
         Msg.message = *R->ShowMessage;
         Msg.type = MessageType::Info;
         notify("window/showMessage", Msg);
-        Reply("Tweak applied.");
       }
+      if (R->ApplyEdit) {
+        WorkspaceEdit WE;
+        WE.changes.emplace();
+        (*WE.changes)[File.uri()] = replacementsToEdits(Code, *R->ApplyEdit);
+        // ApplyEdit will take care of calling Reply().
+        return ApplyEdit(std::move(WE), "Tweak applied.", std::move(Reply));
+      }
+      // When no edit is specified, make sure we Reply().
+      return Reply("Tweak applied.");
     };
     Server->applyTweak(Params.tweakArgs->file.file(),
                        Params.tweakArgs->selection, Params.tweakArgs->tweakID,