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": { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
