[clang-tools-extra] e864f93 - [clangd] Replace raw lexer code with token buffer in prepare rename.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-01-29T12:57:18+01:00
New Revision: e864f937669c996b4dc15db7d0ebe4073527c165

URL: https://github.com/llvm/llvm-project/commit/e864f937669c996b4dc15db7d0ebe4073527c165
DIFF: https://github.com/llvm/llvm-project/commit/e864f937669c996b4dc15db7d0ebe4073527c165.diff

LOG: [clangd] Replace raw lexer code with token buffer in prepare rename.

Summary:
there is a slight behavior change in this patch:

- before: `in^t a;`, returns our internal error message (no symbol at given location)
- after: `in^t a, returns null, and client displays their message (e.g.
   e.g. "the element can't be renamed" in vscode).

both are sensible according LSP, and we'd save one `rename` call in the later case.

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/ClangdServer.cpp
    clang-tools-extra/clangd/test/rename.test

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 7ead45c61458..cf883f130da8 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -316,16 +316,21 @@ void ClangdServer::prepareRename(PathRef File, Position Pos,
       return CB(InpAST.takeError());
     auto &AST = InpAST->AST;
     const auto &SM = AST.getSourceManager();
-    SourceLocation Loc =
-        SM.getMacroArgExpandedLocation(getBeginningOfIdentifier(
-            Pos, AST.getSourceManager(), AST.getLangOpts()));
-    auto Range = getTokenRange(SM, AST.getLangOpts(), Loc);
-    if (!Range)
-      return CB(llvm::None); // "rename" is not valid at the position.
+    auto Loc = sourceLocationInMainFile(SM, Pos);
+    if (!Loc)
+      return CB(Loc.takeError());
+    const auto *TouchingIdentifier =
+        spelledIdentifierTouching(*Loc, AST.getTokens());
+    if (!TouchingIdentifier)
+      return CB(llvm::None); // no rename on non-identifiers.
+
+    auto Range = halfOpenToRange(
+        SM, CharSourceRange::getCharRange(TouchingIdentifier->location(),
+                                          TouchingIdentifier->endLocation()));
 
     if (CrossFileRename)
       // FIXME: we now assume cross-file rename always succeeds, revisit this.
-      return CB(*Range);
+      return CB(Range);
 
     // Performing the local rename isn't substantially more expensive than
     // doing an AST-based check, so we just rename and throw away the results.
@@ -338,7 +343,7 @@ void ClangdServer::prepareRename(PathRef File, Position Pos,
       // the message to users (VSCode does).
       return CB(Changes.takeError());
     }
-    return CB(*Range);
+    return CB(Range);
   };
   WorkScheduler.runWithAST("PrepareRename", File, std::move(Action));
 }

diff  --git a/clang-tools-extra/clangd/test/rename.test b/clang-tools-extra/clangd/test/rename.test
index 527b4263443a..214efb2b5e39 100644
--- a/clang-tools-extra/clangd/test/rename.test
+++ b/clang-tools-extra/clangd/test/rename.test
@@ -21,12 +21,9 @@
 # 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": "Cannot rename symbol: there is no symbol at the given location"
-# CHECK-NEXT:  },
-# CHECK-NEXT:  "id": 2,
-# CHECK-NEXT:  "jsonrpc": "2.0"
+#      CHECK:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": null
 ---
 {"jsonrpc":"2.0","id":4,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}}
 #      CHECK:  "error": {