[clangd] Add doxygen parsing for Hover [1/3]

[tom-anders]

Summary

1/3: Add SymbolDocumentation class to parse Doxygen comments

This commit just adds and tests the a new class for doxygen parsing.
Consumption in Hover and Index will be added in a follow-up commit.

[kadircet]

Hi!

Sorry for letting these series of patches sit around without any comments. We were having some discussions internally to both understand the value proposition and its implications on the infrastructure.
So it'd help a lot if you can provide some more information for use cases, that way we can find a nice scope for this functionality to make sure it provides most of the value and doesn't introduce an unnecessary complexity into rest of the infrastructure and also we should try not to regress indexing of projects that don't have doxygen comments.

So first of all, what are the exact use cases you're planning to address/improve with support for doxygen parsing of comments? Couple that comes to mind:

• obtaining docs about params & return value
• stripping doxygen commands
• treating brief/detail/warning/note differently
• formatting text within comments (bold etc)
• getting linebreaks/indent right clangd#1040

any other use cases that you believe are important?

as you might've noticed, this list already talks about dealing with certain doxygen commands (but not all).
that list is gathered by counting occurrences of those commands in a codebase with lots of open-source third_party code. findings and some initial ideas look like this:

• \brief: ~70k occurrences
  • common but usefulness in practice is unclear
  • can infer for non-doxy too (e.g. first sentence of a regular documentation)
  • maybe just strip (or merge into regular documentation)?
• \return[s]: 30k occurrences
  • unclear if worth separation in hover, because it might be tied to rest of the documentation (re-ordering concerns)
  • can infer for non-doxy maybe?
  • probably just strip the command and keep the rest as-is.
• \param: 28k occurrences
  • useful for signature help. maybe hover on func calls
  • probably worth storing in a structured manner.
• \detail[s]: 2k
• \p: 20k
• \code: 1k
• \warning: 2k
• \note: 9k
  • (for all of the above) just introduce as formatted text?

what do you think about those conclusions? any other commands that you seem worth giving a thought?
One important concern we've noticed around this part is, re-ordering comment components might actually hinder readability. as the comments are usually written with the assumption that they'll be read from top to bottom, but if we re-order them during presentation (e.g. hover has its own layout) we might start referring to concepts/entities in documentation before they're introduced. so we believe it's important to avoid any sort of re-ordering. this is one of the big down sides for preserving parameter comments in a structured way.

another important thing to consider is trying to heuristically infer some of these fields for non-doxygen code bases as well. that way we can provide a similar experience for both.

some other things to discuss about the design overall:

• How to store the extra information?
  • Proposal from our side would be to introduce structured storage for the pieces we want (limited), and keep the rest as part of main documentation text while doing stripping/reformatting.
• What to use as a parser?
  • Clang's doxygen parser actually looks like a great piece of code to re-use, it's unfortunate that it can issue diagnostics, requires AST etc. It'd be great to refactor that into a state where we can use it without any AST or diagnostics, and a minimal SourceManager (this seems feasible to achieve at first glance, as most of these inputs seem to be optional or used in 1 or 2 places).
  • we still need to make sure performance and behaviour on non-doxygen is reasonable though. do you have any numbers here?
• How to store in the index?
  • If we can strip the parser off the dependencies on an astcontext, diagnostics etc. the best option would be to just store as raw text and run the whole pipeline on demand (e.g. do the doxygen parsing and markdown-ization afterwards). This is the simplest approach as it keeps index interfaces the same.

Happy to move the discussion to some other medium as well, if you would like to have them in discourse/github etc.

[tom-anders]

Thanks for the detailed feedback! Unfortunately, I’m sick right now, so I probably won’t be able to give a detailed answer until after Christmas.

In D134130#3992116, @kadircet wrote:

Happy to move the discussion to some other medium as well, if you would like to have them in discourse/github etc.

Yeah let’s maybe copy your comment back to the GitHub Issue and discuss over there, it seems the issue has lots of subscribers already who perhaps also have some more ideas/usecases.

[kadircet]

In D134130#3992215, @tom-anders wrote:

Thanks for the detailed feedback! Unfortunately, I’m sick right now, so I probably won’t be able to give a detailed answer until after Christmas.

Sorry to hear that. I hope you get better soon (and definitely no rush)!

In D134130#3992116, @kadircet wrote:

Happy to move the discussion to some other medium as well, if you would like to have them in discourse/github etc.

Yeah let’s maybe copy your comment back to the GitHub Issue and discuss over there, it seems the issue has lots of subscribers already who perhaps also have some more ideas/usecases.

posted it there, thanks!

[vinniefalco]

