hokein updated this revision to Diff 211080.
hokein added a comment.

Update the patch, and add tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63126/new/

https://reviews.llvm.org/D63126

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/clients/clangd-vscode/package.json
  clang-tools-extra/clangd/test/prepare-rename.test

Index: clang-tools-extra/clangd/test/prepare-rename.test
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/prepare-rename.test
@@ -0,0 +1,30 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"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/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:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": null
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===================================================================
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -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",
Index: clang-tools-extra/clangd/Protocol.h
===================================================================
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -422,6 +422,10 @@
   /// 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 &);
 
Index: clang-tools-extra/clangd/Protocol.cpp
===================================================================
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -331,6 +331,10 @@
         }
       }
     }
+    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")) {
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -241,6 +241,10 @@
                                                      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,
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -285,6 +285,30 @@
   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;
+    // Return null if the "rename" is not valid at the position.
+    auto Changes = renameWithinFile(AST, File, Pos, "dummy", Index);
+    if (!Changes) {
+      llvm::consumeError(Changes.takeError());
+      return CB(llvm::None);
+    }
+    SourceLocation Loc = getBeginningOfIdentifier(
+        AST, Pos, AST.getSourceManager().getMainFileID());
+    if (auto Range = getTokenRange(AST.getSourceManager(),
+                                   AST.getASTContext().getLangOpts(), Loc))
+      return CB(*Range);
+    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,
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -95,6 +95,8 @@
   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>>);
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -409,7 +409,6 @@
             {"definitionProvider", true},
             {"documentHighlightProvider", true},
             {"hoverProvider", true},
-            {"renameProvider", true},
             {"documentSymbolProvider", true},
             {"workspaceSymbolProvider", true},
             {"referencesProvider", true},
@@ -421,6 +420,13 @@
              }},
             {"typeHierarchyProvider", true},
         }}}};
+  if (Params.capabilities.RenamePrepareSupport)
+    Result.getObject("capabilities")
+        ->insert(
+            {"renameProvider", llvm::json::Object{{"prepareProvider", true}}});
+  else
+    Result.getObject("capabilities")->insert({"renameProvider", true});
+
   if (NegotiatedOffsetEncoding)
     Result["offsetEncoding"] = *NegotiatedOffsetEncoding;
   if (Params.capabilities.SemanticHighlighting)
@@ -568,6 +574,19 @@
           std::move(Reply)));
 }
 
+void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params,
+                                      Callback<llvm::Optional<Range>> Reply) {
+  Server->prepareRename(
+      Params.textDocument.uri.file(), Params.position,
+      Bind(
+          [](decltype(Reply) Reply, llvm::Expected<llvm::Optional<Range>> R) {
+            if (!R)
+              return Reply(R.takeError());
+            Reply(std::move(R));
+          },
+          std::move(Reply)));
+}
+
 void ClangdLSPServer::onRename(const RenameParams &Params,
                                Callback<WorkspaceEdit> Reply) {
   Path File = Params.textDocument.uri.file();
@@ -1015,6 +1034,7 @@
   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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to