From 5e5b87c6985c12b8afeea077a723c969e2153f49 Mon Sep 17 00:00:00 2001 From: Tim Cottin Date: Mon, 1 Sep 2025 17:54:53 +0000 Subject: [PATCH] fix note, warning and retval command. Change arguments of doxygen commands to be rendered as monospaced --- clang-tools-extra/clangd/Hover.cpp | 6 +- .../clangd/SymbolDocumentation.cpp | 111 ++++++++------- .../clangd/SymbolDocumentation.h | 33 +---- .../clangd/unittests/HoverTests.cpp | 43 +++++- .../unittests/SymbolDocumentationTests.cpp | 126 ++++++++++++++++-- 5 files changed, 227 insertions(+), 92 deletions(-) diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp index 9eec322fe5963..3b9baed1c2838 100644 --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -1589,13 +1589,11 @@ markup::Document HoverInfo::presentDoxygen() const { P.appendText(" - "); SymbolDoc.returnToMarkup(P); } + + SymbolDoc.retvalsToMarkup(Output); Output.addRuler(); } - // add specially handled doxygen commands. - SymbolDoc.warningsToMarkup(Output); - SymbolDoc.notesToMarkup(Output); - // add any other documentation. SymbolDoc.docToMarkup(Output); diff --git a/clang-tools-extra/clangd/SymbolDocumentation.cpp b/clang-tools-extra/clangd/SymbolDocumentation.cpp index 9ae1ef3f828e0..86bd6162b6f72 100644 --- a/clang-tools-extra/clangd/SymbolDocumentation.cpp +++ b/clang-tools-extra/clangd/SymbolDocumentation.cpp @@ -34,10 +34,20 @@ void commandToMarkup(markup::Paragraph &Out, StringRef Command, StringRef Args) { Out.appendBoldText(commandMarkerAsString(CommandMarker) + Command.str()); Out.appendSpace(); - if (!Args.empty()) { - Out.appendEmphasizedText(Args.str()); + if (!Args.empty()) + Out.appendCode(Args.str()); +} + +template std::string getArgText(const T *Command) { + std::string ArgText; + for (unsigned I = 0; I < Command->getNumArgs(); ++I) { + if (!ArgText.empty()) + ArgText += " "; + ArgText += Command->getArgText(I); } + return ArgText; } + } // namespace class ParagraphToMarkupDocument @@ -70,12 +80,7 @@ class ParagraphToMarkupDocument void visitInlineCommandComment(const comments::InlineCommandComment *C) { if (C->getNumArgs() > 0) { - std::string ArgText; - for (unsigned I = 0; I < C->getNumArgs(); ++I) { - if (!ArgText.empty()) - ArgText += " "; - ArgText += C->getArgText(I); - } + std::string ArgText = getArgText(C); switch (C->getRenderKind()) { case comments::InlineCommandRenderKind::Monospaced: @@ -158,10 +163,9 @@ class ParagraphToString void visitInlineCommandComment(const comments::InlineCommandComment *C) { Out << commandMarkerAsString(C->getCommandMarker()); Out << C->getCommandName(Traits); - if (C->getNumArgs() > 0) { - for (unsigned I = 0; I < C->getNumArgs(); ++I) - Out << " " << C->getArgText(I); - } + std::string ArgText = getArgText(C); + if (!ArgText.empty()) + Out << " " << ArgText; Out << " "; } @@ -210,16 +214,27 @@ class BlockCommentToMarkupDocument Traits) .visit(B->getParagraph()); break; + case comments::CommandTraits::KCI_note: + case comments::CommandTraits::KCI_warning: + commandToHeadedParagraph(B); + break; + case comments::CommandTraits::KCI_retval: { + // The \retval command describes the return value given as its single + // argument in the corresponding paragraph. + // Note: We know that we have exactly one argument but not if it has an + // associated paragraph. + auto &P = Out.addParagraph().appendCode(getArgText(B)); + if (B->getParagraph() && !B->getParagraph()->isWhitespace()) { + P.appendText(" - "); + ParagraphToMarkupDocument(P, Traits).visit(B->getParagraph()); + } + return; + } default: { // Some commands have arguments, like \throws. // The arguments are not part of the paragraph. // We need reconstruct them here. - std::string ArgText; - for (unsigned I = 0; I < B->getNumArgs(); ++I) { - if (!ArgText.empty()) - ArgText += " "; - ArgText += B->getArgText(I); - } + std::string ArgText = getArgText(B); auto &P = Out.addParagraph(); commandToMarkup(P, B->getCommandName(Traits), B->getCommandMarker(), ArgText); @@ -262,6 +277,19 @@ class BlockCommentToMarkupDocument markup::Document &Out; const comments::CommandTraits &Traits; StringRef CommentEscapeMarker; + + /// Emphasize the given command in a paragraph. + /// Uses the command name with the first letter capitalized as the heading. + void commandToHeadedParagraph(const comments::BlockCommandComment *B) { + Out.addRuler(); + auto &P = Out.addParagraph(); + std::string Heading = B->getCommandName(Traits).slice(0, 1).upper() + + B->getCommandName(Traits).drop_front().str(); + P.appendBoldText(Heading + ":"); + P.appendText(" \n"); + ParagraphToMarkupDocument(P, Traits).visit(B->getParagraph()); + Out.addRuler(); + } }; void SymbolDocCommentVisitor::visitBlockCommandComment( @@ -282,36 +310,22 @@ void SymbolDocCommentVisitor::visitBlockCommandComment( } break; case comments::CommandTraits::KCI_retval: - RetvalParagraphs.push_back(B->getParagraph()); - return; - case comments::CommandTraits::KCI_warning: - WarningParagraphs.push_back(B->getParagraph()); - return; - case comments::CommandTraits::KCI_note: - NoteParagraphs.push_back(B->getParagraph()); + // Only consider retval commands having an argument. + // The argument contains the described return value which is needed to + // convert it to markup. + if (B->getNumArgs() == 1) + RetvalCommands.push_back(B); return; default: break; } - // For all other commands, we store them in the UnhandledCommands map. + // For all other commands, we store them in the BlockCommands map. // This allows us to keep the order of the comments. - UnhandledCommands[CommentPartIndex] = B; + BlockCommands[CommentPartIndex] = B; CommentPartIndex++; } -void SymbolDocCommentVisitor::paragraphsToMarkup( - markup::Document &Out, - const llvm::SmallVectorImpl &Paragraphs) - const { - if (Paragraphs.empty()) - return; - - for (const auto *P : Paragraphs) { - ParagraphToMarkupDocument(Out.addParagraph(), Traits).visit(P); - } -} - void SymbolDocCommentVisitor::briefToMarkup(markup::Paragraph &Out) const { if (!BriefParagraph) return; @@ -324,14 +338,6 @@ void SymbolDocCommentVisitor::returnToMarkup(markup::Paragraph &Out) const { ParagraphToMarkupDocument(Out, Traits).visit(ReturnParagraph); } -void SymbolDocCommentVisitor::notesToMarkup(markup::Document &Out) const { - paragraphsToMarkup(Out, NoteParagraphs); -} - -void SymbolDocCommentVisitor::warningsToMarkup(markup::Document &Out) const { - paragraphsToMarkup(Out, WarningParagraphs); -} - void SymbolDocCommentVisitor::parameterDocToMarkup( StringRef ParamName, markup::Paragraph &Out) const { if (ParamName.empty()) @@ -354,7 +360,7 @@ void SymbolDocCommentVisitor::parameterDocToString( void SymbolDocCommentVisitor::docToMarkup(markup::Document &Out) const { for (unsigned I = 0; I < CommentPartIndex; ++I) { - if (const auto *BC = UnhandledCommands.lookup(I)) { + if (const auto *BC = BlockCommands.lookup(I)) { BlockCommentToMarkupDocument(Out, Traits).visit(BC); } else if (const auto *P = FreeParagraphs.lookup(I)) { ParagraphToMarkupDocument(Out.addParagraph(), Traits).visit(P); @@ -382,5 +388,14 @@ void SymbolDocCommentVisitor::templateTypeParmDocToString( } } +void SymbolDocCommentVisitor::retvalsToMarkup(markup::Document &Out) const { + if (RetvalCommands.empty()) + return; + markup::BulletList &BL = Out.addBulletList(); + for (const auto *P : RetvalCommands) { + BlockCommentToMarkupDocument(BL.addItem(), Traits).visit(P); + } +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/SymbolDocumentation.h b/clang-tools-extra/clangd/SymbolDocumentation.h index b0d3428dfce20..1d48ee6d4d3f9 100644 --- a/clang-tools-extra/clangd/SymbolDocumentation.h +++ b/clang-tools-extra/clangd/SymbolDocumentation.h @@ -114,22 +114,14 @@ class SymbolDocCommentVisitor bool hasReturnCommand() const { return ReturnParagraph; } - bool hasRetvalCommands() const { return !RetvalParagraphs.empty(); } - - bool hasNoteCommands() const { return !NoteParagraphs.empty(); } - - bool hasWarningCommands() const { return !WarningParagraphs.empty(); } - /// Converts all unhandled comment commands to a markup document. void docToMarkup(markup::Document &Out) const; /// Converts the "brief" command(s) to a markup document. void briefToMarkup(markup::Paragraph &Out) const; /// Converts the "return" command(s) to a markup document. void returnToMarkup(markup::Paragraph &Out) const; - /// Converts the "note" command(s) to a markup document. - void notesToMarkup(markup::Document &Out) const; - /// Converts the "warning" command(s) to a markup document. - void warningsToMarkup(markup::Document &Out) const; + /// Converts the "retval" command(s) to a markup document. + void retvalsToMarkup(markup::Document &Out) const; void visitBlockCommandComment(const comments::BlockCommandComment *B); @@ -173,19 +165,13 @@ class SymbolDocCommentVisitor /// Paragraph of the "return" command. const comments::ParagraphComment *ReturnParagraph = nullptr; - /// Paragraph(s) of the "note" command(s) - llvm::SmallVector RetvalParagraphs; + /// All the "retval" command(s) + llvm::SmallVector RetvalCommands; - /// Paragraph(s) of the "note" command(s) - llvm::SmallVector NoteParagraphs; - - /// Paragraph(s) of the "warning" command(s) - llvm::SmallVector WarningParagraphs; - - /// All the paragraphs we don't have any special handling for, - /// e.g. "details". + /// All the parsed doxygen block commands. + /// They might have special handling internally like \\note or \\warning llvm::SmallDenseMap - UnhandledCommands; + BlockCommands; /// Parsed paragaph(s) of the "param" comamnd(s) llvm::SmallDenseMap @@ -198,11 +184,6 @@ class SymbolDocCommentVisitor /// All "free" text paragraphs. llvm::SmallDenseMap FreeParagraphs; - - void paragraphsToMarkup( - markup::Document &Out, - const llvm::SmallVectorImpl - &Paragraphs) const; }; } // namespace clangd diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp index 743c0dc0d0187..fbcc2992cf2b7 100644 --- a/clang-tools-extra/clangd/unittests/HoverTests.cpp +++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp @@ -4045,9 +4045,19 @@ brief doc longer doc)"}, {[](HoverInfo &HI) { HI.Kind = index::SymbolKind::Function; - HI.Documentation = "@brief brief doc\n\n" - "longer doc\n@param a this is a param\n@return it " - "returns something"; + HI.Documentation = R"(@brief brief doc + +longer doc +@note this is a note + +As you see, notes are "inlined". +@warning this is a warning + +As well as warnings +@param a this is a param +@return it returns something +@retval 0 if successful +@retval 1 if failed)"; HI.Definition = "int foo(int a)"; HI.ReturnType = "int"; HI.Name = "foo"; @@ -4068,8 +4078,16 @@ longer doc)"}, @brief brief doc longer doc +@note this is a note + +As you see, notes are "inlined". +@warning this is a warning + +As well as warnings @param a this is a param @return it returns something +@retval 0 if successful +@retval 1 if failed --- ```cpp @@ -4095,8 +4113,25 @@ brief doc `int` - it returns something +- `0` - if successful +- `1` - if failed + --- -longer doc)"}, +longer doc + +--- +**Note:** +this is a note + +--- +As you see, notes are "inlined". + +--- +**Warning:** +this is a warning + +--- +As well as warnings)"}, {[](HoverInfo &HI) { HI.Kind = index::SymbolKind::Function; HI.Documentation = "@brief brief doc\n\n" diff --git a/clang-tools-extra/clangd/unittests/SymbolDocumentationTests.cpp b/clang-tools-extra/clangd/unittests/SymbolDocumentationTests.cpp index 31a6a149e33c4..ba692399a7093 100644 --- a/clang-tools-extra/clangd/unittests/SymbolDocumentationTests.cpp +++ b/clang-tools-extra/clangd/unittests/SymbolDocumentationTests.cpp @@ -15,7 +15,7 @@ namespace clang { namespace clangd { -TEST(SymbolDocumentation, UnhandledDocs) { +TEST(SymbolDocumentation, DocToMarkup) { CommentOptions CommentOpts; @@ -63,15 +63,15 @@ TEST(SymbolDocumentation, UnhandledDocs) { }, { "foo \\ref bar baz", - "foo \\*\\*\\\\ref\\*\\* \\*bar\\* baz", - "foo **\\ref** *bar* baz", - "foo **\\ref** *bar* baz", + "foo \\*\\*\\\\ref\\*\\* `bar` baz", + "foo **\\ref** `bar` baz", + "foo **\\ref** bar baz", }, { "foo @ref bar baz", - "foo \\*\\*@ref\\*\\* \\*bar\\* baz", - "foo **@ref** *bar* baz", - "foo **@ref** *bar* baz", + "foo \\*\\*@ref\\*\\* `bar` baz", + "foo **@ref** `bar` baz", + "foo **@ref** bar baz", }, { "\\brief this is a \\n\nbrief description", @@ -81,9 +81,9 @@ TEST(SymbolDocumentation, UnhandledDocs) { }, { "\\throw exception foo", - "\\*\\*\\\\throw\\*\\* \\*exception\\* foo", - "**\\throw** *exception* foo", - "**\\throw** *exception* foo", + "\\*\\*\\\\throw\\*\\* `exception` foo", + "**\\throw** `exception` foo", + "**\\throw** exception foo", }, { R"(\brief this is a brief description @@ -198,6 +198,72 @@ normal text\this is an italic text\ "this is a bold text normal textthis is an italic text " "this is a code block", }, + {"@note This is a note", + R"(\*\*Note:\*\* +This is a note)", + R"(**Note:** +This is a note)", + R"(**Note:** +This is a note)"}, + {R"(Paragraph 1 +@note This is a note + +Paragraph 2)", + R"(Paragraph 1 + +--- +\*\*Note:\*\* +This is a note + +--- +Paragraph 2)", + R"(Paragraph 1 + +--- +**Note:** +This is a note + +--- +Paragraph 2)", + R"(Paragraph 1 + +**Note:** +This is a note + +Paragraph 2)"}, + {"@warning This is a warning", + R"(\*\*Warning:\*\* +This is a warning)", + R"(**Warning:** +This is a warning)", + R"(**Warning:** +This is a warning)"}, + {R"(Paragraph 1 +@warning This is a warning + +Paragraph 2)", + R"(Paragraph 1 + +--- +\*\*Warning:\*\* +This is a warning + +--- +Paragraph 2)", + R"(Paragraph 1 + +--- +**Warning:** +This is a warning + +--- +Paragraph 2)", + R"(Paragraph 1 + +**Warning:** +This is a warning + +Paragraph 2)"}, }; for (const auto &C : Cases) { markup::Document Doc; @@ -211,5 +277,45 @@ normal text\this is an italic text\ } } +TEST(SymbolDocumentation, RetvalCommand) { + + CommentOptions CommentOpts; + + struct Case { + llvm::StringRef Documentation; + llvm::StringRef ExpectedRenderEscapedMarkdown; + llvm::StringRef ExpectedRenderMarkdown; + llvm::StringRef ExpectedRenderPlainText; + } Cases[] = { + {"@retval", "", "", ""}, + {R"(@retval MyReturnVal +@retval MyOtherReturnVal)", + R"(- `MyReturnVal` +- `MyOtherReturnVal`)", + R"(- `MyReturnVal` +- `MyOtherReturnVal`)", + R"(- MyReturnVal +- MyOtherReturnVal)"}, + {R"(@retval MyReturnVal if foo +@retval MyOtherReturnVal if bar)", + R"(- `MyReturnVal` - if foo +- `MyOtherReturnVal` - if bar)", + R"(- `MyReturnVal` - if foo +- `MyOtherReturnVal` - if bar)", + R"(- MyReturnVal - if foo +- MyOtherReturnVal - if bar)"}, + }; + for (const auto &C : Cases) { + markup::Document Doc; + SymbolDocCommentVisitor SymbolDoc(C.Documentation, CommentOpts); + + SymbolDoc.retvalsToMarkup(Doc); + + EXPECT_EQ(Doc.asPlainText(), C.ExpectedRenderPlainText); + EXPECT_EQ(Doc.asMarkdown(), C.ExpectedRenderMarkdown); + EXPECT_EQ(Doc.asEscapedMarkdown(), C.ExpectedRenderEscapedMarkdown); + } +} + } // namespace clangd } // namespace clang