This is a beautiful patch, it has helped me understand the hierarchy of clang comment nodes much better.

I will note that it only supports explicit briefs. That is, those for which the \brief or @brief command has appeared. But implicit briefs are also quite prevalent. An implicit brief is simply the text of the first paragraph, after removing the whitespace starting at the beginning of the FullComment, and the absence of a \brief or @brief command anywhere else in the attached javadoc.

clang-tools-extra/clangd/CMakeLists.txt

add_clang_library(clangDaemon
   Quality.cpp
   ParsedAST.cpp
   Preamble.cpp
   RIFF.cpp
   Selection.cpp
   SemanticHighlighting.cpp
   SemanticSelection.cpp
   SourceCode.cpp
+  SymbolDocumentation.cpp
   QueryDriverDatabase.cpp
   TidyProvider.cpp
   TUScheduler.cpp
   URI.cpp
   XRefs.cpp
   ${CMAKE_CURRENT_BINARY_DIR}/CompletionModel.cpp
 
   index/Background.cpp

clang-tools-extra/clangd/SymbolDocumentation.h

+//===--- SymbolDocumentation.h ==---------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Class to parse doxygen comments into a flat structure for consumption
+// in e.g. Hover and Code Completion
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SYMBOLDOCUMENTATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SYMBOLDOCUMENTATION_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Comment.h"
+#include "clang/AST/CommentVisitor.h"
+
+namespace clang {
+namespace clangd {
+
+template <class String> struct ParameterDocumentation {
+  String Name;
+  String Description;
+
+  ParameterDocumentation<llvm::StringRef> toRef() const;
+  ParameterDocumentation<std::string> toOwned() const;
+};
+
+using ParameterDocumentationRef = ParameterDocumentation<llvm::StringRef>;
+using ParameterDocumentationOwned = ParameterDocumentation<std::string>;
+
+/// @brief Represents a parsed doxygen comment.
+/// @details Currently there's special handling for the "brief", "param"
+/// "returns", "note" and "warning" commands. The content of all other
+/// paragraphs will be appended to the #Description field.
+/// If you're only interested in the full comment, but with comment
+/// markers stripped, use the #CommentText field.
+/// \tparam String When built from a declaration, we're building the strings
+/// by ourselves, so in this case String==std::string.
+/// However, when storing the contents of this class in the index, we need to
+/// use llvm::StringRef. To connvert between std::string and llvm::StringRef
+/// versions of this class, use toRef() and toOwned().
+template <class String> class SymbolDocumentation {
+public:
+  friend class CommentToSymbolDocumentation;
+
+  static SymbolDocumentation<String> descriptionOnly(String &&Description) {
+    SymbolDocumentation<String> Doc;
+    Doc.Description = Description;
+    Doc.CommentText = Description;
+    return Doc;
+  }
+
+  /// Constructs with all fields as empty strings/vectors.
+  SymbolDocumentation() = default;
+
+  SymbolDocumentation<llvm::StringRef> toRef() const;
+  SymbolDocumentation<std::string> toOwned() const;
+
+  bool empty() const { return CommentText.empty(); }
+
+  /// Paragraph of the "brief" command.
+  String Brief;
+
+  /// Paragraph of the "return" command.
+  String Returns;
+
+  /// Paragraph(s) of the "note" command(s)
+  llvm::SmallVector<String, 1> Notes;
+  /// Paragraph(s) of the "warning" command(s)
+  llvm::SmallVector<String, 1> Warnings;
+
+  /// Parsed paragaph(s) of the "param" comamnd(s)
+  llvm::SmallVector<ParameterDocumentation<String>> Parameters;
+
+  /// All the paragraphs we don't have any special handling for,
+  /// e.g. "details".
+  String Description;
+
+  /// The full documentation comment with comment markers stripped.
+  /// See clang::RawComment::getFormattedText() for the detailed
+  /// explanation of how the comment text is transformed.
+  String CommentText;
+};
+
+using SymbolDocumentationOwned = SymbolDocumentation<std::string>;
+using SymbolDocumentationRef = SymbolDocumentation<llvm::StringRef>;
+
+/// @param RC the comment to parse
+/// @param D the declaration that \p RC belongs to
+/// @return parsed doxgen documentation.
+SymbolDocumentationOwned
+parseDoxygenComment(const RawComment &RC, const ASTContext &Ctx, const Decl *D);
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_SYMBOLDOCUMENTATION_H

clang-tools-extra/clangd/SymbolDocumentation.cpp

+//===--- SymbolDocumentation.cpp ==-------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "SymbolDocumentation.h"
+#include "clang/AST/CommentVisitor.h"
+#include "llvm/Support/JSON.h"
+
+namespace clang {
+namespace clangd {
+
+void ensureUTF8(std::string &Str) {
+  if (!llvm::json::isUTF8(Str))
+    Str = llvm::json::fixUTF8(Str);
+}
+
+void ensureUTF8(llvm::MutableArrayRef<std::string> Strings) {
+  for (auto &Str : Strings) {
+    ensureUTF8(Str);
+  }
+}
+
+class BlockCommentToString
+    : public comments::ConstCommentVisitor<BlockCommentToString> {
+public:
+  BlockCommentToString(std::string &Out, const ASTContext &Ctx)
+      : Out(Out), Ctx(Ctx) {}
+
+  void visitParagraphComment(const comments::ParagraphComment *C) {
+    for (const auto *Child = C->child_begin(); Child != C->child_end();
+         ++Child) {
+      visit(*Child);
+    }
+  }
+
+  void visitBlockCommandComment(const comments::BlockCommandComment *B) {
+    Out << (B->getCommandMarker() == (comments::CommandMarkerKind::CMK_At)
+                ? '@'
+                : '\\')
+        << B->getCommandName(Ctx.getCommentCommandTraits());
+
+    visit(B->getParagraph());
+  }
+
+  void visitTextComment(const comments::TextComment *C) {
+    // If this is the very first node, the paragraph has no doxygen command,
+    // so there will be a leading space -> Trim it
+    // Otherwise just trim trailing space
+    if (Out.str().empty())
+      Out << C->getText().trim();
+    else
+      Out << C->getText().rtrim();
+  }
+
+  void visitInlineCommandComment(const comments::InlineCommandComment *C) {
+    const std::string SurroundWith = [C] {
+      switch (C->getRenderKind()) {
+      case comments::InlineCommandComment::RenderKind::RenderMonospaced:
+        return "`";
+      case comments::InlineCommandComment::RenderKind::RenderBold:
+        return "**";
+      case comments::InlineCommandComment::RenderKind::RenderEmphasized:
+        return "*";
+      default:
+        return "";
+      }
+    }();
+
+    Out << " " << SurroundWith;
+    for (unsigned I = 0; I < C->getNumArgs(); ++I) {
+      Out << C->getArgText(I);
+    }
+    Out << SurroundWith;
+  }
+
+private:
+  llvm::raw_string_ostream Out;
+  const ASTContext &Ctx;
+};
+
+class CommentToSymbolDocumentation
+    : public comments::ConstCommentVisitor<CommentToSymbolDocumentation> {
+public:
+  CommentToSymbolDocumentation(const RawComment &RC, const ASTContext &Ctx,
+                               const Decl *D, SymbolDocumentationOwned &Doc)
+      : FullComment(RC.parse(Ctx, nullptr, D)), Output(Doc), Ctx(Ctx) {
+
+    Doc.CommentText =
+        RC.getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+
+    for (auto *Block : FullComment->getBlocks()) {
+      visit(Block);
+    }
+  }
+
+  void visitBlockCommandComment(const comments::BlockCommandComment *B) {
+    const llvm::StringRef CommandName =
+        B->getCommandName(Ctx.getCommentCommandTraits());
+
+    // Visit B->getParagraph() for commands that we have special fields for,
+    // so that the command name won't be included in the string.
+    // Otherwise, we want to keep the command name, so visit B itself.
+    if (CommandName == "brief") {
+      BlockCommentToString(Output.Brief, Ctx).visit(B->getParagraph());
+    } else if (CommandName == "return") {
+      BlockCommentToString(Output.Returns, Ctx).visit(B->getParagraph());
+    } else if (CommandName == "warning") {
+      BlockCommentToString(Output.Warnings.emplace_back(), Ctx)
+          .visit(B->getParagraph());
+    } else if (CommandName == "note") {
+      BlockCommentToString(Output.Notes.emplace_back(), Ctx)
+          .visit(B->getParagraph());
+    } else {
+      if (!Output.Description.empty())
+        Output.Description += "\n\n";
+
+      BlockCommentToString(Output.Description, Ctx).visit(B);
+    }
+  }
+
+  void visitParagraphComment(const comments::ParagraphComment *P) {
+    BlockCommentToString(Output.Description, Ctx).visit(P);
+  }
+
+  void visitParamCommandComment(const comments::ParamCommandComment *P) {
+    if (P->hasParamName() && P->hasNonWhitespaceParagraph()) {
+      ParameterDocumentationOwned Doc;
+      Doc.Name = P->getParamNameAsWritten().str();
+      BlockCommentToString(Doc.Description, Ctx).visit(P->getParagraph());
+      Output.Parameters.push_back(std::move(Doc));
+    }
+  }
+
+private:
+  comments::FullComment *FullComment;
+  SymbolDocumentationOwned &Output;
+  const ASTContext &Ctx;
+};
+
+SymbolDocumentationOwned parseDoxygenComment(const RawComment &RC,
+                                             const ASTContext &Ctx,
+                                             const Decl *D) {
+  SymbolDocumentationOwned Doc;
+  CommentToSymbolDocumentation(RC, Ctx, D, Doc);
+
+  // Clang requires source to be UTF-8, but doesn't enforce this in comments.
+  ensureUTF8(Doc.Brief);
+  ensureUTF8(Doc.Returns);
+
+  ensureUTF8(Doc.Notes);
+  ensureUTF8(Doc.Warnings);
+
+  for (auto &Param : Doc.Parameters) {
+    ensureUTF8(Param.Name);
+    ensureUTF8(Param.Description);
+  }
+
+  ensureUTF8(Doc.Description);
+  ensureUTF8(Doc.CommentText);
+
+  return Doc;
+}
+
+template struct ParameterDocumentation<std::string>;
+template struct ParameterDocumentation<llvm::StringRef>;
+
+template <class StrOut, class StrIn>
+SymbolDocumentation<StrOut> convert(const SymbolDocumentation<StrIn> &In) {
+  SymbolDocumentation<StrOut> Doc;
+
+  Doc.Brief = In.Brief;
+  Doc.Returns = In.Returns;
+
+  Doc.Notes.reserve(In.Notes.size());
+  for (const auto &Note : In.Notes) {
+    Doc.Notes.emplace_back(Note);
+  }
+
+  Doc.Warnings.reserve(In.Warnings.size());
+  for (const auto &Warning : In.Warnings) {
+    Doc.Warnings.emplace_back(Warning);
+  }
+
+  Doc.Parameters.reserve(In.Parameters.size());
+  for (const auto &ParamDoc : In.Parameters) {
+    Doc.Parameters.emplace_back(ParameterDocumentation<StrOut>{
+        StrOut(ParamDoc.Name), StrOut(ParamDoc.Description)});
+  }
+
+  Doc.Description = In.Description;
+  Doc.CommentText = In.CommentText;
+
+  return Doc;
+}
+
+template <> SymbolDocumentationRef SymbolDocumentationOwned::toRef() const {
+  return convert<llvm::StringRef>(*this);
+}
+
+template <> SymbolDocumentationOwned SymbolDocumentationRef::toOwned() const {
+  return convert<std::string>(*this);
+}
+
+template class SymbolDocumentation<std::string>;
+template class SymbolDocumentation<llvm::StringRef>;
+
+} // namespace clangd
+} // namespace clang

clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp

 //===-- CodeCompletionStringsTests.cpp --------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "CodeCompletionStrings.h"
+#include "SymbolDocumentationMatchers.h"
 #include "TestTU.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
 namespace {

 TEST_F(CompletionStringTest, GetDeclCommentBadUTF8) {
   // <ff> is not a valid byte here, should be replaced by encoded <U+FFFD>.
   auto TU = TestTU::withCode("/*x\xffy*/ struct X;");
   auto AST = TU.build();
   EXPECT_EQ("x\xef\xbf\xbdy",
             getDeclComment(AST.getASTContext(), findDecl(AST, "X")));
 }
 
+TEST_F(CompletionStringTest, DoxygenParsing) {
+  struct {
+    const char *const Code;
+    const std::function<void(SymbolDocumentationOwned &)> ExpectedBuilder;
+  } Cases[] = {
+      {R"cpp(
+    // Hello world
+    void foo();
+    )cpp",
+       [](SymbolDocumentationOwned &Doc) { Doc.Description = "Hello world"; }},
+      {R"cpp(
+    /*! 
+     * \brief brief
+     * \details details
+     */
+    void foo();
+    )cpp",
+       [](SymbolDocumentationOwned &Doc) {
+         Doc.Brief = "brief";
+         Doc.Description = "\\details details";
+       }},
+      {R"cpp(
+    /** 
+     * @brief brief
+     * @details details
+     * @see somewhere else
+     */
+    void foo();
+    )cpp",
+       [](SymbolDocumentationOwned &Doc) {
+         Doc.Brief = "brief";
+         Doc.Description = "@details details\n\n@see somewhere else";
+       }},
+      {R"cpp(
+    /*! 
+     * @brief brief
+     * @details details
+     * @param foo foodoc
+     * @throws ball at hoop
+     * @note note1
+     * @warning warning1
+     * @note note2
+     * @warning warning2
+     * @param bar bardoc
+     * @return something
+     */
+    void foo();
+    )cpp",
+       [](SymbolDocumentationOwned &Doc) {
+         Doc.Brief = "brief";
+         Doc.Description = "@details details\n\n@throws ball at hoop";
+         Doc.Parameters = {{"foo", "foodoc"}, {"bar", "bardoc"}};
+         Doc.Warnings = {"warning1", "warning2"};
+         Doc.Notes = {"note1", "note2"};
+         Doc.Returns = "something";
+       }},
+      {R"cpp(
+    /// @brief Here's \b bold \e italic and \p code
+    int foo;
+    )cpp",
+       [](SymbolDocumentationOwned &Doc) {
+         Doc.Brief = "Here's **bold** *italic* and `code`";
+       }}};
+
+  for (const auto &Case : Cases) {
+    SCOPED_TRACE(Case.Code);
+
+    auto TU = TestTU::withCode(Case.Code);
+    auto AST = TU.build();
+    auto &Ctx = AST.getASTContext();
+    const auto &Decl = findDecl(AST, "foo");
+
+    SymbolDocumentationOwned ExpectedDoc;
+    ExpectedDoc.CommentText =
+        getCompletionComment(Ctx, &Decl)
+            ->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+    Case.ExpectedBuilder(ExpectedDoc);
+
+    const RawComment *RC = getCompletionComment(Ctx, &Decl);
+    EXPECT_THAT(RC, testing::NotNull());
+    EXPECT_THAT(parseDoxygenComment(*RC, Ctx, &Decl), matchesDoc(ExpectedDoc));
+  }
+}
+
 TEST_F(CompletionStringTest, MultipleAnnotations) {
   Builder.AddAnnotation("Ano1");
   Builder.AddAnnotation("Ano2");
   Builder.AddAnnotation("Ano3");
 
   EXPECT_EQ(formatDocumentation(*Builder.TakeString(), ""),
             "Annotations: Ano1 Ano2 Ano3\n");
 }

clang-tools-extra/clangd/unittests/SymbolDocumentationMatchers.h

+//===-- SymbolDocumentationMatchers.h ---------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// GMock matchers for the SymbolDocumentation class
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SYMBOLDOCUMENTATION_MATCHERS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SYMBOLDOCUMENTATION_MATCHERS_H
+#include "SymbolDocumentation.h"
+#include "gmock/gmock.h"
+
+namespace clang {
+namespace clangd {
+
+template <class S>
+testing::Matcher<SymbolDocumentation<S>>
+matchesDoc(const SymbolDocumentation<S> &Expected) {
+  using namespace ::testing;
+
+  std::vector<Matcher<ParameterDocumentation<S>>> ParamMatchers;
+  for (const auto &P : Expected.Parameters)
+    ParamMatchers.push_back(
+        AllOf(Field("Name", &ParameterDocumentation<S>::Name, P.Name),
+              Field("Description", &ParameterDocumentation<S>::Description,
+                    P.Description)));
+
+  return AllOf(
+      Field("Brief", &SymbolDocumentation<S>::Brief, Expected.Brief),
+      Field("Returns", &SymbolDocumentation<S>::Returns, Expected.Returns),
+      Field("Notes", &SymbolDocumentation<S>::Notes,
+            ElementsAreArray(Expected.Notes)),
+      Field("Warnings", &SymbolDocumentation<S>::Warnings,
+            ElementsAreArray(Expected.Warnings)),
+      Field("Parameters", &SymbolDocumentation<S>::Parameters,
+            ElementsAreArray(ParamMatchers)),
+      Field("Description", &SymbolDocumentation<S>::Description,
+            Expected.Description),
+      Field("CommentText", &SymbolDocumentation<S>::CommentText,
+            Expected.CommentText));
+}
+
+} // namespace clangd
+} // namespace clang
+
+#endif

llvm/utils/gn/secondary/clang-tools-extra/clangd/BUILD.gn

sources = [
     "Protocol.cpp",
     "Quality.cpp",
     "QueryDriverDatabase.cpp",
     "RIFF.cpp",
     "Selection.cpp",
     "SemanticHighlighting.cpp",
     "SemanticSelection.cpp",
     "SourceCode.cpp",
+    "SymbolDocumentation.cpp"
     "TUScheduler.cpp",
     "TidyProvider.cpp",
     "URI.cpp",
     "XRefs.cpp",
     "index/Background.cpp",
     "index/BackgroundIndexLoader.cpp",
     "index/BackgroundIndexStorage.cpp",
     "index/BackgroundQueue.cpp",