[clang-tools-extra] ffd0f11 - [clangd] Improve type printing in hover

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-01-10T09:51:20+01:00
New Revision: ffd0f116754c36146bb21a01b047782ce8a01e2e

URL: https://github.com/llvm/llvm-project/commit/ffd0f116754c36146bb21a01b047782ce8a01e2e
DIFF: https://github.com/llvm/llvm-project/commit/ffd0f116754c36146bb21a01b047782ce8a01e2e.diff

LOG: [clangd] Improve type printing in hover

Summary:
Do not include tag keywords when printing types for symbol names, as it
will come from SymbolKind.
Also suppress them while printing definitions to prevent them occuring in
template arguments.
Make use of `getAsString`, instead of `print` in all places to have a consistent
style across the file.

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D72450

Added: 
    

Modified: 
    clang-tools-extra/clangd/Hover.cpp
    clang-tools-extra/clangd/unittests/HoverTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 20883b347fdc..a9fa6443b196 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -16,13 +16,13 @@
 #include "Selection.h"
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
-
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/PrettyPrinter.h"
+#include "clang/AST/Type.h"
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
@@ -101,6 +101,7 @@ std::string printDefinition(const Decl *D) {
       printingPolicyForDecls(D->getASTContext().getPrintingPolicy());
   Policy.IncludeTagDefinition = false;
   Policy.SuppressTemplateArgsInCXXConstructors = true;
+  Policy.SuppressTagKeyword = true;
   D->print(OS, Policy);
   OS.flush();
   return Definition;
@@ -233,9 +234,7 @@ void fillFunctionTypeAndParams(HoverInfo &HI, const Decl *D,
     HI.Parameters->emplace_back();
     auto &P = HI.Parameters->back();
     if (!PVD->getType().isNull()) {
-      P.Type.emplace();
-      llvm::raw_string_ostream OS(*P.Type);
-      PVD->getType().print(OS, Policy);
+      P.Type = PVD->getType().getAsString(Policy);
     } else {
       std::string Param;
       llvm::raw_string_ostream OS(Param);
@@ -344,13 +343,10 @@ HoverInfo getHoverContents(const NamedDecl *D, const SymbolIndex *Index) {
   }
 
   // Fill in types and params.
-  if (const FunctionDecl *FD = getUnderlyingFunction(D)) {
+  if (const FunctionDecl *FD = getUnderlyingFunction(D))
     fillFunctionTypeAndParams(HI, D, FD, Policy);
-  } else if (const auto *VD = dyn_cast<ValueDecl>(D)) {
-    HI.Type.emplace();
-    llvm::raw_string_ostream OS(*HI.Type);
-    VD->getType().print(OS, Policy);
-  }
+  else if (const auto *VD = dyn_cast<ValueDecl>(D))
+    HI.Type = VD->getType().getAsString(Policy);
 
   // Fill in value with evaluated initializer if possible.
   if (const auto *Var = dyn_cast<VarDecl>(D)) {
@@ -380,9 +376,9 @@ HoverInfo getHoverContents(QualType T, ASTContext &ASTCtx,
     enhanceFromIndex(HI, *CommentD, Index);
   } else {
     // Builtin types
-    llvm::raw_string_ostream OS(HI.Name);
-    PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
-    T.print(OS, Policy);
+    auto Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
+    Policy.SuppressTagKeyword = true;
+    HI.Name = T.getAsString(Policy);
   }
   return HI;
 }

diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 44337688ff87..91eb5a083ddf 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -446,7 +446,7 @@ class Foo {})cpp";
        [](HoverInfo &HI) {
          HI.Name = "x";
          HI.NamespaceScope = "";
-         HI.Definition = "enum Color x = GREEN";
+         HI.Definition = "Color x = GREEN";
          HI.Kind = index::SymbolKind::Variable;
          HI.Type = "enum Color";
          HI.Value = "GREEN (1)"; // Symbolic when hovering on an expression.
@@ -1427,7 +1427,7 @@ TEST(Hover, All) {
             HI.NamespaceScope = "";
             HI.LocalScope = "test::";
             HI.Type = "struct Test &&";
-            HI.Definition = "struct Test &&test = {}";
+            HI.Definition = "Test &&test = {}";
             HI.Value = "{}";
           }},
       {
@@ -1476,6 +1476,31 @@ TEST(Hover, All) {
             HI.ReturnType = "int";
             HI.Type = "int ()";
           }},
+      {
+          R"cpp(// type of nested templates.
+          template <class T> struct cls {};
+          cls<cls<cls<int>>> [[fo^o]];
+          )cpp",
+          [](HoverInfo &HI) {
+            HI.Definition = "cls<cls<cls<int>>> foo";
+            HI.Kind = index::SymbolKind::Variable;
+            HI.NamespaceScope = "";
+            HI.Name = "foo";
+            HI.Type = "cls<cls<cls<int> > >";
+            HI.Value = "{}";
+          }},
+      {
+          R"cpp(// type of nested templates.
+          template <class T> struct cls {};
+          [[cl^s]]<cls<cls<int>>> foo;
+          )cpp",
+          [](HoverInfo &HI) {
+            HI.Definition = "template <> struct cls<cls<cls<int>>> {}";
+            HI.Kind = index::SymbolKind::Struct;
+            HI.NamespaceScope = "";
+            HI.Name = "cls<cls<cls<int> > >";
+            HI.Documentation = "type of nested templates.";
+          }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.