diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -146,7 +146,16 @@ std::unique_ptr MsgHandler; std::atomic NextCallID = {0}; std::mutex TranspWriter; - void call(StringRef Method, llvm::json::Value Params); + + // A callback that will be invoked when receiving a client reply for a server + // request. + using ReplyCallback = std::function)>; + std::mutex ReplyCallbacksMutex; + // Request ID => reply Callbacks + llvm::StringMap ReplyCallbacks; + + void call(StringRef Method, llvm::json::Value Params, ReplyCallback CB); + void onReply(llvm::json::Value ID, llvm::Expected Result); void notify(StringRef Method, llvm::json::Value Params); const FileSystemProvider &FSProvider; diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -146,11 +146,9 @@ bool onReply(llvm::json::Value ID, llvm::Expected Result) override { WithContext HandlerContext(handlerContext()); - // We ignore replies, just log them. - if (Result) - log("<-- reply({0})", ID); - else - log("<-- reply({0}) error: {1}", ID, llvm::toString(Result.takeError())); + // Log the reply. + log("<-- reply({0})", ID); + Server.onReply(std::move(ID), std::move(Result)); return true; } @@ -310,14 +308,40 @@ }; // call(), notify(), and reply() wrap the Transport, adding logging and locking. -void ClangdLSPServer::call(llvm::StringRef Method, llvm::json::Value Params) { +void ClangdLSPServer::call(llvm::StringRef Method, llvm::json::Value Params, + ReplyCallback CB) { auto ID = NextCallID++; + auto StrID = llvm::to_string(ID); + { + std::lock_guard Lock(ReplyCallbacksMutex); + ReplyCallbacks[StrID] = std::move(CB); + } log("--> {0}({1})", Method, ID); - // We currently don't handle responses, so no need to store ID anywhere. std::lock_guard Lock(TranspWriter); Transp.call(Method, std::move(Params), ID); } +void ClangdLSPServer::onReply(llvm::json::Value ID, + llvm::Expected Result) { + auto StrID = llvm::to_string(ID); + ReplyCallback CB = nullptr; + { + std::lock_guard Lock(ReplyCallbacksMutex); + CB = ReplyCallbacks.lookup(StrID); + ReplyCallbacks.erase(StrID); + } + // No callback for the client reply, just log them. + if (!CB) { + if (Result) + log("no corresponding callback for the reply ({0})", ID); + else + log("no corresponding callback for the reply ({0}) error: {1}", ID, + llvm::toString(Result.takeError())); + return; + } + CB(std::move(Result)); +} + void ClangdLSPServer::notify(llvm::StringRef Method, llvm::json::Value Params) { log("--> {0}", Method); std::lock_guard Lock(TranspWriter); @@ -501,7 +525,7 @@ Edit.edit = std::move(WE); // Ideally, we would wait for the response and if there is no error, we // would reply success/failure to the original RPC. - call("workspace/applyEdit", Edit); + call("workspace/applyEdit", Edit, /*ReplyCallback=*/nullptr); }; if (Params.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND && Params.workspaceEdit) { @@ -527,9 +551,87 @@ auto Action = [this, ApplyEdit](decltype(Reply) Reply, URIForFile File, std::string Code, llvm::Expected R) { + // Function object to apply steps for a tweak. + // Steps are executed in order: for each step, clangd sends a new request + // to the LSP client, waits for the response and continues next step; if + // there is any error during the apply process, clangd stops applying the + // following steps. + // + // Example steps [applyEdit, Rename] + // - apply the first step: send call(applyEdit, Workspace) request + // - ..wait for client response + // - apply the second step: send call(rename, trigger) + // - ..wait for the reply, and finish. + struct CallInSequence { + public: + CallInSequence(ClangdLSPServer *Server, std::vector Steps, + std::string Code, URIForFile MainFile) + : Server(Server), Steps(std::move(Steps)), nextStepIndex(0), + Code(std::move(Code)), MainFile(std::move(MainFile)) {} + + void operator()(llvm::Expected Result) { + if (!Result) { + // FIXME: should send a failed reply to client. + log("receive error: {0}", llvm::toString(Result.takeError())); + return; + } + if (nextStepIndex >= Steps.size()) { + // FIXME: should send a successful reply to client. + log("applied tweak successfully"); + return; + } + auto RequestToClient = takeNextStep(); + nextCall(Server, RequestToClient.first, RequestToClient.second); + } + + std::pair takeNextStep() { + assert(nextStepIndex < Steps.size() && "index out of boundary!"); + return transform(Steps[nextStepIndex++]); + } + + private: + // Transform the step into a LSP request. + std::pair + transform(const Tweak::Step &Step) { + if (Step.ApplyKind == Tweak::Step::ApplyEdit) { + ApplyWorkspaceEditParams Edit; + + Edit.edit.changes.emplace(); + (*Edit.edit.changes)[MainFile.uri()] = + replacementsToEdits(Code, *Step.ApplyEditParams); + return {"workspace/applyEdit", std::move(Edit)}; + } else if (Step.ApplyKind == Tweak::Step::Rename) { + // Clangd extension. + llvm::json::Object Result; + Result["arguments"] = {*Step.RenameParams}; + return {"clangd.triggerRename", std::move(Result)}; + } + llvm_unreachable("unhandled apply kind"); + } + void nextCall(ClangdLSPServer *LSPServer, llvm::StringRef Method, + llvm::json::Value Params) { + LSPServer->call(Method, std::move(Params), std::move(*this)); + } + + ClangdLSPServer *Server; + std::vector Steps; + size_t nextStepIndex; + + // datas for transforming step. + std::string Code; + URIForFile MainFile; + }; + if (!R) return Reply(R.takeError()); + if (R->ApplySteps) { + CallInSequence Calls(this, *R->ApplySteps, Code, File); + auto Request = Calls.takeNextStep(); + // Starts the call chain. + call(Request.first, std::move(Request.second), std::move(Calls)); + } if (R->ApplyEdit) { WorkspaceEdit WE; WE.changes.emplace(); diff --git a/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts b/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts --- a/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts +++ b/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts @@ -128,6 +128,36 @@ status.clear(); } }) + + clangdClient.onReady().then( + () => {clangdClient.onRequest( + "clangd.triggerRename", async (result: any) => { + if (result.arguments) { + return await vscode.commands + .executeCommand( + 'editor.action.rename', + [ + vscode.Uri.file( + vscode.window.activeTextEditor.document.fileName), + result.arguments[0] + ]) + .then((value) => { return Promise.resolve(true); }, + (err) => { + if (err.name == 'Canceled') { + // VSCode will cancel the request automatically + // when it detects there is a change of the file + // (rename) during the process of handling the + // triggerRename request. but we don't want client + // send back a cancel error to clangd, so consume + // the cancel here. + return Promise.resolve(true); + } + + return Promise.resolve(err); + }); + } + return Promise.reject(new Error("Invalid commend arguments")); + })}); // An empty place holder for the activate command, otherwise we'll get an // "command is not registered" error. context.subscriptions.push(vscode.commands.registerCommand( diff --git a/clang-tools-extra/clangd/refactor/Tweak.h b/clang-tools-extra/clangd/refactor/Tweak.h --- a/clang-tools-extra/clangd/refactor/Tweak.h +++ b/clang-tools-extra/clangd/refactor/Tweak.h @@ -64,12 +64,44 @@ /// Provide information to the user. Info, }; + + // The class represents a small and focus refactoring. The purpose is to form + // a chain of different refactorings, so that we could perform a large and + // complicate refactoring. + // + // Steps are applied in sequence. In LSP layer, each step is transformed to a + // call request to the LSP client, LSPServer waits for the client response and + // continues to apply next step when the previous step is finished + // successfully. + // + // clangd doesn't track changes of the source files when apply a step, so + // the following steps are not aware of the changes, Tweak is corresponding to + // pre-calculate the changes for all steps. + // FIXME: we should have a mechanism to track the changes for each step. + struct Step { + enum Kind { + ApplyEdit, + Rename, + }; + Kind ApplyKind; + + llvm::Optional ApplyEditParams; // For ApplyEdit + llvm::Optional RenameParams; // For Rename + }; + struct Effect { /// A message to be displayed to the user. llvm::Optional ShowMessage; /// An edit to apply to the input file. llvm::Optional ApplyEdit; + /// A chain of steps being executed in order when applying the tweak. + llvm::Optional> ApplySteps; + static Effect applySteps(std::vector Steps) { + Effect E; + E.ApplySteps = std::move(Steps); + return E; + } static Effect applyEdit(tooling::Replacements R) { Effect E; E.ApplyEdit = std::move(R); diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp @@ -365,7 +365,43 @@ // replace expression with variable name if (auto Err = Result.add(Target->replaceWithVar(Range, VarName))) return std::move(Err); - return Effect::applyEdit(Result); + // TODO: refine this. + // We do the format internally in order to keep the position of the extracted + // variable not being changed from the caller. + auto &SM = Inputs.AST.getASTContext().getSourceManager(); + auto *FS = &SM.getFileManager().getVirtualFileSystem(); + auto Style = getFormatStyleForFile( + SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName(), + Inputs.Code, FS); + auto Formatted = cleanupAndFormat(Inputs.Code, Result, Style); + if (!Formatted) + return Formatted.takeError(); + Result = *Formatted; + auto NewCode = tooling::applyAllReplacements(Inputs.Code, Result); + if (!NewCode) + return NewCode.takeError(); + + assert(!Result.empty()); + // Calculate the offset of the dummy variable after applying replacements. + size_t ExtractVarOffset = Result.begin()->getOffset(); + for (auto &R : Result) { + auto OffsetInReplacement = R.getReplacementText().find(VarName); + if (OffsetInReplacement != llvm::StringRef::npos) { + ExtractVarOffset += OffsetInReplacement; + break; + } + ExtractVarOffset += R.getReplacementText().size() - R.getLength(); + } + + assert(ExtractVarOffset != llvm::StringRef::npos); + Step Apply; + Apply.ApplyKind = Step::ApplyEdit; + Apply.ApplyEditParams = std::move(Result); + Step Rename; + Rename.ApplyKind = Step::Rename; + Rename.RenameParams = offsetToPosition(*NewCode, ExtractVarOffset); + // TODO: We should not emit Rename if the client doesn't support it. + return Effect::applySteps({std::move(Apply), std::move(Rename)}); } // Find the CallExpr whose callee is an ancestor of the DeclRef