[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 146338.
ioeric marked 3 inline comments as done.
ioeric added a comment.

- Addressed a few comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46751

Files:
  clangd/index/SymbolCollector.cpp
  unittests/clangd/SymbolCollectorTests.cpp


Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -697,6 +697,39 @@
                            AllOf(QName("pörk"), DeclRange(Header.range()))));
 }
 
+TEST_F(SymbolCollectorTest, FilterPrivateProtoSymbols) {
+  TestHeaderName = testPath("x.proto.h");
+  const std::string Header = R"(
+  namespace nx {
+    class Top_Level {};
+    class TopLevel {};
+    enum Kind {
+      KIND_OK,
+      Kind_Not_Ok,
+    };
+  }
+  )";
+  runSymbolCollector(Header, /*Main=*/"");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(QName("nx"), QName("nx::TopLevel"),
+                                            QName("nx::Kind"),
+                                            QName("nx::KIND_OK")));
+}
+
+TEST_F(SymbolCollectorTest, DontFilterNonProtoSymbols) {
+  TestHeaderName = testPath("x.h");
+  const std::string Header = R"(
+  namespace nx {
+    class Top_Level {};
+    enum Kind {
+      Kind_Fine
+    };
+  }
+  )";
+  runSymbolCollector(Header, /*Main=*/"");
+  EXPECT_THAT(Symbols,
+              UnorderedElementsAre(QName("nx"), QName("nx::Top_Level"),
+                                   QName("nx::Kind"), QName("nx::Kind_Fine")));
+}
 
 } // namespace
 } // namespace clangd
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -90,6 +90,24 @@
   return llvm::None;
 }
 
+bool isPrivateProtoSymbol(const NamedDecl *ND, const SourceManager &SM) {
+  auto FileName = SM.getFilename(findNameLoc(ND));
+  if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
+    return false;
+  auto Name = ND->getName();
+  if (!Name.contains('_'))
+    return false;
+  // Nested proto entities (e.g. Message::Nested) have top-level decls
+  // that shouldn't be used (Message_Nested). Ignore them completely.
+  // The nested entities are dangling type aliases, we may want to reconsider
+  // including them in the future.
+  // For enum constants, SOME_ENUM_CONSTANT is not private and should be
+  // indexed. Outer_INNER is private. This heuristic relies on naming style, it
+  // will include OUTER_INNER and exclude some_enum_constant.
+  return (ND->getKind() != Decl::EnumConstant) ||
+         std::any_of(Name.begin(), Name.end(), islower);
+}
+
 bool shouldFilterDecl(const NamedDecl *ND, ASTContext *ASTCtx,
                       const SymbolCollector::Options &Opts) {
   using namespace clang::ast_matchers;
@@ -130,6 +148,10 @@
           .empty())
     return true;
 
+  // Avoid indexing internal symbols in protobuf generated headers.
+  if (isPrivateProtoSymbol(ND, ASTCtx->getSourceManager()))
+    return true;
+
   return false;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46751.146338.patch
Type: text/x-patch
Size: 2857 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180511/9f24b30b/attachment.bin>