[clangd] Introduce a structured hover response

[kadircet]

Summary

Change ClangdServer layer to output a structured response for Hover,
which can be rendered by client according to their needs.

[kadircet]

This is still WIP, just looking for feedbacks on HoverInfo struct and overall layering

[sammccall]

Layering and such looks good. This should compose well with D58547

clang-tools-extra/clangd/XRefs.h

52 ↗

(On Diff #197979)

I'm not sure about reuse of LocatedSymbol - do we want to commit to returning decl/def locations?
Name might be enough here.

54 ↗

(On Diff #197979)

This comes from LSP, and within the scope of C++ I think we might want stronger semantics here.
I think it's likely we actually want to show namespace vs class scope differently, too - maybe these should be separate fields?

55 ↗

(On Diff #197979)

SymbolInfo is a bit of a mess. Maybe we just want SymbolInfo::Kind? (LSP SymbolKind is tempting but loses a lot of info for C++).

I do think we'll need to extend SymbolInfo::Kind or eventually use our own enum, e.g. lambdas may need their own kind (they're variables, but don't have a printable type and need to be displayed more like functions)

57 ↗

(On Diff #197979)

I think we probably want a struct to represent types, so we can annotate it with links later on if needed. (because type can be e.g. mytemplate<mytype>). This wouldn't matter except we should probably reuse it...

57 ↗

(On Diff #197979)

we may want ReturnType too (unless you want to overload Type for that - I'd suggest against it because of e.g. lambdas)

59 ↗

(On Diff #197979)

maybe vector<param> params, where param is struct { string name; Type type }?

We might render as:

---

Parameters:

• value: std::string
• size: int (default = 0)

63 ↗

(On Diff #197979)

TemplateArgs might want to follow the same structure as params?

[kadircet]

• Address comments

[kadircet]

clang-tools-extra/clangd/XRefs.h

52 ↗

(On Diff #197979)

It might be nice to provide editors with enough info to jump to definition(it was brought up during last meeting). But happy to reduce it to just name.

55 ↗

(On Diff #197979)

As you mentioned we might need to extend LSP's enum, but switching to it anyway. Currently it doesn't support "macro" kind exactly.

[ilya-biryukov]

Was passing by, just a small clarifying question...

clang-tools-extra/clangd/XRefs.h

73 ↗

(On Diff #198295)

What does Type mean for non-type and template template parameters?
Could you add a comment?

[kadircet]

clang-tools-extra/clangd/XRefs.h

73 ↗

(On Diff #198295)

added comment for template template parameters.

For non-type template params, isn't it clear that this will hold the type of that param? e.g template <C<int>... X> -> C<int>...

[kadircet]

• Add comments for non-type params

[ilya-biryukov]

clang-tools-extra/clangd/XRefs.h

73 ↗

(On Diff #198295)

Ah, sure, sorry, I meant the type parameters :-)

[ilya-biryukov]

clang-tools-extra/clangd/XRefs.h

57 ↗

(On Diff #198420)

Same about type template parameters.
Probably just holds class or typename?

[kadircet]

I will start adding test cases(and most likely change the way I populate the
struct at some places) if everyone is happy with current representation for
HoverInfo.

clang-tools-extra/clangd/XRefs.h

57 ↗

(On Diff #198420)

Exactly, adding comments.

[ilya-biryukov]

clang-tools-extra/clangd/XRefs.h

53 ↗

(On Diff #198457)

NIT: maybe go with std::string Type at a use-site instead?
The scope of the name is large enough to make single-letter names a bit confusing

[sammccall]

(I only got about halfway through the implementation - it's missing a lot of comments I think ;-))

clang-tools-extra/clangd/XRefs.cpp

539 ↗

(On Diff #198457)

I think this is basically ASTContext::getTypeDeclType()?

549 ↗

(On Diff #198457)

what does this have to do with decls?

563 ↗

(On Diff #198457)

printDefinition?

574 ↗

(On Diff #198457)

I don't think it's reasonable to define this private helper as an overload of operator<<.
Make it a function, or inline it? (I think used only once)

1207 ↗

(On Diff #198457)

avoid emitting the space if T/name are empty?

1214 ↗

(On Diff #198457)

This doesn't seem sufficiently self-documenting.

clang-tools-extra/clangd/XRefs.h

52 ↗

(On Diff #197979)

(this is not done I think - LocatedSymbol is still here)

73 ↗

(On Diff #198295)

I get the curiosity, but this is too much text, and only covers template parameters which are not the most important case.

Can we just say something like The pretty-printed parameter type, e.g. "int", or "typename" (in TemplateParameters)?

We should do something sensible for template template parameters, but it's not important or non-obvious enough to document.

52 ↗

(On Diff #198457)

This needs docs :-)

54 ↗

(On Diff #198457)

one line doc for this struct?

58 ↗

(On Diff #198457)

needs a real name
mention the no-type case (macros)

59 ↗

(On Diff #198457)

mention the unnamed case

60 ↗

(On Diff #198457)

mention the no-default case

64 ↗

(On Diff #198457)

qualiffied -> qualified

65 ↗

(On Diff #198457)

what's the difference between Scope and ParentScope?
Can we give them more obvious names, or a comment?
(The current comment doesn't really say anything)

69 ↗

(On Diff #198457)

This sounds like it's a source location, (e.g. "file.cc:42:7")
But I think it's rather source code?

72 ↗

(On Diff #198457)

I'm not sure this is significant or should even be true (e.g. if T is a function reference?)

Could we explain the relationship between the different fields here instead, maybe briefly with examples? (A function will have return type and parameters set, etc)

73 ↗

(On Diff #198457)

T needs a real name.

[sammccall]

clang-tools-extra/clangd/XRefs.h

53 ↗

(On Diff #198457)

We had discussed making Type a struct, in case we want to add links etc to it later in a back-compatible way.

The typedef doesn't achieve that, I agree with just inlining std::string if we're not going to have the struct.

[kadircet]

• Address comments
• Make little modifications to existing tests

clang-tools-extra/clangd/XRefs.cpp

574 ↗

(On Diff #198457)

it is also used within render, making it a function

clang-tools-extra/clangd/XRefs.h

65 ↗

(On Diff #198457)

We've been using Name and Scope to represent two parts of qualified names across the codebase.
Comment was unfortunately misplaced :/ LMK if you have any other choices for the name

[kadircet]

clang-tools-extra/clangd/XRefs.cpp

539 ↗

(On Diff #198457)

No longer needed

549 ↗

(On Diff #198457)

No longer needed

1214 ↗

(On Diff #198457)

No longer needed

[sammccall]

comments on interface again, will take another pass at implementation

clang-tools-extra/clangd/XRefs.h

65 ↗

(On Diff #198457)

Yes, but here you've got three concepts: name/scope/parentscope (now called containedin?). And this covers lots of cases that our existing Name/Scope doesn't really (there are lots of symbols we don't index).

It wasn't a rhetorical question, I can't suggest better names because I don't know what they are.

My suggestion would be to split into namespace scope/local scope/name, such that Name has no ::, NamespaceScope has exactly the (non-inline) enclosing namespaces, and NamespaceScope + LocalScope + Name == QName.

So for a local in a class member, NamespaceScope == "clang::clangd"", LocalScope == "SymbolCollector::consume:: and Name = Symbols. Or just Name = Symbols and the others are blank, depending on how we choose to display locals.

53 ↗

(On Diff #198630)

I'm having trouble following the "serialized" sentence. Maybe "It can be rendered as a hover panel, or embedding clients can use the structured information to provide their own UI"?

76 ↗

(On Diff #198630)

This seems overlapping/non-orthogonal with scope. It doesn't let us render a member as ClassName::method for instance.
If we really need "In function x::Y" rather than "in x::Y", we could add a member "ImmediateScopeKind" or something. But I'm not sure we do.

89 ↗

(On Diff #198630)

nit: just "templates"?

(e.g. if we hover over a call to std::swap, we might be showing the instantiation rather than the declaration?)

93 ↗

(On Diff #198630)

Even if this struct ends up containing the range, it might be more obvious to have render() produce the MarkupContent only, leaving the caller to pull out the range themselves.

Converting the range isn't closely related to rendering, and I think for cleanest layering/testing you want to return FormattedString after rL360151 (which doesn't have a Hover equivalent).

[sammccall]

Main comment is that I think the code is doing too much work to exactly reproduce the current output, and include as much information as possible.
Minimizing the diff is good all else equal, but one of the goals here is to have richer hovercards that are more consistent across the different codepaths that generate them.

clang-tools-extra/clangd/XRefs.cpp

456 ↗

(On Diff #198630)

This has been pulled from DeclPrinter, so by definition duplicates info in the printed decl. We should only do this for info that's important, and I'm not sure most of this meets the bar.

• that a function is virtual is certainly important (but isVirtual(), not isVirtualAsWritten())
• that the declaration we found for a function is extern is certainly unimportant (it says something about the *decl*, not the function)
• that a function is static means *something* important, but being a non-instance member is pretty different from being an internal helper function. Saying Kind=StaticMethod seems more useful for presentation than putting "static" next to the type again.

Aside from this, the current implementation puts these specifiers in the type, which they're not - functions don't return virtual ints or static strings.

I think we can probably drop these for now, and leave them in the printed decl only. We may want to revisit this as part of having Kind describe results more precisely.

726 ↗

(On Diff #198630)

what are the storage class specifiers that are relevant to function parameters?

737 ↗

(On Diff #198630)

Not totally sure this is the best way to model variadics. We could consider leaving it out for now.

1201 ↗

(On Diff #198630)

This really seems like the wrong idea to me.
"Kind" is a category description suitable to be shown to the user, it's supposed to be useful.
In odd cases we may choose to do things like not show the type if the kind is function.

But HoverInfo is basically a box of facts to render, and using totally different rendering strategies for different kinds (and assuming strings mean macros) cuts against this.

1209 ↗

(On Diff #198630)

why do we have this no-definition case?
Please avoid reinventing code to print C++ syntax if possible...

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

940 ↗

(On Diff #198630)

hmm, this is a bit weird - this uses specialization syntax but there's no actual specialization here right?
I think the old output without template<> is probably better if possible.

[kadircet]

Thanks for also taking a look at the implementation, it is not complete yet. I am rather waiting for a green light on struct itself, so that I can write tests.

clang-tools-extra/clangd/XRefs.cpp

726 ↗

(On Diff #198630)

i suppose only "register" is allowed in the context of a function parameter. but doesn't matter now as requested above, leaving specifiers out.

1209 ↗

(On Diff #198630)

oops, this branch is a leftover :/

clang-tools-extra/clangd/XRefs.h

89 ↗

(On Diff #198630)

Existing hover behavior is to show declaration, except auto, in that case we might show instantiations.

But you are right it is not necessarily only declarations.

93 ↗

(On Diff #198630)

I don't think caller of this method would have enough context to deduce symbol's range so leaving the Range in the struct, but changing the output

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

940 ↗

(On Diff #198630)

The old behavior was inconsistent in the case of auto. We print the decl in all cases, but print the type in the case of auto. For example, if you had
initializer_list<int> i = {1,2} instead of auto i = {1,2} you would get the new response I've provided in this test.

I agree this looks weird in the case of instantiations but I believe it is more important to give a consistent look.

[kadircet]

• Address comments

[kadircet]

• Added tests, and went over implementation.

I am looking for input on:

• Behavior on lambdas, currently it just keeps the old behaviour. I believe they should look more like functions. Do you think it is necessary for first patch?
• Template classes/functions, what we have in definition is not great but I don't think it is too much of a problem since semantic representation carries all the useful information.
• Declared in #SOME_KIND# #SOME_PLACE#, currently we drop SOME_KIND and it needs additional info to be propogated since it is not the type of the symbol but rather it is immediate container. I don't think it is necessary for initial version.
• Discrepencies between hovering over auto/decltype and explicitly spelled types, again this keeps the existing behaviour. You can see details in testcases.

[kadircet]

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

940 ↗

(On Diff #198630)

as discussed offline, leaving this as it is in the initial version and adding tests with both auto and non-auto cases.

[sammccall]

Thanks, I think you're right about all of this. Implementation looks OK too. Please add TODOs for the cases where we're punting to later vs deciding not to fix.

In D61497#1512003, @kadircet wrote:

• Behavior on lambdas, currently it just keeps the old behaviour. I believe they should look more like functions. Do you think it is necessary for first patch?

Agree. Not necessary in first patch IMO. Add a TODO though.

• Template classes/functions, what we have in definition is not great but I don't think it is too much of a problem since semantic representation carries all the useful information.

These look OK to me, though examples are complicated and I might be missing something.

• Declared in #SOME_KIND# #SOME_PLACE#, currently we drop SOME_KIND and it needs additional info to be propogated since it is not the type of the symbol but rather it is immediate container. I don't think it is necessary for initial version.

This seems OK to me. As noted you can keep it in the case that it's "namespace". If you want to propagate the container kind in this patch that's also fine.

• Discrepencies between hovering over auto/decltype and explicitly spelled types, again this keeps the existing behaviour. You can see details in testcases.

Testcases look pretty good I think.

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

567 ↗

(On Diff #200732)

namespace?!

might be a clang-format bug?

574 ↗

(On Diff #200732)

using the constructor/aggregate init here has a couple of downsides:

• vebosity: you need to list every field, even those that are not set for this case (TemplateParameters), or are misleading (SymRange is none)
• brittleness: it makes it hard to change the struct at all
• readability: the /*foo=*/ comments aren't bad, but not ideal either

Using field-by-field initialization would be better I think, though it's a bit awkward here.

but e.g. you could make the member std::function<HoverInfo> ExpectedBuilder, and write

[&](HoverInfo &Expected) {
  Expected.Name = "foo";
  Expected.Kind = SymbolKind::Function;
  ...
}

592 ↗

(On Diff #200732)

I think this should be "ns1::ns2::", as we use scope internally.
This means we can simply concatenate parts to form a qname.

For the current rendering, it's easy to strip ::

599 ↗

(On Diff #200732)

It would be nice to add void() or void (&)() or so if it's easy.
This is what :YcmCompleter GetType would show

678 ↗

(On Diff #200732)

can we have a class template example where the instantiation isn't templated?
e.g.

template<typename T> class vector{};
[[vector]]<int> X;

(just because this is an example that we've discussed a lot, and these edge-case tests make it hard to see the common behavior)

782 ↗

(On Diff #200732)

is it easy to omit the body?

951 ↗

(On Diff #200732)

note that if you want to it's easy to keep "Declared in namespace" - the LocalScope is empty.
(Though we can't provide this info for non-namespace containers)

[kadircet]

• Address comments

[kadircet]

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

567 ↗

(On Diff #200732)

I must have messed the brackets at some point :D

574 ↗

(On Diff #200732)

yeah that looks a lot better, thanks!

592 ↗

(On Diff #200732)

should it also be "::" for global namespace ? which would also result in prefixing any symbol in global namespace with "::" when printing.

599 ↗

(On Diff #200732)

just put the type without any parameter names, but I am not sure whether users would want that. I believe people find current hover behavior a lot more useful then just showing type(which is done by libclang)

782 ↗

(On Diff #200732)

yes, sent out D62487

951 ↗

(On Diff #200732)

I believe it doesn't make sense to add only one, so leaving it until we hear from users that they need the kind information

[sammccall]

Still LG

I forgot to mention - it would be nice to clang-format the definition, what do you think?

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

592 ↗

(On Diff #200732)

We tend to use empty string for global namespace, and explicitly add it where we need it.
Using :: for global causes as many problems as it solves (e.g. in C).

599 ↗

(On Diff #200732)

I don't think we should include it in the actual hover, but we can handle that in rendering (if it's a Function, don't print the type).
It does seem odd not to include it in the structured API, but up to you.

I thought specifically the :GetType might still be useful, but maybe not.

[kadircet]

• Address comments

[kadircet]

• clang-format Definition

clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp

void ClangdLSPServer::onDocumentHighlight(
     Callback<std::vector<DocumentHighlight>> Reply) {
   Server->findDocumentHighlights(Params.textDocument.uri.file(),
                                  Params.position, std::move(Reply));
 }
 
 void ClangdLSPServer::onHover(const TextDocumentPositionParams &Params,
                               Callback<llvm::Optional<Hover>> Reply) {
   Server->findHover(Params.textDocument.uri.file(), Params.position,
-                    std::move(Reply));
+                    Bind(
+                        [](decltype(Reply) Reply,
+                           llvm::Expected<llvm::Optional<HoverInfo>> HIorErr) {
+                          if (!HIorErr)
+                            return Reply(HIorErr.takeError());
+                          const auto &HI = HIorErr.get();
+                          if (!HI)
+                            return Reply(llvm::None);
+                          Hover H;
+                          H.range = HI->SymRange;
+                          H.contents = HI->render();
+                          return Reply(H);
+                        },
+                        std::move(Reply)));
 }
 
 void ClangdLSPServer::onTypeHierarchy(
     const TypeHierarchyParams &Params,
     Callback<Optional<TypeHierarchyItem>> Reply) {
   Server->typeHierarchy(Params.textDocument.uri.file(), Params.position,
                         Params.resolve, Params.direction, std::move(Reply));
 }

clang-tools-extra/trunk/clangd/ClangdServer.h

public:
   llvm::Optional<Path> switchSourceHeader(PathRef Path);
 
   /// Get document highlights for a given position.
   void findDocumentHighlights(PathRef File, Position Pos,
                               Callback<std::vector<DocumentHighlight>> CB);
 
   /// Get code hover for a given position.
   void findHover(PathRef File, Position Pos,
-                 Callback<llvm::Optional<Hover>> CB);
+                 Callback<llvm::Optional<HoverInfo>> CB);
 
   /// Get information about type hierarchy for a given position.
   void typeHierarchy(PathRef File, Position Pos, int Resolve,
                      TypeHierarchyDirection Direction,
                      Callback<llvm::Optional<TypeHierarchyItem>> CB);
 
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,

clang-tools-extra/trunk/clangd/ClangdServer.cpp

if (!InpAST)
       return CB(InpAST.takeError());
     CB(clangd::findDocumentHighlights(InpAST->AST, Pos));
   };
 
   WorkScheduler.runWithAST("Highlights", File, Bind(Action, std::move(CB)));
 }
 
 void ClangdServer::findHover(PathRef File, Position Pos,
-                             Callback<llvm::Optional<Hover>> CB) {
-  auto Action = [Pos](Callback<llvm::Optional<Hover>> CB,
+                             Callback<llvm::Optional<HoverInfo>> CB) {
+  auto Action = [Pos](Callback<llvm::Optional<HoverInfo>> CB, Path File,
                       llvm::Expected<InputsAndAST> InpAST) {
     if (!InpAST)
       return CB(InpAST.takeError());
-    CB(clangd::getHover(InpAST->AST, Pos));
+    format::FormatStyle Style = getFormatStyleForFile(
+        File, InpAST->Inputs.Contents, InpAST->Inputs.FS.get());
+    CB(clangd::getHover(InpAST->AST, Pos, std::move(Style)));
   };
 
-  WorkScheduler.runWithAST("Hover", File, Bind(Action, std::move(CB)));
+  WorkScheduler.runWithAST("Hover", File,
+                           Bind(Action, std::move(CB), File.str()));
 }
 
 void ClangdServer::typeHierarchy(PathRef File, Position Pos, int Resolve,
                                  TypeHierarchyDirection Direction,
                                  Callback<Optional<TypeHierarchyItem>> CB) {
   auto Action = [Pos, Resolve, Direction](decltype(CB) CB,
                                           Expected<InputsAndAST> InpAST) {
     if (!InpAST)

clang-tools-extra/trunk/clangd/Protocol.cpp

 
 #include "Protocol.h"
 #include "Logger.h"
 #include "URI.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace clangd {

clang-tools-extra/trunk/clangd/XRefs.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_XREFS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_XREFS_H
 
 #include "ClangdUnit.h"
 #include "Protocol.h"
 #include "index/Index.h"
+#include "index/SymbolLocation.h"
+#include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/Support/raw_ostream.h"
 #include <vector>
 
 namespace clang {
 namespace clangd {
 
 // Describes where a symbol is declared and defined (as far as clangd knows).
 // There are three cases:
 //  - a declaration only, no definition is known (e.g. only header seen)

 /// Multiple locations may be returned, corresponding to distinct symbols.
 std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
                                           const SymbolIndex *Index = nullptr);
 
 /// Returns highlights for all usages of a symbol at \p Pos.
 std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST,
                                                       Position Pos);
 
+/// Contains detailed information about a Symbol. Especially useful when
+/// generating hover responses. It can be rendered as a hover panel, or
+/// embedding clients can use the structured information to provide their own
+/// UI.
+struct HoverInfo {
+  /// Represents parameters of a function, a template or a macro.
+  /// For example:
+  /// - void foo(ParamType Name = DefaultValue)
+  /// - #define FOO(Name)
+  /// - template <ParamType Name = DefaultType> class Foo {};
+  struct Param {
+    /// The pretty-printed parameter type, e.g. "int", or "typename" (in
+    /// TemplateParameters)
+    llvm::Optional<std::string> Type;
+    /// None for unnamed parameters.
+    llvm::Optional<std::string> Name;
+    /// None if no default is provided.
+    llvm::Optional<std::string> Default;
+  };
+
+  /// For a variable named Bar, declared in clang::clangd::Foo::getFoo the
+  /// following fields will hold:
+  /// - NamespaceScope: clang::clangd::
+  /// - LocalScope: Foo::getFoo::
+  /// - Name: Bar
+
+  /// Scopes might be None in cases where they don't make sense, e.g. macros and
+  /// auto/decltype.
+  /// Contains all of the enclosing namespaces, empty string means global
+  /// namespace.
+  llvm::Optional<std::string> NamespaceScope;
+  /// Remaining named contexts in symbol's qualified name, empty string means
+  /// symbol is not local.
+  std::string LocalScope;
+  /// Name of the symbol, does not contain any "::".
+  std::string Name;
+  llvm::Optional<Range> SymRange;
+  /// Scope containing the symbol. e.g, "global namespace", "function x::Y"
+  /// - None for deduced types, e.g "auto", "decltype" keywords.
+  SymbolKind Kind;
+  std::string Documentation;
+  /// Source code containing the definition of the symbol.
+  std::string Definition;
+
+  /// Pretty-printed variable type.
+  /// Set only for variables.
+  llvm::Optional<std::string> Type;
+  /// Set for functions and lambadas.
+  llvm::Optional<std::string> ReturnType;
+  /// Set for functions, lambdas and macros with parameters.
+  llvm::Optional<std::vector<Param>> Parameters;
+  /// Set for all templates(function, class, variable).
+  llvm::Optional<std::vector<Param>> TemplateParameters;
+
+  /// Lower to LSP struct.
+  MarkupContent render() const;
+};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const HoverInfo::Param &);
+inline bool operator==(const HoverInfo::Param &LHS,
+                       const HoverInfo::Param &RHS) {
+  return std::tie(LHS.Type, LHS.Name, LHS.Default) ==
+         std::tie(RHS.Type, RHS.Name, RHS.Default);
+}
+
 /// Get the hover information when hovering at \p Pos.
-llvm::Optional<Hover> getHover(ParsedAST &AST, Position Pos);
+llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
+                                   format::FormatStyle Style);
 
 /// Returns reference locations of the symbol at a specified \p Pos.
 /// \p Limit limits the number of results returned (0 means no limit).
 std::vector<Location> findReferences(ParsedAST &AST, Position Pos,
                                      uint32_t Limit,
                                      const SymbolIndex *Index = nullptr);
 
 /// Get info about symbols at \p Pos.

clang-tools-extra/trunk/clangd/XRefs.cpp

 //===--- XRefs.cpp -----------------------------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #include "XRefs.h"
 #include "AST.h"
+#include "CodeCompletionStrings.h"
 #include "FindSymbols.h"
 #include "Logger.h"
+#include "Protocol.h"
 #include "SourceCode.h"
 #include "URI.h"
 #include "index/Merge.h"
 #include "index/SymbolCollector.h"
 #include "index/SymbolLocation.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Index/USRGeneration.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 
 // Returns the single definition of the entity declared by D, if visible.
 // In particular:
 // - for non-redeclarable kinds (e.g. local vars), return D
IdentifiedSymbol getSymbolAtPosition(ParsedAST &AST, SourceLocation Pos) {
   IndexOpts.IndexParametersInDeclarations = true;
   IndexOpts.IndexTemplateParameters = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
                      AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 
   return {DeclMacrosFinder.getFoundDecls(), DeclMacrosFinder.takeMacroInfos()};
 }
 
-Range getTokenRange(ParsedAST &AST, SourceLocation TokLoc) {
-  const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
-  SourceLocation LocEnd = Lexer::getLocForEndOfToken(
-      TokLoc, 0, SourceMgr, AST.getASTContext().getLangOpts());
+Range getTokenRange(ASTContext &AST, SourceLocation TokLoc) {
+  const SourceManager &SourceMgr = AST.getSourceManager();
+  SourceLocation LocEnd =
+      Lexer::getLocForEndOfToken(TokLoc, 0, SourceMgr, AST.getLangOpts());
   return {sourceLocToPosition(SourceMgr, TokLoc),
           sourceLocToPosition(SourceMgr, LocEnd)};
 }
 
-llvm::Optional<Location> makeLocation(ParsedAST &AST, SourceLocation TokLoc,
+llvm::Optional<Location> makeLocation(ASTContext &AST, SourceLocation TokLoc,
                                       llvm::StringRef TUPath) {
-  const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
+  const SourceManager &SourceMgr = AST.getSourceManager();
   const FileEntry *F = SourceMgr.getFileEntryForID(SourceMgr.getFileID(TokLoc));
   if (!F)
     return None;
   auto FilePath = getCanonicalPath(F, SourceMgr);
   if (!FilePath) {
     log("failed to get path!");
     return None;
   }
std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
   SourceLocation SourceLocationBeg =
       getBeginningOfIdentifier(AST, Pos, SM.getMainFileID());
   auto Symbols = getSymbolAtPosition(AST, SourceLocationBeg);
 
   // Macros are simple: there's no declaration/definition distinction.
   // As a consequence, there's no need to look them up in the index either.
   std::vector<LocatedSymbol> Result;
   for (auto M : Symbols.Macros) {
-    if (auto Loc =
-            makeLocation(AST, M.Info->getDefinitionLoc(), *MainFilePath)) {
+    if (auto Loc = makeLocation(AST.getASTContext(), M.Info->getDefinitionLoc(),
+                                *MainFilePath)) {
       LocatedSymbol Macro;
       Macro.Name = M.Name;
       Macro.PreferredDeclaration = *Loc;
       Macro.Definition = Loc;
       Result.push_back(std::move(Macro));
     }
   }
 
   // Decls are more complicated.
   // The AST contains at least a declaration, maybe a definition.
   // These are up-to-date, and so generally preferred over index results.
   // We perform a single batch index lookup to find additional definitions.
 
   // Results follow the order of Symbols.Decls.
   // Keep track of SymbolID -> index mapping, to fill in index data later.
   llvm::DenseMap<SymbolID, size_t> ResultIndex;
 
   // Emit all symbol locations (declaration or definition) from AST.
   for (const Decl *D : Symbols.Decls) {
-    auto Loc = makeLocation(AST, findNameLoc(D), *MainFilePath);
+    auto Loc = makeLocation(AST.getASTContext(), findNameLoc(D), *MainFilePath);
     if (!Loc)
       continue;
 
     Result.emplace_back();
     if (auto *ND = dyn_cast<NamedDecl>(D))
       Result.back().Name = printName(AST.getASTContext(), *ND);
     Result.back().PreferredDeclaration = *Loc;
     // DeclInfo.D is always a definition if possible, so this check works.
std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST,
   const SourceManager &SM = AST.getASTContext().getSourceManager();
   auto Symbols = getSymbolAtPosition(
       AST, getBeginningOfIdentifier(AST, Pos, SM.getMainFileID()));
   auto References = findRefs(Symbols.Decls, AST);
 
   std::vector<DocumentHighlight> Result;
   for (const auto &Ref : References) {
     DocumentHighlight DH;
-    DH.range = getTokenRange(AST, Ref.Loc);
+    DH.range = getTokenRange(AST.getASTContext(), Ref.Loc);
     if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Write))
       DH.kind = DocumentHighlightKind::Write;
     else if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Read))
       DH.kind = DocumentHighlightKind::Read;
     else
       DH.kind = DocumentHighlightKind::Text;
     Result.push_back(std::move(DH));
   }
   return Result;
 }
 
 static PrintingPolicy printingPolicyForDecls(PrintingPolicy Base) {
   PrintingPolicy Policy(Base);
 
   Policy.AnonymousTagLocations = false;
   Policy.TerseOutput = true;
   Policy.PolishForDeclaration = true;
   Policy.ConstantsAsWritten = true;
   Policy.SuppressTagKeyword = false;
 
   return Policy;
 }
 
-/// Return a string representation (e.g. "class MyNamespace::MyClass") of
-/// the type declaration \p TD.
-static std::string typeDeclToString(const TypeDecl *TD) {
-  QualType Type = TD->getASTContext().getTypeDeclType(TD);
-
-  PrintingPolicy Policy =
-      printingPolicyForDecls(TD->getASTContext().getPrintingPolicy());
-
-  std::string Name;
-  llvm::raw_string_ostream Stream(Name);
-  Type.print(Stream, Policy);
-
-  return Stream.str();
-}
-
-/// Return a string representation (e.g. "namespace ns1::ns2") of
-/// the named declaration \p ND.
-static std::string namedDeclQualifiedName(const NamedDecl *ND,
-                                          llvm::StringRef Prefix) {
-  PrintingPolicy Policy =
-      printingPolicyForDecls(ND->getASTContext().getPrintingPolicy());
-
-  std::string Name;
-  llvm::raw_string_ostream Stream(Name);
-  Stream << Prefix << ' ';
-  ND->printQualifiedName(Stream, Policy);
-
-  return Stream.str();
-}
-
 /// Given a declaration \p D, return a human-readable string representing the
-/// scope in which it is declared.  If the declaration is in the global scope,
-/// return the string "global namespace".
-static llvm::Optional<std::string> getScopeName(const Decl *D) {
+/// local scope in which it is declared, i.e. class(es) and method name. Returns
+/// an empty string if it is not local.
+static std::string getLocalScope(const Decl *D) {
+  std::vector<std::string> Scopes;
   const DeclContext *DC = D->getDeclContext();
-
-  if (isa<TranslationUnitDecl>(DC))
-    return std::string("global namespace");
+  auto GetName = [](const Decl *D) {
+    const NamedDecl *ND = dyn_cast<NamedDecl>(D);
+    std::string Name = ND->getNameAsString();
+    if (!Name.empty())
+      return Name;
+    if (auto RD = dyn_cast<RecordDecl>(D))
+      return ("(anonymous " + RD->getKindName() + ")").str();
+    return std::string("");
+  };
+  while (DC) {
     if (const TypeDecl *TD = dyn_cast<TypeDecl>(DC))
-    return typeDeclToString(TD);
-  else if (const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC))
-    return namedDeclQualifiedName(ND, "namespace");
+      Scopes.push_back(GetName(TD));
     else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(DC))
-    return namedDeclQualifiedName(FD, "function");
-
-  return None;
+      Scopes.push_back(FD->getNameAsString());
+    DC = DC->getParent();
   }
 
-/// Generate a \p Hover object given the declaration \p D.
-static Hover getHoverContents(const Decl *D) {
-  Hover H;
-  llvm::Optional<std::string> NamedScope = getScopeName(D);
-
-  // Generate the "Declared in" section.
-  if (NamedScope) {
-    assert(!NamedScope->empty());
-
-    H.contents.value += "Declared in ";
-    H.contents.value += *NamedScope;
-    H.contents.value += "\n\n";
-  }
+  return llvm::join(llvm::reverse(Scopes), "::");
 
-  // We want to include the template in the Hover.
-  if (TemplateDecl *TD = D->getDescribedTemplate())
-    D = TD;
+/// Returns the human-readable representation for namespace containing the
+/// declaration \p D. Returns empty if it is contained global namespace.
+static std::string getNamespaceScope(const Decl *D) {
+  const DeclContext *DC = D->getDeclContext();
 
-  std::string DeclText;
-  llvm::raw_string_ostream OS(DeclText);
+  if (const TypeDecl *TD = dyn_cast<TypeDecl>(DC))
+    return getNamespaceScope(TD);
+  if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(DC))
+    return getNamespaceScope(FD);
+  if (const NamedDecl *ND = dyn_cast<NamedDecl>(DC))
+    return ND->getQualifiedNameAsString();
+
+  return "";
+}
 
+static std::string printDefinition(const Decl *D) {
+  std::string Definition;
+  llvm::raw_string_ostream OS(Definition);
   PrintingPolicy Policy =
       printingPolicyForDecls(D->getASTContext().getPrintingPolicy());
-
+  Policy.IncludeTagDefinition = false;
   D->print(OS, Policy);
+  return Definition;
+}
+
+static void printParams(llvm::raw_ostream &OS,
+                        const std::vector<HoverInfo::Param> &Params) {
+  for (size_t I = 0, E = Params.size(); I != E; ++I) {
+    if (I)
+      OS << ", ";
+    OS << Params.at(I);
+  }
+}
+
+static std::vector<HoverInfo::Param>
+fetchTemplateParameters(const TemplateParameterList *Params,
+                        const PrintingPolicy &PP) {
+  assert(Params);
+  std::vector<HoverInfo::Param> TempParameters;
+
+  for (const Decl *Param : *Params) {
+    HoverInfo::Param P;
+    P.Type.emplace();
+    if (const auto TTP = dyn_cast<TemplateTypeParmDecl>(Param)) {
+      P.Type = TTP->wasDeclaredWithTypename() ? "typename" : "class";
+      if (TTP->isParameterPack())
+        *P.Type += "...";
+
+      if (!TTP->getName().empty())
+        P.Name = TTP->getNameAsString();
+      if (TTP->hasDefaultArgument())
+        P.Default = TTP->getDefaultArgument().getAsString(PP);
+    } else if (const auto NTTP = dyn_cast<NonTypeTemplateParmDecl>(Param)) {
+      if (IdentifierInfo *II = NTTP->getIdentifier())
+        P.Name = II->getName().str();
+
+      llvm::raw_string_ostream Out(*P.Type);
+      NTTP->getType().print(Out, PP);
+      if (NTTP->isParameterPack())
+        Out << "...";
+
+      if (NTTP->hasDefaultArgument()) {
+        P.Default.emplace();
+        llvm::raw_string_ostream Out(*P.Default);
+        NTTP->getDefaultArgument()->printPretty(Out, nullptr, PP);
+      }
+    } else if (const auto TTPD = dyn_cast<TemplateTemplateParmDecl>(Param)) {
+      llvm::raw_string_ostream OS(*P.Type);
+      OS << "template <";
+      printParams(OS,
+                  fetchTemplateParameters(TTPD->getTemplateParameters(), PP));
+      OS << "> class"; // FIXME: TemplateTemplateParameter doesn't store the
+                       // info on whether this param was a "typename" or
+                       // "class".
+      if (!TTPD->getName().empty())
+        P.Name = TTPD->getNameAsString();
+      if (TTPD->hasDefaultArgument()) {
+        P.Default.emplace();
+        llvm::raw_string_ostream Out(*P.Default);
+        TTPD->getDefaultArgument().getArgument().print(PP, Out);
+      }
+    }
+    TempParameters.push_back(std::move(P));
+  }
+
+  return TempParameters;
+}
+
+static llvm::Optional<Range> getTokenRange(SourceLocation Loc,
+                                           const ASTContext &Ctx) {
+  if (!Loc.isValid())
+    return llvm::None;
+  SourceLocation End = Lexer::getLocForEndOfToken(
+      Loc, 0, Ctx.getSourceManager(), Ctx.getLangOpts());
+  if (!End.isValid())
+    return llvm::None;
+  return halfOpenToRange(Ctx.getSourceManager(),
+                         CharSourceRange::getCharRange(Loc, End));
+}
+
+/// Generate a \p Hover object given the declaration \p D.
+static HoverInfo getHoverContents(const Decl *D) {
+  HoverInfo HI;
+  const ASTContext &Ctx = D->getASTContext();
+
+  HI.NamespaceScope = getNamespaceScope(D);
+  if (!HI.NamespaceScope->empty())
+    HI.NamespaceScope->append("::");
+  HI.LocalScope = getLocalScope(D);
+  if (!HI.LocalScope.empty())
+    HI.LocalScope.append("::");
+
+  PrintingPolicy Policy = printingPolicyForDecls(Ctx.getPrintingPolicy());
+  if (const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D)) {
+    HI.Documentation = getDeclComment(Ctx, *ND);
+    HI.Name = printName(Ctx, *ND);
+  }
+
+  HI.Kind = indexSymbolKindToSymbolKind(index::getSymbolInfo(D).Kind);
+
+  // Fill in template params.
+  if (const TemplateDecl *TD = D->getDescribedTemplate()) {
+    HI.TemplateParameters =
+        fetchTemplateParameters(TD->getTemplateParameters(), Policy);
+    D = TD;
+  } else if (const FunctionDecl *FD = D->getAsFunction()) {
+    if (const auto FTD = FD->getDescribedTemplate()) {
+      HI.TemplateParameters =
+          fetchTemplateParameters(FTD->getTemplateParameters(), Policy);
+      D = FTD;
+    }
+  }
 
+  // Fill in types and params.
+  if (const FunctionDecl *FD = D->getAsFunction()) {
+    HI.ReturnType.emplace();
+    llvm::raw_string_ostream OS(*HI.ReturnType);
+    FD->getReturnType().print(OS, Policy);
+
+    HI.Type.emplace();
+    llvm::raw_string_ostream TypeOS(*HI.Type);
+    FD->getReturnType().print(TypeOS, Policy);
+    TypeOS << '(';
+
+    HI.Parameters.emplace();
+    for (const ParmVarDecl *PVD : FD->parameters()) {
+      if (HI.Parameters->size())
+        TypeOS << ", ";
+      HI.Parameters->emplace_back();
+      auto &P = HI.Parameters->back();
+      if (!PVD->getType().isNull()) {
+        P.Type.emplace();
+        llvm::raw_string_ostream OS(*P.Type);
+        PVD->getType().print(OS, Policy);
+        PVD->getType().print(TypeOS, Policy);
+      } else {
+        std::string Param;
+        llvm::raw_string_ostream OS(Param);
+        PVD->dump(OS);
         OS.flush();
+        elog("Got param with null type: {0}", Param);
+      }
+      if (!PVD->getName().empty())
+        P.Name = PVD->getNameAsString();
+      if (PVD->hasDefaultArg()) {
+        P.Default.emplace();
+        llvm::raw_string_ostream Out(*P.Default);
+        PVD->getDefaultArg()->printPretty(Out, nullptr, Policy);
+      }
+    }
+    TypeOS << ')';
+    // FIXME: handle variadics.
+  } else if (const auto *VD = dyn_cast<ValueDecl>(D)) {
+    // FIXME: Currently lambdas are also handled as ValueDecls, they should be
+    // more similar to functions.
+    HI.Type.emplace();
+    llvm::raw_string_ostream OS(*HI.Type);
+    VD->getType().print(OS, Policy);
+  }
 
-  H.contents.value += DeclText;
-  return H;
+  HI.Definition = printDefinition(D);
+  return HI;
 }
 
 /// Generate a \p Hover object given the type \p T.
-static Hover getHoverContents(QualType T, ASTContext &ASTCtx) {
-  Hover H;
-  std::string TypeText;
-  llvm::raw_string_ostream OS(TypeText);
+static HoverInfo getHoverContents(QualType T, const Decl *D,
+                                  ASTContext &ASTCtx) {
+  HoverInfo HI;
+  llvm::raw_string_ostream OS(HI.Name);
   PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
   T.print(OS, Policy);
-  OS.flush();
-  H.contents.value += TypeText;
-  return H;
+
+  if (D)
+    HI.Kind = indexSymbolKindToSymbolKind(index::getSymbolInfo(D).Kind);
+  return HI;
 }
 
 /// Generate a \p Hover object given the macro \p MacroDecl.
-static Hover getHoverContents(MacroDecl Decl, ParsedAST &AST) {
+static HoverInfo getHoverContents(MacroDecl Decl, ParsedAST &AST) {
+  HoverInfo HI;
   SourceManager &SM = AST.getASTContext().getSourceManager();
-  std::string Definition = Decl.Name;
+  HI.Name = Decl.Name;
+  HI.Kind = indexSymbolKindToSymbolKind(
+      index::getSymbolInfoForMacro(*Decl.Info).Kind);
+  // FIXME: Populate documentation
+  // FIXME: Pupulate parameters
 
   // Try to get the full definition, not just the name
   SourceLocation StartLoc = Decl.Info->getDefinitionLoc();
   SourceLocation EndLoc = Decl.Info->getDefinitionEndLoc();
   if (EndLoc.isValid()) {
     EndLoc = Lexer::getLocForEndOfToken(EndLoc, 0, SM,
                                         AST.getASTContext().getLangOpts());
     bool Invalid;
     StringRef Buffer = SM.getBufferData(SM.getFileID(StartLoc), &Invalid);
     if (!Invalid) {
       unsigned StartOffset = SM.getFileOffset(StartLoc);
       unsigned EndOffset = SM.getFileOffset(EndLoc);
       if (EndOffset <= Buffer.size() && StartOffset < EndOffset)
-        Definition = Buffer.substr(StartOffset, EndOffset - StartOffset).str();
+        HI.Definition =
+            ("#define " + Buffer.substr(StartOffset, EndOffset - StartOffset))
+                .str();
     }
   }
-
-  Hover H;
-  H.contents.kind = MarkupKind::PlainText;
-  H.contents.value = "#define " + Definition;
-  return H;
+  return HI;
 }
 
 namespace {
 /// Computes the deduced type at a given location by visiting the relevant
 /// nodes. We use this to display the actual type when hovering over an "auto"
 /// keyword or "decltype()" expression.
 /// FIXME: This could have been a lot simpler by visiting AutoTypeLocs but it
 /// seems that the AutoTypeLocs that can be visited along with their AutoType do
 /// not have the deduced type set. Instead, we have to go to the appropriate
 /// DeclaratorDecl/FunctionDecl and work our back to the AutoType that does have
 /// a deduced type set. The AST should be improved to simplify this scenario.
 class DeducedTypeVisitor : public RecursiveASTVisitor<DeducedTypeVisitor> {
   SourceLocation SearchedLocation;
-  llvm::Optional<QualType> DeducedType;
 
 public:
   DeducedTypeVisitor(SourceLocation SearchedLocation)
       : SearchedLocation(SearchedLocation) {}
 
-  llvm::Optional<QualType> getDeducedType() { return DeducedType; }
-
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
   //- auto& i = 1;
   //- auto* i = &a;
   bool VisitDeclaratorDecl(DeclaratorDecl *D) {
     if (!D->getTypeSourceInfo() ||
         D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
       return true;
 
     if (auto *AT = D->getType()->getContainedAutoType()) {
-      if (!AT->getDeducedType().isNull())
+      if (!AT->getDeducedType().isNull()) {
         DeducedType = AT->getDeducedType();
+        this->D = D;
+      }
     }
     return true;
   }
 
   // Handle auto return types:
   //- auto foo() {}
   //- auto& foo() {}
   //- auto foo() -> int {}
bool VisitFunctionDecl(FunctionDecl *D) {
     if (CurLoc.isInvalid())
       CurLoc = D->getSourceRange().getBegin();
     if (CurLoc != SearchedLocation)
       return true;
 
     const AutoType *AT = D->getReturnType()->getContainedAutoType();
     if (AT && !AT->getDeducedType().isNull()) {
       DeducedType = AT->getDeducedType();
+      this->D = D;
     } else if (auto DT = dyn_cast<DecltypeType>(D->getReturnType())) {
       // auto in a trailing return type just points to a DecltypeType and
       // getContainedAutoType does not unwrap it.
-      if (!DT->getUnderlyingType().isNull())
+      if (!DT->getUnderlyingType().isNull()) {
         DeducedType = DT->getUnderlyingType();
+        this->D = D;
+      }
     } else if (!D->getReturnType().isNull()) {
       DeducedType = D->getReturnType();
+      this->D = D;
     }
     return true;
   }
 
   // Handle non-auto decltype, e.g.:
   // - auto foo() -> decltype(expr) {}
   // - decltype(expr);
   bool VisitDecltypeTypeLoc(DecltypeTypeLoc TL) {
     if (TL.getBeginLoc() != SearchedLocation)
       return true;
 
     // A DecltypeType's underlying type can be another DecltypeType! E.g.
     //  int I = 0;
     //  decltype(I) J = I;
     //  decltype(J) K = J;
     const DecltypeType *DT = dyn_cast<DecltypeType>(TL.getTypePtr());
     while (DT && !DT->getUnderlyingType().isNull()) {
       DeducedType = DT->getUnderlyingType();
-      DT = dyn_cast<DecltypeType>(DeducedType->getTypePtr());
+      D = DT->getAsTagDecl();
+      DT = dyn_cast<DecltypeType>(DeducedType.getTypePtr());
     }
     return true;
   }
+
+  QualType DeducedType;
+  const Decl *D = nullptr;
 };
 } // namespace
 
 /// Retrieves the deduced type at a given location (auto, decltype).
-llvm::Optional<QualType> getDeducedType(ParsedAST &AST,
-                                        SourceLocation SourceLocationBeg) {
+bool hasDeducedType(ParsedAST &AST, SourceLocation SourceLocationBeg) {
   Token Tok;
   auto &ASTCtx = AST.getASTContext();
   // Only try to find a deduced type if the token is auto or decltype.
   if (!SourceLocationBeg.isValid() ||
       Lexer::getRawToken(SourceLocationBeg, Tok, ASTCtx.getSourceManager(),
                          ASTCtx.getLangOpts(), false) ||
       !Tok.is(tok::raw_identifier)) {
-    return {};
+    return false;
   }
   AST.getPreprocessor().LookUpIdentifierInfo(Tok);
   if (!(Tok.is(tok::kw_auto) || Tok.is(tok::kw_decltype)))
-    return {};
-
-  DeducedTypeVisitor V(SourceLocationBeg);
-  V.TraverseAST(AST.getASTContext());
-  return V.getDeducedType();
+    return false;
+  return true;
 }
 
-llvm::Optional<Hover> getHover(ParsedAST &AST, Position Pos) {
+llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
+                                   format::FormatStyle Style) {
+  llvm::Optional<HoverInfo> HI;
   const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
   SourceLocation SourceLocationBeg =
       getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID());
   // Identified symbols at a specific position.
   auto Symbols = getSymbolAtPosition(AST, SourceLocationBeg);
 
   if (!Symbols.Macros.empty())
-    return getHoverContents(Symbols.Macros[0], AST);
+    HI = getHoverContents(Symbols.Macros[0], AST);
+  else if (!Symbols.Decls.empty())
+    HI = getHoverContents(Symbols.Decls[0]);
+  else {
+    if (!hasDeducedType(AST, SourceLocationBeg))
+      return None;
 
-  if (!Symbols.Decls.empty())
-    return getHoverContents(Symbols.Decls[0]);
+    DeducedTypeVisitor V(SourceLocationBeg);
+    V.TraverseAST(AST.getASTContext());
+    if (V.DeducedType.isNull())
+      return None;
+    HI = getHoverContents(V.DeducedType, V.D, AST.getASTContext());
+  }
 
-  auto DeducedType = getDeducedType(AST, SourceLocationBeg);
-  if (DeducedType && !DeducedType->isNull())
-    return getHoverContents(*DeducedType, AST.getASTContext());
+  auto Replacements = format::reformat(
+      Style, HI->Definition, tooling::Range(0, HI->Definition.size()));
+  if (auto Formatted =
+          tooling::applyAllReplacements(HI->Definition, Replacements))
+    HI->Definition = *Formatted;
 
-  return None;
+  HI->SymRange = getTokenRange(SourceLocationBeg, AST.getASTContext());
+  return HI;
 }
 
 std::vector<Location> findReferences(ParsedAST &AST, Position Pos,
                                      uint32_t Limit, const SymbolIndex *Index) {
   if (!Limit)
     Limit = std::numeric_limits<uint32_t>::max();
   std::vector<Location> Results;
   const SourceManager &SM = AST.getASTContext().getSourceManager();
   auto MainFilePath =
       getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
   if (!MainFilePath) {
     elog("Failed to get a path for the main file, so no references");
     return Results;
   }
   auto Loc = getBeginningOfIdentifier(AST, Pos, SM.getMainFileID());
   auto Symbols = getSymbolAtPosition(AST, Loc);
 
   // We traverse the AST to find references in the main file.
   // TODO: should we handle macros, too?
   auto MainFileRefs = findRefs(Symbols.Decls, AST);
   for (const auto &Ref : MainFileRefs) {
     Location Result;
-    Result.range = getTokenRange(AST, Ref.Loc);
+    Result.range = getTokenRange(AST.getASTContext(), Ref.Loc);
     Result.uri = URIForFile::canonicalize(*MainFilePath, *MainFilePath);
     Results.push_back(std::move(Result));
   }
 
   // Now query the index for references from other files.
   if (Index && Results.size() < Limit) {
     RefsRequest Req;
     Req.Limit = Limit;
Optional<TypeHierarchyItem> Result =
       getTypeAncestors(*CXXRD, AST.getASTContext(), RPSet);
 
   // FIXME(nridge): Resolve type descendants if direction is Children or Both,
   // and ResolveLevels > 0.
 
   return Result;
 }
 
+MarkupContent HoverInfo::render() const {
+  MarkupContent Content;
+  Content.kind = MarkupKind::PlainText;
+  std::vector<std::string> Output;
+
+  if (NamespaceScope) {
+    llvm::raw_string_ostream Out(Content.value);
+    Out << "Declared in ";
+    // Drop trailing "::".
+    if (!LocalScope.empty())
+      Out << *NamespaceScope << llvm::StringRef(LocalScope).drop_back(2);
+    else if (NamespaceScope->empty())
+      Out << "global namespace";
+    else
+      Out << llvm::StringRef(*NamespaceScope).drop_back(2);
+    Out << "\n\n";
+  }
+
+  if (!Definition.empty()) {
+    Output.push_back(Definition);
+  } else {
+    // Builtin types
+    Output.push_back(Name);
+  }
+  Content.value += llvm::join(Output, " ");
+  return Content;
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+                              const HoverInfo::Param &P) {
+  std::vector<llvm::StringRef> Output;
+  if (P.Type)
+    Output.push_back(*P.Type);
+  if (P.Name)
+    Output.push_back(*P.Name);
+  OS << llvm::join(Output, " ");
+  if (P.Default)
+    OS << " = " << *P.Default;
+  return OS;
+}
+
 } // namespace clangd
 } // namespace clang

clang-tools-extra/trunk/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": "Declared in global namespace\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,
+# CHECK-NEXT:        "line": 0
+# CHECK-NEXT:      }
 # CHECK-NEXT:    }
 # CHECK-NEXT:  }
 # CHECK-NEXT:}
 ---
 {"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":10}}}
 #      CHECK:  "id": 1,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": null
 ---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main2.cpp","languageId":"cpp","version":1,"text":"enum foo{}; int main() { foo f; }\n"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"test:///main2.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": "Declared in global namespace\n\nenum 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,
+# CHECK-NEXT:        "line": 0
+# CHECK-NEXT:      }
+# CHECK-NEXT:    }
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+---
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}

clang-tools-extra/trunk/clangd/unittests/TestTU.cpp

if (Inputs.Index)
     Inputs.Opts.SuggestMissingIncludes = true;
   auto CI = buildCompilerInvocation(Inputs);
   assert(CI && "Failed to build compilation invocation.");
   auto Preamble =
       buildPreamble(FullFilename, *CI,
                     /*OldPreamble=*/nullptr,
                     /*OldCompileCommand=*/Inputs.CompileCommand, Inputs,
                     /*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
-  auto AST = buildAST(FullFilename, createInvocationFromCommandLine(Cmd),
-                      Inputs, Preamble);
+  auto AST = buildAST(FullFilename, std::move(CI), Inputs, Preamble);
   if (!AST.hasValue()) {
     ADD_FAILURE() << "Failed to build code:\n" << Code;
     llvm_unreachable("Failed to build TestTU!");
   }
   return std::move(*AST);
 }
 
 SymbolSlab TestTU::headerSymbols() const {

clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp

 //===-- XRefsTests.cpp  ---------------------------*- C++ -*--------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "Compiler.h"
 #include "Matchers.h"
+#include "Protocol.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
 #include "XRefs.h"
 #include "index/FileIndex.h"
 #include "index/SymbolCollector.h"
 #include "clang/Index/IndexingAction.h"
+#include "llvm/ADT/None.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
 namespace {
)cpp");
   // Go to a definition in header_not_in_preamble.h.
   Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("p3"));
   EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
   EXPECT_THAT(*Locations,
               ElementsAre(Sym("bar_not_preamble",
                               HeaderNotInPreambleAnnotations.range())));
 }
 
+TEST(Hover, Structured) {
+  struct {
+    const char *const Code;
+    const std::function<void(HoverInfo &)> ExpectedBuilder;
+  } Cases[] = {
+      // Global scope.
+      {R"cpp(
+          // Best foo ever.
+          void [[fo^o]]() {}
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "";
+         HI.Name = "foo";
+         HI.Kind = SymbolKind::Function;
+         HI.Documentation = "Best foo ever.";
+         HI.Definition = "void foo()";
+         HI.ReturnType = "void";
+         HI.Type = "void()";
+         HI.Parameters.emplace();
+       }},
+      // Inside namespace
+      {R"cpp(
+          namespace ns1 { namespace ns2 {
+            /// Best foo ever.
+            void [[fo^o]]() {}
+          }}
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "ns1::ns2::";
+         HI.Name = "foo";
+         HI.Kind = SymbolKind::Function;
+         HI.Documentation = "Best foo ever.";
+         HI.Definition = "void foo()";
+         HI.ReturnType = "void";
+         HI.Type = "void()";
+         HI.Parameters.emplace();
+       }},
+      // Field
+      {R"cpp(
+          namespace ns1 { namespace ns2 {
+            struct Foo {
+              int [[b^ar]];
+            };
+          }}
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "ns1::ns2::";
+         HI.LocalScope = "Foo::";
+         HI.Name = "bar";
+         HI.Kind = SymbolKind::Field;
+         HI.Definition = "int bar";
+         HI.Type = "int";
+       }},
+      // Local to class method.
+      {R"cpp(
+          namespace ns1 { namespace ns2 {
+            struct Foo {
+              void foo() {
+                int [[b^ar]];
+              }
+            };
+          }}
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "ns1::ns2::";
+         HI.LocalScope = "Foo::foo::";
+         HI.Name = "bar";
+         HI.Kind = SymbolKind::Variable;
+         HI.Definition = "int bar";
+         HI.Type = "int";
+       }},
+      // Anon namespace and local scope.
+      {R"cpp(
+          namespace ns1 { namespace {
+            struct {
+              int [[b^ar]];
+            } T;
+          }}
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "ns1::(anonymous)::";
+         HI.LocalScope = "(anonymous struct)::";
+         HI.Name = "bar";
+         HI.Kind = SymbolKind::Field;
+         HI.Definition = "int bar";
+         HI.Type = "int";
+       }},
+      // Variable with template type
+      {R"cpp(
+          template <typename T, class... Ts> class Foo {};
+          Foo<int, char, bool> [[fo^o]];
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "";
+         HI.Name = "foo";
+         HI.Kind = SymbolKind::Variable;
+         HI.Definition = "Foo<int, char, bool> foo";
+         HI.Type = "Foo<int, char, bool>";
+       }},
+      // Implicit template instantiation
+      {R"cpp(
+          template <typename T> class vector{};
+          [[vec^tor]]<int> foo;
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "";
+         HI.Name = "vector";
+         HI.Kind = SymbolKind::Class;
+         HI.Definition = "template <typename T> class vector {}";
+         HI.TemplateParameters = {
+             {std::string("typename"), std::string("T"), llvm::None},
+         };
+       }},
+      // Class template
+      {R"cpp(
+          template <template<typename, bool...> class C,
+                    typename = char,
+                    int = 0,
+                    bool Q = false,
+                    class... Ts> class Foo {};
+          template <template<typename, bool...> class T>
+          [[F^oo]]<T> foo;
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "";
+         HI.Name = "Foo";
+         HI.Kind = SymbolKind::Class;
+         HI.Definition =
+             R"cpp(template <template <typename, bool...> class C, typename = char, int = 0,
+          bool Q = false, class... Ts>
+class Foo {})cpp";
+         HI.TemplateParameters = {
+             {std::string("template <typename, bool...> class"),
+              std::string("C"), llvm::None},
+             {std::string("typename"), llvm::None, std::string("char")},
+             {std::string("int"), llvm::None, std::string("0")},
+             {std::string("bool"), std::string("Q"), std::string("false")},
+             {std::string("class..."), std::string("Ts"), llvm::None},
+         };
+       }},
+      // Function template
+      {R"cpp(
+          template <template<typename, bool...> class C,
+                    typename = char,
+                    int = 0,
+                    bool Q = false,
+                    class... Ts> void foo();
+          template<typename, bool...> class Foo;
+
+          void bar() {
+            [[fo^o]]<Foo>();
+          }
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "";
+         HI.Name = "foo";
+         HI.Kind = SymbolKind::Function;
+         HI.Definition =
+             R"cpp(template <template <typename, bool...> class C, typename = char, int = 0,
+          bool Q = false, class... Ts>
+void foo())cpp";
+         HI.ReturnType = "void";
+         HI.Type = "void()";
+         HI.Parameters.emplace();
+         HI.TemplateParameters = {
+             {std::string("template <typename, bool...> class"),
+              std::string("C"), llvm::None},
+             {std::string("typename"), llvm::None, std::string("char")},
+             {std::string("int"), llvm::None, std::string("0")},
+             {std::string("bool"), std::string("Q"), std::string("false")},
+             {std::string("class..."), std::string("Ts"), llvm::None},
+         };
+       }},
+      // Function decl
+      {R"cpp(
+          template<typename, bool...> class Foo {};
+          Foo<bool, true, false> foo(int, bool T = false);
+
+          void bar() {
+            [[fo^o]](3);
+          }
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "";
+         HI.Name = "foo";
+         HI.Kind = SymbolKind::Function;
+         HI.Definition = "Foo<bool, true, false> foo(int, bool T = false)";
+         HI.ReturnType = "Foo<bool, true, false>";
+         HI.Type = "Foo<bool, true, false>(int, bool)";
+         HI.Parameters = {
+             {std::string("int"), llvm::None, llvm::None},
+             {std::string("bool"), std::string("T"), std::string("false")},
+         };
+       }},
+      // Lambda variable
+      {R"cpp(
+        void foo() {
+          int bar = 5;
+          auto lamb = [&bar](int T, bool B) -> bool { return T && B && bar; };
+          bool res = [[lam^b]](bar, false);
+        }
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "";
+         HI.LocalScope = "foo::";
+         HI.Name = "lamb";
+         HI.Kind = SymbolKind::Variable;
+         HI.Definition = "auto lamb = [&bar](int T, bool B) -> bool {}";
+         HI.Type = std::string("class (lambda)");
+         return HI;
+       }},
+      // Local variable in lambda
+      {R"cpp(
+        void foo() {
+          auto lamb = []{int [[te^st]];};
+        }
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "";
+         HI.LocalScope = "foo::(anonymous class)::operator()::";
+         HI.Name = "test";
+         HI.Kind = SymbolKind::Variable;
+         HI.Definition = "int test";
+         HI.Type = "int";
+       }},
+
+      // auto on lambda
+      {R"cpp(
+        void foo() {
+          [[au^to]] lamb = []{};
+        }
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "class (lambda)";
+         HI.Kind = SymbolKind::Variable;
+       }},
+      // auto on template instantiation
+      {R"cpp(
+        template<typename T> class Foo{};
+        void foo() {
+          [[au^to]] x = Foo<int>();
+        }
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "class Foo<int>";
+         HI.Kind = SymbolKind::Variable;
+       }},
+      // auto on specialized template
+      {R"cpp(
+        template<typename T> class Foo{};
+        template<> class Foo<int>{};
+        void foo() {
+          [[au^to]] x = Foo<int>();
+        }
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "class Foo<int>";
+         HI.Kind = SymbolKind::Variable;
+       }},
+
+      // macro
+      {R"cpp(
+        // Best MACRO ever.
+        #define MACRO(x,y,z) void foo(x, y, z);
+        [[MAC^RO]](int, double d, bool z = false);
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "MACRO", HI.Kind = SymbolKind::String,
+         HI.Definition = "#define MACRO(x, y, z) void foo(x, y, z);";
+       }},
+  };
+  for (const auto &Case : Cases) {
+    SCOPED_TRACE(Case.Code);
+
+    Annotations T(Case.Code);
+    TestTU TU = TestTU::withCode(T.code());
+    TU.ExtraArgs.push_back("-std=c++17");
+    auto AST = TU.build();
+    ASSERT_TRUE(AST.getDiagnostics().empty());
+
+    auto H = getHover(AST, T.point(), format::getLLVMStyle());
+    ASSERT_TRUE(H);
+    HoverInfo Expected;
+    Expected.SymRange = T.range();
+    Case.ExpectedBuilder(Expected);
+
+    EXPECT_EQ(H->NamespaceScope, Expected.NamespaceScope);
+    EXPECT_EQ(H->LocalScope, Expected.LocalScope);
+    EXPECT_EQ(H->Name, Expected.Name);
+    EXPECT_EQ(H->Kind, Expected.Kind);
+    EXPECT_EQ(H->Documentation, Expected.Documentation);
+    EXPECT_EQ(H->Definition, Expected.Definition);
+    EXPECT_EQ(H->Type, Expected.Type);
+    EXPECT_EQ(H->ReturnType, Expected.ReturnType);
+    EXPECT_EQ(H->Parameters, Expected.Parameters);
+    EXPECT_EQ(H->TemplateParameters, Expected.TemplateParameters);
+    EXPECT_EQ(H->SymRange, Expected.SymRange);
+  }
+} // namespace clang
+
 TEST(Hover, All) {
   struct OneTest {
     StringRef Input;
     StringRef ExpectedHover;
   };
 
   OneTest Tests[] = {
       {
           R"cpp(// No hover
             ^int main() {
             }
           )cpp",
           "",
       },
       {
           R"cpp(// Local variable
             int main() {
               int bonjour;
               ^bonjour = 2;
               int test1 = bonjour;
             }
           )cpp",
-          "Declared in function main\n\nint bonjour",
+          "Declared in main\n\nint bonjour",
       },
       {
           R"cpp(// Local variable in method
             struct s {
               void method() {
                 int bonjour;
                 ^bonjour = 2;
               }
             };
           )cpp",
-          "Declared in function s::method\n\nint bonjour",
+          "Declared in s::method\n\nint bonjour",
       },
       {
           R"cpp(// Struct
             namespace ns1 {
               struct MyClass {};
             } // namespace ns1
             int main() {
               ns1::My^Class* Params;
             }
           )cpp",
-          "Declared in namespace ns1\n\nstruct MyClass {}",
+          "Declared in ns1\n\nstruct MyClass {}",
       },
       {
           R"cpp(// Class
             namespace ns1 {
               class MyClass {};
             } // namespace ns1
             int main() {
               ns1::My^Class* Params;
             }
           )cpp",
-          "Declared in namespace ns1\n\nclass MyClass {}",
+          "Declared in ns1\n\nclass MyClass {}",
       },
       {
           R"cpp(// Union
             namespace ns1 {
               union MyUnion { int x; int y; };
             } // namespace ns1
             int main() {
               ns1::My^Union Params;
             }
           )cpp",
-          "Declared in namespace ns1\n\nunion MyUnion {}",
+          "Declared in ns1\n\nunion MyUnion {}",
       },
       {
           R"cpp(// Function definition via pointer
             int foo(int) {}
             int main() {
               auto *X = &^foo;
             }
           )cpp",
OneTest Tests[] = {
       {
           R"cpp(// Field
             struct Foo { int x; };
             int main() {
               Foo bar;
               bar.^x;
             }
           )cpp",
-          "Declared in struct Foo\n\nint x",
+          "Declared in Foo\n\nint x",
       },
       {
           R"cpp(// Field with initialization
             struct Foo { int x = 5; };
             int main() {
               Foo bar;
               bar.^x;
             }
           )cpp",
-          "Declared in struct Foo\n\nint x = 5",
+          "Declared in Foo\n\nint x = 5",
       },
       {
           R"cpp(// Static field
             struct Foo { static int x; };
             int main() {
               Foo::^x;
             }
           )cpp",
-          "Declared in struct Foo\n\nstatic int x",
+          "Declared in Foo\n\nstatic int x",
       },
       {
           R"cpp(// Field, member initializer
             struct Foo {
               int x;
               Foo() : ^x(0) {}
             };
           )cpp",
-          "Declared in struct Foo\n\nint x",
+          "Declared in Foo\n\nint x",
       },
       {
           R"cpp(// Field, GNU old-style field designator
             struct Foo { int x; };
             int main() {
               Foo bar = { ^x : 1 };
             }
           )cpp",
-          "Declared in struct Foo\n\nint x",
+          "Declared in Foo\n\nint x",
       },
       {
           R"cpp(// Field, field designator
             struct Foo { int x; };
             int main() {
               Foo bar = { .^x = 2 };
             }
           )cpp",
-          "Declared in struct Foo\n\nint x",
+          "Declared in Foo\n\nint x",
       },
       {
           R"cpp(// Method call
             struct Foo { int x(); };
             int main() {
               Foo bar;
               bar.^x();
             }
           )cpp",
-          "Declared in struct Foo\n\nint x()",
+          "Declared in Foo\n\nint x()",
       },
       {
           R"cpp(// Static method call
             struct Foo { static int x(); };
             int main() {
               Foo::^x();
             }
           )cpp",
-          "Declared in struct Foo\n\nstatic int x()",
+          "Declared in Foo\n\nstatic int x()",
       },
       {
           R"cpp(// Typedef
             typedef int Foo;
             int main() {
               ^Foo bar;
             }
           )cpp",
           "Declared in global namespace\n\ntypedef int Foo",
       },
       {
           R"cpp(// Namespace
             namespace ns {
             struct Foo { static void bar(); }
             } // namespace ns
             int main() { ^ns::Foo::bar(); }
           )cpp",
-          "Declared in global namespace\n\nnamespace ns {\n}",
+          "Declared in global namespace\n\nnamespace ns {}",
       },
       {
           R"cpp(// Anonymous namespace
             namespace ns {
               namespace {
                 int foo;
               } // anonymous namespace
             } // namespace ns
             int main() { ns::f^oo++; }
           )cpp",
-          "Declared in namespace ns::(anonymous)\n\nint foo",
+          "Declared in ns::(anonymous)\n\nint foo",
       },
       {
           R"cpp(// Macro
             #define MACRO 0
             #define MACRO 1
             int main() { return ^MACRO; }
             #define MACRO 2
             #undef macro
OneTest Tests[] = {
       },
       {
           R"cpp(// Macro
             #define MACRO {\
               return 0;\
             }
             int main() ^MACRO
           )cpp",
-          R"cpp(#define MACRO {\
-              return 0;\
-            })cpp",
+          "#define MACRO                                                       "
+          "           \\\n  { return 0; }",
       },
       {
           R"cpp(// Forward class declaration
             class Foo;
             class Foo {};
             F^oo* foo();
           )cpp",
           "Declared in global namespace\n\nclass Foo {}",
OneTest Tests[] = {
           R"cpp(// Enum declaration
             enum Hello {
               ONE, TWO, THREE,
             };
             void foo() {
               Hel^lo hello = ONE;
             }
           )cpp",
-          "Declared in global namespace\n\nenum Hello {\n}",
+          "Declared in global namespace\n\nenum Hello {}",
       },
       {
           R"cpp(// Enumerator
             enum Hello {
               ONE, TWO, THREE,
             };
             void foo() {
               Hello hello = O^NE;
             }
           )cpp",
-          "Declared in enum Hello\n\nONE",
+          "Declared in Hello\n\nONE",
       },
       {
           R"cpp(// Enumerator in anonymous enum
             enum {
               ONE, TWO, THREE,
             };
             void foo() {
               int hello = O^NE;
             }
           )cpp",
-          "Declared in enum (anonymous)\n\nONE",
+          "Declared in global namespace\n\nONE",
       },
       {
           R"cpp(// Global variable
             static int hey = 10;
             void foo() {
               he^y++;
             }
           )cpp",
           "Declared in global namespace\n\nstatic int hey = 10",
       },
       {
           R"cpp(// Global variable in namespace
             namespace ns1 {
               static int hey = 10;
             }
             void foo() {
               ns1::he^y++;
             }
           )cpp",
-          "Declared in namespace ns1\n\nstatic int hey = 10",
+          "Declared in ns1\n\nstatic int hey = 10",
       },
       {
           R"cpp(// Field in anonymous struct
             static struct {
               int hello;
             } s;
             void foo() {
               s.he^llo++;
             }
           )cpp",
-          "Declared in struct (anonymous)\n\nint hello",
+          "Declared in (anonymous struct)\n\nint hello",
       },
       {
           R"cpp(// Templated function
             template <typename T>
             T foo() {
               return 17;
             }
             void g() { auto x = f^oo<int>(); }
           )cpp",
           "Declared in global namespace\n\ntemplate <typename T> T foo()",
       },
       {
           R"cpp(// Anonymous union
             struct outer {
               union {
                 int abc, def;
               } v;
             };
             void g() { struct outer o; o.v.d^ef++; }
           )cpp",
-          "Declared in union outer::(anonymous)\n\nint def",
+          "Declared in outer::(anonymous union)\n\nint def",
       },
       {
           R"cpp(// Nothing
             void foo() {
               ^
             }
           )cpp",
           "",
OneTest Tests[] = {
       },
   };
 
   for (const OneTest &Test : Tests) {
     Annotations T(Test.Input);
     TestTU TU = TestTU::withCode(T.code());
     TU.ExtraArgs.push_back("-std=c++17");
     auto AST = TU.build();
-    if (auto H = getHover(AST, T.point())) {
+    if (auto H = getHover(AST, T.point(), format::getLLVMStyle())) {
       EXPECT_NE("", Test.ExpectedHover) << Test.Input;
-      EXPECT_EQ(H->contents.value, Test.ExpectedHover.str()) << Test.Input;
+      EXPECT_EQ(H->render().value, Test.ExpectedHover.str()) << Test.Input;
     } else
       EXPECT_EQ("", Test.ExpectedHover.str()) << Test.Input;
   }
 }
 
 TEST(GoToInclude, All) {
   MockFSProvider FS;
   IgnoreDiagnostics DiagConsumer;