[clangd] Fix unknown doxygen command parsing in parameter documentation (#202121)

This patch mainly fixes a bug with parsing of unknown doxygen commands
in function parameter documentation.

To extract the parameter documentation from the function documentation,
the whole function documentation is parsed first.
Then the documentation paragraph for the requested parameter is
"converted" to a string and stored as the documentation for the
parameter. The string is converted by visiting and dumping all chunks of
the parsed paragraph.

When unknown doxygen commands are parsed (during the function
documentation parsing step), they are registered in a
`clang::comments::CommandTraits` object.
Visiting the unknown command requires to query the registered commands
through the `clang::comments::CommandTraits` object to get the command
name.

The bug was that the function documentation parsing and the visiting
step used 2 different `clang::comments::CommandTraits` objects. Hence
the visiting step fails (array access out of bounds) when trying to
retrieve the command names for unknown commands.

The patch moves the function documentation parsing step to the
construction of the `SymbolDocCommentVisitor` which is also responsible
for converting the parameter documentation paragraph to a string.
This way the same `clang::comments::CommandTraits` is used and the query
for unknown command names is correct.

Additional fixes:

- correct some whitespace behaviour for doxygen inline commands
- add a new token kind for the clang comment parser to distinguish
unknown "backslash" and "at" commands to correctly show them in the
clangd hover info

Related issue: clangd/clangd#2671
This commit is contained in:
tcottin 2026-06-25 14:21:45 +02:00 committed by GitHub
parent 5314be5a74
commit 94ae3dd241
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 321 additions and 60 deletions

View file

@ -127,11 +127,12 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &Decl) {
// not write them into PCH, because they are racy and slow to load.
assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getBeginLoc()));
comments::FullComment *FC = RC->parse(Ctx, /*PP=*/nullptr, ND);
if (!FC)
std::string DeclDoc =
RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
if (!looksLikeDocComment(DeclDoc))
return "";
SymbolDocCommentVisitor V(FC, Ctx.getLangOpts().CommentOpts);
SymbolDocCommentVisitor V(DeclDoc, Ctx.getLangOpts().CommentOpts);
std::string RawDoc;
llvm::raw_string_ostream OS(RawDoc);

View file

@ -34,9 +34,10 @@ void commandToMarkup(markup::Paragraph &Out, StringRef Command,
comments::CommandMarkerKind CommandMarker,
StringRef Args) {
Out.appendBoldText(commandMarkerAsString(CommandMarker) + Command.str());
Out.appendSpace();
if (!Args.empty())
if (!Args.empty()) {
Out.appendSpace();
Out.appendCode(Args.str());
}
}
template <typename T> std::string getArgText(const T *Command) {
@ -108,6 +109,7 @@ public:
commandToMarkup(Out, C->getCommandName(Traits), C->getCommandMarker(),
"");
LastChunkEndsWithNewline = false;
}
}
@ -159,7 +161,11 @@ public:
}
}
void visitTextComment(const comments::TextComment *C) { Out << C->getText(); }
void visitTextComment(const comments::TextComment *C) {
Out << C->getText();
if (C->hasTrailingNewline())
Out << "\n";
}
void visitInlineCommandComment(const comments::InlineCommandComment *C) {
Out << commandMarkerAsString(C->getCommandMarker());
@ -167,7 +173,6 @@ public:
std::string ArgText = getArgText(C);
if (!ArgText.empty())
Out << " " << ArgText;
Out << " ";
}
void visitHTMLStartTagComment(const comments::HTMLStartTagComment *STC) {
@ -251,10 +256,7 @@ public:
commandToMarkup(P, B->getCommandName(Traits), B->getCommandMarker(),
ArgText);
if (B->getParagraph() && !B->getParagraph()->isWhitespace()) {
// For commands with arguments, the paragraph starts after the first
// space. Therefore we need to append a space manually in this case.
if (!ArgText.empty())
P.appendSpace();
P.appendSpace();
ParagraphToMarkupDocument(P, Traits).visit(B->getParagraph());
}
}

View file

@ -31,17 +31,6 @@ namespace clangd {
class SymbolDocCommentVisitor
: public comments::ConstCommentVisitor<SymbolDocCommentVisitor> {
public:
SymbolDocCommentVisitor(comments::FullComment *FC,
const CommentOptions &CommentOpts)
: Traits(Allocator, CommentOpts), Allocator() {
if (!FC)
return;
for (auto *Block : FC->getBlocks()) {
visit(Block);
}
}
SymbolDocCommentVisitor(llvm::StringRef Documentation,
const CommentOptions &CommentOpts)
: Traits(Allocator, CommentOpts), Allocator() {

View file

@ -537,16 +537,28 @@ void Paragraph::renderEscapedMarkdown(llvm::raw_ostream &OS) const {
void Paragraph::renderMarkdown(llvm::raw_ostream &OS) const {
bool NeedsSpace = false;
bool HasChunks = false;
bool TextEndsWithNewline = false;
std::string ParagraphText;
ParagraphText.reserve(EstimatedStringSize);
llvm::raw_string_ostream ParagraphTextOS(ParagraphText);
for (auto &C : Chunks) {
if (TextEndsWithNewline) {
ParagraphTextOS << "\n";
TextEndsWithNewline = false;
}
if (C.SpaceBefore || NeedsSpace)
ParagraphTextOS << " ";
switch (C.Kind) {
case ChunkKind::PlainText:
ParagraphTextOS << renderText(C.Contents, !HasChunks,
/*EscapeMarkdown=*/false);
// renderText removes trailing newlines, but in case there are additional
// chunks to process, we need to keep track of the trailing newline and
// add it in the next iteration.
TextEndsWithNewline = llvm::StringRef(C.Contents).ends_with("\n");
break;
case ChunkKind::InlineCode:
ParagraphTextOS << renderInlineBlock(C.Contents);

View file

@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "CodeCompletionStrings.h"
#include "Config.h"
#include "TestTU.h"
#include "clang/Sema/CodeCompleteConsumer.h"
#include "gmock/gmock.h"
@ -69,6 +70,59 @@ TEST_F(CompletionStringTest, GetDeclCommentBadUTF8) {
getDeclComment(AST.getASTContext(), findDecl(AST, "X")));
}
TEST_F(CompletionStringTest, GetDeclCommentForParam) {
auto TU =
TestTU::withCode("/** @param a this is param a */\nvoid func(int a);");
auto AST = TU.build();
Config Cfg;
Cfg.Documentation.CommentFormat = Config::CommentFormatPolicy::Doxygen;
WithContextValue WithCfg(Config::Key, std::move(Cfg));
const auto &FD = llvm::cast<FunctionDecl>(findDecl(AST, "func"));
EXPECT_EQ(FD.getNumParams(), 1UL);
EXPECT_EQ("this is param a",
getDeclComment(AST.getASTContext(), *FD.parameters()[0]));
}
TEST_F(CompletionStringTest, GetDeclCommentForMultipleParams) {
auto TU = TestTU::withCode("/** @param a this is param a\n * @param b this "
"is param b\n */\nvoid func(int a, int b);");
auto AST = TU.build();
Config Cfg;
Cfg.Documentation.CommentFormat = Config::CommentFormatPolicy::Doxygen;
WithContextValue WithCfg(Config::Key, std::move(Cfg));
const auto &FD = llvm::cast<FunctionDecl>(findDecl(AST, "func"));
EXPECT_EQ(FD.getNumParams(), 2UL);
EXPECT_EQ("this is param a",
getDeclComment(AST.getASTContext(), *FD.parameters()[0]));
EXPECT_EQ("this is param b",
getDeclComment(AST.getASTContext(), *FD.parameters()[1]));
}
TEST_F(CompletionStringTest, GetDeclCommentForUndocumentedParam) {
auto TU =
TestTU::withCode("/** @param c this is param c */\nvoid func(int a);");
auto AST = TU.build();
Config Cfg;
Cfg.Documentation.CommentFormat = Config::CommentFormatPolicy::Doxygen;
WithContextValue WithCfg(Config::Key, std::move(Cfg));
const auto &FD = llvm::cast<FunctionDecl>(findDecl(AST, "func"));
EXPECT_EQ(FD.getNumParams(), 1UL);
EXPECT_EQ("", getDeclComment(AST.getASTContext(), *FD.parameters()[0]));
}
TEST_F(CompletionStringTest, GetDeclCommentForParamWithUnknownDoxygenCommand) {
auto TU = TestTU::withCode("/** @param a this is param a and an\n * @unknown "
"command\n */\nvoid func(int a);");
auto AST = TU.build();
Config Cfg;
Cfg.Documentation.CommentFormat = Config::CommentFormatPolicy::Doxygen;
WithContextValue WithCfg(Config::Key, std::move(Cfg));
const auto &FD = llvm::cast<FunctionDecl>(findDecl(AST, "func"));
EXPECT_EQ(FD.getNumParams(), 1UL);
EXPECT_EQ("this is param a and an\n@unknown command",
getDeclComment(AST.getASTContext(), *FD.parameters()[0]));
}
TEST_F(CompletionStringTest, MultipleAnnotations) {
Builder.AddAnnotation("Ano1");
Builder.AddAnnotation("Ano2");

View file

@ -4431,6 +4431,56 @@ brief doc
### Details
longer doc)"},
{[](HoverInfo &HI) {
HI.Kind = index::SymbolKind::Function;
HI.Documentation = "@brief brief doc\n"
"@unknown command is treated as an inline command";
HI.Definition = "int foo(int a)";
HI.ReturnType = "int";
HI.Name = "foo";
HI.Parameters.emplace();
HI.Parameters->emplace_back();
HI.Parameters->back().Type = "int";
HI.Parameters->back().Name = "a";
},
R"(### function `foo`
---
→ `int`
Parameters:
- `int a`
@brief brief doc
@unknown command is treated as an inline command
---
```cpp
int foo(int a)
```)",
R"(### function
---
```cpp
int foo(int a)
```
---
### Brief
brief doc
**@unknown** command is treated as an inline command
---
### Parameters
- `int a`
---
### Returns
`int`)"},
};
for (const auto &C : Cases) {
@ -5203,6 +5253,22 @@ TEST(Hover, FunctionParameters) {
"### param\n\n---\n```cpp\n// In foo\nint b\n```\n\n---\nthis is "
"\\<b>doc\\</b> \\<html-tag attribute/> \\<another-html-tag "
"attribute=\"value\">for\\</another-html-tag> `b`\n\n---\nType: `int`"},
{R"cpp(/// Function doc
/// @param a the next command is an
/// @unknown command.
void foo(int [[^a]]);
)cpp",
[](HoverInfo &HI) {
HI.Name = "a";
HI.Kind = index::SymbolKind::Parameter;
HI.NamespaceScope = "";
HI.LocalScope = "foo::";
HI.Type = "int";
HI.Definition = "int a";
HI.Documentation = "the next command is an\n @unknown command.";
},
"### param\n\n---\n```cpp\n// In foo\nint a\n```\n\n---\nthe next "
"command is an\n**@unknown** command.\n\n---\nType: `int`"},
};
// Create a tiny index, so tests above can verify documentation is fetched.

View file

@ -10,6 +10,7 @@
#include "support/Markup.h"
#include "clang/Basic/CommentOptions.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/raw_ostream.h"
#include "gtest/gtest.h"
namespace clang {
@ -732,5 +733,104 @@ line
}
}
TEST(SymbolDocumentation, ParameterDocToString) {
CommentOptions CommentOpts;
struct Case {
llvm::StringRef Documentation;
llvm::StringRef ExpectedOutputString;
llvm::StringRef ParameterName;
} Cases[] = {
{"This documentation does not contain parameter docs", "", "a"},
{"@param a this is a parameter", "", "not_exists"},
{"@param a this is a parameter", " this is a parameter", "a"},
{R"(@param a parameter doc with an \p inline command)",
R"( parameter doc with an \p inline command)", "a"},
{R"(@param a parameter doc with an \unknown command)",
R"( parameter doc with an \unknown command)", "a"},
{"@param a parameter doc with an @unknown command",
" parameter doc with an @unknown command", "a"},
{R"(@param a parameter doc with
multiple lines)",
R"( parameter doc with
multiple lines)",
"a"},
{R"(@param a parameter doc with an
@unknown command starting a new line)",
R"( parameter doc with an
@unknown command starting a new line)",
"a"},
{R"(@param a parameter doc with a
@note command which is a new block command and therefore ends the parameter doc paragraph)",
R"( parameter doc with a
)",
"a"},
{R"(Unrelated docs
@param a parameter doc
New paragraph with unrelated docs)",
" parameter doc", "a"},
};
for (const auto &C : Cases) {
std::string Result;
llvm::raw_string_ostream OS(Result);
SymbolDocCommentVisitor SymbolDoc(C.Documentation, CommentOpts);
SymbolDoc.parameterDocToString(C.ParameterName, OS);
EXPECT_EQ(Result, C.ExpectedOutputString);
}
}
TEST(SymbolDocumentation, TemplateParameterDocToString) {
CommentOptions CommentOpts;
struct Case {
llvm::StringRef Documentation;
llvm::StringRef ExpectedOutputString;
llvm::StringRef TemplateParameterName;
} Cases[] = {
{"This documentation does not contain parameter docs", "", "a"},
{"@tparam a this is a template type parameter", "", "not_exists"},
{"@tparam a this is a template type parameter",
" this is a template type parameter", "a"},
{R"(@tparam a template type parameter doc with an \p inline command)",
R"( template type parameter doc with an \p inline command)", "a"},
{R"(@tparam a template type parameter doc with an \unknown command)",
R"( template type parameter doc with an \unknown command)", "a"},
{"@tparam a template type parameter doc with an @unknown command",
" template type parameter doc with an @unknown command", "a"},
{R"(@tparam a template type parameter doc with
multiple lines)",
R"( template type parameter doc with
multiple lines)",
"a"},
{R"(@tparam a template type parameter doc with an
@unknown command starting a new line)",
R"( template type parameter doc with an
@unknown command starting a new line)",
"a"},
{R"(@tparam a template type parameter doc with a
@note command which is a new block command and therefore ends the template type parameter doc paragraph)",
R"( template type parameter doc with a
)",
"a"},
{R"(Unrelated docs
@tparam a template type parameter doc
New paragraph with unrelated docs)",
" template type parameter doc", "a"},
};
for (const auto &C : Cases) {
std::string Result;
llvm::raw_string_ostream OS(Result);
SymbolDocCommentVisitor SymbolDoc(C.Documentation, CommentOpts);
SymbolDoc.templateTypeParmDocToString(C.TemplateParameterName, OS);
EXPECT_EQ(Result, C.ExpectedOutputString);
}
}
} // namespace clangd
} // namespace clang

View file

@ -344,15 +344,6 @@ protected:
ArrayRef<Argument> Args;
public:
InlineCommandComment(SourceLocation LocBegin, SourceLocation LocEnd,
unsigned CommandID, InlineCommandRenderKind RK,
ArrayRef<Argument> Args)
: InlineContentComment(CommentKind::InlineCommandComment, LocBegin,
LocEnd),
Args(Args) {
InlineCommandCommentBits.RenderKind = llvm::to_underlying(RK);
InlineCommandCommentBits.CommandID = CommandID;
}
InlineCommandComment(SourceLocation LocBegin, SourceLocation LocEnd,
unsigned CommandID, InlineCommandRenderKind RK,
CommandMarkerKind CommandMarker, ArrayRef<Argument> Args)

View file

@ -33,9 +33,12 @@ enum TokenKind {
eof,
newline,
text,
unknown_command, // Command that does not have an ID.
backslash_command, // Command with an ID, that used backslash marker.
at_command, // Command with an ID, that used 'at' marker.
unknown_backslash_command, // Command that does not have an ID, that used
// backslash marker.
unknown_at_command, // Command that does not have an ID, that used 'at'
// marker.
backslash_command, // Command with an ID, that used backslash marker.
at_command, // Command with an ID, that used 'at' marker.
verbatim_block_begin,
verbatim_block_line,
verbatim_block_end,
@ -107,12 +110,12 @@ public:
}
StringRef getUnknownCommandName() const LLVM_READONLY {
assert(is(tok::unknown_command));
assert(is(tok::unknown_backslash_command) || is(tok::unknown_at_command));
return StringRef(TextPtr, IntVal);
}
void setUnknownCommandName(StringRef Name) {
assert(is(tok::unknown_command));
assert(is(tok::unknown_backslash_command) || is(tok::unknown_at_command));
TextPtr = Name.data();
IntVal = Name.size();
}

View file

@ -136,11 +136,13 @@ public:
InlineContentComment *actOnUnknownCommand(SourceLocation LocBegin,
SourceLocation LocEnd,
StringRef CommandName);
StringRef CommandName,
CommandMarkerKind CommandMarker);
InlineContentComment *actOnUnknownCommand(SourceLocation LocBegin,
SourceLocation LocEnd,
unsigned CommandID);
unsigned CommandID,
CommandMarkerKind CommandMarker);
TextComment *actOnText(SourceLocation LocBegin,
SourceLocation LocEnd,

View file

@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "clang/AST/CommentLexer.h"
#include "clang/AST/Comment.h"
#include "clang/AST/CommentCommandTraits.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/DiagnosticComment.h"
@ -420,7 +421,10 @@ void Lexer::lexCommentText(Token &T) {
<< FullRange << CommandName << CorrectedName
<< FixItHint::CreateReplacement(CommandRange, CorrectedName);
} else {
formTokenWithChars(T, TokenPtr, tok::unknown_command);
formTokenWithChars(T, TokenPtr,
CommandKind == tok::backslash_command
? tok::unknown_backslash_command
: tok::unknown_at_command);
T.setUnknownCommandName(CommandName);
Diag(T.getLocation(), diag::warn_unknown_comment_command_name)
<< SourceRange(T.getLocation(), T.getEndLocation());

View file

@ -726,10 +726,12 @@ BlockContentComment *Parser::parseParagraphOrBlockCommand() {
case tok::eof:
break; // Block content or EOF ahead, finish this parapgaph.
case tok::unknown_command:
Content.push_back(S.actOnUnknownCommand(Tok.getLocation(),
Tok.getEndLocation(),
Tok.getUnknownCommandName()));
case tok::unknown_backslash_command:
case tok::unknown_at_command:
Content.push_back(S.actOnUnknownCommand(
Tok.getLocation(), Tok.getEndLocation(), Tok.getUnknownCommandName(),
Tok.getKind() == tok::unknown_backslash_command ? CMK_Backslash
: CMK_At));
consumeToken();
continue;
@ -751,9 +753,9 @@ BlockContentComment *Parser::parseParagraphOrBlockCommand() {
continue;
}
if (Info->IsUnknownCommand) {
Content.push_back(S.actOnUnknownCommand(Tok.getLocation(),
Tok.getEndLocation(),
Info->getID()));
Content.push_back(S.actOnUnknownCommand(
Tok.getLocation(), Tok.getEndLocation(), Info->getID(),
Tok.getKind() == tok::backslash_command ? CMK_Backslash : CMK_At));
consumeToken();
continue;
}
@ -892,7 +894,8 @@ VerbatimLineComment *Parser::parseVerbatimLine() {
BlockContentComment *Parser::parseBlockContent() {
switch (Tok.getKind()) {
case tok::text:
case tok::unknown_command:
case tok::unknown_backslash_command:
case tok::unknown_at_command:
case tok::backslash_command:
case tok::at_command:
case tok::html_start_tag:

View file

@ -376,19 +376,21 @@ Sema::actOnInlineCommand(SourceLocation CommandLocBegin,
getInlineCommandRenderKind(CommandName), CommandMarker, Args);
}
InlineContentComment *Sema::actOnUnknownCommand(SourceLocation LocBegin,
SourceLocation LocEnd,
StringRef CommandName) {
InlineContentComment *
Sema::actOnUnknownCommand(SourceLocation LocBegin, SourceLocation LocEnd,
StringRef CommandName,
CommandMarkerKind CommandMarker) {
unsigned CommandID = Traits.registerUnknownCommand(CommandName)->getID();
return actOnUnknownCommand(LocBegin, LocEnd, CommandID);
return actOnUnknownCommand(LocBegin, LocEnd, CommandID, CommandMarker);
}
InlineContentComment *Sema::actOnUnknownCommand(SourceLocation LocBegin,
SourceLocation LocEnd,
unsigned CommandID) {
InlineContentComment *
Sema::actOnUnknownCommand(SourceLocation LocBegin, SourceLocation LocEnd,
unsigned CommandID, CommandMarkerKind CommandMarker) {
ArrayRef<InlineCommandComment::Argument> Args;
return new (Allocator) InlineCommandComment(
LocBegin, LocEnd, CommandID, InlineCommandRenderKind::Normal, Args);
return new (Allocator) InlineCommandComment(LocBegin, LocEnd, CommandID,
InlineCommandRenderKind::Normal,
CommandMarker, Args);
}
TextComment *Sema::actOnText(SourceLocation LocBegin,

View file

@ -447,22 +447,22 @@ TEST_F(CommentLexerTest, DoxygenCommand9) {
ASSERT_EQ(tok::text, Toks[0].getKind());
ASSERT_EQ(StringRef(" "), Toks[0].getText());
ASSERT_EQ(tok::unknown_command, Toks[1].getKind());
ASSERT_EQ(tok::unknown_backslash_command, Toks[1].getKind());
ASSERT_EQ(StringRef("aaa"), Toks[1].getUnknownCommandName());
ASSERT_EQ(tok::unknown_command, Toks[2].getKind());
ASSERT_EQ(tok::unknown_backslash_command, Toks[2].getKind());
ASSERT_EQ(StringRef("bbb"), Toks[2].getUnknownCommandName());
ASSERT_EQ(tok::text, Toks[3].getKind());
ASSERT_EQ(StringRef(" "), Toks[3].getText());
ASSERT_EQ(tok::unknown_command, Toks[4].getKind());
ASSERT_EQ(tok::unknown_backslash_command, Toks[4].getKind());
ASSERT_EQ(StringRef("ccc"), Toks[4].getUnknownCommandName());
ASSERT_EQ(tok::text, Toks[5].getKind());
ASSERT_EQ(StringRef("\t"), Toks[5].getText());
ASSERT_EQ(tok::unknown_command, Toks[6].getKind());
ASSERT_EQ(tok::unknown_backslash_command, Toks[6].getKind());
ASSERT_EQ(StringRef("ddd"), Toks[6].getUnknownCommandName());
ASSERT_EQ(tok::newline, Toks[7].getKind());
@ -485,6 +485,38 @@ TEST_F(CommentLexerTest, DoxygenCommand10) {
ASSERT_EQ(tok::newline, Toks[2].getKind());
}
TEST_F(CommentLexerTest, DoxygenCommand11) {
const char *Source = "/// @aaa@bbb @ccc\t@ddd\n";
std::vector<Token> Toks;
lexString(Source, Toks);
ASSERT_EQ(8U, Toks.size());
ASSERT_EQ(tok::text, Toks[0].getKind());
ASSERT_EQ(StringRef(" "), Toks[0].getText());
ASSERT_EQ(tok::unknown_at_command, Toks[1].getKind());
ASSERT_EQ(StringRef("aaa"), Toks[1].getUnknownCommandName());
ASSERT_EQ(tok::unknown_at_command, Toks[2].getKind());
ASSERT_EQ(StringRef("bbb"), Toks[2].getUnknownCommandName());
ASSERT_EQ(tok::text, Toks[3].getKind());
ASSERT_EQ(StringRef(" "), Toks[3].getText());
ASSERT_EQ(tok::unknown_at_command, Toks[4].getKind());
ASSERT_EQ(StringRef("ccc"), Toks[4].getUnknownCommandName());
ASSERT_EQ(tok::text, Toks[5].getKind());
ASSERT_EQ(StringRef("\t"), Toks[5].getText());
ASSERT_EQ(tok::unknown_at_command, Toks[6].getKind());
ASSERT_EQ(StringRef("ddd"), Toks[6].getUnknownCommandName());
ASSERT_EQ(tok::newline, Toks[7].getKind());
}
TEST_F(CommentLexerTest, RegisterCustomBlockCommand) {
const char *Source =
"/// \\NewBlockCommand Aaa.\n"