This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd3606a3375d2: [clangd] Provide documentation as
MarkupContent in signaturehelp (authored by kadircet).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115442/new/
https://reviews.llvm.org/D115442
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/CodeComplete.h
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/test/signature-help.test
clang-tools-extra/clangd/unittests/ClangdTests.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang-tools-extra/clangd/unittests/SyncAPI.cpp
clang-tools-extra/clangd/unittests/SyncAPI.h
Index: clang-tools-extra/clangd/unittests/SyncAPI.h
===================================================================
--- clang-tools-extra/clangd/unittests/SyncAPI.h
+++ clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -16,6 +16,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_SYNCAPI_H
#include "ClangdServer.h"
+#include "Protocol.h"
#include "index/Index.h"
namespace clang {
@@ -32,7 +33,8 @@
clangd::CodeCompleteOptions Opts);
llvm::Expected<SignatureHelp> runSignatureHelp(ClangdServer &Server,
- PathRef File, Position Pos);
+ PathRef File, Position Pos,
+ MarkupKind DocumentationFormat);
llvm::Expected<std::vector<LocatedSymbol>>
runLocateSymbolAt(ClangdServer &Server, PathRef File, Position Pos);
Index: clang-tools-extra/clangd/unittests/SyncAPI.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "SyncAPI.h"
+#include "Protocol.h"
#include "index/Index.h"
namespace clang {
@@ -77,9 +78,10 @@
}
llvm::Expected<SignatureHelp> runSignatureHelp(ClangdServer &Server,
- PathRef File, Position Pos) {
+ PathRef File, Position Pos,
+ MarkupKind DocumentationFormat) {
llvm::Optional<llvm::Expected<SignatureHelp>> Result;
- Server.signatureHelp(File, Pos, capture(Result));
+ Server.signatureHelp(File, Pos, DocumentationFormat, capture(Result));
return std::move(*Result);
}
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -24,6 +24,7 @@
#include "support/Threading.h"
#include "clang/Sema/CodeCompleteConsumer.h"
#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/Path.h"
#include "llvm/Testing/Support/Annotations.h"
@@ -1171,8 +1172,10 @@
MainFileRefs(0u), ScopeRefs(3u))));
}
-SignatureHelp signatures(llvm::StringRef Text, Position Point,
- std::vector<Symbol> IndexSymbols = {}) {
+SignatureHelp
+signatures(llvm::StringRef Text, Position Point,
+ std::vector<Symbol> IndexSymbols = {},
+ MarkupKind DocumentationFormat = MarkupKind::PlainText) {
std::unique_ptr<SymbolIndex> Index;
if (!IndexSymbols.empty())
Index = memIndex(IndexSymbols);
@@ -1193,13 +1196,16 @@
ADD_FAILURE() << "Couldn't build Preamble";
return {};
}
- return signatureHelp(testPath(TU.Filename), Point, *Preamble, Inputs);
+ return signatureHelp(testPath(TU.Filename), Point, *Preamble, Inputs,
+ DocumentationFormat);
}
-SignatureHelp signatures(llvm::StringRef Text,
- std::vector<Symbol> IndexSymbols = {}) {
+SignatureHelp
+signatures(llvm::StringRef Text, std::vector<Symbol> IndexSymbols = {},
+ MarkupKind DocumentationFormat = MarkupKind::PlainText) {
Annotations Test(Text);
- return signatures(Test.code(), Test.point(), std::move(IndexSymbols));
+ return signatures(Test.code(), Test.point(), std::move(IndexSymbols),
+ DocumentationFormat);
}
struct ExpectedParameter {
@@ -1216,7 +1222,7 @@
}
return true;
}
-MATCHER_P(SigDoc, Doc, "") { return arg.documentation == Doc; }
+MATCHER_P(SigDoc, Doc, "") { return arg.documentation.value == Doc; }
/// \p AnnotatedLabel is a signature label with ranges marking parameters, e.g.
/// foo([[int p1]], [[double p2]]) -> void
@@ -1388,8 +1394,9 @@
#include "a.h"
void bar() { foo(^2); })cpp");
TU.Code = Test.code().str();
- auto Results = signatureHelp(testPath(TU.Filename), Test.point(),
- *EmptyPreamble, TU.inputs(FS));
+ auto Results =
+ signatureHelp(testPath(TU.Filename), Test.point(), *EmptyPreamble,
+ TU.inputs(FS), MarkupKind::PlainText);
EXPECT_THAT(Results.signatures, ElementsAre(Sig("foo([[int x]]) -> int")));
EXPECT_EQ(0, Results.activeSignature);
EXPECT_EQ(0, Results.activeParameter);
@@ -2261,10 +2268,10 @@
Server.addDocument(File, FileContent.code());
// Wait for the dynamic index being built.
ASSERT_TRUE(Server.blockUntilIdleForTest());
- EXPECT_THAT(
- llvm::cantFail(runSignatureHelp(Server, File, FileContent.point()))
- .signatures,
- ElementsAre(AllOf(Sig("foo() -> int"), SigDoc("Member doc"))));
+ EXPECT_THAT(llvm::cantFail(runSignatureHelp(Server, File, FileContent.point(),
+ MarkupKind::PlainText))
+ .signatures,
+ ElementsAre(AllOf(Sig("foo() -> int"), SigDoc("Member doc"))));
}
TEST(CompletionTest, CompletionFunctionArgsDisabled) {
@@ -3431,6 +3438,21 @@
IsEmpty());
}
+TEST(SignatureHelp, DocFormat) {
+ Annotations Code(R"cpp(
+ // Comment `with` markup.
+ void foo(int);
+ void bar() { foo(^); }
+ )cpp");
+ for (auto DocumentationFormat :
+ {MarkupKind::PlainText, MarkupKind::Markdown}) {
+ auto Sigs = signatures(Code.code(), Code.point(), /*IndexSymbols=*/{},
+ DocumentationFormat);
+ ASSERT_EQ(Sigs.signatures.size(), 1U);
+ EXPECT_EQ(Sigs.signatures[0].documentation.kind, DocumentationFormat);
+ }
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -633,7 +633,8 @@
EXPECT_ERROR(runFindDocumentHighlights(Server, FooCpp, Position()));
EXPECT_ERROR(runRename(Server, FooCpp, Position(), "new_name",
clangd::RenameOptions()));
- EXPECT_ERROR(runSignatureHelp(Server, FooCpp, Position()));
+ EXPECT_ERROR(
+ runSignatureHelp(Server, FooCpp, Position(), MarkupKind::PlainText));
// Identifier-based fallback completion.
EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, Position(),
clangd::CodeCompleteOptions()))
Index: clang-tools-extra/clangd/test/signature-help.test
===================================================================
--- clang-tools-extra/clangd/test/signature-help.test
+++ clang-tools-extra/clangd/test/signature-help.test
@@ -1,10 +1,10 @@
# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
# Start a session.
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument": {"signatureHelp": {"signatureInformation": {"documentationFormat": ["markdown", "plaintext"]}}}},"trace":"off"}}
---
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void x(int);\nint main(){\nx("}}}
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"// comment `markdown` _escape_\nvoid x(int);\nint main(){\nx("}}}
---
-{"jsonrpc":"2.0","id":1,"method":"textDocument/signatureHelp","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":2}}}
+{"jsonrpc":"2.0","id":1,"method":"textDocument/signatureHelp","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":3,"character":2}}}
# CHECK: "id": 1,
# CHECK-NEXT: "jsonrpc": "2.0",
# CHECK-NEXT: "result": {
@@ -12,6 +12,10 @@
# CHECK-NEXT: "activeSignature": 0,
# CHECK-NEXT: "signatures": [
# CHECK-NEXT: {
+# CHECK-NEXT: "documentation": {
+# CHECK-NEXT: "kind": "markdown",
+# CHECK-NEXT: "value": "comment `markdown` \\_escape\\_"
+# CHECK-NEXT: },
# CHECK-NEXT: "label": "x(int) -> void",
# CHECK-NEXT: "parameters": [
# CHECK-NEXT: {
Index: clang-tools-extra/clangd/Protocol.h
===================================================================
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -438,6 +438,11 @@
/// textDocument.signatureHelp.signatureInformation.parameterInformation.labelOffsetSupport
bool OffsetsInSignatureHelp = false;
+ /// The documentation format that should be used for
+ /// textDocument/signatureHelp.
+ /// textDocument.signatureHelp.signatureInformation.documentationFormat
+ MarkupKind SignatureHelpDocumentationFormat = MarkupKind::PlainText;
+
/// The supported set of CompletionItemKinds for textDocument/completion.
/// textDocument.completion.completionItemKind.valueSet
llvm::Optional<CompletionItemKindBitset> CompletionItemKinds;
@@ -1283,7 +1288,7 @@
std::string label;
/// The documentation of this signature. Optional.
- std::string documentation;
+ MarkupContent documentation;
/// The parameters of this signature.
std::vector<ParameterInformation> parameters;
Index: clang-tools-extra/clangd/Protocol.cpp
===================================================================
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -383,6 +383,13 @@
if (auto OffsetSupport = Parameter->getBoolean("labelOffsetSupport"))
R.OffsetsInSignatureHelp = *OffsetSupport;
}
+ if (const auto *DocumentationFormat =
+ Info->getArray("documentationFormat")) {
+ for (const auto &Format : *DocumentationFormat) {
+ if (fromJSON(Format, R.SignatureHelpDocumentationFormat, P))
+ break;
+ }
+ }
}
}
if (auto *Rename = TextDocument->getObject("rename")) {
@@ -1031,7 +1038,7 @@
{"label", SI.label},
{"parameters", llvm::json::Array(SI.parameters)},
};
- if (!SI.documentation.empty())
+ if (!SI.documentation.value.empty())
Result["documentation"] = SI.documentation;
return std::move(Result);
}
Index: clang-tools-extra/clangd/CodeComplete.h
===================================================================
--- clang-tools-extra/clangd/CodeComplete.h
+++ clang-tools-extra/clangd/CodeComplete.h
@@ -284,7 +284,8 @@
/// Get signature help at a specified \p Pos in \p FileName.
SignatureHelp signatureHelp(PathRef FileName, Position Pos,
const PreambleData &Preamble,
- const ParseInputs &ParseInput);
+ const ParseInputs &ParseInput,
+ MarkupKind DocumentationFormat);
// For index-based completion, we only consider:
// * symbols in namespaces or translation unit scopes (e.g. no class
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -37,6 +37,7 @@
#include "index/Symbol.h"
#include "index/SymbolOrigin.h"
#include "support/Logger.h"
+#include "support/Markup.h"
#include "support/Threading.h"
#include "support/ThreadsafeFS.h"
#include "support/Trace.h"
@@ -157,6 +158,21 @@
llvm_unreachable("Unhandled CodeCompletionResult::ResultKind.");
}
+// FIXME: find a home for this (that can depend on both markup and Protocol).
+MarkupContent renderDoc(const markup::Document &Doc, MarkupKind Kind) {
+ MarkupContent Result;
+ Result.kind = Kind;
+ switch (Kind) {
+ case MarkupKind::PlainText:
+ Result.value.append(Doc.asPlainText());
+ break;
+ case MarkupKind::Markdown:
+ Result.value.append(Doc.asMarkdown());
+ break;
+ }
+ return Result;
+}
+
// Identifier code completion result.
struct RawIdentifier {
llvm::StringRef Name;
@@ -897,10 +913,12 @@
class SignatureHelpCollector final : public CodeCompleteConsumer {
public:
SignatureHelpCollector(const clang::CodeCompleteOptions &CodeCompleteOpts,
+ MarkupKind DocumentationFormat,
const SymbolIndex *Index, SignatureHelp &SigHelp)
: CodeCompleteConsumer(CodeCompleteOpts), SigHelp(SigHelp),
Allocator(std::make_shared<clang::GlobalCodeCompletionAllocator>()),
- CCTUInfo(Allocator), Index(Index) {}
+ CCTUInfo(Allocator), Index(Index),
+ DocumentationFormat(DocumentationFormat) {}
void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
OverloadCandidate *Candidates,
@@ -969,9 +987,9 @@
if (!S.Documentation.empty())
FetchedDocs[S.ID] = std::string(S.Documentation);
});
- log("SigHelp: requested docs for {0} symbols from the index, got {1} "
- "symbols with non-empty docs in the response",
- IndexRequest.IDs.size(), FetchedDocs.size());
+ vlog("SigHelp: requested docs for {0} symbols from the index, got {1} "
+ "symbols with non-empty docs in the response",
+ IndexRequest.IDs.size(), FetchedDocs.size());
}
llvm::sort(ScoredSignatures, [](const ScoredSignature &L,
@@ -1009,8 +1027,12 @@
for (auto &SS : ScoredSignatures) {
auto IndexDocIt =
SS.IDForDoc ? FetchedDocs.find(SS.IDForDoc) : FetchedDocs.end();
- if (IndexDocIt != FetchedDocs.end())
- SS.Signature.documentation = IndexDocIt->second;
+ if (IndexDocIt != FetchedDocs.end()) {
+ markup::Document SignatureComment;
+ parseDocumentation(IndexDocIt->second, SignatureComment);
+ SS.Signature.documentation =
+ renderDoc(SignatureComment, DocumentationFormat);
+ }
SigHelp.signatures.push_back(std::move(SS.Signature));
}
@@ -1071,7 +1093,9 @@
SignatureQualitySignals Signal;
const char *ReturnType = nullptr;
- Signature.documentation = formatDocumentation(CCS, DocComment);
+ markup::Document OverloadComment;
+ parseDocumentation(formatDocumentation(CCS, DocComment), OverloadComment);
+ Signature.documentation = renderDoc(OverloadComment, DocumentationFormat);
Signal.Kind = Candidate.getKind();
for (const auto &Chunk : CCS) {
@@ -1110,7 +1134,7 @@
Result.Signature = std::move(Signature);
Result.Quality = Signal;
const FunctionDecl *Func = Candidate.getFunction();
- if (Func && Result.Signature.documentation.empty()) {
+ if (Func && Result.Signature.documentation.value.empty()) {
// Computing USR caches linkage, which may change after code completion.
if (!hasUnstableLinkage(Func))
Result.IDForDoc = clangd::getSymbolID(Func);
@@ -1122,6 +1146,7 @@
std::shared_ptr<clang::GlobalCodeCompletionAllocator> Allocator;
CodeCompletionTUInfo CCTUInfo;
const SymbolIndex *Index;
+ MarkupKind DocumentationFormat;
}; // SignatureHelpCollector
// Used only for completion of C-style comments in function call (i.e.
@@ -2020,7 +2045,8 @@
SignatureHelp signatureHelp(PathRef FileName, Position Pos,
const PreambleData &Preamble,
- const ParseInputs &ParseInput) {
+ const ParseInputs &ParseInput,
+ MarkupKind DocumentationFormat) {
auto Offset = positionToOffset(ParseInput.Contents, Pos);
if (!Offset) {
elog("Signature help position was invalid {0}", Offset.takeError());
@@ -2033,8 +2059,8 @@
Options.IncludeCodePatterns = false;
Options.IncludeBriefComments = false;
semaCodeComplete(
- std::make_unique<SignatureHelpCollector>(Options, ParseInput.Index,
- Result),
+ std::make_unique<SignatureHelpCollector>(Options, DocumentationFormat,
+ ParseInput.Index, Result),
Options,
{FileName, *Offset, Preamble,
PreamblePatch::createFullPatch(FileName, ParseInput, Preamble),
@@ -2075,21 +2101,6 @@
return false;
}
-// FIXME: find a home for this (that can depend on both markup and Protocol).
-static MarkupContent renderDoc(const markup::Document &Doc, MarkupKind Kind) {
- MarkupContent Result;
- Result.kind = Kind;
- switch (Kind) {
- case MarkupKind::PlainText:
- Result.value.append(Doc.asPlainText());
- break;
- case MarkupKind::Markdown:
- Result.value.append(Doc.asMarkdown());
- break;
- }
- return Result;
-}
-
CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {
CompletionItem LSP;
const auto *InsertInclude = Includes.empty() ? nullptr : &Includes[0];
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -225,7 +225,8 @@
/// Provide signature help for \p File at \p Pos. This method should only be
/// called for tracked files.
- void signatureHelp(PathRef File, Position Pos, Callback<SignatureHelp> CB);
+ void signatureHelp(PathRef File, Position Pos, MarkupKind DocumentationFormat,
+ Callback<SignatureHelp> CB);
/// Find declaration/definition locations of symbol at a specified position.
void locateSymbolAt(PathRef File, Position Pos,
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -403,9 +403,11 @@
}
void ClangdServer::signatureHelp(PathRef File, Position Pos,
+ MarkupKind DocumentationFormat,
Callback<SignatureHelp> CB) {
auto Action = [Pos, File = File.str(), CB = std::move(CB),
+ DocumentationFormat,
this](llvm::Expected<InputsAndPreamble> IP) mutable {
if (!IP)
return CB(IP.takeError());
@@ -416,7 +418,8 @@
ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
ParseInput.Index = Index;
- CB(clangd::signatureHelp(File, Pos, *PreambleData, ParseInput));
+ CB(clangd::signatureHelp(File, Pos, *PreambleData, ParseInput,
+ DocumentationFormat));
};
// Unlike code completion, we wait for a preamble here.
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -56,6 +56,7 @@
/// Per-feature options. Generally ClangdServer lets these vary
/// per-request, but LSP allows limited/no customizations.
clangd::CodeCompleteOptions CodeComplete;
+ MarkupKind SignatureHelpDocumentationFormat = MarkupKind::PlainText;
clangd::RenameOptions Rename;
/// Returns true if the tweak should be enabled.
std::function<bool(const Tweak &)> TweakFilter = [](const Tweak &T) {
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -500,6 +500,8 @@
Opts.CodeComplete.BundleOverloads = Params.capabilities.HasSignatureHelp;
Opts.CodeComplete.DocumentationFormat =
Params.capabilities.CompletionDocumentationFormat;
+ Opts.SignatureHelpDocumentationFormat =
+ Params.capabilities.SignatureHelpDocumentationFormat;
DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes;
DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory;
DiagOpts.EmitRelatedLocations =
@@ -1058,6 +1060,7 @@
void ClangdLSPServer::onSignatureHelp(const TextDocumentPositionParams &Params,
Callback<SignatureHelp> Reply) {
Server->signatureHelp(Params.textDocument.uri.file(), Params.position,
+ Opts.SignatureHelpDocumentationFormat,
[Reply = std::move(Reply), this](
llvm::Expected<SignatureHelp> Signature) mutable {
if (!Signature)
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
