diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -25,9 +25,11 @@ #include "clang/AST/PrettyPrinter.h" #include "clang/Index/IndexSymbol.h" #include "llvm/ADT/STLExtras.h" -#include "llvm/ADT/iterator_range.h" -#include "llvm/Support/Casting.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/raw_ostream.h" +#include namespace clang { namespace clangd { @@ -224,8 +226,8 @@ // Populates Type, ReturnType, and Parameters for function-like decls. void fillFunctionTypeAndParams(HoverInfo &HI, const Decl *D, - const FunctionDecl *FD, - const PrintingPolicy &Policy) { + const FunctionDecl *FD, + const PrintingPolicy &Policy) { HI.Parameters.emplace(); for (const ParmVarDecl *PVD : FD->parameters()) { HI.Parameters->emplace_back(); @@ -250,11 +252,11 @@ } } - if (const auto* CCD = llvm::dyn_cast(FD)) { + if (const auto *CCD = llvm::dyn_cast(FD)) { // Constructor's "return type" is the class type. HI.ReturnType = declaredType(CCD->getParent()).getAsString(Policy); // Don't provide any type for the constructor itself. - } else if (llvm::isa(FD)){ + } else if (llvm::isa(FD)) { HI.ReturnType = "void"; } else { HI.ReturnType = FD->getReturnType().getAsString(Policy); @@ -309,7 +311,7 @@ } /// Generate a \p Hover object given the declaration \p D. -HoverInfo getHoverContents(const Decl *D, const SymbolIndex *Index) { +HoverInfo getHoverContents(const NamedDecl *D, const SymbolIndex *Index) { HoverInfo HI; const ASTContext &Ctx = D->getASTContext(); @@ -321,12 +323,10 @@ HI.LocalScope.append("::"); PrintingPolicy Policy = printingPolicyForDecls(Ctx.getPrintingPolicy()); - if (const NamedDecl *ND = llvm::dyn_cast(D)) { - HI.Name = printName(Ctx, *ND); - ND = getDeclForComment(ND); - HI.Documentation = getDeclComment(Ctx, *ND); - enhanceFromIndex(HI, *ND, Index); - } + HI.Name = printName(Ctx, *D); + const auto *CommentD = getDeclForComment(D); + HI.Documentation = getDeclComment(Ctx, *CommentD); + enhanceFromIndex(HI, *CommentD, Index); HI.Kind = index::getSymbolInfo(D).Kind; @@ -460,34 +460,70 @@ tooling::applyAllReplacements(HI->Definition, Replacements)) HI->Definition = *Formatted; - HI->SymRange = getTokenRange(AST.getSourceManager(), - AST.getLangOpts(), SourceLocationBeg); + HI->SymRange = getTokenRange(AST.getSourceManager(), AST.getLangOpts(), + SourceLocationBeg); return HI; } markup::Document HoverInfo::present() const { markup::Document Output; - if (NamespaceScope) { - auto &P = Output.addParagraph(); - P.appendText("Declared in"); - // Drop trailing "::". - if (!LocalScope.empty()) - P.appendCode(llvm::StringRef(LocalScope).drop_back(2)); - else if (NamespaceScope->empty()) - P.appendCode("global namespace"); - else - P.appendCode(llvm::StringRef(*NamespaceScope).drop_back(2)); + // Header contains a text of the form: + // variable `var` : `int` + // + // class `X` + // + // function `foo` → `int` + markup::Paragraph &Header = Output.addParagraph(); + 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); } - if (!Definition.empty()) { - Output.addCodeBlock(Definition); - } else { - // Builtin types - Output.addCodeBlock(Name); + // For functions we display signature in a list form, e.g.: + // - `bool param1` + // - `int param2 = 5` + if (Parameters && !Parameters->empty()) { + 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())); + } + } + + if (Value) { + markup::Paragraph &P = Output.addParagraph(); + P.appendText("Value: "); + P.appendCode(*Value); } if (!Documentation.empty()) Output.addParagraph().appendText(Documentation); + + if (!Definition.empty()) { + std::string ScopeComment; + // Drop trailing "::". + if (!LocalScope.empty()) { + // Container name, e.g. class, method, function. + // We might want to propogate some info about container type to print + // function foo, class X, method X::bar, etc. + ScopeComment = + "// In " + llvm::StringRef(LocalScope).rtrim(':').str() + '\n'; + } else if (NamespaceScope && !NamespaceScope->empty()) { + ScopeComment = "// In namespace " + + llvm::StringRef(*NamespaceScope).rtrim(':').str() + '\n'; + } + // Note that we don't print anything for global namespace, to not annoy + // non-c++ projects or projects that are not making use of namespaces. + Output.addCodeBlock(ScopeComment + Definition); + } return Output; } diff --git a/clang-tools-extra/clangd/test/hover.test b/clang-tools-extra/clangd/test/hover.test --- a/clang-tools-extra/clangd/test/hover.test +++ b/clang-tools-extra/clangd/test/hover.test @@ -9,7 +9,7 @@ # CHECK-NEXT: "result": { # CHECK-NEXT: "contents": { # CHECK-NEXT: "kind": "plaintext", -# CHECK-NEXT: "value": "Declared in global namespace\n\nvoid foo()" +# CHECK-NEXT: "value": "function foo → void\n\nvoid foo()" # CHECK-NEXT: }, # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { @@ -37,7 +37,7 @@ # CHECK-NEXT: "result": { # CHECK-NEXT: "contents": { # CHECK-NEXT: "kind": "plaintext", -# CHECK-NEXT: "value": "Declared in global namespace\n\nenum foo {}" +# CHECK-NEXT: "value": "enum foo\n\nenum foo {}" # CHECK-NEXT: }, # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp --- a/clang-tools-extra/clangd/unittests/HoverTests.cpp +++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp @@ -1606,6 +1606,95 @@ } } } +TEST(Hover, Present) { + struct { + const std::function Builder; + llvm::StringRef ExpectedRender; + } Cases[] = { + { + [](HoverInfo &HI) { + HI.Kind = index::SymbolKind::Unknown; + HI.Name = "X"; + }, + R"( X)", + }, + { + [](HoverInfo &HI) { + HI.Kind = index::SymbolKind::NamespaceAlias; + HI.Name = "foo"; + }, + R"(namespace-alias foo)", + }, + { + [](HoverInfo &HI) { + HI.Kind = index::SymbolKind::Class; + HI.TemplateParameters = { + {std::string("typename"), std::string("T"), llvm::None}, + {std::string("typename"), std::string("C"), + std::string("bool")}, + }; + HI.Documentation = "documentation"; + HI.Definition = + "template class Foo {}"; + HI.Name = "foo"; + HI.NamespaceScope.emplace(); + }, + R"(class foo +documentation + +template class Foo {})", + }, + { + [](HoverInfo &HI) { + HI.Kind = index::SymbolKind::Function; + HI.Name = "foo"; + HI.Type = "type"; + HI.ReturnType = "ret_type"; + HI.Parameters.emplace(); + HoverInfo::Param P; + HI.Parameters->push_back(P); + P.Type = "type"; + 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 +- +- 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 +Value: value + +// In test::bar +def)", + }, + }; + + for (const auto &C : Cases) { + HoverInfo HI; + C.Builder(HI); + EXPECT_EQ(HI.present().asPlainText(), C.ExpectedRender); + } +} + } // namespace } // namespace clangd } // namespace clang