[clangd] Show parameters for construct.

[lh123]

Summary

Show parameters for construct.

[kadircet]

clang-tools-extra/clangd/Hover.cpp

1074

it's a subtle invariant that we only have parameters for functions (which has a return type) or constructor/destructor/conversion-operators (which doesn't have either a type or a return type).

I think we should assert on Type not being present here, as otherwise we would probably duplicate parameters in both places. can you also add a condition around !Type and have a comment saying // Don't print parameters after Type, as they're likely to be mentioned there.

[lh123]

address comment.

[lh123]

clang-tools-extra/clangd/Hover.cpp

1074

We cannot assert here because the function type has both ReturnType, Type, and Parameters. I think we only need to not print Type when ReturnType or Parameters are present.

[kadircet]

thanks LGTM!
let me know if I should land this for you.

clang-tools-extra/clangd/Hover.cpp

1054

can you add braces around here?

1074

sorry by asserting i meant having it inside the check. this LG, thanks!

1075

can you move the comment above the if and also include as this will just duplicate the information.

[lh123]

address comment.

clang-tools-extra/clangd/Hover.cpp

if (Kind != index::SymbolKind::Unknown)
     Header.appendText(index::getSymbolKindString(Kind)).appendSpace();
   assert(!Name.empty() && "hover triggered on a nameless symbol");
   Header.appendCode(Name);
 
   // 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) {
+  }
+
+
+  // Don't print Type after Parameters or ReturnType as this will just duplicate
+  // the information
+  if (Type && !ReturnType && !Parameters)
     Output.addParagraph().appendText("Type: ").appendCode(*Type);
-  }
 
   if (Value) {
     markup::Paragraph &P = Output.addParagraph();
     P.appendText("Value = ");
     P.appendCode(*Value);
   }
 
   if (Offset)

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

 
 → int
 
 // In cls<int>
 protected: int method())",
       },
       {
           [](HoverInfo &HI) {
+            HI.Definition = "cls(int a, int b = 5)";
+            HI.AccessSpecifier = "public";
+            HI.Kind = index::SymbolKind::Constructor;
+            HI.NamespaceScope = "";
+            HI.LocalScope = "cls";
+            HI.Name = "cls";
+            HI.Parameters.emplace();
+            HI.Parameters->emplace_back();
+            HI.Parameters->back().Type = "int";
+            HI.Parameters->back().Name = "a";
+            HI.Parameters->emplace_back();
+            HI.Parameters->back().Type = "int";
+            HI.Parameters->back().Name = "b";
+            HI.Parameters->back().Default = "5";
+          },
+          R"(constructor cls
+
+Parameters:
+- int a
+- int b = 5
+
+// In cls
+public: cls(int a, int b = 5))",
+      },
+      {
+          [](HoverInfo &HI) {
             HI.Kind = index::SymbolKind::Union;
             HI.AccessSpecifier = "private";
             HI.Name = "foo";
             HI.NamespaceScope = "ns1::";
             HI.Definition = "union foo {}";
           },
           R"(union foo