[clangd] Rearrange type, returntype and parameters in hover card

[kadircet]

Summary

Moves type/returntype into its own line as it is more readable in cases
where the type is long.

Also gives parameter lists a heading, Parameters: to make them stand out.

Leaves the right arrow instead of Returns: before Return Type to make
output more symmetric.

function foo

Returns: ret_type
Parameters:
- int x

vs

function foo

🡺 ret_type
Parameters:
- int x

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

[sammccall]

clang-tools-extra/clangd/Hover.cpp

535

Comment needs updating

551

consider ∈, which would be short and symmetrical with the arrow for return type...

Too obscure?

[kadircet]

• Address comments and rebase on top of ruler patch

clang-tools-extra/clangd/Hover.cpp

551

it is a good way to make the output shorter, not sure about the symmetry though as type and returntype never show up in the same hovercard.
but I believe it is too obscure.

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

[sammccall]

clang-tools-extra/clangd/Hover.cpp

528–529

as discussed offline, better IMO to allow the middle section to be empty, emit both rulers, and fix it in post :-)

551

The symmetry I mean is two kinds of hover cards for functions vs values that when scanned have approximately the same layout (type information is introduced by a similar-sized symbol) but can still be distinguished.

but I believe it is too obscure.

:-( fair enough

[lh123]

I think the character "🡺" should be avoided, as it may not display properly in some environments.

[kadircet]

• Rebase

[kadircet]

In D72623#1819153, @lh123 wrote:

I think the character "🡺" should be avoided, as it may not display properly in some environments.

I believe most of the editors should be able to display unicode characters, please let us know if you've got any concrete examples of corrupt rendering.

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

clang-tools-extra/clangd/Hover.cpp

llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
   HI->SymRange = getTokenRange(AST.getSourceManager(), AST.getLangOpts(),
                                SourceLocationBeg);
   return HI;
 }
 
 markup::Document HoverInfo::present() const {
   markup::Document Output;
   // Header contains a text of the form:
-  // variable `var` : `int`
+  // variable `var`
   //
   // class `X`
   //
-  // function `foo` → `int`
+  // function `foo`
+  //
+  // expression
   //
-  // expression : `int`
   // Note that we are making use of a level-3 heading because VSCode renders
   // level 1 and 2 headers in a huge font, see
   // https://github.com/microsoft/vscode/issues/88417 for details.
   markup::Paragraph &Header = Output.addHeading(3);
   Header.appendText(index::getSymbolKindString(Kind));
   assert(!Name.empty() && "hover triggered on a nameless symbol");
   Header.appendCode(Name);
-  if (ReturnType) {
-    Header.appendText("→");
-    Header.appendCode(*ReturnType);
-  } else if (Type) {
-    Header.appendText(":");
-    Header.appendCode(*Type);
-  }
 
   // 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(*ReturnType);
     if (Parameters && !Parameters->empty()) {
+      Output.addParagraph().appendText("Parameters:");
       markup::BulletList &L = Output.addBulletList();
       for (const auto &Param : *Parameters) {
         std::string Buffer;
         llvm::raw_string_ostream OS(Buffer);
         OS << Param;
         L.addItem().addParagraph().appendCode(std::move(OS.str()));
       }
     }
+  } else if (Type) {
+    Output.addParagraph().appendText("Type: ").appendCode(*Type);
+  }
 
   if (Value) {
     markup::Paragraph &P = Output.addParagraph();
     P.appendText("Value =");
     P.appendCode(*Value);
   }
 
   if (!Documentation.empty())

clang-tools-extra/clangd/test/hover.test

 # RUN: clangd -lit-test < %s | FileCheck %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void foo(); int main() { foo(); }\n"}}}
 ---
 {"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":27}}}
 #      CHECK:  "id": 1,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": {
 # CHECK-NEXT:    "contents": {
 # CHECK-NEXT:      "kind": "plaintext",
-# CHECK-NEXT:      "value": "function foo → void\n\nvoid foo()"
+# CHECK-NEXT:      "value": "function foo\n\n🡺 void\n\nvoid foo()"
 # CHECK-NEXT:    },
 # CHECK-NEXT:    "range": {
 # CHECK-NEXT:      "end": {
 # CHECK-NEXT:        "character": 28,
 # CHECK-NEXT:        "line": 0
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "start": {
 # CHECK-NEXT:        "character": 25,

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

template <typename T, typename C = bool> class Foo {})",
             HI.Parameters->push_back(P);
             P.Name = "foo";
             HI.Parameters->push_back(P);
             P.Default = "default";
             HI.Parameters->push_back(P);
             HI.NamespaceScope = "ns::";
             HI.Definition = "ret_type foo(params) {}";
           },
-          R"(function foo → ret_type
+          R"(function foo
 
+🡺 ret_type
+Parameters:
 - 
 - type
 - type foo
 - type foo = default
 
 // In namespace ns
 ret_type foo(params) {})",
       },
       {
           [](HoverInfo &HI) {
             HI.Kind = index::SymbolKind::Variable;
             HI.LocalScope = "test::bar::";
             HI.Value = "value";
             HI.Name = "foo";
             HI.Type = "type";
             HI.Definition = "def";
           },
-          R"(variable foo : type
+          R"(variable foo
 
+Type: type
 Value = value
 
 // In test::bar
 def)",
       },
   };
 
   for (const auto &C : Cases) {
     HoverInfo HI;
     C.Builder(HI);
     EXPECT_EQ(HI.present().asPlainText(), C.ExpectedRender);
   }
 }
 
 // This is a separate test as headings don't create any differences in plaintext
 // mode.
 TEST(Hover, PresentHeadings) {
   HoverInfo HI;
   HI.Kind = index::SymbolKind::Variable;
   HI.Name = "foo";
-  HI.Type = "type";
 
-  EXPECT_EQ(HI.present().asMarkdown(), "### variable `foo` \\: `type`");
+  EXPECT_EQ(HI.present().asMarkdown(), "### variable `foo`");
 }
 
 // This is a separate test as rulers behave differently in markdown vs
 // plaintext.
 TEST(Hover, PresentRulers) {
   HoverInfo HI;
   HI.Kind = index::SymbolKind::Variable;
   HI.Name = "foo";