diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/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 TweakFilter = [](const Tweak &T) { diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/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 Reply) { Server->signatureHelp(Params.textDocument.uri.file(), Params.position, + Opts.SignatureHelpDocumentationFormat, [Reply = std::move(Reply), this]( llvm::Expected Signature) mutable { if (!Signature) diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/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 CB); + void signatureHelp(PathRef File, Position Pos, MarkupKind DocumentationFormat, + Callback CB); /// Find declaration/definition locations of symbol at a specified position. void locateSymbolAt(PathRef File, Position Pos, diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -403,9 +403,11 @@ } void ClangdServer::signatureHelp(PathRef File, Position Pos, + MarkupKind DocumentationFormat, Callback CB) { auto Action = [Pos, File = File.str(), CB = std::move(CB), + DocumentationFormat, this](llvm::Expected 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. diff --git a/clang-tools-extra/clangd/CodeComplete.h b/clang-tools-extra/clangd/CodeComplete.h --- a/clang-tools-extra/clangd/CodeComplete.h +++ b/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 diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/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()), - 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 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(Options, ParseInput.Index, - Result), + std::make_unique(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]; diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h --- a/clang-tools-extra/clangd/Protocol.h +++ b/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 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 parameters; diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/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); } diff --git a/clang-tools-extra/clangd/test/signature-help.test b/clang-tools-extra/clangd/test/signature-help.test --- a/clang-tools-extra/clangd/test/signature-help.test +++ b/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: { diff --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp --- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp +++ b/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())) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/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 IndexSymbols = {}) { +SignatureHelp +signatures(llvm::StringRef Text, Position Point, + std::vector IndexSymbols = {}, + MarkupKind DocumentationFormat = MarkupKind::PlainText) { std::unique_ptr 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 IndexSymbols = {}) { +SignatureHelp +signatures(llvm::StringRef Text, std::vector 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 diff --git a/clang-tools-extra/clangd/unittests/SyncAPI.h b/clang-tools-extra/clangd/unittests/SyncAPI.h --- a/clang-tools-extra/clangd/unittests/SyncAPI.h +++ b/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 runSignatureHelp(ClangdServer &Server, - PathRef File, Position Pos); + PathRef File, Position Pos, + MarkupKind DocumentationFormat); llvm::Expected> runLocateSymbolAt(ClangdServer &Server, PathRef File, Position Pos); diff --git a/clang-tools-extra/clangd/unittests/SyncAPI.cpp b/clang-tools-extra/clangd/unittests/SyncAPI.cpp --- a/clang-tools-extra/clangd/unittests/SyncAPI.cpp +++ b/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 runSignatureHelp(ClangdServer &Server, - PathRef File, Position Pos) { + PathRef File, Position Pos, + MarkupKind DocumentationFormat) { llvm::Optional> Result; - Server.signatureHelp(File, Pos, capture(Result)); + Server.signatureHelp(File, Pos, DocumentationFormat, capture(Result)); return std::move(*Result); }