[clangd] Include the underlying decls in go-to-definition.

[hokein]

Summary

Fixes https://github.com/clangd/clangd/issues/277

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: CMakeCache.txt, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[hokein]

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

674

we need to avoid the underlying decl in this case.

[hokein]

no regression on non-definition of renaming alias.

[hokein]

remove an accident change.

[hokein]

friendly ping, this patch should be ready for review.

[nridge]

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

695

Why is it useful to give the using-declaration itself as a result?

It seems like the more useful behaviour from the user's point of view is to jump directly to the definition of class Foo, without having to choose from a popup first.

[sammccall]

Sorry about being late coming back to this.

clang-tools-extra/clangd/XRefs.cpp

306

I believe this is going to do the wrong thing on overload sets.

namespace ns { int x(char); int x(double); }
using ns::x;
int y = ^x('a');

getDeclAtPosition(..., TemplatePattern|Alias) will yield UsingDecl, and then targetDecl(UsingDecl, Underlying) will yield both overloads of x.
However getDeclAtPosition(..., Underlying) would correctly return only the desired overload.
(Please add a test like this!)

This is awkward to work around, it's a failure of the targetDecl API. Maybe for now, call getDeclAtPosition with TemplatePattern|Alias, and if there's exactly one result, replace D with it? And leave a fixme to address more complicated cases somehow.

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

695

+1, that would mean adding a continue after the addresultdecl

[hokein]

address review comment: fix the wrong result for multiple overloads.

[sammccall]

Thanks!

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

720

this is basically the same as the above case (which is fine)
but what about class [[Foo]]; using Bar = ^Foo;?

(maybe this is tested elsewhere already, but it should be moved here)

[hokein]

address comment.

clang-tools-extra/clangd/XRefs.cpp

if (Def == Preferred)
       Result.back().Definition = *Loc;
 
     // Record SymbolID for index lookup later.
     if (auto ID = getSymbolID(Preferred))
       ResultIndex[*ID] = Result.size() - 1;
   };
 
   // Emit all symbol locations (declaration or definition) from AST.
-  DeclRelationSet Relations =
-      DeclRelation::TemplatePattern | DeclRelation::Alias;
+  DeclRelationSet Relations = DeclRelation::TemplatePattern |
+                              DeclRelation::Alias | DeclRelation::Underlying;
   for (const NamedDecl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
     // Special case: void foo() ^override: jump to the overridden method.
     if (const auto *CMD = llvm::dyn_cast<CXXMethodDecl>(D)) {
       const InheritableAttr* Attr = D->getAttr<OverrideAttr>();
       if (!Attr)
         Attr = D->getAttr<FinalAttr>();
       const syntax::Token *Tok =
           spelledIdentifierTouching(SourceLoc, AST.getTokens());
if (SM.getMacroArgExpandedLocation(D->getLocation()) == IdentStartLoc) {
       if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
         AddResultDecl(CTSD->getSpecializedTemplate());
         continue;
       }
     }
 
     // Otherwise the target declaration is the right one.
     AddResultDecl(D);
   }
 
   // Now query the index for all Symbol IDs we found in the AST.
   if (Index && !ResultIndex.empty()) {
     LookupRequest QueryRequest;
     for (auto It : ResultIndex)
       QueryRequest.IDs.insert(It.first);
     std::string Scratch;
     Index->lookup(QueryRequest, [&](const Symbol &Sym) {

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

 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
 using ::testing::Matcher;
 using ::testing::UnorderedElementsAreArray;
 
 MATCHER_P2(FileRange, File, Range, "") {
   return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
 }
+MATCHER(DeclRange, "") {
+  const LocatedSymbol &Sym = ::testing::get<0>(arg);
+  const Range &Range = ::testing::get<1>(arg);
+  return Sym.PreferredDeclaration.range == Range;
+}
 
 // Extracts ranges from an annotated example, and constructs a matcher for a
 // highlight set. Ranges should be named $read/$write as appropriate.
 Matcher<const std::vector<DocumentHighlight> &>
 HighlightsFrom(const Annotations &Test) {
   std::vector<DocumentHighlight> Expected;
   auto Add = [&](const Range &R, DocumentHighlightKind K) {
     Expected.emplace_back();
TEST(LocateSymbol, Ambiguous) {
   EXPECT_THAT(locateSymbolAt(AST, T.point("12")),
               UnorderedElementsAre(Sym("bar", T.range("NonstaticOverload1")),
                                    Sym("bar", T.range("NonstaticOverload2"))));
   EXPECT_THAT(locateSymbolAt(AST, T.point("13")),
               UnorderedElementsAre(Sym("baz", T.range("StaticOverload1")),
                                    Sym("baz", T.range("StaticOverload2"))));
 }
 
-TEST(LocateSymbol, TemplateTypedefs) {
-  auto T = Annotations(R"cpp(
-    template <class T> struct function {};
-    template <class T> using callback = function<T()>;
+TEST(LocateSymbol, Alias) {
+  const char *Tests[] = {
+    R"cpp(
+      template <class T> struct [[function]] {};
+      template <class T> using [[callback]] = function<T()>;
 
       c^allback<int> foo;
-  )cpp");
+    )cpp",
+
+    R"cpp(
+      class [[Foo]] {};
+      typedef Foo [[Ba^r]];
+    )cpp",
+
+    R"cpp(
+      namespace ns { class [[Foo]] {}; }
+      using ns::[[F^oo]];
+    )cpp",
+
+    R"cpp(
+      class [[Foo]] {};
+      using [[B^ar]] = Foo;
+    )cpp",
+  };
+
+  for (const auto* Case : Tests) {
+    SCOPED_TRACE(Case);
+    auto T = Annotations(Case);
     auto AST = TestTU::withCode(T.code()).build();
-  EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+    EXPECT_THAT(locateSymbolAt(AST, T.point()),
+                ::testing::UnorderedPointwise(DeclRange(), T.ranges()));
+  }
 }
 
 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");
 
   Annotations HeaderInPreambleAnnotations(R"cpp(
 int [[bar_preamble]];
 )cpp");
 
   Annotations HeaderNotInPreambleAnnotations(R"cpp(
 int [[bar_not_preamble]];
 )cpp");
 
   // Make the compilation paths appear as ../src/foo.cpp in the compile
   // commands.
   SmallString<32> RelPathPrefix("..");
   llvm::sys::path::append(RelPathPrefix, "src");
   std::string BuildDir = testPath("build");
   MockCompilationDatabase CDB(BuildDir, RelPathPrefix);
 
   MockFSProvider FS;