[clang-tools-extra] a3a27a7 - [clangd] Render code complete documentation as plaintext/markdown.

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 30 10:01:02 PDT 2020


Author: Sam McCall
Date: 2020-04-30T19:00:49+02:00
New Revision: a3a27a7aeed6d9fee447e1c589811017b091384c

URL: https://github.com/llvm/llvm-project/commit/a3a27a7aeed6d9fee447e1c589811017b091384c
DIFF: https://github.com/llvm/llvm-project/commit/a3a27a7aeed6d9fee447e1c589811017b091384c.diff

LOG: [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

Added: 
    

Modified: 
    clang-tools-extra/clangd/ClangdLSPServer.cpp
    clang-tools-extra/clangd/CodeComplete.cpp
    clang-tools-extra/clangd/CodeComplete.h
    clang-tools-extra/clangd/FormattedString.cpp
    clang-tools-extra/clangd/FormattedString.h
    clang-tools-extra/clangd/Hover.cpp
    clang-tools-extra/clangd/Hover.h
    clang-tools-extra/clangd/Protocol.cpp
    clang-tools-extra/clangd/Protocol.h
    clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 4ca4f83ae395..882df4abf1ce 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 d218d6b34599..abf9e1fddaf6 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 1e2370486862..8c2908e445ae 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<markup::Document> 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 81de6a8be93c..96a5290780cb 100644
--- a/clang-tools-extra/clangd/FormattedString.cpp
+++ b/clang-tools-extra/clangd/FormattedString.cpp
@@ -271,6 +271,9 @@ class Ruler : public Block {
     OS << "\n---\n";
   }
   void renderPlainText(llvm::raw_ostream &OS) const override { OS << '\n'; }
+  std::unique_ptr<Block> clone() const override {
+    return std::make_unique<Ruler>(*this);
+  }
   bool isRuler() const override { return true; }
 };
 
@@ -287,6 +290,10 @@ class CodeBlock : public Block {
     OS << '\n' << Contents << "\n\n";
   }
 
+  std::unique_ptr<Block> clone() const override {
+    return std::make_unique<CodeBlock>(*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<Block> Paragraph::clone() const {
+  return std::make_unique<Paragraph>(*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<Block> BulletList::clone() const {
+  return std::make_unique<BulletList>(*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<Paragraph>());
   return *static_cast<Paragraph *>(Children.back().get());

diff  --git a/clang-tools-extra/clangd/FormattedString.h b/clang-tools-extra/clangd/FormattedString.h
index 6558f265a687..ccb758f294c9 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<Block> 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<Block> 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<Block> clone() const override;
 
   class Document &addItem();
 
@@ -79,6 +82,12 @@ class BulletList : public Block {
 /// 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 3a66130ea153..ebacca823afb 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 4476ed874305..73138c284b6c 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 314e6b0bbabc..675ba18e80ce 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<int>(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 8a0e5b44e057..0177ee262ae3 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<CompletionItemKindBitset> 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<MarkupContent> 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 d8926d4fd3f5..c9799525de3f 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<const std::vector<CodeCompletion> &> 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<SymbolIndex> memIndex(std::vector<Symbol> 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) {


        


More information about the cfe-commits mailing list