[clangd] Match AST and Index label for template Symbols

[ilya-biryukov]

Summary

Previsouly, class completions items from the index were missing
template parameters in both the snippet and the label.

[sammccall]

Nice, I'd been wondering about that...

clangd/index/SymbolCollector.cpp

36 ↗

(On Diff #141821)

uber-nit: these three return statements are a bit confusing to me. Maybe omit them and if/elseif, so the default case falls through to the bottom.

331 ↗

(On Diff #141821)

"an actual run" confused me here. Maybe "We use the primary template, as clang does during code completion"?

unittests/clangd/FileIndexTests.cpp

218 ↗

(On Diff #141821)

If snippets are off, we'll get "vector", not "vector<>", right?

(Probably no need to test this explicitly, but I just want to be sure)

[ilya-biryukov]

• Also test plain text completion text
• Clarify the comment
• Simplify conditions in getTemplateOrThis

[ilya-biryukov]

unittests/clangd/FileIndexTests.cpp

218 ↗

(On Diff #141821)

Yes, that's exactly the case.
Testing doesn't hurt too, added a test.

clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp

 //===--- SymbolCollector.cpp -------------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "SymbolCollector.h"
 #include "../AST.h"
 #include "../CodeCompletionStrings.h"
 #include "../Logger.h"
 #include "../URI.h"
 #include "CanonicalIncludes.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/USRGeneration.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 
 namespace clang {
 namespace clangd {
 
 namespace {
+/// If \p ND is a template specialization, returns the primary template.
+/// Otherwise, returns \p ND.
+const NamedDecl &getTemplateOrThis(const NamedDecl &ND) {
+  if (auto Cls = dyn_cast<CXXRecordDecl>(&ND)) {
+    if (auto T = Cls->getDescribedTemplate())
+      return *T;
+  } else if (auto Func = dyn_cast<FunctionDecl>(&ND)) {
+    if (auto T = Func->getPrimaryTemplate())
+      return *T;
+  }
+  return ND;
+}
+
 // Returns a URI of \p Path. Firstly, this makes the \p Path absolute using the
 // current working directory of the given SourceManager if the Path is not an
 // absolute path. If failed, this resolves relative paths against \p FallbackDir
 // to get an absolute path. Then, this tries creating an URI for the absolute
 // path with schemes specified in \p Opts. This returns an URI with the first
 // working scheme, if there is any; otherwise, this returns None.
 //
 // The Path can be a path relative to the build directory, or retrieved from
const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
   std::string FileURI;
   if (auto DeclLoc =
           getSymbolLocation(ND, SM, Opts, ASTCtx->getLangOpts(), FileURI))
     S.CanonicalDeclaration = *DeclLoc;
 
   // Add completion info.
   // FIXME: we may want to choose a different redecl, or combine from several.
   assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
-  CodeCompletionResult SymbolCompletion(&ND, 0);
+  // We use the primary template, as clang does during code completion.
+  CodeCompletionResult SymbolCompletion(&getTemplateOrThis(ND), 0);
   const auto *CCS = SymbolCompletion.CreateCodeCompletionString(
       *ASTCtx, *PP, CodeCompletionContext::CCC_Name, *CompletionAllocator,
       *CompletionTUInfo,
       /*IncludeBriefComments*/ true);
   std::string Label;
   std::string SnippetInsertText;
   std::string IgnoredLabel;
   std::string PlainInsertText;

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

TEST(FileIndexTest, NoIncludeCollected) {
   bool SeenSymbol = false;
   M.fuzzyFind(Req, [&](const Symbol &Sym) {
     EXPECT_TRUE(Sym.Detail->IncludeHeader.empty());
     SeenSymbol = true;
   });
   EXPECT_TRUE(SeenSymbol);
 }
 
+TEST(FileIndexTest, TemplateParamsInLabel) {
+  auto Source = R"cpp(
+template <class Ty>
+class vector {
+};
+
+template <class Ty>
+vector<Ty> make_vector(Ty* begin, Ty* end) {}
+)cpp";
+
+  FileIndex M;
+  M.update("f", build("f", Source).getPointer());
+
+  FuzzyFindRequest Req;
+  Req.Query = "";
+  bool SeenVector = false;
+  bool SeenMakeVector = false;
+  M.fuzzyFind(Req, [&](const Symbol &Sym) {
+    if (Sym.Name == "vector") {
+      EXPECT_EQ(Sym.CompletionLabel, "vector<class Ty>");
+      EXPECT_EQ(Sym.CompletionSnippetInsertText, "vector<${1:class Ty}>");
+      EXPECT_EQ(Sym.CompletionPlainInsertText, "vector");
+      SeenVector = true;
+      return;
+    }
+
+    if (Sym.Name == "make_vector") {
+      EXPECT_EQ(Sym.CompletionLabel, "make_vector(Ty *begin, Ty *end)");
+      EXPECT_EQ(Sym.CompletionSnippetInsertText,
+                "make_vector(${1:Ty *begin}, ${2:Ty *end})");
+      EXPECT_EQ(Sym.CompletionPlainInsertText, "make_vector");
+      SeenMakeVector = true;
+    }
+  });
+  EXPECT_TRUE(SeenVector);
+  EXPECT_TRUE(SeenMakeVector);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang