[clangd] Implement textDocument/declaration from LSP 3.14

[sammccall]

Summary

LSP now reflects the declaration/definition distinction.

Language server changes:

• textDocument/definition now returns a definition if one is found, otherwise the declaration. It no longer returns declaration + definition if they are distinct.
• textDocument/declaration returns the best declaration we can find.
• For macros, the active macro definition is returned for both methods.
• For include directive, the top of the target file is returned for both.

There doesn't appear to be a discovery mechanism (we can't return everything to
clients that only know about definition), so this changes existing behavior.
In practice, it should greatly reduce the fraction of the time we need to show
the user a menu of options.

C++ API changes:

• findDefinitions is replaced by locateSymbolAt, which returns a vector<LocatedSymbol> - one for each symbol under the cursor.
• this contains the preferred declaration, the definition (if found), and the symbol name

This API enables some potentially-neat extensions, like swapping between decl
and def, and exposing the symbol name to the UI in the case of multiple symbols.

[sammccall]

Add server capability declarationProvider.
Technically an extension, but that's probably just a protocol bug.

[hokein]

Looks good overall, most are nits.

textDocument/definition now returns a definition if one is found, otherwise the declaration. It no longer returns declaration + definition if they are distinct.
textDocument/declaration returns the best declaration we can find.
For macros, the active macro definition is returned for both methods.
For include directive, the top of the target file is returned for both.

Just realized these at the last minutes, I think these are well-documented (and very helpful for future-code readers), I'd suggest putting them to the code comments as well.

clangd/ClangdLSPServer.cpp

357

I believe missing this is a LSP protocol bug, vscode uses this term https://github.com/Microsoft/vscode/blob/248aea7fd51a703a7ab94ae5a582c85c11fbff33/src/vs/vscode.d.ts#L2301.

clangd/ClangdLSPServer.h

84

nit: maybe put it before onGoToDefinition, it seems to me more natural that declaration comes first.

clangd/ClangdServer.h

170–171

nit: this comment seems out-of-date?

clangd/XRefs.cpp

40

