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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
