From a3a27a7aeed6d9fee447e1c589811017b091384c Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Thu, 30 Apr 2020 10:49:32 +0200 Subject: [PATCH] [clangd] Render code complete documentation as plaintext/markdown. Summary: Structure is parsed from the raw comment using the existing heuristics used for hover. Reviewers: kadircet Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D79157 --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 2 ++ clang-tools-extra/clangd/CodeComplete.cpp | 33 +++++++++++++++++----- clang-tools-extra/clangd/CodeComplete.h | 7 ++++- clang-tools-extra/clangd/FormattedString.cpp | 22 +++++++++++++++ clang-tools-extra/clangd/FormattedString.h | 9 ++++++ clang-tools-extra/clangd/Hover.cpp | 1 + clang-tools-extra/clangd/Hover.h | 1 + clang-tools-extra/clangd/Protocol.cpp | 13 +++++---- clang-tools-extra/clangd/Protocol.h | 6 +++- .../clangd/unittests/CodeCompleteTests.cpp | 23 ++++++++++----- 10 files changed, 96 insertions(+), 21 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 4ca4f83..882df4a 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -528,6 +528,8 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params, CCOpts.IncludeFixIts = Params.capabilities.CompletionFixes; if (!CCOpts.BundleOverloads.hasValue()) CCOpts.BundleOverloads = Params.capabilities.HasSignatureHelp; + CCOpts.DocumentationFormat = + Params.capabilities.CompletionDocumentationFormat; DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes; DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory; DiagOpts.EmitRelatedLocations = diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index d218d6b..abf9e1f 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -26,6 +26,7 @@ #include "FileDistance.h" #include "FuzzyMatch.h" #include "Headers.h" +#include "Hover.h" #include "Preamble.h" #include "Protocol.h" #include "Quality.h" @@ -371,12 +372,19 @@ struct CodeCompletionBuilder { S.SnippetSuffix = std::string(C.IndexResult->CompletionSnippetSuffix); S.ReturnType = std::string(C.IndexResult->ReturnType); } - if (ExtractDocumentation && Completion.Documentation.empty()) { - if (C.IndexResult) - Completion.Documentation = std::string(C.IndexResult->Documentation); - else if (C.SemaResult) - Completion.Documentation = getDocComment(*ASTCtx, *C.SemaResult, - /*CommentsFromHeader=*/false); + if (ExtractDocumentation && !Completion.Documentation) { + auto SetDoc = [&](llvm::StringRef Doc) { + if (!Doc.empty()) { + Completion.Documentation.emplace(); + parseDocumentation(Doc, *Completion.Documentation); + } + }; + if (C.IndexResult) { + SetDoc(C.IndexResult->Documentation); + } else if (C.SemaResult) { + SetDoc(getDocComment(*ASTCtx, *C.SemaResult, + /*CommentsFromHeader=*/false)); + } } } @@ -1832,7 +1840,18 @@ CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const { LSP.deprecated = Deprecated; if (InsertInclude) LSP.detail += "\n" + InsertInclude->Header; - LSP.documentation = Documentation; + if (Documentation) { + LSP.documentation.emplace(); + switch (Opts.DocumentationFormat) { + case MarkupKind::PlainText: + LSP.documentation->value = Documentation->asPlainText(); + break; + case MarkupKind::Markdown: + LSP.documentation->value = Documentation->asMarkdown(); + break; + } + LSP.documentation->kind = Opts.DocumentationFormat; + } LSP.sortText = sortText(Score.Total, Name); LSP.filterText = Name; LSP.textEdit = {CompletionTokenRange, RequiredQualifier + Name}; diff --git a/clang-tools-extra/clangd/CodeComplete.h b/clang-tools-extra/clangd/CodeComplete.h index 1e23704..8c2908e 100644 --- a/clang-tools-extra/clangd/CodeComplete.h +++ b/clang-tools-extra/clangd/CodeComplete.h @@ -15,6 +15,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H +#include "FormattedString.h" #include "Headers.h" #include "Protocol.h" #include "Quality.h" @@ -73,6 +74,9 @@ struct CodeCompleteOptions { /// If more results are available, we set CompletionList.isIncomplete. size_t Limit = 0; + /// Whether to present doc comments as plain-text or markdown. + MarkupKind DocumentationFormat = MarkupKind::PlainText; + enum IncludeInsertion { IWYU, NeverInsert, @@ -161,7 +165,8 @@ struct CodeCompletion { std::string SnippetSuffix; // Type to be displayed for this completion. std::string ReturnType; - std::string Documentation; + // The parsed documentation comment. + llvm::Optional Documentation; CompletionItemKind Kind = CompletionItemKind::Missing; // This completion item may represent several symbols that can be inserted in // the same way, such as function overloads. In this case BundleSize > 1, and diff --git a/clang-tools-extra/clangd/FormattedString.cpp b/clang-tools-extra/clangd/FormattedString.cpp index 81de6a8b..96a5290 100644 --- a/clang-tools-extra/clangd/FormattedString.cpp +++ b/clang-tools-extra/clangd/FormattedString.cpp @@ -271,6 +271,9 @@ public: OS << "\n---\n"; } void renderPlainText(llvm::raw_ostream &OS) const override { OS << '\n'; } + std::unique_ptr clone() const override { + return std::make_unique(*this); + } bool isRuler() const override { return true; } }; @@ -287,6 +290,10 @@ public: OS << '\n' << Contents << "\n\n"; } + std::unique_ptr clone() const override { + return std::make_unique(*this); + } + CodeBlock(std::string Contents, std::string Language) : Contents(std::move(Contents)), Language(std::move(Language)) {} @@ -358,6 +365,10 @@ void Paragraph::renderMarkdown(llvm::raw_ostream &OS) const { OS << " \n"; } +std::unique_ptr Paragraph::clone() const { + return std::make_unique(*this); +} + void Paragraph::renderPlainText(llvm::raw_ostream &OS) const { llvm::StringRef Sep = ""; for (auto &C : Chunks) { @@ -407,11 +418,22 @@ Paragraph &Paragraph::appendCode(llvm::StringRef Code) { return *this; } +std::unique_ptr BulletList::clone() const { + return std::make_unique(*this); +} + class Document &BulletList::addItem() { Items.emplace_back(); return Items.back(); } +Document &Document::operator=(const Document &Other) { + Children.clear(); + for (const auto &C : Other.Children) + Children.push_back(C->clone()); + return *this; +} + Paragraph &Document::addParagraph() { Children.push_back(std::make_unique()); return *static_cast(Children.back().get()); diff --git a/clang-tools-extra/clangd/FormattedString.h b/clang-tools-extra/clangd/FormattedString.h index 6558f26..ccb758f 100644 --- a/clang-tools-extra/clangd/FormattedString.h +++ b/clang-tools-extra/clangd/FormattedString.h @@ -30,6 +30,7 @@ class Block { public: virtual void renderMarkdown(llvm::raw_ostream &OS) const = 0; virtual void renderPlainText(llvm::raw_ostream &OS) const = 0; + virtual std::unique_ptr clone() const = 0; std::string asMarkdown() const; std::string asPlainText() const; @@ -44,6 +45,7 @@ class Paragraph : public Block { public: void renderMarkdown(llvm::raw_ostream &OS) const override; void renderPlainText(llvm::raw_ostream &OS) const override; + std::unique_ptr clone() const override; /// Append plain text to the end of the string. Paragraph &appendText(llvm::StringRef Text); @@ -68,6 +70,7 @@ class BulletList : public Block { public: void renderMarkdown(llvm::raw_ostream &OS) const override; void renderPlainText(llvm::raw_ostream &OS) const override; + std::unique_ptr clone() const override; class Document &addItem(); @@ -79,6 +82,12 @@ private: /// markdown and plaintext. class Document { public: + Document() = default; + Document(const Document &Other) { *this = Other; } + Document &operator=(const Document &); + Document(Document &&) = default; + Document &operator=(Document &&) = default; + /// Adds a semantical block that will be separate from others. Paragraph &addParagraph(); /// Inserts a horizontal separator to the document. diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp index 3a66130..ebacca8 100644 --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -648,6 +648,7 @@ bool isHardLineBreakIndicator(llvm::StringRef Rest) { } bool isHardLineBreakAfter(llvm::StringRef Line, llvm::StringRef Rest) { + // Should we also consider whether Line is short? return punctuationIndicatesLineBreak(Line) || isHardLineBreakIndicator(Rest); } diff --git a/clang-tools-extra/clangd/Hover.h b/clang-tools-extra/clangd/Hover.h index 4476ed8..73138c2 100644 --- a/clang-tools-extra/clangd/Hover.h +++ b/clang-tools-extra/clangd/Hover.h @@ -80,6 +80,7 @@ struct HoverInfo { }; // Try to infer structure of a documentation comment (e.g. line breaks). +// FIXME: move to another file so CodeComplete doesn't depend on Hover. void parseDocumentation(llvm::StringRef Input, markup::Document &Output); llvm::raw_ostream &operator<<(llvm::raw_ostream &, const HoverInfo::Param &); diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index 314e6b0..675ba18 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -311,6 +311,12 @@ bool fromJSON(const llvm::json::Value &Params, ClientCapabilities &R) { if (auto *Item = Completion->getObject("completionItem")) { if (auto SnippetSupport = Item->getBoolean("snippetSupport")) R.CompletionSnippets = *SnippetSupport; + if (auto DocumentationFormat = Item->getArray("documentationFormat")) { + for (const auto &Format : *DocumentationFormat) { + if (fromJSON(Format, R.CompletionDocumentationFormat)) + break; + } + } } if (auto *ItemKind = Completion->getObject("completionItemKind")) { if (auto *ValueSet = ItemKind->get("valueSet")) { @@ -334,11 +340,8 @@ bool fromJSON(const llvm::json::Value &Params, ClientCapabilities &R) { if (auto *Hover = TextDocument->getObject("hover")) { if (auto *ContentFormat = Hover->getArray("contentFormat")) { for (const auto &Format : *ContentFormat) { - MarkupKind K = MarkupKind::PlainText; - if (fromJSON(Format, K)) { - R.HoverContentFormat = K; + if (fromJSON(Format, R.HoverContentFormat)) break; - } } } } @@ -891,7 +894,7 @@ llvm::json::Value toJSON(const CompletionItem &CI) { Result["kind"] = static_cast(CI.kind); if (!CI.detail.empty()) Result["detail"] = CI.detail; - if (!CI.documentation.empty()) + if (CI.documentation) Result["documentation"] = CI.documentation; if (!CI.sortText.empty()) Result["sortText"] = CI.sortText; diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 8a0e5b4..0177ee2 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -430,6 +430,10 @@ struct ClientCapabilities { /// textDocument.completion.completionItemKind.valueSet llvm::Optional CompletionItemKinds; + /// The documentation format that should be used for textDocument/completion. + /// textDocument.completion.completionItem.documentationFormat + MarkupKind CompletionDocumentationFormat = MarkupKind::PlainText; + /// Client supports CodeAction return value for textDocument/codeAction. /// textDocument.codeAction.codeActionLiteralSupport. bool CodeActionStructure = false; @@ -1105,7 +1109,7 @@ struct CompletionItem { std::string detail; /// A human-readable string that represents a doc-comment. - std::string documentation; + llvm::Optional documentation; /// A string that should be used when comparing this item with other items. /// When `falsy` the label is used. diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index d8926d4..c979952 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -59,7 +59,9 @@ MATCHER_P(Labeled, Label, "") { } MATCHER_P(SigHelpLabeled, Label, "") { return arg.label == Label; } MATCHER_P(Kind, K, "") { return arg.Kind == K; } -MATCHER_P(Doc, D, "") { return arg.Documentation == D; } +MATCHER_P(Doc, D, "") { + return arg.Documentation && arg.Documentation->asPlainText() == D; +} MATCHER_P(ReturnType, D, "") { return arg.ReturnType == D; } MATCHER_P(HasInclude, IncludeHeader, "") { return !arg.Includes.empty() && arg.Includes[0].Header == IncludeHeader; @@ -83,7 +85,7 @@ Matcher &> Has(std::string Name, CompletionItemKind K) { return Contains(AllOf(Named(std::move(Name)), Kind(K))); } -MATCHER(IsDocumented, "") { return !arg.Documentation.empty(); } +MATCHER(IsDocumented, "") { return arg.Documentation.hasValue(); } MATCHER(Deprecated, "") { return arg.Deprecated; } std::unique_ptr memIndex(std::vector Symbols) { @@ -842,7 +844,7 @@ TEST(CompletionTest, Documentation) { Results.Completions, Contains(AllOf(Named("bar"), Doc("Doxygen comment.\n\\param int a")))); EXPECT_THAT(Results.Completions, - Contains(AllOf(Named("baz"), Doc("Multi-line\nblock comment")))); + Contains(AllOf(Named("baz"), Doc("Multi-line block comment")))); } TEST(CompletionTest, CommentsFromSystemHeaders) { @@ -1506,8 +1508,10 @@ TEST(CompletionTest, OverloadBundling) { EXPECT_EQ(A.Kind, CompletionItemKind::Method); EXPECT_EQ(A.ReturnType, "int"); // All overloads return int. // For now we just return one of the doc strings arbitrarily. - EXPECT_THAT(A.Documentation, AnyOf(HasSubstr("Overload with int"), - HasSubstr("Overload with bool"))); + ASSERT_TRUE(A.Documentation); + EXPECT_THAT( + A.Documentation->asPlainText(), + AnyOf(HasSubstr("Overload with int"), HasSubstr("Overload with bool"))); EXPECT_EQ(A.SnippetSuffix, "($0)"); } @@ -1641,7 +1645,8 @@ TEST(CompletionTest, Render) { C.ReturnType = "int"; C.RequiredQualifier = "Foo::"; C.Scope = "ns::Foo::"; - C.Documentation = "This is x()."; + C.Documentation.emplace(); + C.Documentation->addParagraph().appendText("This is ").appendCode("x()"); C.Includes.emplace_back(); auto &Include = C.Includes.back(); Include.Header = "\"foo.h\""; @@ -1661,7 +1666,7 @@ TEST(CompletionTest, Render) { EXPECT_EQ(R.insertTextFormat, InsertTextFormat::PlainText); EXPECT_EQ(R.filterText, "x"); EXPECT_EQ(R.detail, "int\n\"foo.h\""); - EXPECT_EQ(R.documentation, "This is x()."); + EXPECT_EQ(R.documentation->value, "This is x()"); EXPECT_THAT(R.additionalTextEdits, IsEmpty()); EXPECT_EQ(R.sortText, sortText(1.0, "x")); EXPECT_FALSE(R.deprecated); @@ -1688,6 +1693,10 @@ TEST(CompletionTest, Render) { C.Deprecated = true; R = C.render(Opts); EXPECT_TRUE(R.deprecated); + + Opts.DocumentationFormat = MarkupKind::Markdown; + R = C.render(Opts); + EXPECT_EQ(R.documentation->value, "This is `x()`"); } TEST(CompletionTest, IgnoreRecoveryResults) { -- 2.7.4