[clangd] Replace shortenNamespace with getQualification

[merge_guards_bot]

Unit tests: fail. 60978 tests passed, 1 failed and 727 were skipped.

failed: lit.lit/shtest-format.py

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[ilya-biryukov]

clang-tools-extra/clangd/AST.cpp

307

Why not use explicitTargetReferences here? This should handle typedefs (and potentially other things) as well.
I'm not sure we can tests those, as auto might have all typedefs stripped of, though

[kadircet]

• Use explicitTargetReferences
• Added some testing

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

[kadircet]

ping

[ilya-biryukov]

LGTM

clang-tools-extra/clangd/AST.cpp

307

This is such a terrible hack... Could you put a FIXME mentioning this should be done when printing names of the underlying decls instead?

clang-tools-extra/clangd/AST.h

 /// could change these semantics in the future by reimplementing this funcure
 /// (e.g. avoid USR for macros).
 llvm::Optional<SymbolID> getSymbolID(const llvm::StringRef MacroName,
                                      const MacroInfo *MI,
                                      const SourceManager &SM);
 
 /// Returns a QualType as string. The result doesn't contain unwritten scopes
 /// like annoymous/inline namespace.
-std::string printType(const QualType QT, const DeclContext &Context);
-
-/// Try to shorten the OriginalName by removing namespaces from the left of
-/// the string that are redundant in the CurrentNamespace. This way the type
-/// idenfier become shorter and easier to read.
-/// Limitation: It only handles the qualifier of the type itself, not that of
-/// templates.
-/// FIXME: change type of parameter CurrentNamespace to DeclContext ,
-/// take in to account using directives etc
-/// Example: shortenNamespace("ns1::MyClass<ns1::OtherClass>", "ns1")
-///    --> "MyClass<ns1::OtherClass>"
-std::string shortenNamespace(const llvm::StringRef OriginalName,
-                             const llvm::StringRef CurrentNamespace);
+std::string printType(const QualType QT, const DeclContext &CurContext);
 
 /// Indicates if \p D is a template instantiation implicitly generated by the
 /// compiler, e.g.
 ///     template <class T> struct vector {};
 ///     vector<int> v; // 'vector<int>' is an implicit instantiation
 bool isImplicitTemplateInstantiation(const NamedDecl *D);
 /// Indicates if \p D is an explicit template specialization, e.g.
 ///   template <class T> struct vector {};

 /// empty.
 /// Elements in VisibleNamespaces should be in the form: `ns::`, with trailing
 /// "::".
 /// Note that this is just textual and might be incorrect. e.g. when there are
 /// two namespaces ns1::a and ns2::a, the function will early exit if "a::" is
 /// present in \p VisibleNamespaces, no matter whether it is from ns1:: or ns2::
 std::string getQualification(ASTContext &Context,
                              const DeclContext *DestContext,
-                             SourceLocation InsertionPoint, const NamedDecl *ND,
+                             const NamedDecl *ND,
                              llvm::ArrayRef<std::string> VisibleNamespaces);
 
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_

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 "FindTarget.h"
 #include "SourceCode.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
llvm::Optional<SymbolID> getSymbolID(const llvm::StringRef MacroName,
   if (MI == nullptr)
     return None;
   llvm::SmallString<128> USR;
   if (index::generateUSRForMacro(MacroName, MI->getDefinitionLoc(), SM, USR))
     return None;
   return SymbolID(USR);
 }
 
