hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73610

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/test/rename.test

Index: clang-tools-extra/clangd/test/rename.test
===================================================================
--- clang-tools-extra/clangd/test/rename.test
+++ 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": {
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -448,11 +448,8 @@
     return (*Content)->getBuffer().str();
   };
   // Try to find the tokens adjacent to the cursor position.
-  auto Loc = sourceLocationInMainFile(SM, RInputs.Pos);
-  if (!Loc)
-    return Loc.takeError();
   const syntax::Token *IdentifierToken =
-      spelledIdentifierTouching(*Loc, AST.getTokens());
+      getTouchingIdentifier(RInputs.Pos, SM, AST.getTokens());
   // Renames should only triggered on identifiers.
   if (!IdentifierToken)
     return makeError(ReasonToReject::NoSymbolFound);
Index: clang-tools-extra/clangd/SourceCode.h
===================================================================
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -21,6 +21,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
@@ -298,6 +299,12 @@
 bool isHeaderFile(llvm::StringRef FileName,
                   llvm::Optional<LangOptions> LangOpts = llvm::None);
 
+/// Returns the identifier token that touches the given \p Pos
+/// Redturns nullptr if there is none.
+const syntax::Token *getTouchingIdentifier(Position Pos,
+                                           const SourceManager &SM,
+                                           const syntax::TokenBuffer &Tokens);
+
 } // namespace clangd
 } // namespace clang
 #endif
Index: clang-tools-extra/clangd/SourceCode.cpp
===================================================================
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -1127,5 +1127,15 @@
   return Lang != types::TY_INVALID && types::onlyPrecompileType(Lang);
 }
 
+const syntax::Token *getTouchingIdentifier(Position Pos,
+                                           const SourceManager &SM,
+                                           const syntax::TokenBuffer &Tokens) {
+  auto Loc = sourceLocationInMainFile(SM, Pos);
+  if (!Loc) {
+    elog("Failed to convert position to source location: {0}", Loc.takeError());
+    return nullptr;
+  }
+  return spelledIdentifierTouching(*Loc, Tokens);
+}
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -316,10 +316,13 @@
       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);
+    const auto *TouchingIdentifier =
+        getTouchingIdentifier(Pos, SM, AST.getTokens());
+    if (!TouchingIdentifier)
+      return CB(llvm::None); // no rename on non-identifiers.
+
+    auto Range = getTokenRange(AST.getSourceManager(), AST.getLangOpts(),
+                               TouchingIdentifier->location());
     if (!Range)
       return CB(llvm::None); // "rename" is not valid at the position.
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to