[clangd] Implement hover for string literals

[v1nh1shungry]

Summary

Show string-literals' type and size in a hover card

Issue related: https://github.com/clangd/clangd/issues/1016

[tom-anders]

clang-tools-extra/clangd/Hover.cpp

785

Is it really useful to show the contents inside the Hover? You already see the contents of the string literal anyway, so I don't think this adds any value

788

(nit)

814

Hmm so what's stopping us from adding support for C here?

820

I think you should prefer llvm::formatv here

[v1nh1shungry]

@tom-anders Thanks for reviewing and for your suggestions!

clang-tools-extra/clangd/Hover.cpp

785

I added this comment only because others show the contents. I think you're right.

814

Because I'm not that familiar with C's data types and standards.

[v1nh1shungry]

Improve the patch

[v1nh1shungry]

Support different C++ standards

[v1nh1shungry]

Improve the patch

[v1nh1shungry]

Seems like I have made things too complex :(

Should work for C, C++, and other languages now.

[v1nh1shungry]

Improve the patch

[kadircet]

clang-tools-extra/clangd/Hover.cpp

868

can you extract this into a function, similar to the cases below?

871

i think it's a good idea to also include the size here (e.g. L"XYZ", would be 8 bytes).

[v1nh1shungry]

Apply the reviewing suggestions

[v1nh1shungry]

@kadircet Thanks for your suggestions!

[v1nh1shungry]

Modify the name in the hover info

[kadircet]

thanks, lgtm!

[v1nh1shungry]

@kadircet Could you please help me commit it? Thanks a lot!

[kadircet]

sure i am happy to land, @tom-anders do you have anything else to add ?

[tom-anders]

In D137650#3919089, @kadircet wrote:

sure i am happy to land, @tom-anders do you have anything else to add ?

LGTM!

clang-tools-extra/clangd/Hover.cpp

if (const auto *D = QT->getAsTagDecl()) {
       HI.Documentation = getDeclComment(ASTCtx, *CommentD);
       enhanceFromIndex(HI, *CommentD, Index);
     }
   }
 
   return HI;
 }
 
+llvm::Optional<HoverInfo>
+getStringLiteralHoverContents(const StringLiteral *SL,
+                              const LangOptions &LangOpts) {
+  HoverInfo HI;
+
+  // TODO: Show string literal's contents
+  HI.Name = "String Literal";
+
+  // In C++ string literals have `const` qualifier, but in C it don't.
+  std::string Qualifier;
+
+  std::string CharType;
+  // C++
+  // TODO: Support different C++ standards
+  if (LangOpts.CPlusPlus) {
+    Qualifier = "const ";
+    switch (SL->getKind()) {
+    case StringLiteral::Ordinary:
+      CharType = "char";
+      break;
+    case StringLiteral::Wide:
+      CharType = "wchar_t";
+      break;
+    case StringLiteral::UTF8:
+      CharType = "char8_t";
+      break;
+    case StringLiteral::UTF16:
+      CharType = "char16_t";
+      break;
+    case StringLiteral::UTF32:
+      CharType = "char32_t";
+      break;
+    }
+  } else {
+    // TODO: For other languages
+    return llvm::None;
+  }
+
+  HoverInfo::PrintedType Type;
+  Type.Type =
+      Qualifier + CharType + "[" + std::to_string(SL->getLength() + 1) + "]";
+  HI.Type = Type;
+
+  return HI;
+}
+
 bool isLiteral(const Expr *E) {
   // Unfortunately there's no common base Literal classes inherits from
   // (apart from Expr), therefore these exclusions.
   return llvm::isa<CompoundLiteralExpr>(E) ||
          llvm::isa<CXXBoolLiteralExpr>(E) ||
          llvm::isa<CXXNullPtrLiteralExpr>(E) ||
          llvm::isa<FixedPointLiteral>(E) || llvm::isa<FloatingLiteral>(E) ||
          llvm::isa<ImaginaryLiteral>(E) || llvm::isa<IntegerLiteral>(E) ||

 
 // Generates hover info for `this` and evaluatable expressions.
 // FIXME: Support hover for literals (esp user-defined)
 llvm::Optional<HoverInfo> getHoverContents(const Expr *E, ParsedAST &AST,
                                            const PrintingPolicy &PP,
                                            const SymbolIndex *Index) {
   // There's not much value in hovering over "42" and getting a hover card
   // saying "42 is an int", similar for other literals.
-  if (isLiteral(E))
+  if (isLiteral(E)) {
+    // Generate hover info for string literals showing
+    // its character's type and length
+    if (const StringLiteral *SL = dyn_cast<StringLiteral>(E)) {
+      return getStringLiteralHoverContents(SL, AST.getLangOpts());
+    }
     return llvm::None;
+  }
 
   HoverInfo HI;
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast<CXXThisExpr>(E))
     return getThisExprHoverContents(CTE, AST.getASTContext(), PP);
   if (const PredefinedExpr *PE = dyn_cast<PredefinedExpr>(E))
     return getPredefinedExprHoverContents(*PE, AST.getASTContext(), PP);
   // For expressions we currently print the type and the value, iff it is
   // evaluatable.
   if (auto Val = printExprValue(E, AST.getASTContext())) {
     HI.Type = printType(E->getType(), AST.getASTContext(), PP);
     HI.Value = *Val;
     HI.Name = std::string(getNameForExpr(E));
     return HI;
   }
   return llvm::None;
 }

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

llvm::StringRef Tests[] = {
           )cpp",
       // literals
       "auto x = t^rue;",
       "auto x = ^(int){42};",
       "auto x = ^42.;",
       "auto x = ^42.0i;",
       "auto x = ^42;",
       "auto x = ^nullptr;",
-      "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
     SCOPED_TRACE(Test);
 
     Annotations T(Test);
     TestTU TU = TestTU::withCode(T.code());
     TU.ExtraArgs.push_back("-std=c++17");
struct {
     const std::function<void(HoverInfo &)> ExpectedBuilder;
   } Cases[] = {
       {"auto x = [['^A']]; // character literal",
        [](HoverInfo &HI) {
          HI.Name = "expression";
          HI.Type = "char";
          HI.Value = "65 (0x41)";
        }},
+      {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+       [](HoverInfo &HI) {
+         HI.Name = "String Literal";
+         HI.Type = "const char[14]";
+       }},
       {
           R"cpp(// Local variable
             int main() {
               int bonjour;
               ^[[bonjour]] = 2;
               int test1 = bonjour;
             }
           )cpp",