[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)

Nathan Ridge via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 25 19:56:35 PDT 2024


https://github.com/HighCommander4 updated https://github.com/llvm/llvm-project/pull/89557

>From fcb2ac4c68554d9c708b3db779b5570ff94725e8 Mon Sep 17 00:00:00 2001
From: Nathan Ridge <zeratul976 at hotmail.com>
Date: Sun, 21 Apr 2024 20:30:16 -0400
Subject: [PATCH] [clangd] Show struct fields and enum members in hovers

Fixes https://github.com/clangd/clangd/issues/959
---
 clang-tools-extra/clangd/Config.h             |   3 +
 clang-tools-extra/clangd/ConfigCompile.cpp    |   6 +
 clang-tools-extra/clangd/ConfigFragment.h     |   5 +-
 clang-tools-extra/clangd/ConfigYAML.cpp       |   4 +
 clang-tools-extra/clangd/Hover.cpp            |  41 +++++
 .../clangd/unittests/ConfigYAMLTests.cpp      |  13 ++
 .../clangd/unittests/HoverTests.cpp           | 157 +++++++++++++++++-
 clang/include/clang/AST/PrettyPrinter.h       |  13 ++
 clang/lib/AST/DeclPrinter.cpp                 |  32 +++-
 clang/unittests/AST/DeclPrinterTest.cpp       |  34 +++-
 10 files changed, 295 insertions(+), 13 deletions(-)

diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index 4371c80a6c5877..3b6658e7ff034f 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -136,6 +136,9 @@ struct Config {
   struct {
     /// Whether hover show a.k.a type.
     bool ShowAKA = true;
+
+    /// Whether to show struct fields
+    bool ShowFields = false;
   } Hover;
 
   struct {
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index 5bb2eb4a9f803f..714a04fc77a5fc 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -616,6 +616,12 @@ struct FragmentCompiler {
         C.Hover.ShowAKA = ShowAKA;
       });
     }
+    if (F.ShowFields) {
+      Out.Apply.push_back(
+          [ShowFields(**F.ShowFields)](const Params &, Config &C) {
+            C.Hover.ShowFields = ShowFields;
+          });
+    }
   }
 
   void compile(Fragment::InlayHintsBlock &&F) {
diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h
index 7fa61108c78a05..ccbc412f4fc533 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -309,8 +309,11 @@ struct Fragment {
 
   /// Describes hover preferences.
   struct HoverBlock {
-    /// Whether hover show a.k.a type.
+    /// Whether hovers show a.k.a type.
     std::optional<Located<bool>> ShowAKA;
+
+    /// Whether hovers show struct fields
+    std::optional<Located<bool>> ShowFields;
   };
   HoverBlock Hover;
 
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp
index ce09af819247ae..68e4d66763d16c 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -235,6 +235,10 @@ class Parser {
       if (auto ShowAKA = boolValue(N, "ShowAKA"))
         F.ShowAKA = *ShowAKA;
     });
+    Dict.handle("ShowFields", [&](Node &N) {
+      if (auto ShowFields = boolValue(N, "ShowFields"))
+        F.ShowFields = *ShowFields;
+    });
     Dict.parse(N);
   }
 
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 06b949bc4a2b55..db4049402f8fc0 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -66,12 +66,53 @@ namespace clang {
 namespace clangd {
 namespace {
 
+// A PrintingCallbacks implementation that prints public fields of
+// record types and enum members. Meant to be used with TerseOutput=true.
+class ClangdPrintingCallbacks : public PrintingCallbacks {
+public:
+  virtual ~ClangdPrintingCallbacks() = default;
+
+  void summarizeTagDecl(raw_ostream &Out, const TagDecl *D,
+                        const PrintingPolicy &PP) const override {
+    if (!Config::current().Hover.ShowFields) {
+      Out << " {}";
+      return;
+    }
+
+    // Don't worry about the details of the whitespace, e.g. space vs. newline,
+    // as the printed definition will be passed through clang-format before
+    // being sent to the client.
+    Out << " { ";
+    const bool IsRecord = isa<RecordDecl>(D);
+    for (DeclContext::decl_iterator Member = D->decls_begin(),
+                                    MemberEnd = D->decls_end();
+         Member != MemberEnd; ++Member) {
+      if (isa<EnumConstantDecl>(*Member) ||
+          (isa<FieldDecl>(*Member) &&
+           dyn_cast<FieldDecl>(*Member)->getAccess() == AS_public)) {
+        Member->print(Out, PP, 1);
+        if (IsRecord) {
+          Out << "; ";
+        } else {
+          DeclContext::decl_iterator Next = Member;
+          ++Next;
+          if (Next != MemberEnd)
+            Out << ", ";
+        }
+      }
+    }
+    Out << "}";
+  }
+};
+
 PrintingPolicy getPrintingPolicy(PrintingPolicy Base) {
+  static ClangdPrintingCallbacks Callbacks;
   Base.AnonymousTagLocations = false;
   Base.TerseOutput = true;
   Base.PolishForDeclaration = true;
   Base.ConstantsAsWritten = true;
   Base.SuppressTemplateArgsInCXXConstructors = true;
+  Base.Callbacks = &Callbacks;
   return Base;
 }
 
diff --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
index 44a6647d4c0a81..42cca3c6197602 100644
--- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -230,6 +230,19 @@ TEST(ParseYAML, ShowAKA) {
   EXPECT_THAT(Results[0].Hover.ShowAKA, llvm::ValueIs(val(true)));
 }
 
+TEST(ParseYAML, ShowFields) {
+  CapturedDiags Diags;
+  Annotations YAML(R"yaml(
+Hover:
+  ShowFields: True
+  )yaml");
+  auto Results =
+      Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_EQ(Results.size(), 1u);
+  EXPECT_THAT(Results[0].Hover.ShowFields, llvm::ValueIs(val(true)));
+}
+
 TEST(ParseYAML, InlayHints) {
   CapturedDiags Diags;
   Annotations YAML(R"yaml(
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 35db757b9c15b5..13d3993a592be3 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1645,7 +1645,11 @@ TEST(Hover, All) {
       {
           R"cpp(// Struct
             namespace ns1 {
-              struct MyClass {};
+              struct MyClass {
+                // Fields not shown without ShowFields=true
+                int field1;
+                int field2;
+              };
             } // namespace ns1
             int main() {
               ns1::[[My^Class]]* Params;
@@ -3941,6 +3945,157 @@ TEST(Hover, DisableShowAKA) {
   EXPECT_EQ(H->Type, HoverInfo::PrintedType("m_int"));
 }
 
+TEST(Hover, ShowFields) {
+  struct {
+    const char *const Code;
+    const std::function<void(HoverInfo &)> ExpectedBuilder;
+  } Cases[] = {
+    {
+      R"cpp(// Struct
+        namespace ns1 {
+          struct MyClass {
+            // Public fields shown in hover
+            int field1;
+            int field2;
+
+            // Methods and private fields not shown
+            void method();
+          private:
+            bool private_field;
+          };
+        } // namespace ns1
+        int main() {
+          ns1::[[My^Class]]* Params;
+        }
+      )cpp",
+      [](HoverInfo &HI) {
+        HI.Definition = "struct MyClass {\n  int field1;\n  int field2;\n}";
+      }
+    },
+    {
+      R"cpp(// Union
+        namespace ns1 {
+          union MyUnion { int x; int y; };
+        } // namespace ns1
+        int main() {
+          ns1::[[My^Union]] Params;
+        }
+      )cpp",
+      [](HoverInfo &HI) {
+        HI.Definition = "union MyUnion {\n  int x;\n  int y;\n}";
+      }
+    },
+    {
+      R"cpp(// Enum declaration
+        enum Hello {
+          ONE, TWO, THREE,
+        };
+        void foo() {
+          [[Hel^lo]] hello = ONE;
+        }
+      )cpp",
+      [](HoverInfo &HI) {
+        HI.Definition = "enum Hello { ONE, TWO, THREE }";
+      }
+    },
+  };
+  for (const auto &Case : Cases) {
+    SCOPED_TRACE(Case.Code);
+
+    Annotations T(Case.Code);
+    TestTU TU = TestTU::withCode(T.code());
+    TU.ExtraArgs.push_back("-std=c++20");
+    TU.ExtraArgs.push_back("-xobjective-c++");
+
+    // Types might be different depending on the target triplet, we chose a
+    // fixed one to make sure tests passes on different platform.
+    TU.ExtraArgs.push_back("--target=x86_64-pc-linux-gnu");
+    auto AST = TU.build();
+    Config Cfg;
+    Cfg.Hover.ShowFields = true;
+    WithContextValue WithCfg(Config::Key, std::move(Cfg));
+    auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+    ASSERT_TRUE(H);
+    HoverInfo Expected;
+    Expected.SymRange = T.range();
+    Case.ExpectedBuilder(Expected);
+
+    SCOPED_TRACE(H->present().asPlainText());
+    EXPECT_EQ(H->Definition, Expected.Definition);
+  }
+}
+
+TEST(Hover, ShowFields_CLanguage) {
+  struct {
+    const char *const Code;
+    const std::function<void(HoverInfo &)> ExpectedBuilder;
+  } Cases[] = {
+    {
+      R"cpp(// Struct
+        struct MyStruct {
+          // Public fields shown in hover
+          int field1;
+          int field2;
+        };
+        int main() {
+          struct [[My^Struct]]* Params;
+        }
+      )cpp",
+      [](HoverInfo &HI) {
+        HI.Definition = "struct MyStruct {\n  int field1;\n  int field2;\n}";
+      }
+    },
+    {
+      R"cpp(// Union
+        union MyUnion { int x; int y; };
+        int main() {
+          union [[My^Union]] Params;
+        }
+      )cpp",
+      [](HoverInfo &HI) {
+        HI.Definition = "union MyUnion {\n  int x;\n  int y;\n}";
+      }
+    },
+    {
+      R"cpp(// Enum declaration
+        enum Hello {
+          ONE, TWO, THREE,
+        };
+        void foo() {
+          enum [[Hel^lo]] hello = ONE;
+        }
+      )cpp",
+      [](HoverInfo &HI) {
+        HI.Definition = "enum Hello { ONE, TWO, THREE }";
+      }
+    },
+  };
+  for (const auto &Case : Cases) {
+    SCOPED_TRACE(Case.Code);
+
+    Annotations T(Case.Code);
+    TestTU TU = TestTU::withCode(T.code());
+    TU.ExtraArgs.push_back("-std=c99");
+    TU.ExtraArgs.push_back("-xc");
+
+    // Types might be different depending on the target triplet, we chose a
+    // fixed one to make sure tests passes on different platform.
+    TU.ExtraArgs.push_back("--target=x86_64-pc-linux-gnu");
+    auto AST = TU.build();
+    Config Cfg;
+    Cfg.Hover.ShowFields = true;
+    WithContextValue WithCfg(Config::Key, std::move(Cfg));
+    auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+    ASSERT_TRUE(H);
+    HoverInfo Expected;
+    Expected.SymRange = T.range();
+    Case.ExpectedBuilder(Expected);
+
+    SCOPED_TRACE(H->present().asPlainText());
+    EXPECT_EQ(H->Definition, Expected.Definition);
+  }
+}
+
 TEST(Hover, HideBigInitializers) {
   Annotations T(R"cpp(
   #define A(x) x, x, x, x
diff --git a/clang/include/clang/AST/PrettyPrinter.h b/clang/include/clang/AST/PrettyPrinter.h
index da276e26049b00..7e8ab32ceb5d72 100644
--- a/clang/include/clang/AST/PrettyPrinter.h
+++ b/clang/include/clang/AST/PrettyPrinter.h
@@ -21,6 +21,7 @@ namespace clang {
 class DeclContext;
 class LangOptions;
 class Stmt;
+class TagDecl;
 
 class PrinterHelper {
 public:
@@ -29,6 +30,9 @@ class PrinterHelper {
 };
 
 /// Callbacks to use to customize the behavior of the pretty-printer.
+
+struct PrintingPolicy;
+
 class PrintingCallbacks {
 protected:
   ~PrintingCallbacks() = default;
@@ -47,6 +51,13 @@ class PrintingCallbacks {
   /// The printing stops at the first isScopeVisible() == true, so there will
   /// be no calls with outer scopes.
   virtual bool isScopeVisible(const DeclContext *DC) const { return false; }
+
+  /// Print a summary of the body of a TagDecl when PrintingPolicy::TerseOutput
+  /// is true.
+  virtual void summarizeTagDecl(raw_ostream &Out, const TagDecl *D,
+                                const PrintingPolicy &PP) const {
+    Out << " {}";
+  }
 };
 
 /// Describes how types, statements, expressions, and declarations should be
@@ -249,6 +260,8 @@ struct PrintingPolicy {
   /// For example, in this mode we don't print function bodies, class members,
   /// declarations inside namespaces etc.  Effectively, this should print
   /// only the requested declaration.
+  /// The behaviour for printing the bodies of record and enum decls can be
+  /// customized by overriding PrintingCallbacks::summarizeTagDecl.
   LLVM_PREFERRED_TYPE(bool)
   unsigned TerseOutput : 1;
 
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 43d221968ea3fb..27679eb87e1278 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/Basic/Module.h"
+#include "clang/Basic/Specifiers.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace clang;
 
@@ -631,9 +632,16 @@ void DeclPrinter::VisitEnumDecl(EnumDecl *D) {
     Out << " : " << D->getIntegerType().stream(Policy);
 
   if (D->isCompleteDefinition()) {
-    Out << " {\n";
-    VisitDeclContext(D);
-    Indent() << "}";
+    if (Policy.TerseOutput) {
+      if (Policy.Callbacks)
+        Policy.Callbacks->summarizeTagDecl(Out, D, Policy);
+      else
+        Out << " {}";
+    } else {
+      Out << " {\n";
+      VisitDeclContext(D);
+      Indent() << "}";
+    }
   }
 }
 
@@ -648,9 +656,16 @@ void DeclPrinter::VisitRecordDecl(RecordDecl *D) {
     Out << ' ' << *D;
 
   if (D->isCompleteDefinition()) {
-    Out << " {\n";
-    VisitDeclContext(D);
-    Indent() << "}";
+    if (Policy.TerseOutput) {
+      if (Policy.Callbacks)
+        Policy.Callbacks->summarizeTagDecl(Out, D, Policy);
+      else
+        Out << " {}";
+    } else {
+      Out << " {\n";
+      VisitDeclContext(D);
+      Indent() << "}";
+    }
   }
 }
 
@@ -1183,7 +1198,10 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) {
     // Print the class definition
     // FIXME: Doesn't print access specifiers, e.g., "public:"
     if (Policy.TerseOutput) {
-      Out << " {}";
+      if (Policy.Callbacks)
+        Policy.Callbacks->summarizeTagDecl(Out, D, Policy);
+      else
+        Out << " {}";
     } else {
       Out << " {\n";
       VisitDeclContext(D);
diff --git a/clang/unittests/AST/DeclPrinterTest.cpp b/clang/unittests/AST/DeclPrinterTest.cpp
index 0e09ab2a7bba88..60a92ec77a0f83 100644
--- a/clang/unittests/AST/DeclPrinterTest.cpp
+++ b/clang/unittests/AST/DeclPrinterTest.cpp
@@ -20,6 +20,7 @@
 
 #include "ASTPrint.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/PrettyPrinter.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
@@ -78,12 +79,13 @@ PrintedDeclCXX98Matches(StringRef Code, const DeclarationMatcher &NodeMatch,
                             PolicyModifier);
 }
 
-::testing::AssertionResult PrintedDeclCXX11Matches(StringRef Code,
-                                                   StringRef DeclName,
-                                                   StringRef ExpectedPrinted) {
+::testing::AssertionResult
+PrintedDeclCXX11Matches(StringRef Code, StringRef DeclName,
+                        StringRef ExpectedPrinted,
+                        PrintingPolicyAdjuster PolicyModifier = nullptr) {
   std::vector<std::string> Args(1, "-std=c++11");
   return PrintedDeclMatches(Code, Args, namedDecl(hasName(DeclName)).bind("id"),
-                            ExpectedPrinted, "input.cc");
+                            ExpectedPrinted, "input.cc", PolicyModifier);
 }
 
 ::testing::AssertionResult PrintedDeclCXX11Matches(
@@ -1430,3 +1432,27 @@ TEST(DeclPrinter, VarDeclWithInitializer) {
       PrintedDeclCXX17Matches("void foo() {int arr[42]; for(int a : arr);}",
                               namedDecl(hasName("a")).bind("id"), "int a"));
 }
+
+TEST(DeclPrinter, SummarizeTagDecl) {
+  class TestPrintingCallbacks : public PrintingCallbacks {
+  public:
+    virtual ~TestPrintingCallbacks() = default;
+    virtual void summarizeTagDecl(raw_ostream &Out, const TagDecl *D,
+                                  const PrintingPolicy &PP) const {
+      Out << " { Custom Tag Decl Summary }";
+    }
+  };
+  TestPrintingCallbacks Callbacks;
+  auto UseCallbacks = [&](PrintingPolicy &Policy) {
+    Policy.Callbacks = &Callbacks;
+  };
+  ASSERT_TRUE(PrintedDeclCXX11Matches(
+      "struct MyStruct { int x; int y; };", "MyStruct",
+      "struct MyStruct { Custom Tag Decl Summary }", UseCallbacks));
+  ASSERT_TRUE(PrintedDeclCXX11Matches(
+      "union MyUnion { int x; int y; };", "MyUnion",
+      "union MyUnion { Custom Tag Decl Summary }", UseCallbacks));
+  ASSERT_TRUE(PrintedDeclCXX11Matches(
+      "enum MyEnum { one, two, three };", "MyEnum",
+      "enum MyEnum { Custom Tag Decl Summary }", UseCallbacks));
+}



More information about the cfe-commits mailing list