-std::string shortenNamespace(const llvm::StringRef OriginalName,
-                             const llvm::StringRef CurrentNamespace) {
-  llvm::SmallVector<llvm::StringRef, 8> OriginalParts;
-  llvm::SmallVector<llvm::StringRef, 8> CurrentParts;
-  llvm::SmallVector<llvm::StringRef, 8> Result;
-  OriginalName.split(OriginalParts, "::");
-  CurrentNamespace.split(CurrentParts, "::");
-  auto MinLength = std::min(CurrentParts.size(), OriginalParts.size());
-
-  unsigned DifferentAt = 0;
-  while (DifferentAt < MinLength &&
-         CurrentParts[DifferentAt] == OriginalParts[DifferentAt]) {
-    DifferentAt++;
-  }
-
-  for (unsigned i = DifferentAt; i < OriginalParts.size(); ++i) {
-    Result.push_back(OriginalParts[i]);
-  }
-  return join(Result, "::");
-}
-
-std::string printType(const QualType QT, const DeclContext &Context) {
-  PrintingPolicy PP(Context.getParentASTContext().getPrintingPolicy());
-  PP.SuppressUnwrittenScope = 1;
-  PP.SuppressTagKeyword = 1;
-  return shortenNamespace(QT.getAsString(PP), printNamespaceScope(Context));
+// FIXME: This should be handled while printing underlying decls instead.
+std::string printType(const QualType QT, const DeclContext &CurContext) {
+  std::string Result;
+  llvm::raw_string_ostream OS(Result);
+  auto Decls = explicitReferenceTargets(
+      ast_type_traits::DynTypedNode::create(QT), DeclRelation::Alias);
+  if (!Decls.empty())
+    OS << getQualification(CurContext.getParentASTContext(), &CurContext,
+                           Decls.front(),
+                           /*VisibleNamespaces=*/llvm::ArrayRef<std::string>{});
+  PrintingPolicy PP(CurContext.getParentASTContext().getPrintingPolicy());
+  PP.SuppressScope = true;
+  PP.SuppressTagKeyword = true;
+  QT.print(OS, PP);
+  return OS.str();
 }
 
 QualType declaredType(const TypeDecl *D) {
   if (const auto *CTSD = llvm::dyn_cast<ClassTemplateSpecializationDecl>(D))
     if (const auto *TSI = CTSD->getTypeAsWritten())
       return TSI->getType();
   return D->getASTContext().getTypeDeclType(D);
 }
return getQualification(
                             [CanonNSD](const NamespaceDecl *NSD) {
                               return NSD->getCanonicalDecl() == CanonNSD;
                             });
       });
 }
 
 std::string getQualification(ASTContext &Context,
                              const DeclContext *DestContext,
-                             SourceLocation InsertionPoint, const NamedDecl *ND,
+                             const NamedDecl *ND,
                              llvm::ArrayRef<std::string> VisibleNamespaces) {
   for (llvm::StringRef NS : VisibleNamespaces) {
     assert(NS.endswith("::"));
     (void)NS;
   }
   return getQualification(
       Context, DestContext, ND->getDeclContext(),
       [&](NestedNameSpecifier *NNS) {

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

 
 #include "Annotations.h"
 #include "ParsedAST.h"
 #include "TestTU.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <cstddef>
 #include <string>
 #include <vector>
 
 namespace clang {
 namespace clangd {
 namespace {
 
-TEST(ShortenNamespace, All) {
-  ASSERT_EQ("TestClass", shortenNamespace("TestClass", ""));
-
-  ASSERT_EQ("TestClass", shortenNamespace(
-      "testnamespace::TestClass", "testnamespace"));
-
-  ASSERT_EQ(
-      "namespace1::TestClass",
-      shortenNamespace("namespace1::TestClass", "namespace2"));
-
-  ASSERT_EQ("TestClass",
-            shortenNamespace("testns1::testns2::TestClass",
-                             "testns1::testns2"));
-
-  ASSERT_EQ(
-      "testns2::TestClass",
-      shortenNamespace("testns1::testns2::TestClass", "testns1"));
-
-  ASSERT_EQ("TestClass<testns1::OtherClass>",
-            shortenNamespace(
-                "testns1::TestClass<testns1::OtherClass>", "testns1"));
-}
-
 TEST(GetDeducedType, KwAutoExpansion) {
   struct Test {
     StringRef AnnotatedCode;
     const char *DeducedType;
   } Tests[] = {
       {"^auto i = 0;", "int"},
       {"^auto f(){ return 1;};", "int"},
   };
for (size_t I = 0, E = InsertionPoints.size(); I != E; ++I) {
       const Decl *D = InsertionPoints[I];
       if (Case.VisibleNamespaces.empty()) {
         EXPECT_EQ(getQualification(AST.getASTContext(),
                                    D->getLexicalDeclContext(), D->getBeginLoc(),
                                    TargetDecl),
                   Case.Qualifications[I]);
       } else {
         EXPECT_EQ(getQualification(AST.getASTContext(),
-                                   D->getLexicalDeclContext(), D->getBeginLoc(),
-                                   TargetDecl, Case.VisibleNamespaces),
+                                   D->getLexicalDeclContext(), TargetDecl,
+                                   Case.VisibleNamespaces),
                   Case.Qualifications[I]);
       }
     }
   }
 }
 
+TEST(ClangdAST, PrintType) {
+  const struct {
+    llvm::StringRef Test;
+    std::vector<llvm::StringRef> Types;
+  } Cases[] = {
+      {
+          R"cpp(
+            namespace ns1 { namespace ns2 { class Foo {}; } }
+            void insert(); // ns1::ns2::Foo
+            namespace ns1 {
+              void insert(); // ns2::Foo
+              namespace ns2 {
+                void insert(); // Foo
+              }
+            }
+          )cpp",
+          {"ns1::ns2::Foo", "ns2::Foo", "Foo"},
+      },
+      {
+          R"cpp(
+            namespace ns1 {
+              typedef int Foo;
+            }
+            void insert(); // ns1::Foo
+            namespace ns1 {
+              void insert(); // Foo
+            }
+          )cpp",
+          {"ns1::Foo", "Foo"},
+      },
+  };
+  for (const auto &Case : Cases) {
+    Annotations Test(Case.Test);
+    TestTU TU = TestTU::withCode(Test.code());
+    ParsedAST AST = TU.build();
+    std::vector<const DeclContext *> InsertionPoints;
+    const TypeDecl *TargetDecl = nullptr;
+    findDecl(AST, [&](const NamedDecl &ND) {
+      if (ND.getNameAsString() == "Foo") {
+        if (const auto *TD = llvm::dyn_cast<TypeDecl>(&ND)) {
+          TargetDecl = TD;
+          return true;
+        }
+      } else if (ND.getNameAsString() == "insert")
+        InsertionPoints.push_back(ND.getDeclContext());
+      return false;
+    });
+
+    ASSERT_EQ(InsertionPoints.size(), Case.Types.size());
+    for (size_t I = 0, E = InsertionPoints.size(); I != E; ++I) {
+      const auto *DC = InsertionPoints[I];
+      EXPECT_EQ(printType(AST.getASTContext().getTypeDeclType(TargetDecl), *DC),
+                Case.Types[I]);
+    }
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang