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