Author: hokein Date: Wed Jul 24 00:49:23 2019 New Revision: 366873 URL: http://llvm.org/viewvc/llvm-project?rev=366873&view=rev Log: [clangd] Implement "prepareRename"
Summary: - "prepareRename" request is added in LSP v3.12.0 - also update the vscode-client dependency to pick-up the rename bug fix[1] [1]: https://github.com/microsoft/vscode-languageserver-node/issues/447 Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D63126 Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp clang-tools-extra/trunk/clangd/ClangdLSPServer.h clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.h clang-tools-extra/trunk/clangd/Protocol.cpp clang-tools-extra/trunk/clangd/Protocol.h clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json clang-tools-extra/trunk/clangd/test/rename.test Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=366873&r1=366872&r2=366873&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Jul 24 00:49:23 2019 @@ -382,6 +382,14 @@ void ClangdLSPServer::onInitialize(const SupportFileStatus = Params.initializationOptions.FileStatus; HoverContentFormat = Params.capabilities.HoverContentFormat; SupportsOffsetsInSignatureHelp = Params.capabilities.OffsetsInSignatureHelp; + + // Per LSP, renameProvider can be either boolean or RenameOptions. + // RenameOptions will be specified if the client states it supports prepare. + llvm::json::Value RenameProvider = + llvm::json::Object{{"prepareProvider", true}}; + if (!Params.capabilities.RenamePrepareSupport) // Only boolean allowed per LSP + RenameProvider = true; + llvm::json::Object Result{ {{"capabilities", llvm::json::Object{ @@ -409,7 +417,7 @@ void ClangdLSPServer::onInitialize(const {"definitionProvider", true}, {"documentHighlightProvider", true}, {"hoverProvider", true}, - {"renameProvider", true}, + {"renameProvider", std::move(RenameProvider)}, {"documentSymbolProvider", true}, {"workspaceSymbolProvider", true}, {"referencesProvider", true}, @@ -568,6 +576,12 @@ void ClangdLSPServer::onWorkspaceSymbol( std::move(Reply))); } +void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params, + Callback<llvm::Optional<Range>> Reply) { + Server->prepareRename(Params.textDocument.uri.file(), Params.position, + std::move(Reply)); +} + void ClangdLSPServer::onRename(const RenameParams &Params, Callback<WorkspaceEdit> Reply) { Path File = Params.textDocument.uri.file(); @@ -1015,6 +1029,7 @@ ClangdLSPServer::ClangdLSPServer( MsgHandler->bind("textDocument/declaration", &ClangdLSPServer::onGoToDeclaration); MsgHandler->bind("textDocument/references", &ClangdLSPServer::onReference); MsgHandler->bind("textDocument/switchSourceHeader", &ClangdLSPServer::onSwitchSourceHeader); + MsgHandler->bind("textDocument/prepareRename", &ClangdLSPServer::onPrepareRename); MsgHandler->bind("textDocument/rename", &ClangdLSPServer::onRename); MsgHandler->bind("textDocument/hover", &ClangdLSPServer::onHover); MsgHandler->bind("textDocument/documentSymbol", &ClangdLSPServer::onDocumentSymbol); Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=366873&r1=366872&r2=366873&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Jul 24 00:49:23 2019 @@ -95,6 +95,8 @@ private: void onCommand(const ExecuteCommandParams &, Callback<llvm::json::Value>); void onWorkspaceSymbol(const WorkspaceSymbolParams &, Callback<std::vector<SymbolInformation>>); + void onPrepareRename(const TextDocumentPositionParams &, + Callback<llvm::Optional<Range>>); void onRename(const RenameParams &, Callback<WorkspaceEdit>); void onHover(const TextDocumentPositionParams &, Callback<llvm::Optional<Hover>>); Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=366873&r1=366872&r2=366873&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Jul 24 00:49:23 2019 @@ -292,6 +292,35 @@ ClangdServer::formatOnType(llvm::StringR return Result; } +void ClangdServer::prepareRename(PathRef File, Position Pos, + Callback<llvm::Optional<Range>> CB) { + auto Action = [Pos, this](Path File, Callback<llvm::Optional<Range>> CB, + llvm::Expected<InputsAndAST> InpAST) { + if (!InpAST) + return CB(InpAST.takeError()); + auto &AST = InpAST->AST; + // Performing the rename isn't substantially more expensive than doing an + // AST-based check, so we just rename and throw away the results. We may + // have to revisit this when we support cross-file rename. + auto Changes = renameWithinFile(AST, File, Pos, "dummy", Index); + if (!Changes) { + // LSP says to return null on failure, but that will result in a generic + // failure message. If we send an LSP error response, clients can surface + // the message to users (VSCode does). + return CB(Changes.takeError()); + } + SourceLocation Loc = getBeginningOfIdentifier( + AST, Pos, AST.getSourceManager().getMainFileID()); + if (auto Range = getTokenRange(AST.getSourceManager(), + AST.getASTContext().getLangOpts(), Loc)) + return CB(*Range); + // Return null if the "rename" is not valid at the position. + CB(llvm::None); + }; + WorkScheduler.runWithAST("PrepareRename", File, + Bind(Action, File.str(), std::move(CB))); +} + void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName, bool WantFormat, Callback<std::vector<TextEdit>> CB) { auto Action = [Pos, WantFormat, this](Path File, std::string NewName, Modified: clang-tools-extra/trunk/clangd/ClangdServer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=366873&r1=366872&r2=366873&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.h (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.h Wed Jul 24 00:49:23 2019 @@ -241,6 +241,10 @@ public: PathRef File, Position Pos, StringRef TriggerText); + /// Test the validity of a rename operation. + void prepareRename(PathRef File, Position Pos, + Callback<llvm::Optional<Range>> CB); + /// Rename all occurrences of the symbol at the \p Pos in \p File to /// \p NewName. /// If WantFormat is false, the final TextEdit will be not formatted, Modified: clang-tools-extra/trunk/clangd/Protocol.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=366873&r1=366872&r2=366873&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Protocol.cpp (original) +++ clang-tools-extra/trunk/clangd/Protocol.cpp Wed Jul 24 00:49:23 2019 @@ -331,6 +331,10 @@ bool fromJSON(const llvm::json::Value &P } } } + if (auto *Rename = TextDocument->getObject("rename")) { + if (auto RenameSupport = Rename->getBoolean("prepareSupport")) + R.RenamePrepareSupport = *RenameSupport; + } } if (auto *Workspace = O->getObject("workspace")) { if (auto *Symbol = Workspace->getObject("symbol")) { Modified: clang-tools-extra/trunk/clangd/Protocol.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=366873&r1=366872&r2=366873&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Protocol.h (original) +++ clang-tools-extra/trunk/clangd/Protocol.h Wed Jul 24 00:49:23 2019 @@ -422,6 +422,10 @@ struct ClientCapabilities { /// The content format that should be used for Hover requests. /// textDocument.hover.contentEncoding MarkupKind HoverContentFormat = MarkupKind::PlainText; + + /// The client supports testing for validity of rename operations + /// before execution. + bool RenamePrepareSupport = false; }; bool fromJSON(const llvm::json::Value &, ClientCapabilities &); Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=366873&r1=366872&r2=366873&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original) +++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Wed Jul 24 00:49:23 2019 @@ -6,7 +6,7 @@ "publisher": "llvm-vs-code-extensions", "homepage": "https://clang.llvm.org/extra/clangd.html", "engines": { - "vscode": "^1.30.0" + "vscode": "^1.36.0" }, "categories": [ "Programming Languages", @@ -35,8 +35,8 @@ "test": "node ./node_modules/vscode/bin/test" }, "dependencies": { - "vscode-languageclient": "^5.2.0", - "vscode-languageserver": "^5.2.0" + "vscode-languageclient": "^5.3.0-next.6", + "vscode-languageserver": "^5.3.0-next.6" }, "devDependencies": { "@types/mocha": "^2.2.32", Modified: clang-tools-extra/trunk/clangd/test/rename.test URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/rename.test?rev=366873&r1=366872&r2=366873&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/test/rename.test (original) +++ clang-tools-extra/trunk/clangd/test/rename.test Wed Jul 24 00:49:23 2019 @@ -1,12 +1,45 @@ # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s -{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument": {"rename": {"dynamicRegistration": true, "prepareSupport": true}}},"trace":"off"}} +# CHECK: "renameProvider": { +# CHECK-NEXT: "prepareProvider": true +# CHECK-NEXT: }, --- {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.cpp","languageId":"cpp","version":1,"text":"int foo;"}}} --- -{"jsonrpc":"2.0","id":1,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":5},"newName":"bar"}} +{"jsonrpc":"2.0","id":1,"method":"textDocument/prepareRename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":5}}} # CHECK: "id": 1, # CHECK-NEXT: "jsonrpc": "2.0", # CHECK-NEXT: "result": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 7, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 4, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +--- +{"jsonrpc":"2.0","id":2,"method":"textDocument/prepareRename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2}}} +# CHECK: "error": { +# CHECK-NEXT: "code": -32001, +# CHECK-NEXT: "message": "there is no symbol at the given location" +# CHECK-NEXT: }, +# CHECK-NEXT: "id": 2, +# CHECK-NEXT: "jsonrpc": "2.0" +--- +{"jsonrpc":"2.0","id":4,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}} +# CHECK: "error": { +# CHECK-NEXT: "code": -32001, +# CHECK-NEXT: "message": "there is no symbol at the given location" +# CHECK-NEXT: }, +# CHECK-NEXT: "id": 4, +# CHECK-NEXT: "jsonrpc": "2.0" +--- +{"jsonrpc":"2.0","id":3,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":5},"newName":"bar"}} +# CHECK: "id": 3, +# CHECK-NEXT: "jsonrpc": "2.0", +# CHECK-NEXT: "result": { # CHECK-NEXT: "changes": { # CHECK-NEXT: "file://{{.*}}/foo.cpp": [ # CHECK-NEXT: { @@ -26,14 +59,6 @@ # CHECK-NEXT: } # CHECK-NEXT: } --- -{"jsonrpc":"2.0","id":2,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}} -# CHECK: "error": { -# CHECK-NEXT: "code": -32001, -# CHECK-NEXT: "message": "there is no symbol at the given location" -# CHECK-NEXT: }, -# CHECK-NEXT: "id": 2, -# CHECK-NEXT: "jsonrpc": "2.0" ---- -{"jsonrpc":"2.0","id":3,"method":"shutdown"} +{"jsonrpc":"2.0","id":5,"method":"shutdown"} --- {"jsonrpc":"2.0","method":"exit"} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits