[clangd] Provide documentation as MarkupContent in signaturehelp

This unifies the behaviour we have in code completion item
documentations and signaturehelp. Providing better line wrapping and detection
of inline code blocks in comments to be renedered appropriately in markdown.

Differential Revision: https://reviews.llvm.org/D115442

clang-tools-extra/clangd/ClangdLSPServer.cpp

@@ -500,6 +500,8 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params,
     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::onCompletion(const CompletionParams &Params,
 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)

clang-tools-extra/clangd/ClangdLSPServer.h

@@ -56,6 +56,7 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
     /// 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) {

clang-tools-extra/clangd/ClangdServer.cpp

@@ -403,9 +403,11 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
 }
 
 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 @@ void ClangdServer::signatureHelp(PathRef File, Position Pos,
 
     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.

clang-tools-extra/clangd/ClangdServer.h

@@ -225,7 +225,8 @@ class ClangdServer {
 
   /// 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,

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 @@ toCompletionItemKind(CodeCompletionResult::ResultKind ResKind,
   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 @@ int paramIndexForArg(const CodeCompleteConsumer::OverloadCandidate &Candidate,
 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 @@ class SignatureHelpCollector final : public CodeCompleteConsumer {
         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 @@ class SignatureHelpCollector final : public CodeCompleteConsumer {
     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 @@ class SignatureHelpCollector final : public CodeCompleteConsumer {
     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 @@ class SignatureHelpCollector final : public CodeCompleteConsumer {
     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 @@ class SignatureHelpCollector final : public CodeCompleteConsumer {
   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 @@ CodeCompleteResult codeComplete(PathRef FileName, Position Pos,
 
 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 @@ SignatureHelp signatureHelp(PathRef FileName, Position Pos,
   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 @@ bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx) {
   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];

clang-tools-extra/clangd/CodeComplete.h

@@ -284,7 +284,8 @@ CodeCompleteResult codeComplete(PathRef FileName, Position Pos,
 /// 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

clang-tools-extra/clangd/Protocol.cpp

@@ -383,6 +383,13 @@ bool fromJSON(const llvm::json::Value &Params, ClientCapabilities &R,
           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 @@ llvm::json::Value toJSON(const SignatureInformation &SI) {
       {"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);
 }

clang-tools-extra/clangd/Protocol.h

@@ -438,6 +438,11 @@ struct ClientCapabilities {
   /// 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 @@ struct SignatureInformation {
   std::string label;
 
   /// The documentation of this signature. Optional.
-  std::string documentation;
+  MarkupContent documentation;
 
   /// The parameters of this signature.
   std::vector<ParameterInformation> parameters;

clang-tools-extra/clangd/test/signature-help.test

@@ -1,17 +1,21 @@
 # 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": {
 # CHECK-NEXT:   "activeParameter": 0,
 # 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:         {

clang-tools-extra/clangd/unittests/ClangdTests.cpp

@@ -633,7 +633,8 @@ TEST(ClangdServerTest, InvalidCompileCommand) {
   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()))

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 @@ TEST(CompletionTest, ASTSignals) {
                         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 @@ SignatureHelp signatures(llvm::StringRef Text, Position Point,
     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 @@ MATCHER_P(ParamsAre, P, "") {
   }
   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 @@ TEST(SignatureHelpTest, StalePreamble) {
     #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 @@ TEST(SignatureHelpTest, DynamicIndexDocumentation) {
   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 @@ TEST(CompletionTest, CommentParamName) {
               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