llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangd Author: None (tcottin) <details> <summary>Changes</summary> This is the final patch to solve clangd/clangd#<!-- -->529. This uses the patches from [llvm/llvm-project#<!-- -->140498](https://github.com/llvm/llvm-project/pull/140498) and [llvm/llvm-project#<!-- -->150790](https://github.com/llvm/llvm-project/pull/150790) to improve the hover content for doxygen documented code. The patch uses the information from the parsed doxygen comment to extend and rearrange the hover documentation: - own section for brief, notes and warning documentation - for functions add parameter/template parameter documentation - for functions add return documentation Note: this requires to configure ```yaml Documentation: CommentFormat: Doxygen ``` in your `.clangd` configuration. --- Patch is 34.76 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/152918.diff 7 Files Affected: - (modified) clang-tools-extra/clangd/CodeCompletionStrings.cpp (+6-2) - (modified) clang-tools-extra/clangd/Hover.cpp (+78-22) - (modified) clang-tools-extra/clangd/Hover.h (+3) - (modified) clang-tools-extra/clangd/SymbolDocumentation.cpp (+96-7) - (modified) clang-tools-extra/clangd/SymbolDocumentation.h (+66-10) - (modified) clang-tools-extra/clangd/unittests/HoverTests.cpp (+340-37) - (modified) clang-tools-extra/clangd/unittests/SymbolDocumentationTests.cpp (+94-40) ``````````diff diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp b/clang-tools-extra/clangd/CodeCompletionStrings.cpp index d6579640cb0fb..02c3ea8abd8af 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp +++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp @@ -112,7 +112,7 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &Decl) { std::string Doc; if (Cfg.Documentation.CommentFormat == Config::CommentFormatPolicy::Doxygen && - isa<ParmVarDecl>(Decl)) { + (isa<ParmVarDecl>(Decl) || isa<TemplateTypeParmDecl>(Decl))) { // Parameters are documented in their declaration context (function or // template function). const NamedDecl *ND = dyn_cast<NamedDecl>(Decl.getDeclContext()); @@ -135,7 +135,11 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &Decl) { std::string RawDoc; llvm::raw_string_ostream OS(RawDoc); - V.parameterDocToString(dyn_cast<ParmVarDecl>(&Decl)->getName(), OS); + if (isa<ParmVarDecl>(Decl)) + V.parameterDocToString(dyn_cast<ParmVarDecl>(&Decl)->getName(), OS); + else + V.templateTypeParmDocToString( + dyn_cast<TemplateTypeParmDecl>(&Decl)->getName(), OS); Doc = StringRef(RawDoc).trim().str(); } else { diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp index 0afa90285db52..45e47233c9e1f 100644 --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -1276,6 +1276,7 @@ std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos, HI.Definition = URIForFile::canonicalize(Inc.Resolved, AST.tuPath()).file().str(); HI.DefinitionLanguage = ""; + HI.IsIncludeDirective = true; maybeAddUsedSymbols(AST, HI, Inc); return HI; } @@ -1504,7 +1505,26 @@ markup::Document HoverInfo::presentDoxygen() const { Header.appendText(index::getSymbolKindString(Kind)).appendSpace(); assert(!Name.empty() && "hover triggered on a nameless symbol"); - Header.appendCode(Name); + if (IsIncludeDirective) { + Header.appendCode(Name); + + if (!Definition.empty()) + Output.addParagraph().appendCode(Definition); + + if (!UsedSymbolNames.empty()) { + Output.addRuler(); + usedSymbolNamesToMarkup(Output); + } + + return Output; + } + + if (!Definition.empty()) { + Output.addRuler(); + definitionScopeToMarkup(Output); + } else { + Header.appendCode(Name); + } if (!Provider.empty()) { providerToMarkupParagraph(Output); @@ -1512,33 +1532,76 @@ markup::Document HoverInfo::presentDoxygen() const { // Put a linebreak after header to increase readability. Output.addRuler(); - // Print Types on their own lines to reduce chances of getting line-wrapped by - // editor, as they might be long. - if (ReturnType) { - // For functions we display signature in a list form, e.g.: - // → `x` - // Parameters: - // - `bool param1` - // - `int param2 = 5` - Output.addParagraph().appendText("→ ").appendCode( - llvm::to_string(*ReturnType)); - } SymbolDocCommentVisitor SymbolDoc(Documentation, CommentOpts); + if (SymbolDoc.hasBriefCommand()) { + SymbolDoc.briefToMarkup(Output.addParagraph()); + Output.addRuler(); + } + + // For functions we display signature in a list form, e.g.: + // Template Parameters: + // - `typename T` - description + // Parameters: + // - `bool param1` - description + // - `int param2 = 5` - description + // Returns + // `type` - description + if (TemplateParameters && !TemplateParameters->empty()) { + Output.addParagraph().appendBoldText("Template Parameters:"); + markup::BulletList &L = Output.addBulletList(); + for (const auto &Param : *TemplateParameters) { + markup::Paragraph &P = L.addItem().addParagraph(); + P.appendCode(llvm::to_string(Param)); + if (SymbolDoc.isTemplateTypeParmDocumented(llvm::to_string(Param.Name))) { + P.appendText(" - "); + SymbolDoc.templateTypeParmDocToMarkup(llvm::to_string(Param.Name), P); + } + } + Output.addRuler(); + } + if (Parameters && !Parameters->empty()) { - Output.addParagraph().appendText("Parameters:"); + Output.addParagraph().appendBoldText("Parameters:"); markup::BulletList &L = Output.addBulletList(); for (const auto &Param : *Parameters) { markup::Paragraph &P = L.addItem().addParagraph(); P.appendCode(llvm::to_string(Param)); if (SymbolDoc.isParameterDocumented(llvm::to_string(Param.Name))) { - P.appendText(" -"); + P.appendText(" - "); SymbolDoc.parameterDocToMarkup(llvm::to_string(Param.Name), P); } } + Output.addRuler(); + } + + // Print Types on their own lines to reduce chances of getting line-wrapped by + // editor, as they might be long. + if (ReturnType && + ((ReturnType->Type != "void" && !ReturnType->AKA.has_value()) || + (ReturnType->AKA.has_value() && ReturnType->AKA != "void"))) { + Output.addParagraph().appendBoldText("Returns:"); + markup::Paragraph &P = Output.addParagraph(); + P.appendCode(llvm::to_string(*ReturnType)); + + if (SymbolDoc.hasReturnCommand()) { + P.appendText(" - "); + SymbolDoc.returnToMarkup(P); + } + Output.addRuler(); } + + // add specially handled doxygen commands. + SymbolDoc.warningsToMarkup(Output); + SymbolDoc.notesToMarkup(Output); + + // add any other documentation. + SymbolDoc.docToMarkup(Output); + + Output.addRuler(); + // Don't print Type after Parameters or ReturnType as this will just duplicate // the information if (Type && !ReturnType && !Parameters) @@ -1559,13 +1622,6 @@ markup::Document HoverInfo::presentDoxygen() const { calleeArgInfoToMarkupParagraph(Output.addParagraph()); } - SymbolDoc.docToMarkup(Output); - - if (!Definition.empty()) { - Output.addRuler(); - definitionScopeToMarkup(Output); - } - if (!UsedSymbolNames.empty()) { Output.addRuler(); usedSymbolNamesToMarkup(Output); @@ -1613,7 +1669,7 @@ markup::Document HoverInfo::presentDefault() const { } if (Parameters && !Parameters->empty()) { - Output.addParagraph().appendText("Parameters: "); + Output.addParagraph().appendText("Parameters:"); markup::BulletList &L = Output.addBulletList(); for (const auto &Param : *Parameters) L.addItem().addParagraph().appendCode(llvm::to_string(Param)); diff --git a/clang-tools-extra/clangd/Hover.h b/clang-tools-extra/clangd/Hover.h index 614180a7b9846..2eb3dd99ab3fc 100644 --- a/clang-tools-extra/clangd/Hover.h +++ b/clang-tools-extra/clangd/Hover.h @@ -120,6 +120,9 @@ struct HoverInfo { // alphabetical order. std::vector<std::string> UsedSymbolNames; + /// True if this symbol is a include directive. + bool IsIncludeDirective = false; + /// Produce a user-readable information based on the specified markup kind. std::string present(MarkupKind Kind) const; diff --git a/clang-tools-extra/clangd/SymbolDocumentation.cpp b/clang-tools-extra/clangd/SymbolDocumentation.cpp index dea637b9100da..9152355cd0f79 100644 --- a/clang-tools-extra/clangd/SymbolDocumentation.cpp +++ b/clang-tools-extra/clangd/SymbolDocumentation.cpp @@ -33,8 +33,8 @@ void commandToMarkup(markup::Paragraph &Out, StringRef Command, comments::CommandMarkerKind CommandMarker, StringRef Args) { Out.appendBoldText(commandMarkerAsString(CommandMarker) + Command.str()); + Out.appendSpace(); if (!Args.empty()) { - Out.appendSpace(); Out.appendEmphasizedText(Args.str()); } } @@ -132,7 +132,8 @@ class ParagraphToMarkupDocument const comments::CommandTraits &Traits; /// If true, the next leading space after a new line is trimmed. - bool LastChunkEndsWithNewline = false; + /// Initially set it to true, to always trim the first text line. + bool LastChunkEndsWithNewline = true; }; class ParagraphToString @@ -263,8 +264,76 @@ class BlockCommentToMarkupDocument StringRef CommentEscapeMarker; }; -void SymbolDocCommentVisitor::parameterDocToMarkup(StringRef ParamName, - markup::Paragraph &Out) { +void SymbolDocCommentVisitor::visitBlockCommandComment( + const comments::BlockCommandComment *B) { + switch (B->getCommandID()) { + case comments::CommandTraits::KCI_brief: { + if (!BriefParagraph) { + BriefParagraph = B->getParagraph(); + return; + } + break; + } + case comments::CommandTraits::KCI_return: + case comments::CommandTraits::KCI_returns: + if (!ReturnParagraph) { + ReturnParagraph = B->getParagraph(); + return; + } + 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()); + return; + default: + break; + } + + // For all other commands, we store them in the UnhandledCommands map. + // This allows us to keep the order of the comments. + UnhandledCommands[CommentPartIndex] = B; + CommentPartIndex++; +} + +void SymbolDocCommentVisitor::paragraphsToMarkup( + markup::Document &Out, + const llvm::SmallVector<const comments::ParagraphComment *> &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; + ParagraphToMarkupDocument(Out, Traits).visit(BriefParagraph); +} + +void SymbolDocCommentVisitor::returnToMarkup(markup::Paragraph &Out) const { + if (!ReturnParagraph) + return; + 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()) return; @@ -274,7 +343,7 @@ void SymbolDocCommentVisitor::parameterDocToMarkup(StringRef ParamName, } void SymbolDocCommentVisitor::parameterDocToString( - StringRef ParamName, llvm::raw_string_ostream &Out) { + StringRef ParamName, llvm::raw_string_ostream &Out) const { if (ParamName.empty()) return; @@ -283,9 +352,9 @@ void SymbolDocCommentVisitor::parameterDocToString( } } -void SymbolDocCommentVisitor::docToMarkup(markup::Document &Out) { +void SymbolDocCommentVisitor::docToMarkup(markup::Document &Out) const { for (unsigned I = 0; I < CommentPartIndex; ++I) { - if (const auto *BC = BlockCommands.lookup(I)) { + if (const auto *BC = UnhandledCommands.lookup(I)) { BlockCommentToMarkupDocument(Out, Traits).visit(BC); } else if (const auto *P = FreeParagraphs.lookup(I)) { ParagraphToMarkupDocument(Out.addParagraph(), Traits).visit(P); @@ -293,5 +362,25 @@ void SymbolDocCommentVisitor::docToMarkup(markup::Document &Out) { } } +void SymbolDocCommentVisitor::templateTypeParmDocToMarkup( + StringRef TemplateParamName, markup::Paragraph &Out) const { + if (TemplateParamName.empty()) + return; + + if (const auto *TP = TemplateParameters.lookup(TemplateParamName)) { + ParagraphToMarkupDocument(Out, Traits).visit(TP->getParagraph()); + } +} + +void SymbolDocCommentVisitor::templateTypeParmDocToString( + StringRef TemplateParamName, llvm::raw_string_ostream &Out) const { + if (TemplateParamName.empty()) + return; + + if (const auto *P = TemplateParameters.lookup(TemplateParamName)) { + ParagraphToString(Out, Traits).visit(P->getParagraph()); + } +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/SymbolDocumentation.h b/clang-tools-extra/clangd/SymbolDocumentation.h index b5120ba04e8f1..9d173745965e2 100644 --- a/clang-tools-extra/clangd/SymbolDocumentation.h +++ b/clang-tools-extra/clangd/SymbolDocumentation.h @@ -106,16 +106,43 @@ class SymbolDocCommentVisitor return Parameters.contains(ParamName); } - void parameterDocToMarkup(StringRef ParamName, markup::Paragraph &Out); + bool isTemplateTypeParmDocumented(StringRef ParamName) const { + return TemplateParameters.contains(ParamName); + } - void parameterDocToString(StringRef ParamName, llvm::raw_string_ostream &Out); + bool hasBriefCommand() const { return BriefParagraph; } - void docToMarkup(markup::Document &Out); + bool hasReturnCommand() const { return ReturnParagraph; } - void visitBlockCommandComment(const comments::BlockCommandComment *B) { - BlockCommands[CommentPartIndex] = B; - CommentPartIndex++; - } + 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; + + void visitBlockCommandComment(const comments::BlockCommandComment *B); + + void templateTypeParmDocToMarkup(StringRef TemplateParamName, + markup::Paragraph &Out) const; + + void templateTypeParmDocToString(StringRef TemplateParamName, + llvm::raw_string_ostream &Out) const; + + void parameterDocToMarkup(StringRef ParamName, markup::Paragraph &Out) const; + + void parameterDocToString(StringRef ParamName, + llvm::raw_string_ostream &Out) const; void visitParagraphComment(const comments::ParagraphComment *P) { FreeParagraphs[CommentPartIndex] = P; @@ -126,6 +153,10 @@ class SymbolDocCommentVisitor Parameters[P->getParamNameAsWritten()] = P; } + void visitTParamCommandComment(const comments::TParamCommandComment *TP) { + TemplateParameters[TP->getParamNameAsWritten()] = std::move(TP); + } + private: comments::CommandTraits Traits; llvm::BumpPtrAllocator Allocator; @@ -136,17 +167,42 @@ class SymbolDocCommentVisitor /// This index allows us to keep the order of the other comment parts. unsigned CommentPartIndex = 0; + /// Paragraph of the "brief" command. + const comments::ParagraphComment *BriefParagraph = nullptr; + + /// Paragraph of the "return" command. + const comments::ParagraphComment *ReturnParagraph = nullptr; + + /// Paragraph(s) of the "note" command(s) + llvm::SmallVector<const comments::ParagraphComment *> RetvalParagraphs; + + /// Paragraph(s) of the "note" command(s) + llvm::SmallVector<const comments::ParagraphComment *> NoteParagraphs; + + /// Paragraph(s) of the "warning" command(s) + llvm::SmallVector<const comments::ParagraphComment *> WarningParagraphs; + + /// All the paragraphs we don't have any special handling for, + /// e.g. "details". + llvm::SmallDenseMap<unsigned, const comments::BlockCommandComment *> + UnhandledCommands; + /// Parsed paragaph(s) of the "param" comamnd(s) llvm::SmallDenseMap<StringRef, const comments::ParamCommandComment *> Parameters; - /// All the block commands. - llvm::SmallDenseMap<unsigned, const comments::BlockCommandComment *> - BlockCommands; + /// Parsed paragaph(s) of the "tparam" comamnd(s) + llvm::SmallDenseMap<StringRef, const comments::TParamCommandComment *> + TemplateParameters; /// All "free" text paragraphs. llvm::SmallDenseMap<unsigned, const comments::ParagraphComment *> FreeParagraphs; + + void + paragraphsToMarkup(markup::Document &Out, + const llvm::SmallVector<const comments::ParagraphComment *> + &Paragraphs) const; }; } // namespace clangd diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp index 3331164ab0024..f7e88c66973f3 100644 --- a/clang-tools-extra/clangd/unittests/HoverTests.cpp +++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp @@ -3409,7 +3409,8 @@ TEST(Hover, DocsFromMostSpecial) { TEST(Hover, Present) { struct { const std::function<void(HoverInfo &)> Builder; - llvm::StringRef ExpectedRender; + llvm::StringRef ExpectedMarkdownRender; + llvm::StringRef ExpectedDoxygenRender; } Cases[] = { { [](HoverInfo &HI) { @@ -3417,6 +3418,7 @@ TEST(Hover, Present) { HI.Name = "X"; }, R"(X)", + R"(### `X`)", }, { [](HoverInfo &HI) { @@ -3424,6 +3426,7 @@ TEST(Hover, Present) { HI.Name = "foo"; }, R"(namespace-alias foo)", + R"(### namespace-alias `foo`)", }, { [](HoverInfo &HI) { @@ -3446,6 +3449,24 @@ Size: 10 bytes documentation template <typename T, typename C = bool> class Foo {})", + R"(### class + +--- +```cpp +template <typename T, typename C = bool> class Foo {} +``` + +--- +**Template Parameters:** + +- `typename T` +- `typename C = bool` + +--- +documentation + +--- +Size: 10 bytes)", }, { [](HoverInfo &HI) { @@ -3476,6 +3497,26 @@ template <typename T, typename C = bool> class Foo {})", "\n" "// In namespace ns\n" "ret_type foo(params) {}", + R"(### function + +--- +```cpp +// In namespace ns +ret_type foo(params) {} +``` + +--- +**Parameters:** + +- +- `type (aka can_type)` +- `type foo (aka can_type)` +- `type foo = default (aka can_type)` + +--- +**Returns:** + +`ret_type (aka can_ret_type)`)", }, { [](HoverInfo &HI) { @@ -3502,6 +3543,22 @@ Size: 4 bytes (+4 bytes padding), alignment 4 bytes // In test::Bar def)", + R"(### field + +--- +```cpp +// In test::Bar +def +``` + +--- +Type: `type (aka can_type)` + +Value = `value` + +Offset: 12 bytes + +Size: 4 bytes (+4 bytes padding), alignment 4 bytes)", }, { [](HoverInfo &HI) { @@ -3528,6 +3585,22 @@ Size: 25 bits (+4 bits padding), alignment 8 bytes // In test::Bar def)", + R"(### field + +--- +```cpp +// In test::Bar +def +``` + +--- +Type: `type (aka can_type)` + +Value = `value` + +Offset: 4 bytes and 3 bits + +Size: 25 bits (+4 bits padding), alignment 8 bytes)", }, { [](HoverInfo &HI) { @@ -3541,6 +3614,13 @@ def)", // In test::Bar public: def)", + R"(### field + +--- +```cpp +// In test::Bar +public: def +```)", }, { [](HoverInfo &HI) { @@ -3560,6 +3640,18 @@ public: def)", // In cls<int> protected: size_t method())", + R"(### instance-method + +--- +```cpp +// In cls<int> +protected: size_t method() +``` + +--- +**Returns:** + +`size_t (aka unsigned long)`)", }, { [](HoverInfo &HI) { @@ -3587,6 +3679,19 @@ protected: size_t method())", // In cls public: cls(int a, int b = 5))", + R"(### constructor + +--- +```cpp +// In cls +public: cls(int a, int b = 5) +``` + +--- +**Parameters:** + +- `int a` +- `int b = 5`)", }, { [](HoverInfo &HI) { @@ -3600,6 +3705,13 @@ public: cls(int a, int b = 5))", // In namespace ns1 private: union foo {})", + R"(### union + +--- +```cpp +// In namespace ns1 +private: union foo {} +```)", }, { [](HoverInfo &HI) { @@ -3625,6 +3737,20 @@ Passed as arg_a // In test::Bar int foo = 3)", + R"(### variable + +--- +```cpp +// In test::Bar +int foo = 3 +``` + +--- +Type: `int` + +Value = `3` + +Passed as arg_a)", }, { [](HoverInfo &HI) { @@ -3636,6 +3762,10 @@ int foo = 3)", }, R"(variable foo... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/152918 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
