Index: clang-tools-extra/clangd/ClangdServer.cpp =================================================================== --- clang-tools-extra/clangd/ClangdServer.cpp +++ clang-tools-extra/clangd/ClangdServer.cpp @@ -499,13 +499,13 @@ void ClangdServer::findHover(PathRef File, Position Pos, Callback> CB) { - auto Action = [Pos](decltype(CB) CB, Path File, - llvm::Expected InpAST) { + auto Action = [Pos, this](decltype(CB) CB, Path File, + llvm::Expected 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, Index: clang-tools-extra/clangd/FormattedString.cpp =================================================================== --- clang-tools-extra/clangd/FormattedString.cpp +++ clang-tools-extra/clangd/FormattedString.cpp @@ -89,14 +89,10 @@ } // 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 @@ return; R += " "; }; + Optional 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; } Index: clang-tools-extra/clangd/XRefs.h =================================================================== --- clang-tools-extra/clangd/XRefs.h +++ clang-tools-extra/clangd/XRefs.h @@ -118,7 +118,8 @@ /// Get the hover information when hovering at \p Pos. llvm::Optional 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). Index: clang-tools-extra/clangd/XRefs.cpp =================================================================== --- clang-tools-extra/clangd/XRefs.cpp +++ clang-tools-extra/clangd/XRefs.cpp @@ -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 @@ 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(D)) + return; + const NamedDecl &ND = *cast(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 @@ } 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 @@ } llvm::Optional getHover(ParsedAST &AST, Position Pos, - format::FormatStyle Style) { + format::FormatStyle Style, + const SymbolIndex *Index) { llvm::Optional HI; SourceLocation SourceLocationBeg = getBeginningOfIdentifier( AST, Pos, AST.getSourceManager().getMainFileID()); @@ -869,7 +898,7 @@ 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 @@ 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 @@ // Builtin types Output.appendCodeBlock(Name); } + + if (!Documentation.empty()) + Output.appendText(Documentation); return Output; } Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp +++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp @@ -21,9 +21,10 @@ 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 @@ 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(); Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -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 @@ 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 @@ "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 @@ "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 @@ "text[Declared in]code[global namespace]\n" "codeblock(cpp) [\n" "typedef int Foo\n" - "]", + "]\n" + "text[Typedef]", }, { R"cpp(// Namespace @@ -1294,7 +1299,8 @@ "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 @@ "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 @@ "text[Declared in]code[global namespace]\n" "codeblock(cpp) [\n" "enum Hello {}\n" - "]", + "]\n" + "text[Enum declaration]", }, { R"cpp(// Enumerator @@ -1359,7 +1367,8 @@ "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 @@ "text[Declared in]code[global namespace]\n" "codeblock(cpp) [\n" "template T foo()\n" - "]", + "]\n" + "text[Templated function]", }, { R"cpp(// Anonymous union @@ -1416,6 +1426,18 @@ "int def\n" "]", }, + { + 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 @@ }, }; + // 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;