[clang-tools-extra] r365522 - [clangd] Show documentation in hover, and fetch docs from index if needed.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 9 10:59:50 PDT 2019
Author: sammccall
Date: Tue Jul 9 10:59:50 2019
New Revision: 365522
URL: http://llvm.org/viewvc/llvm-project?rev=365522&view=rev
Log:
[clangd] Show documentation in hover, and fetch docs from index if needed.
Summary:
I assume showing docs is going to be part of structured hover rendering, but
it's unclear whether that's going to make clangd 9 so this is low-hanging fruit.
(Also fixes a bug uncovered in FormattedString's plain text output: need blank
lines when text follows codeblocks)
Reviewers: kadircet
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D64296
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/FormattedString.cpp
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/XRefs.h
clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=365522&r1=365521&r2=365522&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Jul 9 10:59:50 2019
@@ -499,13 +499,13 @@ void ClangdServer::findDocumentHighlight
void ClangdServer::findHover(PathRef File, Position Pos,
Callback<llvm::Optional<HoverInfo>> CB) {
- auto Action = [Pos](decltype(CB) CB, Path File,
- llvm::Expected<InputsAndAST> InpAST) {
+ auto Action = [Pos, this](decltype(CB) CB, Path File,
+ llvm::Expected<InputsAndAST> InpAST) {
if (!InpAST)
return CB(InpAST.takeError());
format::FormatStyle Style = getFormatStyleForFile(
File, InpAST->Inputs.Contents, InpAST->Inputs.FS.get());
- CB(clangd::getHover(InpAST->AST, Pos, std::move(Style)));
+ CB(clangd::getHover(InpAST->AST, Pos, std::move(Style), Index));
};
WorkScheduler.runWithAST("Hover", File,
Modified: clang-tools-extra/trunk/clangd/FormattedString.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FormattedString.cpp?rev=365522&r1=365521&r2=365522&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/FormattedString.cpp (original)
+++ clang-tools-extra/trunk/clangd/FormattedString.cpp Tue Jul 9 10:59:50 2019
@@ -89,14 +89,10 @@ static std::string renderCodeBlock(llvm:
} // namespace
void FormattedString::appendText(std::string Text) {
- // We merge consecutive blocks of text to simplify the overall structure.
- if (Chunks.empty() || Chunks.back().Kind != ChunkKind::PlainText) {
- Chunk C;
- C.Kind = ChunkKind::PlainText;
- Chunks.push_back(C);
- }
- // FIXME: ensure there is a whitespace between the chunks.
- Chunks.back().Contents += Text;
+ Chunk C;
+ C.Kind = ChunkKind::PlainText;
+ C.Contents = Text;
+ Chunks.push_back(C);
}
void FormattedString::appendCodeBlock(std::string Code, std::string Language) {
@@ -146,28 +142,30 @@ std::string FormattedString::renderAsPla
return;
R += " ";
};
+ Optional<bool> LastWasBlock;
for (const auto &C : Chunks) {
+ bool IsBlock = C.Kind == ChunkKind::CodeBlock;
+ if (LastWasBlock.hasValue() && (IsBlock || *LastWasBlock))
+ R += "\n\n";
+ LastWasBlock = IsBlock;
+
switch (C.Kind) {
case ChunkKind::PlainText:
EnsureWhitespace();
R += C.Contents;
- continue;
+ break;
case ChunkKind::InlineCodeBlock:
EnsureWhitespace();
R += C.Contents;
- continue;
+ break;
case ChunkKind::CodeBlock:
- if (!R.empty())
- R += "\n\n";
R += C.Contents;
- if (!llvm::StringRef(C.Contents).endswith("\n"))
- R += "\n";
- continue;
+ break;
}
- llvm_unreachable("unhanlded ChunkKind");
+ // Trim trailing whitespace in chunk.
+ while (!R.empty() && isWhitespace(R.back()))
+ R.pop_back();
}
- while (!R.empty() && isWhitespace(R.back()))
- R.pop_back();
return R;
}
Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=365522&r1=365521&r2=365522&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Tue Jul 9 10:59:50 2019
@@ -14,6 +14,7 @@
#include "Protocol.h"
#include "SourceCode.h"
#include "URI.h"
+#include "index/Index.h"
#include "index/Merge.h"
#include "index/SymbolCollector.h"
#include "index/SymbolLocation.h"
@@ -601,8 +602,32 @@ static const FunctionDecl *getUnderlying
return D->getAsFunction();
}
+// Look up information about D from the index, and add it to Hover.
+static void enhanceFromIndex(HoverInfo &Hover, const Decl *D,
+ const SymbolIndex *Index) {
+ if (!Index || !llvm::isa<NamedDecl>(D))
+ return;
+ const NamedDecl &ND = *cast<NamedDecl>(D);
+ // We only add documentation, so don't bother if we already have some.
+ if (!Hover.Documentation.empty())
+ return;
+ // Skip querying for non-indexable symbols, there's no point.
+ // We're searching for symbols that might be indexed outside this main file.
+ if (!SymbolCollector::shouldCollectSymbol(ND, ND.getASTContext(),
+ SymbolCollector::Options(),
+ /*IsMainFileOnly=*/false))
+ return;
+ auto ID = getSymbolID(&ND);
+ if (!ID)
+ return;
+ LookupRequest Req;
+ Req.IDs.insert(*ID);
+ Index->lookup(
+ Req, [&](const Symbol &S) { Hover.Documentation = S.Documentation; });
+}
+
/// Generate a \p Hover object given the declaration \p D.
-static HoverInfo getHoverContents(const Decl *D) {
+static HoverInfo getHoverContents(const Decl *D, const SymbolIndex *Index) {
HoverInfo HI;
const ASTContext &Ctx = D->getASTContext();
@@ -697,19 +722,22 @@ static HoverInfo getHoverContents(const
}
HI.Definition = printDefinition(D);
+ enhanceFromIndex(HI, D, Index);
return HI;
}
/// Generate a \p Hover object given the type \p T.
-static HoverInfo getHoverContents(QualType T, const Decl *D,
- ASTContext &ASTCtx) {
+static HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx,
+ const SymbolIndex *Index) {
HoverInfo HI;
llvm::raw_string_ostream OS(HI.Name);
PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
T.print(OS, Policy);
- if (D)
+ if (D) {
HI.Kind = indexSymbolKindToSymbolKind(index::getSymbolInfo(D).Kind);
+ enhanceFromIndex(HI, D, Index);
+ }
return HI;
}
@@ -859,7 +887,8 @@ bool hasDeducedType(ParsedAST &AST, Sour
}
llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
- format::FormatStyle Style) {
+ format::FormatStyle Style,
+ const SymbolIndex *Index) {
llvm::Optional<HoverInfo> HI;
SourceLocation SourceLocationBeg = getBeginningOfIdentifier(
AST, Pos, AST.getSourceManager().getMainFileID());
@@ -869,7 +898,7 @@ llvm::Optional<HoverInfo> getHover(Parse
if (!Symbols.Macros.empty())
HI = getHoverContents(Symbols.Macros[0], AST);
else if (!Symbols.Decls.empty())
- HI = getHoverContents(Symbols.Decls[0]);
+ HI = getHoverContents(Symbols.Decls[0], Index);
else {
if (!hasDeducedType(AST, SourceLocationBeg))
return None;
@@ -878,7 +907,7 @@ llvm::Optional<HoverInfo> getHover(Parse
V.TraverseAST(AST.getASTContext());
if (V.DeducedType.isNull())
return None;
- HI = getHoverContents(V.DeducedType, V.D, AST.getASTContext());
+ HI = getHoverContents(V.DeducedType, V.D, AST.getASTContext(), Index);
}
auto Replacements = format::reformat(
@@ -1225,6 +1254,9 @@ FormattedString HoverInfo::present() con
// Builtin types
Output.appendCodeBlock(Name);
}
+
+ if (!Documentation.empty())
+ Output.appendText(Documentation);
return Output;
}
Modified: clang-tools-extra/trunk/clangd/XRefs.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.h?rev=365522&r1=365521&r2=365522&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.h (original)
+++ clang-tools-extra/trunk/clangd/XRefs.h Tue Jul 9 10:59:50 2019
@@ -118,7 +118,8 @@ inline bool operator==(const HoverInfo::
/// Get the hover information when hovering at \p Pos.
llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
- format::FormatStyle Style);
+ format::FormatStyle Style,
+ const SymbolIndex *Index);
/// Returns reference locations of the symbol at a specified \p Pos.
/// \p Limit limits the number of results returned (0 means no limit).
Modified: clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp?rev=365522&r1=365521&r2=365522&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp Tue Jul 9 10:59:50 2019
@@ -21,9 +21,10 @@ TEST(FormattedString, Basic) {
EXPECT_EQ(S.renderAsPlainText(), "");
EXPECT_EQ(S.renderAsMarkdown(), "");
- S.appendText("foobar");
- EXPECT_EQ(S.renderAsPlainText(), "foobar");
- EXPECT_EQ(S.renderAsMarkdown(), "foobar");
+ S.appendText("foobar ");
+ S.appendText("baz");
+ EXPECT_EQ(S.renderAsPlainText(), "foobar baz");
+ EXPECT_EQ(S.renderAsMarkdown(), "foobar baz");
S = FormattedString();
S.appendInlineCode("foobar");
@@ -42,15 +43,21 @@ TEST(FormattedString, CodeBlocks) {
FormattedString S;
S.appendCodeBlock("foobar");
S.appendCodeBlock("bazqux", "javascript");
+ S.appendText("after");
+
+ std::string ExpectedText = R"(foobar
+
+bazqux
- EXPECT_EQ(S.renderAsPlainText(), "foobar\n\n\nbazqux");
+after)";
+ EXPECT_EQ(S.renderAsPlainText(), ExpectedText);
std::string ExpectedMarkdown = R"md(```cpp
foobar
```
```javascript
bazqux
```
-)md";
+after)md";
EXPECT_EQ(S.renderAsMarkdown(), ExpectedMarkdown);
S = FormattedString();
Modified: clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp?rev=365522&r1=365521&r2=365522&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp Tue Jul 9 10:59:50 2019
@@ -12,9 +12,11 @@
#include "Protocol.h"
#include "SyncAPI.h"
#include "TestFS.h"
+#include "TestIndex.h"
#include "TestTU.h"
#include "XRefs.h"
#include "index/FileIndex.h"
+#include "index/MemIndex.h"
#include "index/SymbolCollector.h"
#include "clang/Index/IndexingAction.h"
#include "llvm/ADT/None.h"
@@ -987,7 +989,7 @@ void foo())cpp";
auto AST = TU.build();
ASSERT_TRUE(AST.getDiagnostics().empty());
- auto H = getHover(AST, T.point(), format::getLLVMStyle());
+ auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
ASSERT_TRUE(H);
HoverInfo Expected;
Expected.SymRange = T.range();
@@ -1101,7 +1103,8 @@ TEST(Hover, All) {
"text[Declared in]code[global namespace]\n"
"codeblock(cpp) [\n"
"int foo(int)\n"
- "]",
+ "]\n"
+ "text[Function definition via pointer]",
},
{
R"cpp(// Function declaration via call
@@ -1113,7 +1116,8 @@ TEST(Hover, All) {
"text[Declared in]code[global namespace]\n"
"codeblock(cpp) [\n"
"int foo(int)\n"
- "]",
+ "]\n"
+ "text[Function declaration via call]",
},
{
R"cpp(// Field
@@ -1224,7 +1228,8 @@ TEST(Hover, All) {
"text[Declared in]code[global namespace]\n"
"codeblock(cpp) [\n"
"typedef int Foo\n"
- "]",
+ "]\n"
+ "text[Typedef]",
},
{
R"cpp(// Namespace
@@ -1294,7 +1299,8 @@ TEST(Hover, All) {
"text[Declared in]code[global namespace]\n"
"codeblock(cpp) [\n"
"class Foo {}\n"
- "]",
+ "]\n"
+ "text[Forward class declaration]",
},
{
R"cpp(// Function declaration
@@ -1305,7 +1311,8 @@ TEST(Hover, All) {
"text[Declared in]code[global namespace]\n"
"codeblock(cpp) [\n"
"void foo()\n"
- "]",
+ "]\n"
+ "text[Function declaration]",
},
{
R"cpp(// Enum declaration
@@ -1319,7 +1326,8 @@ TEST(Hover, All) {
"text[Declared in]code[global namespace]\n"
"codeblock(cpp) [\n"
"enum Hello {}\n"
- "]",
+ "]\n"
+ "text[Enum declaration]",
},
{
R"cpp(// Enumerator
@@ -1359,7 +1367,8 @@ TEST(Hover, All) {
"text[Declared in]code[global namespace]\n"
"codeblock(cpp) [\n"
"static int hey = 10\n"
- "]",
+ "]\n"
+ "text[Global variable]",
},
{
R"cpp(// Global variable in namespace
@@ -1400,7 +1409,8 @@ TEST(Hover, All) {
"text[Declared in]code[global namespace]\n"
"codeblock(cpp) [\n"
"template <typename T> T foo()\n"
- "]",
+ "]\n"
+ "text[Templated function]",
},
{
R"cpp(// Anonymous union
@@ -1417,6 +1427,18 @@ TEST(Hover, All) {
"]",
},
{
+ R"cpp(// documentation from index
+ int nextSymbolIsAForwardDeclFromIndexWithNoLocalDocs;
+ void indexSymbol();
+ void g() { ind^exSymbol(); }
+ )cpp",
+ "text[Declared in]code[global namespace]\n"
+ "codeblock(cpp) [\n"
+ "void indexSymbol()\n"
+ "]\n"
+ "text[comment from index]",
+ },
+ {
R"cpp(// Nothing
void foo() {
^
@@ -1773,12 +1795,21 @@ TEST(Hover, All) {
},
};
+ // Create a tiny index, so tests above can verify documentation is fetched.
+ Symbol IndexSym = func("indexSymbol");
+ IndexSym.Documentation = "comment from index";
+ SymbolSlab::Builder Symbols;
+ Symbols.insert(IndexSym);
+ auto Index =
+ MemIndex::build(std::move(Symbols).build(), RefSlab(), RelationSlab());
+
for (const OneTest &Test : Tests) {
Annotations T(Test.Input);
TestTU TU = TestTU::withCode(T.code());
TU.ExtraArgs.push_back("-std=c++17");
auto AST = TU.build();
- if (auto H = getHover(AST, T.point(), format::getLLVMStyle())) {
+ if (auto H =
+ getHover(AST, T.point(), format::getLLVMStyle(), Index.get())) {
EXPECT_NE("", Test.ExpectedHover) << Test.Input;
EXPECT_EQ(H->present().renderForTests(), Test.ExpectedHover.str())
<< Test.Input;
More information about the cfe-commits
mailing list