[clang-tools-extra] r333881 - [clangd] Hover should return null when not hovering over anything.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 4 03:37:16 PDT 2018
Author: sammccall
Date: Mon Jun 4 03:37:16 2018
New Revision: 333881
URL: http://llvm.org/viewvc/llvm-project?rev=333881&view=rev
Log:
[clangd] Hover should return null when not hovering over anything.
Summary: Also made JSON serialize Optional<T> to simplify this.
Reviewers: ioeric
Subscribers: klimek, ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47701
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/JSONExpr.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/XRefs.h
clang-tools-extra/trunk/test/clangd/hover.test
clang-tools-extra/trunk/unittests/clangd/JSONExprTests.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=333881&r1=333880&r2=333881&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Mon Jun 4 03:37:16 2018
@@ -365,7 +365,7 @@ void ClangdLSPServer::onDocumentHighligh
void ClangdLSPServer::onHover(TextDocumentPositionParams &Params) {
Server.findHover(Params.textDocument.uri.file(), Params.position,
- [](llvm::Expected<Hover> H) {
+ [](llvm::Expected<llvm::Optional<Hover>> H) {
if (!H) {
replyError(ErrorCode::InternalError,
llvm::toString(H.takeError()));
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=333881&r1=333880&r2=333881&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Jun 4 03:37:16 2018
@@ -403,9 +403,10 @@ void ClangdServer::findDocumentHighlight
WorkScheduler.runWithAST("Highlights", File, Bind(Action, std::move(CB)));
}
-void ClangdServer::findHover(PathRef File, Position Pos, Callback<Hover> CB) {
+void ClangdServer::findHover(PathRef File, Position Pos,
+ Callback<llvm::Optional<Hover>> CB) {
auto FS = FSProvider.getFileSystem();
- auto Action = [Pos, FS](Callback<Hover> CB,
+ auto Action = [Pos, FS](Callback<llvm::Optional<Hover>> CB,
llvm::Expected<InputsAndAST> InpAST) {
if (!InpAST)
return CB(InpAST.takeError());
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=333881&r1=333880&r2=333881&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Mon Jun 4 03:37:16 2018
@@ -158,7 +158,8 @@ public:
Callback<std::vector<DocumentHighlight>> CB);
/// Get code hover for a given position.
- void findHover(PathRef File, Position Pos, Callback<Hover> CB);
+ void findHover(PathRef File, Position Pos,
+ Callback<llvm::Optional<Hover>> CB);
/// Retrieve the top symbols from the workspace matching a query.
void workspaceSymbols(StringRef Query, int Limit,
Modified: clang-tools-extra/trunk/clangd/JSONExpr.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/JSONExpr.h?rev=333881&r1=333880&r2=333881&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/JSONExpr.h (original)
+++ clang-tools-extra/trunk/clangd/JSONExpr.h Mon Jun 4 03:37:16 2018
@@ -22,6 +22,8 @@
namespace clang {
namespace clangd {
namespace json {
+class Expr;
+template <typename T> Expr toJSON(const llvm::Optional<T> &Opt);
// An Expr is an JSON value of unknown type.
// They can be copied, but should generally be moved.
@@ -516,6 +518,11 @@ bool fromJSON(const json::Expr &E, std::
return false;
}
+template <typename T>
+json::Expr toJSON(const llvm::Optional<T>& Opt) {
+ return Opt ? json::Expr(*Opt) : json::Expr(nullptr);
+}
+
// Helper for mapping JSON objects onto protocol structs.
// See file header for example.
class ObjectMapper {
Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=333881&r1=333880&r2=333881&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Mon Jun 4 03:37:16 2018
@@ -526,7 +526,7 @@ static Hover getHoverContents(StringRef
return H;
}
-Hover getHover(ParsedAST &AST, Position Pos) {
+Optional<Hover> getHover(ParsedAST &AST, Position Pos) {
const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
SourceLocation SourceLocationBeg =
getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID());
@@ -539,7 +539,7 @@ Hover getHover(ParsedAST &AST, Position
if (!Symbols.Decls.empty())
return getHoverContents(Symbols.Decls[0]);
- return Hover();
+ return None;
}
} // namespace clangd
Modified: clang-tools-extra/trunk/clangd/XRefs.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.h?rev=333881&r1=333880&r2=333881&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.h (original)
+++ clang-tools-extra/trunk/clangd/XRefs.h Mon Jun 4 03:37:16 2018
@@ -16,6 +16,7 @@
#include "ClangdUnit.h"
#include "Protocol.h"
#include "index/Index.h"
+#include "llvm/ADT/Optional.h"
#include <vector>
namespace clang {
@@ -30,7 +31,7 @@ std::vector<DocumentHighlight> findDocum
Position Pos);
/// Get the hover information when hovering at \p Pos.
-Hover getHover(ParsedAST &AST, Position Pos);
+llvm::Optional<Hover> getHover(ParsedAST &AST, Position Pos);
} // namespace clangd
} // namespace clang
Modified: clang-tools-extra/trunk/test/clangd/hover.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/hover.test?rev=333881&r1=333880&r2=333881&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/hover.test (original)
+++ clang-tools-extra/trunk/test/clangd/hover.test Mon Jun 4 03:37:16 2018
@@ -14,6 +14,11 @@
# CHECK-NEXT: }
# CHECK-NEXT:}
---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":10}}}
+# CHECK: "id": 1,
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": null
+---
{"jsonrpc":"2.0","id":3,"method":"shutdown"}
---
{"jsonrpc":"2.0","method":"exit"}
Modified: clang-tools-extra/trunk/unittests/clangd/JSONExprTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/JSONExprTests.cpp?rev=333881&r1=333880&r2=333881&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/JSONExprTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/JSONExprTests.cpp Mon Jun 4 03:37:16 2018
@@ -38,6 +38,8 @@ TEST(JSONExprTests, Constructors) {
EXPECT_EQ(R"({"A":{"B":{}}})", s(obj{{"A", obj{{"B", obj{}}}}}));
EXPECT_EQ(R"({"A":{"B":{"X":"Y"}}})",
s(obj{{"A", obj{{"B", obj{{"X", "Y"}}}}}}));
+ EXPECT_EQ("null", s(llvm::Optional<double>()));
+ EXPECT_EQ("2.5", s(llvm::Optional<double>(2.5)));
}
TEST(JSONExprTests, StringOwnership) {
Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=333881&r1=333880&r2=333881&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Mon Jun 4 03:37:16 2018
@@ -629,14 +629,24 @@ TEST(Hover, All) {
)cpp",
"Declared in union outer::(anonymous)\n\nint def",
},
+ {
+ R"cpp(// Nothing
+ void foo() {
+ ^
+ }
+ )cpp",
+ "",
+ },
};
for (const OneTest &Test : Tests) {
Annotations T(Test.Input);
auto AST = TestTU::withCode(T.code()).build();
- Hover H = getHover(AST, T.point());
-
- EXPECT_EQ(H.contents.value, Test.ExpectedHover) << Test.Input;
+ if (auto H = getHover(AST, T.point())) {
+ EXPECT_EQ("", Test.ExpectedHover) << Test.Input;
+ EXPECT_EQ(H->contents.value, Test.ExpectedHover) << Test.Input;
+ } else
+ EXPECT_EQ("", Test.ExpectedHover) << Test.Input;
}
}
More information about the cfe-commits
mailing list