[clangd] Print qualifiers of out-of-line definitions in document outline

[ilya-biryukov]

Summary

To improve the UX around navigating and searching through the results.

[hokein]

clang-tools-extra/clangd/AST.cpp

106

if .. out-of-line definition seems a bit confusing, I think here we just return a nested name specifier if it is written/present in the source code?

[ilya-biryukov]

• Rename the helper function
• Remove mention of 'out-of-line definition' from a comment

[ilya-biryukov]

clang-tools-extra/clangd/AST.cpp

106

Good point, probably too many related concepts (and even synonyms) mentioned in one function.
Renamed to getQualifier and not mentioning the out-of-line definition anymore.

With the examples, it should be clear what this function is doing.

clang-tools-extra/clangd/AST.cpp

 //===--- AST.cpp - Utility AST functions  -----------------------*- 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 "AST.h"
 
 #include "SourceCode.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/DeclarationName.h"
+#include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Index/USRGeneration.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ScopedPrinter.h"
std::string printQualifiedName(const NamedDecl &ND) {
   // namespaces to query: the preamble doesn't have a dedicated list.
   Policy.SuppressUnwrittenScope = true;
   ND.printQualifiedName(OS, Policy);
   OS.flush();
   assert(!StringRef(QName).startswith("::"));
   return QName;
 }
 