is this a case that we were missing before? can we add a unittest for it (I didn't find a related change in the unittest).

303–304

nit: this is not used anymore.

clangd/XRefs.h

29

nit: maybe also mention marco? it is unclear to me how the declaration and definition would be like for macro without reading the implementation.

unittests/clangd/XRefsTests.cpp

339

I think this should be WantDef?

[sammccall]

Address review comments

[sammccall]

clangd/ClangdLSPServer.cpp

357

Yeah, agree.

clangd/XRefs.cpp

40

Previously, this function only had to be correct for things that can be declared and defined separately.
For some decls that are always definitions (e.g. member variables) we would return nullptr here, and treat them as decl-only ... but that was OK, because the return value was just "a list of locations" and it didn't matter whether we treated them as decls or defs when there's just one.

However now the return type is "here's the decl, and here's the def". Without this change, we have Definition == llvm::None for e.g. member variables. While the LSP method falls back from def->decl so you probably can't observe this problem, API users can.

This is in fact covered by the tests, though it's kind of indirect (this is a private helper function, after all).

[hokein]

clangd/XRefs.cpp

40

Thanks for the explanation. Maybe add more in the comment?

unittests/clangd/XRefsTests.cpp

339

what about this comment?

[sammccall]

clangd/XRefs.cpp

40

Added a little bit - there's not really so much to say, because the new behavior is kind of the obvious one.
The old behavior ("just return nullptr if it doesn't matter") maybe deserved a longer comment, but we didn't notice.

[sammccall]

address review comments

clangd/ClangdLSPServer.h

private:
   // SupportsHierarchicalDocumentSymbol is true and 'vector<SymbolInformation>'
   // otherwise.
   void onDocumentSymbol(const DocumentSymbolParams &,
                         Callback<llvm::json::Value>);
   void onCodeAction(const CodeActionParams &, Callback<llvm::json::Value>);
   void onCompletion(const CompletionParams &, Callback<CompletionList>);
   void onSignatureHelp(const TextDocumentPositionParams &,
                        Callback<SignatureHelp>);
+  void onGoToDeclaration(const TextDocumentPositionParams &,
+                         Callback<std::vector<Location>>);
   void onGoToDefinition(const TextDocumentPositionParams &,
                         Callback<std::vector<Location>>);
   void onReference(const ReferenceParams &, Callback<std::vector<Location>>);
   void onSwitchSourceHeader(const TextDocumentIdentifier &,
                             Callback<std::string>);
   void onDocumentHighlight(const TextDocumentPositionParams &,
                            Callback<std::vector<DocumentHighlight>>);
   void onFileEvent(const DidChangeWatchedFilesParams &);
   void onCommand(const ExecuteCommandParams &, Callback<llvm::json::Value>);
   void onWorkspaceSymbol(const WorkspaceSymbolParams &,
                          Callback<std::vector<SymbolInformation>>);

clangd/ClangdLSPServer.cpp

Reply(llvm::json::Object{
                  // We do extra checks for '>' and ':' in completion to only
                  // trigger on '->' and '::'.
                  {"triggerCharacters", {".", ">", ":"}},
              }},
             {"signatureHelpProvider",
              llvm::json::Object{
                  {"triggerCharacters", {"(", ","}},
              }},
+            {"declarationProvider", true},
             {"definitionProvider", true},
             {"documentHighlightProvider", true},
             {"hoverProvider", true},
             {"renameProvider", true},
             {"documentSymbolProvider", true},
             {"workspaceSymbolProvider", true},
             {"referencesProvider", true},
             {"executeCommandProvider",

 void ClangdLSPServer::onSignatureHelp(const TextDocumentPositionParams &Params,
                                       Callback<SignatureHelp> Reply) {
   Server->signatureHelp(Params.textDocument.uri.file(), Params.position,
                         std::move(Reply));
 }
 
 void ClangdLSPServer::onGoToDefinition(const TextDocumentPositionParams &Params,
                                        Callback<std::vector<Location>> Reply) {
-  Server->findDefinitions(Params.textDocument.uri.file(), Params.position,
-                          std::move(Reply));
+  Server->locateSymbolAt(
+      Params.textDocument.uri.file(), Params.position,
+      Bind(
+          [&](decltype(Reply) Reply,
+              llvm::Expected<std::vector<LocatedSymbol>> Symbols) {
+            if (!Symbols)
+              return Reply(Symbols.takeError());
+            std::vector<Location> Defs;
+            for (const auto &S : *Symbols)
+              Defs.push_back(S.Definition.getValueOr(S.PreferredDeclaration));
+            Reply(std::move(Defs));
+          },
+          std::move(Reply)));
+}
+
+void ClangdLSPServer::onGoToDeclaration(
+    const TextDocumentPositionParams &Params,
+    Callback<std::vector<Location>> Reply) {
+  Server->locateSymbolAt(
+      Params.textDocument.uri.file(), Params.position,
+      Bind(
+          [&](decltype(Reply) Reply,
+              llvm::Expected<std::vector<LocatedSymbol>> Symbols) {
+            if (!Symbols)
+              return Reply(Symbols.takeError());
+            std::vector<Location> Decls;
+            for (const auto &S : *Symbols)
+              Decls.push_back(S.PreferredDeclaration);
+            Reply(std::move(Decls));
+          },
+          std::move(Reply)));
 }
 
 void ClangdLSPServer::onSwitchSourceHeader(const TextDocumentIdentifier &Params,
                                            Callback<std::string> Reply) {
   llvm::Optional<Path> Result = Server->switchSourceHeader(Params.uri.file());
   Reply(Result ? URI::createFile(*Result).toString() : "");
 }
 
ClangdLSPServer::ClangdLSPServer(class Transport &Transp,
   MsgHandler->bind("sync", &ClangdLSPServer::onSync);
   MsgHandler->bind("textDocument/rangeFormatting", &ClangdLSPServer::onDocumentRangeFormatting);
   MsgHandler->bind("textDocument/onTypeFormatting", &ClangdLSPServer::onDocumentOnTypeFormatting);
   MsgHandler->bind("textDocument/formatting", &ClangdLSPServer::onDocumentFormatting);
   MsgHandler->bind("textDocument/codeAction", &ClangdLSPServer::onCodeAction);
   MsgHandler->bind("textDocument/completion", &ClangdLSPServer::onCompletion);
   MsgHandler->bind("textDocument/signatureHelp", &ClangdLSPServer::onSignatureHelp);
   MsgHandler->bind("textDocument/definition", &ClangdLSPServer::onGoToDefinition);
+  MsgHandler->bind("textDocument/declaration", &ClangdLSPServer::onGoToDeclaration);
   MsgHandler->bind("textDocument/references", &ClangdLSPServer::onReference);
   MsgHandler->bind("textDocument/switchSourceHeader", &ClangdLSPServer::onSwitchSourceHeader);
   MsgHandler->bind("textDocument/rename", &ClangdLSPServer::onRename);
   MsgHandler->bind("textDocument/hover", &ClangdLSPServer::onHover);
   MsgHandler->bind("textDocument/documentSymbol", &ClangdLSPServer::onDocumentSymbol);
   MsgHandler->bind("workspace/executeCommand", &ClangdLSPServer::onCommand);
   MsgHandler->bind("textDocument/documentHighlight", &ClangdLSPServer::onDocumentHighlight);
   MsgHandler->bind("workspace/symbol", &ClangdLSPServer::onWorkspaceSymbol);

clangd/ClangdServer.h

 #include "Cancellation.h"
 #include "ClangdUnit.h"
 #include "CodeComplete.h"
 #include "FSProvider.h"
 #include "Function.h"
 #include "GlobalCompilationDatabase.h"
 #include "Protocol.h"
 #include "TUScheduler.h"
+#include "XRefs.h"
 #include "index/Background.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "refactor/Tweak.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
public:
   /// when codeComplete results become available.
   void codeComplete(PathRef File, Position Pos,
                     const clangd::CodeCompleteOptions &Opts,
                     Callback<CodeCompleteResult> CB);
 
   /// Provide signature help for \p File at \p Pos.  This method should only be
   /// called for tracked files.
   void signatureHelp(PathRef File, Position Pos, Callback<SignatureHelp> CB);
 
-  /// Get definition of symbol at a specified \p Line and \p Column in \p File.
-  void findDefinitions(PathRef File, Position Pos,
-                       Callback<std::vector<Location>> CB);
+  /// Find declaration/definition locations of symbol at a specified position.
+  void locateSymbolAt(PathRef File, Position Pos,
+                      Callback<std::vector<LocatedSymbol>> CB);
 
   /// Helper function that returns a path to the corresponding source file when
   /// given a header file and vice versa.
   llvm::Optional<Path> switchSourceHeader(PathRef Path);
 
   /// Get document highlights for a given position.
   void findDocumentHighlights(PathRef File, Position Pos,
                               Callback<std::vector<DocumentHighlight>> CB);

clangd/ClangdServer.cpp

 //===--- ClangdServer.cpp - Main clangd server code --------------*- 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 "ClangdServer.h"
 #include "ClangdUnit.h"
 #include "CodeComplete.h"
 #include "FindSymbols.h"
 #include "Headers.h"
 #include "SourceCode.h"
 #include "Trace.h"
-#include "XRefs.h"
 #include "index/FileIndex.h"
 #include "index/Merge.h"
 #include "refactor/Tweak.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
auto Action = [](decltype(Callback) Callback,
     ResultOS.flush();
 
     Callback(Result);
   };
 
   WorkScheduler.runWithAST("DumpAST", File, Bind(Action, std::move(Callback)));
 }
 
-void ClangdServer::findDefinitions(PathRef File, Position Pos,
-                                   Callback<std::vector<Location>> CB) {
-  auto Action = [Pos, this](Callback<std::vector<Location>> CB,
+void ClangdServer::locateSymbolAt(PathRef File, Position Pos,
+                                  Callback<std::vector<LocatedSymbol>> CB) {
+  auto Action = [Pos, this](decltype(CB) CB,
                             llvm::Expected<InputsAndAST> InpAST) {
     if (!InpAST)
       return CB(InpAST.takeError());
-    CB(clangd::findDefinitions(InpAST->AST, Pos, Index));
+    CB(clangd::locateSymbolAt(InpAST->AST, Pos, Index));
   };
 
   WorkScheduler.runWithAST("Definitions", File, Bind(Action, std::move(CB)));
 }
 
 llvm::Optional<Path> ClangdServer::switchSourceHeader(PathRef Path) {
 
   llvm::StringRef SourceExtensions[] = {".cpp", ".c", ".cc", ".cxx",

clangd/XRefs.h

 #include "Protocol.h"
 #include "index/Index.h"
 #include "llvm/ADT/Optional.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)
+//  - a declaration and a distinct definition (e.g. function declared in header)
+//  - a declaration and an equal definition (e.g. inline function, or class)
+// For some types of symbol, e.g. macros, definition == declaration always.
+struct LocatedSymbol {
+  // The (unqualified) name of the symbol.
+  std::string Name;
+  // The canonical or best declaration: where most users find its interface.
+  Location PreferredDeclaration;
+  // Where the symbol is defined, if known. May equal PreferredDeclaration.
+  llvm::Optional<Location> Definition;
+};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const LocatedSymbol &);
 /// Get definition of symbol at a specified \p Pos.
-std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos,
+/// 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);
 
 /// Get the hover information when hovering at \p Pos.
 llvm::Optional<Hover> getHover(ParsedAST &AST, Position Pos);
 

clangd/XRefs.cpp

 #include "clang/Index/IndexingAction.h"
 #include "clang/Index/USRGeneration.h"
 #include "llvm/Support/Path.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 
-// Get the definition from a given declaration `D`.
-// Return nullptr if no definition is found, or the declaration type of `D` is
-// not supported.
+// Returns the single definition of the entity declared by D, if visible.
+// In particular:
+// - for non-redeclarable kinds (e.g. local vars), return D
+// - for kinds that allow multiple definitions (e.g. namespaces), return nullptr
+// Kinds of nodes that always return nullptr here will not have definitions
+// reported by locateSymbolAt().
 const Decl *getDefinition(const Decl *D) {
   assert(D);
+  // Decl has one definition that we can find.
   if (const auto *TD = dyn_cast<TagDecl>(D))
     return TD->getDefinition();
-  else if (const auto *VD = dyn_cast<VarDecl>(D))
+  if (const auto *VD = dyn_cast<VarDecl>(D))
     return VD->getDefinition();
-  else if (const auto *FD = dyn_cast<FunctionDecl>(D))
+  if (const auto *FD = dyn_cast<FunctionDecl>(D))
     return FD->getDefinition();
-  return nullptr;
+  // Only a single declaration is allowed.
+  if (isa<ValueDecl>(D)) // except cases above
+    return D;
+  // Multiple definitions are allowed.
+  return nullptr; // except cases above
 }
 
 void logIfOverflow(const SymbolLocation &Loc) {
   if (Loc.Start.hasOverflow() || Loc.End.hasOverflow())
     log("Possible overflow in symbol location: {0}", Loc);
 }
 
 // Convert a SymbolLocation to LSP's Location.
llvm::Optional<Location> makeLocation(ParsedAST &AST, SourceLocation TokLoc,
   Location L;
   L.uri = URIForFile::canonicalize(*FilePath, TUPath);
   L.range = getTokenRange(AST, TokLoc);
   return L;
 }
 
 } // namespace
 
-std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos,
+std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
                                           const SymbolIndex *Index) {
   const auto &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 {};
   }
 
-  std::vector<Location> Result;
-  // Handle goto definition for #include.
+  // Treat #included files as symbols, to enable go-to-definition on them.
   for (auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
-    if (!Inc.Resolved.empty() && Inc.R.start.line == Pos.line)
-      Result.push_back(
-          Location{URIForFile::canonicalize(Inc.Resolved, *MainFilePath), {}});
+    if (!Inc.Resolved.empty() && Inc.R.start.line == Pos.line) {
+      LocatedSymbol File;
+      File.Name = llvm::sys::path::filename(Inc.Resolved);
+      File.PreferredDeclaration = {
+          URIForFile::canonicalize(Inc.Resolved, *MainFilePath), Range{}};
+      File.Definition = File.PreferredDeclaration;
+      // We're not going to find any further symbols on #include lines.
+      return {std::move(File)};
+    }
   }
-  if (!Result.empty())
-    return Result;
 
-  // Identified symbols at a specific position.
   SourceLocation SourceLocationBeg =
       getBeginningOfIdentifier(AST, Pos, SM.getMainFileID());
   auto Symbols = getSymbolAtPosition(AST, SourceLocationBeg);
 
-  for (auto Item : Symbols.Macros) {
-    auto Loc = Item.Info->getDefinitionLoc();
-    auto L = makeLocation(AST, Loc, *MainFilePath);
-    if (L)
-      Result.push_back(*L);
+  // 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)) {
+      LocatedSymbol Macro;
+      Macro.Name = M.Name;
+      Macro.PreferredDeclaration = *Loc;
+      Macro.Definition = Loc;
+      Result.push_back(std::move(Macro));
+    }
   }
 
-  // Declaration and definition are different terms in C-family languages, and
-  // LSP only defines the "GoToDefinition" specification, so we try to perform
-  // the "most sensible" GoTo operation:
-  //
-  //  - We use the location from AST and index (if available) to provide the
-  //    final results. When there are duplicate results, we prefer AST over
-  //    index because AST is more up-to-date.
-  //
-  //  - For each symbol, we will return a location of the canonical declaration
-  //    (e.g. function declaration in header), and a location of definition if
-  //    they are available.
-  //
-  // So the work flow:
-  //
-  //   1. Identify the symbols being search for by traversing the AST.
-  //   2. Populate one of the locations with the AST location.
-  //   3. Use the AST information to query the index, and populate the index
-  //      location (if available).
-  //   4. Return all populated locations for all symbols, definition first (
-  //      which  we think is the users wants most often).
-  struct CandidateLocation {
-    llvm::Optional<Location> Def;
-    llvm::Optional<Location> Decl;
-  };
-  // We respect the order in Symbols.Decls.
-  llvm::SmallVector<CandidateLocation, 8> ResultCandidates;
-  llvm::DenseMap<SymbolID, size_t> CandidatesIndex;
+  // 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 DeclInfo &DI : Symbols.Decls) {
     const Decl *D = DI.D;
-    // Fake key for symbols don't have USR (no SymbolID).
-    // Ideally, there should be a USR for each identified symbols. Symbols
-    // without USR are rare and unimportant cases, we use the a fake holder to
-    // minimize the invasiveness of these cases.
-    SymbolID Key("");
-    if (auto ID = getSymbolID(D))
-      Key = *ID;
+    auto Loc = makeLocation(AST, findNameLoc(D), *MainFilePath);
+    if (!Loc)
+      continue;
 
-    auto R = CandidatesIndex.try_emplace(Key, ResultCandidates.size());
-    if (R.second) // new entry
-      ResultCandidates.emplace_back();
-    auto &Candidate = ResultCandidates[R.first->second];
-
-    auto Loc = findNameLoc(D);
-    auto L = makeLocation(AST, Loc, *MainFilePath);
-    // The declaration in the identified symbols is a definition if possible
-    // otherwise it is declaration.
-    bool IsDef = getDefinition(D) == D;
-    // Populate one of the slots with location for the AST.
-    if (!IsDef)
-      Candidate.Decl = L;
-    else
-      Candidate.Def = L;
+    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.
+    if (getDefinition(D) == D)
+      Result.back().Definition = *Loc;
+
+    // Record SymbolID for index lookup later.
+    if (auto ID = getSymbolID(D))
+      ResultIndex[*ID] = Result.size() - 1;
   }
 
-  if (Index) {
+  // Now query the index for all Symbol IDs we found in the AST.
+  if (Index && !ResultIndex.empty()) {
     LookupRequest QueryRequest;
-    // Build request for index query, using SymbolID.
-    for (auto It : CandidatesIndex)
+    for (auto It : ResultIndex)
       QueryRequest.IDs.insert(It.first);
-    std::string TUPath;
-    const FileEntry *FE = SM.getFileEntryForID(SM.getMainFileID());
-    if (auto Path = getCanonicalPath(FE, SM))
-      TUPath = *Path;
-    // Query the index and populate the empty slot.
-    Index->lookup(QueryRequest, [&TUPath, &ResultCandidates,
-                                 &CandidatesIndex](const Symbol &Sym) {
-      auto It = CandidatesIndex.find(Sym.ID);
-      assert(It != CandidatesIndex.end());
-      auto &Value = ResultCandidates[It->second];
-
-      if (!Value.Def)
-        Value.Def = toLSPLocation(Sym.Definition, TUPath);
-      if (!Value.Decl)
-        Value.Decl = toLSPLocation(Sym.CanonicalDeclaration, TUPath);
-    });
-  }
+    Index->lookup(QueryRequest, [&](const Symbol &Sym) {
+      auto &R = Result[ResultIndex.lookup(Sym.ID)];
+
+      // Special case: if the AST yielded a definition, then it may not be
+      // the right *declaration*. Prefer the one from the index.
+      if (R.Definition) // from AST
+        if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration, *MainFilePath))
+          R.PreferredDeclaration = *Loc;
 
-  // Populate the results, definition first.
-  for (const auto &Candidate : ResultCandidates) {
-    if (Candidate.Def)
-      Result.push_back(*Candidate.Def);
-    if (Candidate.Decl &&
-        Candidate.Decl != Candidate.Def) // Decl and Def might be the same
-      Result.push_back(*Candidate.Decl);
+      if (!R.Definition)
+        R.Definition = toLSPLocation(Sym.Definition, *MainFilePath);
+    });
   }
 
   return Result;
 }
 
 namespace {
 
 /// Collects references to symbols within the main file.
if (!index::generateUSRForMacro(NewMacro.name,
       NewMacro.ID = SymbolID(NewMacro.USR);
     }
     Results.push_back(std::move(NewMacro));
   }
 
   return Results;
 }
 
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const LocatedSymbol &S) {
+  OS << S.Name << ": " << S.PreferredDeclaration;
+  if (S.Definition)
+    OS << " def=" << *S.Definition;
+  return OS;
+}
+
 } // namespace clangd
 } // namespace clang

test/clangd/initialize-params.test

 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
 # Test initialize request parameters with rootUri
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootUri":"test:///workspace","capabilities":{},"trace":"off"}}
 #      CHECK:  "id": 0,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": {
 # CHECK-NEXT:    "capabilities": {
 # CHECK-NEXT:      "codeActionProvider": true,
 # CHECK-NEXT:      "completionProvider": {
 # CHECK-NEXT:        "resolveProvider": false,
 # CHECK-NEXT:        "triggerCharacters": [
 # CHECK-NEXT:          ".",
 # CHECK-NEXT:          ">",
 # CHECK-NEXT:          ":"
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
+# CHECK-NEXT:      "declarationProvider": true,
 # CHECK-NEXT:      "definitionProvider": true,
 # CHECK-NEXT:      "documentFormattingProvider": true,
 # CHECK-NEXT:      "documentHighlightProvider": true,
 # CHECK-NEXT:      "documentOnTypeFormattingProvider": {
 # CHECK-NEXT:        "firstTriggerCharacter": "}",
 # CHECK-NEXT:        "moreTriggerCharacter": []
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "documentRangeFormattingProvider": true,

unittests/clangd/ClangdTests.cpp

 using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::Field;
 using ::testing::Gt;
 using ::testing::IsEmpty;
 using ::testing::Pair;
 using ::testing::UnorderedElementsAre;
 
-MATCHER_P2(FileRange, File, Range, "") {
-  return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
+MATCHER_P2(DeclAt, File, Range, "") {
+  return arg.PreferredDeclaration ==
+         Location{URIForFile::canonicalize(File, testRoot()), Range};
 }
 
 bool diagsContainErrors(const std::vector<Diag> &Diagnostics) {
   for (auto D : Diagnostics) {
     if (D.Severity == DiagnosticsEngine::Error ||
         D.Severity == DiagnosticsEngine::Fatal)
       return true;
   }
)cpp");
   Server.addDocument(BarCpp, BarSource.code());
   Server.addDocument(BazCpp, BazSource.code());
   ASSERT_TRUE(Server.blockUntilIdleForTest());
 
   EXPECT_THAT(DiagConsumer.filesWithDiags(),
               UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, true),
                                    Pair(BazCpp, false)));
 
-  auto Locations = runFindDefinitions(Server, FooCpp, FooSource.point());
+  auto Locations = runLocateSymbolAt(Server, FooCpp, FooSource.point());
   EXPECT_TRUE(bool(Locations));
-  EXPECT_THAT(*Locations,
-              ElementsAre(FileRange(FooCpp, FooSource.range("one"))));
+  EXPECT_THAT(*Locations, ElementsAre(DeclAt(FooCpp, FooSource.range("one"))));
 
   // Undefine MACRO, close baz.cpp.
   CDB.ExtraClangFlags.clear();
   DiagConsumer.clear();
   Server.removeDocument(BazCpp);
   Server.addDocument(FooCpp, FooSource.code(), WantDiagnostics::Auto);
   Server.addDocument(BarCpp, BarSource.code(), WantDiagnostics::Auto);
   ASSERT_TRUE(Server.blockUntilIdleForTest());
 
   EXPECT_THAT(DiagConsumer.filesWithDiags(),
               UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, false)));
 
-  Locations = runFindDefinitions(Server, FooCpp, FooSource.point());
+  Locations = runLocateSymbolAt(Server, FooCpp, FooSource.point());
   EXPECT_TRUE(bool(Locations));
-  EXPECT_THAT(*Locations,
-              ElementsAre(FileRange(FooCpp, FooSource.range("two"))));
+  EXPECT_THAT(*Locations, ElementsAre(DeclAt(FooCpp, FooSource.range("two"))));
 }
 
 TEST_F(ClangdVFSTest, MemoryUsage) {
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB;
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
TEST_F(ClangdVFSTest, InvalidCompileCommand) {
   // CompileCommand. We pass the file in ExtraFlags once and CDB adds another
   // one in getCompileCommand().
   CDB.ExtraClangFlags.push_back(FooCpp);
 
   // Clang can't parse command args in that case, but we shouldn't crash.
   runAddDocument(Server, FooCpp, "int main() {}");
 
   EXPECT_EQ(runDumpAST(Server, FooCpp), "<no-ast>");
-  EXPECT_ERROR(runFindDefinitions(Server, FooCpp, Position()));
+  EXPECT_ERROR(runLocateSymbolAt(Server, FooCpp, Position()));
   EXPECT_ERROR(runFindDocumentHighlights(Server, FooCpp, Position()));
   EXPECT_ERROR(runRename(Server, FooCpp, Position(), "new_name"));
   // FIXME: codeComplete and signatureHelp should also return errors when they
   // can't parse the file.
   EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, Position(),
                                        clangd::CodeCompleteOptions()))
                   .Completions,
               IsEmpty());
auto CodeCompletionRequest = [&]() {
       // tests slow, since there's no way to cancel previous completion
       // requests as opposed to AddDocument/RemoveDocument, which are implicitly
       // cancelled by any subsequent AddDocument/RemoveDocument request to the
       // same file.
       cantFail(runCodeComplete(Server, FilePaths[FileIndex], Pos,
                                clangd::CodeCompleteOptions()));
     };
 
-    auto FindDefinitionsRequest = [&]() {
+    auto LocateSymbolRequest = [&]() {
       unsigned FileIndex = FileIndexDist(RandGen);
       // Make sure we don't violate the ClangdServer's contract.
       if (ReqStats[FileIndex].FileIsRemoved)
         AddDocument(FileIndex, /*SkipCache=*/false);
 
       Position Pos;
       Pos.line = LineDist(RandGen);
       Pos.character = ColumnDist(RandGen);
 
-      ASSERT_TRUE(!!runFindDefinitions(Server, FilePaths[FileIndex], Pos));
+      ASSERT_TRUE(!!runLocateSymbolAt(Server, FilePaths[FileIndex], Pos));
     };
 
     std::vector<std::function<void()>> AsyncRequests = {
         AddDocumentRequest, ForceReparseRequest, RemoveDocumentRequest};
     std::vector<std::function<void()>> BlockingRequests = {
-        CodeCompletionRequest, FindDefinitionsRequest};
+        CodeCompletionRequest, LocateSymbolRequest};
 
     // Bash requests to ClangdServer in a loop.
     std::uniform_int_distribution<int> AsyncRequestIndexDist(
         0, AsyncRequests.size() - 1);
     std::uniform_int_distribution<int> BlockingRequestIndexDist(
         0, BlockingRequests.size() - 1);
     for (unsigned I = 1; I <= RequestsCount; ++I) {
       if (I % BlockingRequestInterval != 0) {

unittests/clangd/SyncAPI.h

 
 llvm::Expected<CodeCompleteResult>
 runCodeComplete(ClangdServer &Server, PathRef File, Position Pos,
                 clangd::CodeCompleteOptions Opts);
 
 llvm::Expected<SignatureHelp> runSignatureHelp(ClangdServer &Server,
                                                PathRef File, Position Pos);
 
-llvm::Expected<std::vector<Location>>
-runFindDefinitions(ClangdServer &Server, PathRef File, Position Pos);
+llvm::Expected<std::vector<LocatedSymbol>>
+runLocateSymbolAt(ClangdServer &Server, PathRef File, Position Pos);
 
 llvm::Expected<std::vector<DocumentHighlight>>
 runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos);
 
 llvm::Expected<std::vector<tooling::Replacement>>
 runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName);
 
 std::string runDumpAST(ClangdServer &Server, PathRef File);

unittests/clangd/SyncAPI.cpp

 
 llvm::Expected<SignatureHelp> runSignatureHelp(ClangdServer &Server,
                                                PathRef File, Position Pos) {
   llvm::Optional<llvm::Expected<SignatureHelp>> Result;
   Server.signatureHelp(File, Pos, capture(Result));
   return std::move(*Result);
 }
 
-llvm::Expected<std::vector<Location>>
-runFindDefinitions(ClangdServer &Server, PathRef File, Position Pos) {
-  llvm::Optional<llvm::Expected<std::vector<Location>>> Result;
-  Server.findDefinitions(File, Pos, capture(Result));
+llvm::Expected<std::vector<LocatedSymbol>>
+runLocateSymbolAt(ClangdServer &Server, PathRef File, Position Pos) {
+  llvm::Optional<llvm::Expected<std::vector<LocatedSymbol>>> Result;
+  Server.locateSymbolAt(File, Pos, capture(Result));
   return std::move(*Result);
 }
 
 llvm::Expected<std::vector<DocumentHighlight>>
 runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos) {
   llvm::Optional<llvm::Expected<std::vector<DocumentHighlight>>> Result;
   Server.findDocumentHighlights(File, Pos, capture(Result));
   return std::move(*Result);

unittests/clangd/TUSchedulerTests.cpp

TEST_F(TUSchedulerTests, TUStatus) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   ClangdServer Server(CDB, FS, CaptureTUStatus, ClangdServer::optsForTest());
   Annotations Code("int m^ain () {}");
 
   // We schedule the following tasks in the queue:
   //   [Update] [GoToDefinition]
   Server.addDocument(testPath("foo.cpp"), Code.code(), WantDiagnostics::Yes);
-  Server.findDefinitions(testPath("foo.cpp"), Code.point(),
-                         [](Expected<std::vector<Location>> Result) {
-                           ASSERT_TRUE((bool)Result);
-                         });
+  Server.locateSymbolAt(testPath("foo.cpp"), Code.point(),
+                        [](Expected<std::vector<LocatedSymbol>> Result) {
 
   ASSERT_TRUE(Server.blockUntilIdleForTest());
 
   EXPECT_THAT(CaptureTUStatus.AllStatus,
               ElementsAre(
                   // Statuses of "Update" action.
                   TUState(TUAction::RunningAction, "Update"),
                   TUState(TUAction::BuildingPreamble, "Update"),

unittests/clangd/XRefsTests.cpp

 #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/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 
 using testing::ElementsAre;
-using testing::Field;
 using testing::IsEmpty;
 using testing::Matcher;
 using testing::UnorderedElementsAreArray;
 
 class IgnoreDiagnostics : public DiagnosticsConsumer {
   void onDiagnosticsReady(PathRef File,
                           std::vector<Diag> Diagnostics) override {}
 };
TEST(HighlightsTest, All) {
   for (const char *Test : Tests) {
     Annotations T(Test);
     auto AST = TestTU::withCode(T.code()).build();
     EXPECT_THAT(findDocumentHighlights(AST, T.point()), HighlightsFrom(T))
         << Test;
   }
 }
 
+MATCHER_P3(Sym, Name, Decl, DefOrNone, "") {
+  llvm::Optional<Range> Def = DefOrNone;
+  if (Name != arg.Name) {
+    *result_listener << "Name is " << arg.Name;
+    return false;
+  }
+  if (Decl != arg.PreferredDeclaration.range) {
+    *result_listener << "Declaration is "
+                     << llvm::to_string(arg.PreferredDeclaration);
+    return false;
+  }
+  if (Def && !arg.Definition) {
+    *result_listener << "Has no definition";
+    return false;
+  }
+  if (Def && arg.Definition->range != *Def) {
+    *result_listener << "Definition is " << llvm::to_string(arg.Definition);
+    return false;
+  }
+  return true;
+}
+testing::Matcher<LocatedSymbol> Sym(std::string Name, Range Decl) {
+  return Sym(Name, Decl, llvm::None);
+}
+MATCHER_P(Sym, Name, "") { return arg.Name == Name; }
+
 MATCHER_P(RangeIs, R, "") { return arg.range == R; }
 
-TEST(GoToDefinition, WithIndex) {
+TEST(LocateSymbol, WithIndex) {
   Annotations SymbolHeader(R"cpp(
         class $forward[[Forward]];
         class $foo[[Foo]] {};
 
         void $f1[[f1]]();
 
         inline void $f2[[f2]]() {}
       )cpp");
   Annotations SymbolCpp(R"cpp(
       class $forward[[forward]] {};
       void  $f1[[f1]]() {}
     )cpp");
 
   TestTU TU;
   TU.Code = SymbolCpp.code();
   TU.HeaderCode = SymbolHeader.code();
   auto Index = TU.index();
-  auto runFindDefinitionsWithIndex = [&Index](const Annotations &Main) {
+  auto LocateWithIndex = [&Index](const Annotations &Main) {
     auto AST = TestTU::withCode(Main.code()).build();
-    return clangd::findDefinitions(AST, Main.point(), Index.get());
+    return clangd::locateSymbolAt(AST, Main.point(), Index.get());
   };
 
   Annotations Test(R"cpp(// only declaration in AST.
         void [[f1]]();
         int main() {
           ^f1();
         }
       )cpp");
-  EXPECT_THAT(runFindDefinitionsWithIndex(Test),
-              testing::ElementsAreArray(
-                  {RangeIs(SymbolCpp.range("f1")), RangeIs(Test.range())}));
+  EXPECT_THAT(LocateWithIndex(Test),
+              ElementsAre(Sym("f1", Test.range(), SymbolCpp.range("f1"))));
 
   Test = Annotations(R"cpp(// definition in AST.
         void [[f1]]() {}
         int main() {
           ^f1();
         }
       )cpp");
-  EXPECT_THAT(runFindDefinitionsWithIndex(Test),
-              testing::ElementsAreArray(
-                  {RangeIs(Test.range()), RangeIs(SymbolHeader.range("f1"))}));
+  EXPECT_THAT(LocateWithIndex(Test),
+              ElementsAre(Sym("f1", SymbolHeader.range("f1"), Test.range())));
 
   Test = Annotations(R"cpp(// forward declaration in AST.
         class [[Foo]];
         F^oo* create();
       )cpp");
-  EXPECT_THAT(runFindDefinitionsWithIndex(Test),
-              testing::ElementsAreArray(
-                  {RangeIs(SymbolHeader.range("foo")), RangeIs(Test.range())}));
+  EXPECT_THAT(LocateWithIndex(Test),
+              ElementsAre(Sym("Foo", Test.range(), SymbolHeader.range("foo"))));
 
   Test = Annotations(R"cpp(// defintion in AST.
         class [[Forward]] {};
         F^orward create();
       )cpp");
-  EXPECT_THAT(runFindDefinitionsWithIndex(Test),
-              testing::ElementsAreArray({
-                  RangeIs(Test.range()),
-                  RangeIs(SymbolHeader.range("forward")),
-              }));
+  EXPECT_THAT(
+      LocateWithIndex(Test),
+      ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range())));
 }
 
-TEST(GoToDefinition, All) {
+TEST(LocateSymbol, All) {
+  // Ranges in tests:
+  //   $decl is the declaration location (if absent, no symbol is located)
+  //   $def is the definition location (if absent, symbol has no definition)
+  //   unnamed range becomes both $decl and $def.
   const char *Tests[] = {
       R"cpp(// Local variable
         int main() {
           int [[bonjour]];
           ^bonjour = 2;
           int test1 = bonjour;
         }
       )cpp",
const char *Tests[] = {
       R"cpp(// Function definition via pointer
         int [[foo]](int) {}
         int main() {
           auto *X = &^foo;
         }
       )cpp",
 
       R"cpp(// Function declaration via call
-        int [[foo]](int);
+        int $decl[[foo]](int);
         int main() {
           return ^foo(42);
         }
       )cpp",
 
       R"cpp(// Field
         struct Foo { int [[x]]; };
         int main() {
const char *Tests[] = {
       R"cpp(// Field, field designator
         struct Foo { int [[x]]; };
         int main() {
           Foo bar = { .^x = 2 };
         }
       )cpp",
 
       R"cpp(// Method call
-        struct Foo { int [[x]](); };
+        struct Foo { int $decl[[x]](); };
         int main() {
           Foo bar;
           bar.^x();
         }
       )cpp",
 
       R"cpp(// Typedef
-        typedef int [[Foo]];
+        typedef int $decl[[Foo]];
         int main() {
           ^Foo bar;
         }
       )cpp",
 
       /* FIXME: clangIndex doesn't handle template type parameters
       R"cpp(// Template type parameter
         template <[[typename T]]>
         void foo() { ^T t; }
       )cpp", */
 
       R"cpp(// Namespace
-        namespace [[ns]] {
+        namespace $decl[[ns]] {
         struct Foo { static void bar(); }
         } // namespace ns
         int main() { ^ns::Foo::bar(); }
       )cpp",
 
       R"cpp(// Macro
         #define MACRO 0
         #define [[MACRO]] 1
const char *Tests[] = {
       R"cpp(
          #define FF() class [[Test]] {};
          FF();
          void f() { T^est a; }
       )cpp",
   };
   for (const char *Test : Tests) {
     Annotations T(Test);
+    llvm::Optional<Range> WantDecl;
+    llvm::Optional<Range> WantDef;
+    if (!T.ranges().empty())
+      WantDecl = WantDef = T.range();
+    if (!T.ranges("decl").empty())
+      WantDecl = T.range("decl");
+    if (!T.ranges("def").empty())
+      WantDef = T.range("def");
+
     auto AST = TestTU::withCode(T.code()).build();
-    std::vector<Matcher<Location>> ExpectedLocations;
-    for (const auto &R : T.ranges())
-      ExpectedLocations.push_back(RangeIs(R));
-    EXPECT_THAT(findDefinitions(AST, T.point()),
-                ElementsAreArray(ExpectedLocations))
-        << Test;
+    auto Results = locateSymbolAt(AST, T.point());
+
+    if (!WantDecl) {
+      EXPECT_THAT(Results, IsEmpty()) << Test;
+    } else {
+      ASSERT_THAT(Results, ::testing::SizeIs(1)) << Test;
+      EXPECT_EQ(Results[0].PreferredDeclaration.range, *WantDecl) << Test;
+      llvm::Optional<Range> GotDef;
+      if (Results[0].Definition)
+        GotDef = Results[0].Definition->range;
+      EXPECT_EQ(WantDef, GotDef) << Test;
+    }
   }
 }
 
-TEST(GoToDefinition, Rank) {
+TEST(LocateSymbol, Ambiguous) {
   auto T = Annotations(R"cpp(
-    struct $foo1[[Foo]] {
-      $foo2[[Foo]]();
-      $foo3[[Foo]](Foo&&);
-      $foo4[[Foo]](const char*);
+    struct Foo {
+      Foo();
+      Foo(Foo&&);
+      Foo(const char*);
     };
 
-    Foo $f[[f]]();
+    Foo f();
 
-    void $g[[g]](Foo foo);
+    void g(Foo foo);
 
     void call() {
-      const char* $str[[str]] = "123";
+      const char* str = "123";
       Foo a = $1^str;
       Foo b = Foo($2^str);
       Foo c = $3^f();
       $4^g($5^f());
       g($6^str);
     }
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
-  EXPECT_THAT(findDefinitions(AST, T.point("1")),
-              ElementsAre(RangeIs(T.range("str")), RangeIs(T.range("foo4"))));
-  EXPECT_THAT(findDefinitions(AST, T.point("2")),
-              ElementsAre(RangeIs(T.range("str"))));
-  EXPECT_THAT(findDefinitions(AST, T.point("3")),
-              ElementsAre(RangeIs(T.range("f")), RangeIs(T.range("foo3"))));
-  EXPECT_THAT(findDefinitions(AST, T.point("4")),
-              ElementsAre(RangeIs(T.range("g"))));
-  EXPECT_THAT(findDefinitions(AST, T.point("5")),
-              ElementsAre(RangeIs(T.range("f")), RangeIs(T.range("foo3"))));
-
-  auto DefinitionAtPoint6 = findDefinitions(AST, T.point("6"));
-  EXPECT_EQ(3ul, DefinitionAtPoint6.size());
-  EXPECT_THAT(DefinitionAtPoint6, HasSubsequence(RangeIs(T.range("str")),
-                                                 RangeIs(T.range("foo4"))));
-  EXPECT_THAT(DefinitionAtPoint6, HasSubsequence(RangeIs(T.range("str")),
-                                                 RangeIs(T.range("foo3"))));
+  // Ordered assertions are deliberate: we expect a predictable order.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("1")),
+              ElementsAre(Sym("str"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("2")), ElementsAre(Sym("str")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("3")),
+              ElementsAre(Sym("f"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("4")), ElementsAre(Sym("g")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("5")),
+              ElementsAre(Sym("f"), Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("6")),
+              ElementsAre(Sym("str"), Sym("Foo"), Sym("Foo")));
 }
 
-TEST(GoToDefinition, RelPathsInCompileCommand) {
+TEST(LocateSymbol, RelPathsInCompileCommand) {
   // The source is in "/clangd-test/src".
   // We build in "/clangd-test/build".
 
   Annotations SourceAnnotations(R"cpp(
 #include "header_in_preamble.h"
 int [[foo]];
 #include "header_not_in_preamble.h"
 int baz = f$p1^oo + bar_pre$p2^amble + bar_not_pre$p3^amble;
)cpp");
   FS.Files[HeaderInPreambleH] = HeaderInPreambleAnnotations.code();
   auto HeaderNotInPreambleH = testPath("src/header_not_in_preamble.h");
   FS.Files[HeaderNotInPreambleH] = HeaderNotInPreambleAnnotations.code();
 
   runAddDocument(Server, FooCpp, SourceAnnotations.code());
 
   // Go to a definition in main source file.
   auto Locations =
-      runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p1"));
+      runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("p1"));
   EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations,
-              ElementsAre(FileRange(FooCpp, SourceAnnotations.range())));
+  EXPECT_THAT(*Locations, ElementsAre(Sym("foo", SourceAnnotations.range())));
 
   // Go to a definition in header_in_preamble.h.
-  Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p2"));
+  Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("p2"));
   EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations,
-              ElementsAre(FileRange(HeaderInPreambleH,
-                                    HeaderInPreambleAnnotations.range())));
+  EXPECT_THAT(
+      *Locations,
+      ElementsAre(Sym("bar_preamble", HeaderInPreambleAnnotations.range())));
 
   // Go to a definition in header_not_in_preamble.h.
-  Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p3"));
+  Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("p3"));
   EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
   EXPECT_THAT(*Locations,
-              ElementsAre(FileRange(HeaderNotInPreambleH,
-                                    HeaderNotInPreambleAnnotations.range())));
+              ElementsAre(Sym("bar_not_preamble",
 }
 
 TEST(Hover, All) {
   struct OneTest {
     StringRef Input;
     StringRef ExpectedHover;
   };
 
const char *HeaderContents = R"cpp([[]]#pragma once
                                      )cpp";
   Annotations HeaderAnnotations(HeaderContents);
   FS.Files[FooH] = HeaderAnnotations.code();
 
   Server.addDocument(FooH, HeaderAnnotations.code());
   Server.addDocument(FooCpp, SourceAnnotations.code());
 
   // Test include in preamble.
-  auto Locations =
-      runFindDefinitions(Server, FooCpp, SourceAnnotations.point());
-  ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations,
-              ElementsAre(FileRange(FooH, HeaderAnnotations.range())));
+  auto Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point());
+  ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
+  EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range())));
 
   // Test include in preamble, last char.
-  Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("2"));
-  ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations,
-              ElementsAre(FileRange(FooH, HeaderAnnotations.range())));
-
-  Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("3"));
-  ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations,
-              ElementsAre(FileRange(FooH, HeaderAnnotations.range())));
+  Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("2"));
+  ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
+  EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range())));
+
+  Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("3"));
+  ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
+  EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range())));
 
   // Test include outside of preamble.
-  Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("6"));
-  ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations,
-              ElementsAre(FileRange(FooH, HeaderAnnotations.range())));
+  Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("6"));
+  ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
+  EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range())));
 
   // Test a few positions that do not result in Locations.
-  Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("4"));
-  ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+  Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("4"));
+  ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
   EXPECT_THAT(*Locations, IsEmpty());
 
-  Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("5"));
-  ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations,
-              ElementsAre(FileRange(FooH, HeaderAnnotations.range())));
+  Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("5"));
+  ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
+  EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range())));
 
-  Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("7"));
-  ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-  EXPECT_THAT(*Locations,
-              ElementsAre(FileRange(FooH, HeaderAnnotations.range())));
+  Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("7"));
+  ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
+  EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range())));
 }
 
-TEST(GoToDefinition, WithPreamble) {
+TEST(LocateSymbol, WithPreamble) {
   // Test stragety: AST should always use the latest preamble instead of last
   // good preamble.
   MockFSProvider FS;
   IgnoreDiagnostics DiagConsumer;
   MockCompilationDatabase CDB;
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   auto FooCpp = testPath("foo.cpp");
   // The trigger locations must be the same.
   Annotations FooWithHeader(R"cpp(#include "fo^o.h")cpp");
   Annotations FooWithoutHeader(R"cpp(double    [[fo^o]]();)cpp");
 
   FS.Files[FooCpp] = FooWithHeader.code();
 
   auto FooH = testPath("foo.h");
   Annotations FooHeader(R"cpp([[]])cpp");
   FS.Files[FooH] = FooHeader.code();
 
   runAddDocument(Server, FooCpp, FooWithHeader.code());
-  // GoToDefinition goes to a #include file: the result comes from the preamble.
+  // LocateSymbol goes to a #include file: the result comes from the preamble.
   EXPECT_THAT(
-      cantFail(runFindDefinitions(Server, FooCpp, FooWithHeader.point())),
-      ElementsAre(FileRange(FooH, FooHeader.range())));
+      cantFail(runLocateSymbolAt(Server, FooCpp, FooWithHeader.point())),
+      ElementsAre(Sym("foo.h", FooHeader.range())));
 
   // Only preamble is built, and no AST is built in this request.
   Server.addDocument(FooCpp, FooWithoutHeader.code(), WantDiagnostics::No);
   // We build AST here, and it should use the latest preamble rather than the
   // stale one.
   EXPECT_THAT(
-      cantFail(runFindDefinitions(Server, FooCpp, FooWithoutHeader.point())),
-      ElementsAre(FileRange(FooCpp, FooWithoutHeader.range())));
+      cantFail(runLocateSymbolAt(Server, FooCpp, FooWithoutHeader.point())),
+      ElementsAre(Sym("foo", FooWithoutHeader.range())));
 
   // Reset test environment.
   runAddDocument(Server, FooCpp, FooWithHeader.code());
   // Both preamble and AST are built in this request.
   Server.addDocument(FooCpp, FooWithoutHeader.code(), WantDiagnostics::Yes);
   // Use the AST being built in above request.
   EXPECT_THAT(
-      cantFail(runFindDefinitions(Server, FooCpp, FooWithoutHeader.point())),
-      ElementsAre(FileRange(FooCpp, FooWithoutHeader.range())));
+      cantFail(runLocateSymbolAt(Server, FooCpp, FooWithoutHeader.point())),
+      ElementsAre(Sym("foo", FooWithoutHeader.range())));
 }
 
 TEST(FindReferences, WithinAST) {
   const char *Tests[] = {
       R"cpp(// Local variable
         int main() {
           int [[foo]];
           [[^foo]] = 2;