[clangd] Collect enum constants in SymbolCollector

[hokein]

Summary

• ignore nameless symbols
• include enum constant declarataion

[ilya-biryukov]

clangd/index/SymbolCollector.cpp

75

What are those declarations exactly?

unittests/clangd/SymbolCollectorTests.cpp

174

I'd say we should drop enum constants inside stongly-typed enums (i.e. enum class).
They can't be found inside namespace by code completion, therefore being similar to static method in that regard. (And we don't include static methods now, right?)

[hokein]

Ignore enum constants in scoped enum.

[hokein]

clangd/index/SymbolCollector.cpp

75

This would ignore anonymous declarations, e.g. anonymous class/enum. See the unittest.

unittests/clangd/SymbolCollectorTests.cpp

174

yeah, that makes sense.

[ioeric]

unittests/clangd/SymbolCollectorTests.cpp

164

Could you add a test case like the following and check whether ns::X is in the result?

namespace ns {
  enum {
     X
  };
}

[hokein]

Add one more test.

[ilya-biryukov]

LGTM. (See the comment about changing the comment, though)

clangd/index/SymbolCollector.cpp

75

Thanks for clarifying. Maybe we could change the comment to say "anonymous" instead of "nameless"? This is what bit got me confused.

[hokein]

Update comment.

clangd/index/SymbolCollector.cpp

splitQualifiedName(llvm::StringRef QName) {
   return {QName.substr(0, Pos), QName.substr(Pos + 2)};
 }
 
 bool shouldFilterDecl(const NamedDecl *ND, ASTContext *ASTCtx,
                       const SymbolCollector::Options &Opts) {
   using namespace clang::ast_matchers;
   if (ND->isImplicit())
     return true;
+  // Skip nameless declarations.
+  if (ND->getDeclName().isEmpty())
+    return true;
+
   // FIXME: figure out a way to handle internal linkage symbols (e.g. static
   // variables, function) defined in the .cc files. Also we skip the symbols
   // in anonymous namespace as the qualifier names of these symbols are like
   // `foo::<anonymous>::bar`, which need a special handling.
   // In real world projects, we have a relatively large set of header files
   // that define static variables (like "static const int A = 1;"), we still
   // want to collect these symbols, although they cause potential ODR
   // violations.
   if (ND->isInAnonymousNamespace())
     return true;
 
-  // We only want symbols in namespaces or translation unit scopes (e.g. no
-  // class members).
-  if (match(decl(allOf(
-                Opts.IndexMainFiles ? decl()
-                                    : decl(unless(isExpansionInMainFile())),
-                hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl())))),
+  // We only want:
+  //   * symbols in namespaces or translation unit scopes (e.g. no class
+  //     members)
+  //   * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")
+  auto InTopLevelScope =
+      hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl()));
+  if (match(decl(allOf(Opts.IndexMainFiles
+                           ? decl()
+                       anyOf(InTopLevelScope,
+                             hasDeclContext(enumDecl(InTopLevelScope,
+                                                     unless(isScoped())))))),
             *ND, *ASTCtx)
           .empty())
     return true;
 
   return false;
 }
 
 } // namespace

unittests/clangd/SymbolCollectorTests.cpp

EXPECT_THAT(Symbols,
                    QName("foo::bar"),
                    QName("foo::int32"),
                    QName("foo::int32_t"),
                    QName("foo::v1"),
                    QName("foo::bar::v2"),
                    QName("foo::baz")}));
 }
 
+TEST_F(SymbolCollectorTest, IncludeEnums) {
+  CollectorOpts.IndexMainFiles = false;
+  const std::string Header = R"(
+    enum {
+      Red
+    };
+    enum Color {
+      Green
+    };
+    enum class Color2 {
+      Yellow // ignore
+    };
+    namespace ns {
+    enum {
+      Black
+    };
+    }
+  )";
+  runSymbolCollector(Header, /*Main=*/"");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Red"), QName("Color"),
+                                            QName("Green"), QName("Color2"),
+                                            QName("ns"),
+                                            QName("ns::Black")));
+}
+
+TEST_F(SymbolCollectorTest, IgnoreNamelessSymbols) {
+  CollectorOpts.IndexMainFiles = false;
+  const std::string Header = R"(
+    struct {
+      int a;
+    } Foo;
+  )";
+  runSymbolCollector(Header, /*Main=*/"");
+  EXPECT_THAT(Symbols,
+              UnorderedElementsAre(QName("Foo")));
+}
+
 TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) {
   CollectorOpts.IndexMainFiles = false;
   const std::string Header = R"(
     class Foo {};
     void f1();
     inline void f2() {}
   )";
   const std::string Main = R"(