+static bool isAnonymous(const DeclarationName &N) {
+  return N.isIdentifier() && !N.getAsIdentifierInfo();
+}
+
+/// Returns a nested name specifier of \p ND if it was present in the source,
+/// e.g.
+///     void ns::something::foo() -> returns 'ns::something'
+///     void foo() -> returns null
+static NestedNameSpecifier *getQualifier(const NamedDecl &ND) {
+  if (auto *V = llvm::dyn_cast<DeclaratorDecl>(&ND))
+    return V->getQualifier();
+  if (auto *T = llvm::dyn_cast<TagDecl>(&ND))
+    return T->getQualifier();
+  return nullptr;
+}
+
 std::string printName(const ASTContext &Ctx, const NamedDecl &ND) {
   std::string Name;
   llvm::raw_string_ostream Out(Name);
   PrintingPolicy PP(Ctx.getLangOpts());
+
   // Handle 'using namespace'. They all have the same name - <using-directive>.
   if (auto *UD = llvm::dyn_cast<UsingDirectiveDecl>(&ND)) {
     Out << "using namespace ";
     if (auto *Qual = UD->getQualifier())
       Qual->print(Out, PP);
     UD->getNominatedNamespaceAsWritten()->printName(Out);
     return Out.str();
   }
-  ND.getDeclName().print(Out, PP);
-  if (!Out.str().empty()) {
-    Out << printTemplateSpecializationArgs(ND);
-    return Out.str();
-  }
+
+  // Handle anonymous names.
+  if (isAnonymous(ND.getDeclName())) {
     // The name was empty, so present an anonymous entity.
     if (isa<NamespaceDecl>(ND))
       return "(anonymous namespace)";
     if (auto *Cls = llvm::dyn_cast<RecordDecl>(&ND))
       return ("(anonymous " + Cls->getKindName() + ")").str();
     if (isa<EnumDecl>(ND))
       return "(anonymous enum)";
     return "(anonymous)";
   }
 
+  // Print nested name qualifier if it was written in the source code.
+  if (auto *Qualifier = getQualifier(ND))
+    Qualifier->print(Out, PP);
+  // Print the name itself.
+  ND.getDeclName().print(Out, PP);
+  // Print template arguments.
+  Out << printTemplateSpecializationArgs(ND);
+
+  return Out.str();
+}
+
 std::string printTemplateSpecializationArgs(const NamedDecl &ND) {
   std::string TemplateArgs;
   llvm::raw_string_ostream OS(TemplateArgs);
   PrintingPolicy Policy(ND.getASTContext().getLangOpts());
   if (llvm::Optional<llvm::ArrayRef<TemplateArgumentLoc>> Args =
           getTemplateSpecializationArgLocs(ND)) {
     printTemplateArgumentList(OS, *Args, Policy);
   } else if (auto *Cls = llvm::dyn_cast<ClassTemplateSpecializationDecl>(&ND)) {

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

 #include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
 
 namespace {
 
 using ::testing::AllOf;
-using ::testing::AnyOf;
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
 using ::testing::Field;
 using ::testing::IsEmpty;
 using ::testing::UnorderedElementsAre;
 
 class IgnoreDiagnostics : public DiagnosticsConsumer {
   void onDiagnosticsReady(PathRef File,
EXPECT_THAT(
                                                WithKind(SymbolKind::Method),
                                                Children()))))),
            AllOf(WithName("Friend"), WithKind(SymbolKind::Class), Children()),
            AllOf(WithName("f1"), WithKind(SymbolKind::Function), Children()),
            AllOf(WithName("f2"), WithKind(SymbolKind::Function), Children()),
            AllOf(WithName("KInt"), WithKind(SymbolKind::Variable), Children()),
            AllOf(WithName("kStr"), WithKind(SymbolKind::Variable), Children()),
            AllOf(WithName("f1"), WithKind(SymbolKind::Function), Children()),
-           AllOf(WithName("foo"), WithKind(SymbolKind::Namespace),
-                 Children(
-                     AllOf(WithName("int32"), WithKind(SymbolKind::Class),
-                           Children()),
-                     AllOf(WithName("int32_t"), WithKind(SymbolKind::Class),
-                           Children()),
-                     AllOf(WithName("v1"), WithKind(SymbolKind::Variable),
-                           Children()),
-                     AllOf(WithName("bar"), WithKind(SymbolKind::Namespace),
-                           Children(AllOf(WithName("v2"),
-                                          WithKind(SymbolKind::Variable),
-                                          Children()))),
-                     AllOf(WithName("baz"), WithKind(SymbolKind::Namespace),
-                           Children()),
-                     AllOf(WithName("v2"), WithKind(SymbolKind::Namespace))))}));
+           AllOf(
+               WithName("foo"), WithKind(SymbolKind::Namespace),
 }
 
 TEST_F(DocumentSymbolsTest, DeclarationDefinition) {
   std::string FilePath = testPath("foo.cpp");
   Annotations Main(R"(
       class Foo {
         void $decl[[f]]();
       };
       void Foo::$def[[f]]() {
       }
     )");
 
   addFile(FilePath, Main.code());
-  EXPECT_THAT(getSymbols(FilePath),
-              ElementsAre(AllOf(WithName("Foo"), WithKind(SymbolKind::Class),
-                                Children(AllOf(
-                                    WithName("f"), WithKind(SymbolKind::Method),
-                                    SymNameRange(Main.range("decl"))))),
-                          AllOf(WithName("f"), WithKind(SymbolKind::Method),
-                                SymNameRange(Main.range("def")))));
+  EXPECT_THAT(
+      getSymbols(FilePath),
+      ElementsAre(
+          AllOf(WithName("Foo"), WithKind(SymbolKind::Class),
+                Children(AllOf(WithName("f"), WithKind(SymbolKind::Method),
+          AllOf(WithName("Foo::f"), WithKind(SymbolKind::Method),
 }
 
 TEST_F(DocumentSymbolsTest, ExternSymbol) {
   std::string FilePath = testPath("foo.cpp");
   addFile(testPath("foo.h"), R"cpp(
       extern int var;
       )cpp");
   addFile(FilePath, R"cpp(
EXPECT_THAT(
       getSymbols("foo.cpp"),
       UnorderedElementsAre(
           AllOf(WithName("Foo"), WithKind(SymbolKind::Class)),
           AllOf(WithName("Foo<int, T>"), WithKind(SymbolKind::Class)),
           AllOf(WithName("Foo<bool, int>"), WithKind(SymbolKind::Class)),
           AllOf(WithName("Foo<bool, int, 3>"), WithKind(SymbolKind::Class))));
 }
 
+TEST_F(DocumentSymbolsTest, Qualifiers) {
+  addFile("foo.cpp", R"cpp(
+    namespace foo { namespace bar {
+      struct Cls;
+
+      int func1();
+      int func2();
+      int func3();
+      int func4();
+    }}
+
+    struct foo::bar::Cls { };
+
+    int foo::bar::func1() { return 10; }
+    int ::foo::bar::func2() { return 20; }
+
+    using namespace foo;
+    int bar::func3() { return 30; }
+
+    namespace alias = foo::bar;
+    int ::alias::func4() { return 40; }
+  )cpp");
+
+  // All the qualifiers should be preserved exactly as written.
+  EXPECT_THAT(getSymbols("foo.cpp"),
+              UnorderedElementsAre(
+                  WithName("foo"), WithName("foo::bar::Cls"),
+                  WithName("foo::bar::func1"), WithName("::foo::bar::func2"),
+                  WithName("using namespace foo"), WithName("bar::func3"),
+                  WithName("alias"), WithName("::alias::func4")));
+}
+
+TEST_F(DocumentSymbolsTest, QualifiersWithTemplateArgs) {
+  addFile("foo.cpp", R"cpp(
+      template <typename T, typename U = double> class Foo;
+
+      template <>
+      class Foo<int, double> {
+        int method1();
+        int method2();
+        int method3();
+      };
+
+      using int_type = int;
+
+      // Typedefs should be preserved!
+      int Foo<int_type, double>::method1() { return 10; }
+
+      // Default arguments should not be shown!
+      int Foo<int>::method2() { return 20; }
+
+      using Foo_type = Foo<int>;
+      // If the whole type is aliased, this should be preserved too!
+      int Foo_type::method3() { return 30; }
+      )cpp");
+  EXPECT_THAT(
+      getSymbols("foo.cpp"),
+      UnorderedElementsAre(WithName("Foo"), WithName("Foo<int, double>"),
+                           WithName("int_type"),
+                           WithName("Foo<int_type, double>::method1"),
+                           WithName("Foo<int>::method2"), WithName("Foo_type"),
+                           WithName("Foo_type::method3")));
+}
+
 } // namespace clangd
 } // namespace clang