diff options
author | Sam McCall <sam.mccall@gmail.com> | 2019-02-02 05:56:00 +0000 |
---|---|---|
committer | Sam McCall <sam.mccall@gmail.com> | 2019-02-02 05:56:00 +0000 |
commit | b5ed3914b4b13626cbbf7fda55e99c88a3c29b27 (patch) | |
tree | df023af59bce7b72d123dfe1569c9e1671e68df0 | |
parent | 3531ea17aee51e2b7fc38e477204d33ae6660ba1 (diff) | |
download | clang-tools-extra-b5ed3914b4b13626cbbf7fda55e99c88a3c29b27.tar.gz |
[Clangd] textDocument/definition and textDocument/declaration "bounce" between definition and declaration location when they are distinct.
Summary:
This helps minimize the disruption of not returning declarations as part of
a find-definition response (r352864).
Reviewers: hokein
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, ilya-biryukov
Tags: #clang
Differential Revision: https://reviews.llvm.org/D57580
git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@352953 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r-- | clangd/ClangdLSPServer.cpp | 40 | ||||
-rw-r--r-- | test/clangd/xrefs.test | 49 |
2 files changed, 74 insertions, 15 deletions
diff --git a/clangd/ClangdLSPServer.cpp b/clangd/ClangdLSPServer.cpp index 7e7acf94..893d85ab 100644 --- a/clangd/ClangdLSPServer.cpp +++ b/clangd/ClangdLSPServer.cpp @@ -726,18 +726,41 @@ void ClangdLSPServer::onSignatureHelp(const TextDocumentPositionParams &Params, std::move(Reply)); } +// Go to definition has a toggle function: if def and decl are distinct, then +// the first press gives you the def, the second gives you the matching def. +// getToggle() returns the counterpart location that under the cursor. +// +// We return the toggled location alone (ignoring other symbols) to encourage +// editors to "bounce" quickly between locations, without showing a menu. +static Location *getToggle(const TextDocumentPositionParams &Point, + LocatedSymbol &Sym) { + // Toggle only makes sense with two distinct locations. + if (!Sym.Definition || *Sym.Definition == Sym.PreferredDeclaration) + return nullptr; + if (Sym.Definition->uri.file() == Point.textDocument.uri.file() && + Sym.Definition->range.contains(Point.position)) + return &Sym.PreferredDeclaration; + if (Sym.PreferredDeclaration.uri.file() == Point.textDocument.uri.file() && + Sym.PreferredDeclaration.range.contains(Point.position)) + return &*Sym.Definition; + return nullptr; +} + void ClangdLSPServer::onGoToDefinition(const TextDocumentPositionParams &Params, Callback<std::vector<Location>> Reply) { Server->locateSymbolAt( Params.textDocument.uri.file(), Params.position, Bind( - [&](decltype(Reply) Reply, - llvm::Expected<std::vector<LocatedSymbol>> Symbols) { + [&, Params](decltype(Reply) Reply, + llvm::Expected<std::vector<LocatedSymbol>> Symbols) { if (!Symbols) return Reply(Symbols.takeError()); std::vector<Location> Defs; - for (const auto &S : *Symbols) + for (auto &S : *Symbols) { + if (Location *Toggle = getToggle(Params, S)) + return Reply(std::vector<Location>{std::move(*Toggle)}); Defs.push_back(S.Definition.getValueOr(S.PreferredDeclaration)); + } Reply(std::move(Defs)); }, std::move(Reply))); @@ -749,13 +772,16 @@ void ClangdLSPServer::onGoToDeclaration( Server->locateSymbolAt( Params.textDocument.uri.file(), Params.position, Bind( - [&](decltype(Reply) Reply, - llvm::Expected<std::vector<LocatedSymbol>> Symbols) { + [&, Params](decltype(Reply) Reply, + llvm::Expected<std::vector<LocatedSymbol>> Symbols) { if (!Symbols) return Reply(Symbols.takeError()); std::vector<Location> Decls; - for (const auto &S : *Symbols) - Decls.push_back(S.PreferredDeclaration); + for (auto &S : *Symbols) { + if (Location *Toggle = getToggle(Params, S)) + return Reply(std::vector<Location>{std::move(*Toggle)}); + Decls.push_back(std::move(S.PreferredDeclaration)); + } Reply(std::move(Decls)); }, std::move(Reply))); diff --git a/test/clangd/xrefs.test b/test/clangd/xrefs.test index 58ca44cb..128c97ff 100644 --- a/test/clangd/xrefs.test +++ b/test/clangd/xrefs.test @@ -1,9 +1,9 @@ # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} --- -{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"int x = 0;\nint y = x;"}}} +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"extern int x;\nint x = 0;\nint y = x;"}}} --- -{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":1,"character":8}}} +{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":8}}} # CHECK: "id": 1, # CHECK-NEXT: "jsonrpc": "2.0", # CHECK-NEXT: "result": [ @@ -11,10 +11,30 @@ # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { # CHECK-NEXT: "character": 5, -# CHECK-NEXT: "line": 0 +# CHECK-NEXT: "line": 1 # CHECK-NEXT: }, # CHECK-NEXT: "start": { # CHECK-NEXT: "character": 4, +# CHECK-NEXT: "line": 1 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "uri": "file://{{.*}}/{{([A-Z]:/)?}}main.cpp" +# CHECK-NEXT: } +# CHECK-NEXT: ] +--- +# Toggle: we're on the definition, so jump to the declaration. +{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":1,"character":4}}} +# CHECK: "id": 1, +# CHECK-NEXT: "jsonrpc": "2.0", +# CHECK-NEXT: "result": [ +# CHECK-NEXT: { +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 12, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 11, # CHECK-NEXT: "line": 0 # CHECK-NEXT: } # CHECK-NEXT: }, @@ -22,7 +42,7 @@ # CHECK-NEXT: } # CHECK-NEXT: ] --- -{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":1,"character":8}}} +{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":8}}} # CHECK: "id": 1 # CHECK-NEXT: "jsonrpc": "2.0", # CHECK-NEXT: "result": [ @@ -30,25 +50,38 @@ # CHECK-NEXT: "kind": 1, # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 5, +# CHECK-NEXT: "character": 12, # CHECK-NEXT: "line": 0 # CHECK-NEXT: }, # CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 4, +# CHECK-NEXT: "character": 11, # CHECK-NEXT: "line": 0 # CHECK-NEXT: } # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: { +# CHECK-NEXT: "kind": 1, +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 5, +# CHECK-NEXT: "line": 1 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 4, +# CHECK-NEXT: "line": 1 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: { # CHECK-NEXT: "kind": 2, # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { # CHECK-NEXT: "character": 9, -# CHECK-NEXT: "line": 1 +# CHECK-NEXT: "line": 2 # CHECK-NEXT: }, # CHECK-NEXT: "start": { # CHECK-NEXT: "character": 8, -# CHECK-NEXT: "line": 1 +# CHECK-NEXT: "line": 2 # CHECK-NEXT: } # CHECK-NEXT: } # CHECK-NEXT: } |