Repository: kythe/kythe
Pull request #5392 (2022-10-10 23:11:44)

chore(cxx_indexer): add a test for protobuf enum generated code
Turns out the generally-preferred alias isn't current annotated.


Patch:
diff --git a/kythe/cxx/tools/BUILD b/kythe/cxx/tools/BUILD
index 6a5c26559..60a43d206 100644
--- a/kythe/cxx/tools/BUILD
+++ b/kythe/cxx/tools/BUILD
@@ -15,6 +15,7 @@ cc_binary(
         "//kythe/cxx/common:init",
         "//kythe/cxx/common:kzip_reader",
         "//kythe/cxx/common:lib",
+        "//kythe/cxx/common:re2_flag",
         "//kythe/proto:analysis_cc_proto",
         "//kythe/proto:claim_cc_proto",
         "//kythe/proto:filecontext_cc_proto",
diff --git a/kythe/cxx/tools/static_claim_main.cc b/kythe/cxx/tools/static_claim_main.cc
index 3b168395c..22524d4ad 100644
--- a/kythe/cxx/tools/static_claim_main.cc
+++ b/kythe/cxx/tools/static_claim_main.cc
@@ -38,6 +38,7 @@
 #include "google/protobuf/io/zero_copy_stream_impl.h"
 #include "kythe/cxx/common/init.h"
 #include "kythe/cxx/common/kzip_reader.h"
+#include "kythe/cxx/common/re2_flag.h"
 #include "kythe/cxx/common/vname_ordering.h"
 #include "kythe/proto/analysis.pb.h"
 #include "kythe/proto/claim.pb.h"
@@ -49,6 +50,8 @@ using kythe::proto::VName;
 
 ABSL_FLAG(bool, text, false, "Dump output as text instead of protobuf.");
 ABSL_FLAG(bool, show_stats, false, "Show some statistics.");
+ABSL_FLAG(kythe::RE2Flag, include_files, {},
+          "If set, a RE2 pattern of file VName paths to claim.");
 
 struct Claimable;
 
@@ -215,21 +218,13 @@ class ClaimTool {
           // the file included at h with context c (otherwise the index file
           // isn't well-formed). We therefore only need to claim each unique
           // row.
-          ++total_include_count_;
           VName cxt_vname = input_vname;
           cxt_vname.set_signature(row.source_context() +
                                   input_vname.signature());
-          auto input_insert_result =
-              claimables_.emplace(cxt_vname, Claimable{cxt_vname, nullptr});
-          input_insert_result.first->second.claimants.insert(
-              &insert_result.first->second);
+          Claim(cxt_vname, &insert_result.first->second);
         }
       } else {
-        ++total_include_count_;
-        auto input_insert_result = claimables_.emplace(
-            input.v_name(), Claimable{input.v_name(), nullptr});
-        input_insert_result.first->second.claimants.insert(
-            &insert_result.first->second);
+        Claim(input.v_name(), &insert_result.first->second);
       }
     }
   }
@@ -238,8 +233,22 @@ class ClaimTool {
   const ClaimableMap& claimables() const { return claimables_; }
   size_t total_include_count() const { return total_include_count_; }
   size_t total_input_count() const { return total_input_count_; }
+  size_t skipped_input_count() const { return skipped_input_count_; }
 
  private:
+  void Claim(const VName& vname, Claimant* claimant) {
+    ++total_include_count_;
+    if (auto accept = absl::GetFlag(FLAGS_include_files);
+        accept.value != nullptr &&
+        !RE2::FullMatch(vname.path(), *accept.value)) {
+      ++skipped_input_count_;
+      return;
+    }
+
+    auto input_insert_result =
+        claimables_.emplace(vname, Claimable{vname, nullptr});
+    input_insert_result.first->second.claimants.insert(claimant);
+  }
   /// Objects that may claim resources.
   ClaimantMap claimants_;
   /// Resources that may be claimed.
@@ -248,6 +257,8 @@ class ClaimTool {
   size_t total_include_count_ = 0;
   /// Number of #includes.
   size_t total_input_count_ = 0;
+  /// Number of required inputs skipped due for failure to match.
+  size_t skipped_input_count_ = 0;
 };
 
 int main(int argc, char* argv[]) {
@@ -261,7 +272,7 @@ int main(int argc, char* argv[]) {
     if (next_index_file.empty()) {
       continue;
     }
-    for (CompilationUnit unit : ReadCompilationUnits(next_index_file)) {
+    for (const auto& unit : ReadCompilationUnits(next_index_file)) {
       tool.HandleCompilationUnit(unit);
     }
   }
@@ -276,6 +287,7 @@ int main(int argc, char* argv[]) {
     absl::PrintF(" Number of claimants: %lu\n", tool.claimants().size());
     absl::PrintF("   Total input count: %lu\n", tool.total_input_count());
     absl::PrintF(" Total include count: %lu\n", tool.total_include_count());
+    absl::PrintF(" Skipped input count: %lu\n", tool.skipped_input_count());
     absl::PrintF("%%claimables/includes: %f\n",
                  tool.claimables().size() * 100.0 / tool.total_include_count());
   }


Test patch:
diff --git a/kythe/cxx/indexer/cxx/testdata/proto/BUILD b/kythe/cxx/indexer/cxx/testdata/proto/BUILD
index 2d19b3456..9eb1c5c1d 100644
--- a/kythe/cxx/indexer/cxx/testdata/proto/BUILD
+++ b/kythe/cxx/indexer/cxx/testdata/proto/BUILD
@@ -60,6 +60,15 @@ cc_proto_verifier_test(
     ],
 )
 
+cc_proto_verifier_test(
+    name = "enum_test",
+    size = "medium",
+    srcs = ["enum.cc"],
+    proto_libs = [
+        "//kythe/testdata/indexers/proto:enum_proto",
+    ],
+)
+
 bzl_library(
     name = "cc_proto_verifier_test_bzl",
     srcs = ["cc_proto_verifier_test.bzl"],
diff --git a/kythe/cxx/indexer/cxx/testdata/proto/cc_proto_verifier_test.bzl b/kythe/cxx/indexer/cxx/testdata/proto/cc_proto_verifier_test.bzl
index c536863a7..e2d668c41 100644
--- a/kythe/cxx/indexer/cxx/testdata/proto/cc_proto_verifier_test.bzl
+++ b/kythe/cxx/indexer/cxx/testdata/proto/cc_proto_verifier_test.bzl
@@ -31,7 +31,8 @@ def cc_proto_verifier_test(
         ],
         size = "small",
         experimental_guess_proto_semantics = False,
-        experimental_record_dataflow_edges = False):
+        experimental_record_dataflow_edges = False,
+        minimal_claiming = True):
     """Verify cross-language references between C++ and Proto.
 
     Args:
@@ -42,6 +43,7 @@ def cc_proto_verifier_test(
       size: Size of the test.
       experimental_guess_proto_semantics: guess proto semantics?
       experimental_record_dataflow_edges: record dataflow edges?
+      minimal_claiming: If true, only index the `srcs` and protobuf header files.
 
     Returns:
       The label of the test.
@@ -75,6 +77,33 @@ def cc_proto_verifier_test(
         deps = cc_proto_libs,
     )
 
+    claim_file = None
+    if minimal_claiming:
+        claim_file = name + ".claim"
+        native.genrule(
+            name = name + "_static_claim",
+            outs = [claim_file],
+            srcs = [cc_kzip],
+            exec_tools = ["//kythe/cxx/tools:static_claim"],
+            cmd = " ".join([
+                "echo \"$<\" |",
+                "$(location //kythe/cxx/tools:static_claim)",
+                # Only claim protocol buffer headers and direct sources.
+                # This won't work for things which aren't direct, immediate source files.
+                "--include_files='.*/({}|[^/]+.(pb|proto).h)$$'".format("|".join(srcs)),
+                "> \"$@\"",
+            ]),
+        )
+
+    claim_opt = []
+    claim_deps = []
+    if minimal_claiming:
+        claim_opt = [
+            "--static_claim=$(execpath {})".format(claim_file),
+            "--claim_unknown=false",
+        ]
+        claim_deps = [claim_file]
+
     guess_opt = []
     if experimental_guess_proto_semantics:
         guess_opt = ["--experimental_guess_proto_semantics"]
@@ -86,6 +115,7 @@ def cc_proto_verifier_test(
         cc_index,
         name = name + "_cc_entries",
         srcs = [cc_kzip],
+        deps = claim_deps,
         opts = [
             # Avoid emitting some nodes with conflicting facts.
             "--experimental_index_lite",
@@ -94,7 +124,7 @@ def cc_proto_verifier_test(
             "--noindex_template_instantiations",
             "--experimental_drop_instantiation_independent_data",
             "--noemit_anchors_on_builtins",
-        ] + guess_opt + df_opt,
+        ] + guess_opt + df_opt + claim_opt,
     )
 
     return _invoke(
diff --git a/kythe/cxx/indexer/cxx/testdata/proto/enum.cc b/kythe/cxx/indexer/cxx/testdata/proto/enum.cc
new file mode 100644
index 000000000..f5240eea1
--- /dev/null
+++ b/kythe/cxx/indexer/cxx/testdata/proto/enum.cc
@@ -0,0 +1,21 @@
+#include "kythe/testdata/indexers/proto/enum.pb.h"
+
+//- WrapperMessageProto generates WrapperMessageCpp
+//- EnumerationTypeProto generates EnumerationTypeCpp
+//- EnumeratorProto generates EnumeratorCpp
+
+//- @Wrapper ref WrapperMessageCpp
+//- // TODO(shahms): Uncomment the below when the alias is annotated.
+//- //@EnumerationType ref EnumerationAliasCpp
+//- //EnumerationTypeProto generates EnumerationAliasCpp
+::pkg::proto::Wrapper::EnumerationType get_alias();
+//- @Wrapper_EnumerationType ref EnumerationTypeCpp
+::pkg::proto::Wrapper_EnumerationType get_raw();
+
+void fn() {
+  //- @Wrapper ref WrapperMessageCpp
+  ::pkg::proto::Wrapper message;
+  //- @Wrapper ref WrapperMessageCpp
+  //- @ENUMERATOR ref EnumeratorCpp
+  message.set_field(::pkg::proto::Wrapper::ENUMERATOR);
+}
diff --git a/kythe/testdata/indexers/proto/BUILD b/kythe/testdata/indexers/proto/BUILD
index 85830feef..8a96bfe0f 100644
--- a/kythe/testdata/indexers/proto/BUILD
+++ b/kythe/testdata/indexers/proto/BUILD
@@ -1,36 +1,33 @@
 load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
 load("@rules_proto//proto:defs.bzl", "proto_library")
 
+package(default_visibility = ["//visibility:public"])
+
 exports_files(glob(["*.proto"]))
 
 proto_library(
     name = "testdata_proto",
     srcs = ["testdata.proto"],
-    visibility = ["//visibility:public"],
 )
 
 proto_library(
     name = "testdata2_proto",
     srcs = ["testdata2.proto"],
-    visibility = ["//visibility:public"],
 )
 
 proto_library(
     name = "testdata3a_proto",
     srcs = ["testdata3a.proto"],
-    visibility = ["//visibility:public"],
 )
 
 proto_library(
     name = "testdata3b_proto",
     srcs = ["testdata3b.proto"],
-    visibility = ["//visibility:public"],
 )
 
 proto_library(
     name = "testdata3_proto",
     srcs = ["testdata3.proto"],
-    visibility = ["//visibility:public"],
     deps = [
         ":testdata3a_proto",
         ":testdata3b_proto",
@@ -40,31 +37,33 @@ proto_library(
 proto_library(
     name = "testdata4a_proto",
     srcs = ["testdata4a.proto"],
-    visibility = ["//visibility:public"],
     deps = [":testdata4c_proto"],
 )
 
 proto_library(
     name = "testdata4b_proto",
     srcs = ["testdata4b.proto"],
-    visibility = ["//visibility:public"],
     deps = [":testdata4c_proto"],
 )
 
 proto_library(
     name = "testdata4c_proto",
     srcs = ["testdata4c.proto"],
-    visibility = ["//visibility:public"],
 )
 
 copy_file(
     name = "testdata5_generated",
     src = "testdata5.proto",
     out = "gen/testdata5.gen.proto",
+    visibility = ["//visibility:private"],
 )
 
 proto_library(
     name = "testdata5_proto",
     srcs = [":testdata5_generated"],
-    visibility = ["//visibility:public"],
+)
+
+proto_library(
+    name = "enum_proto",
+    srcs = ["enum.proto"],
 )
diff --git a/kythe/testdata/indexers/proto/enum.proto b/kythe/testdata/indexers/proto/enum.proto
new file mode 100644
index 000000000..99d1d86a6
--- /dev/null
+++ b/kythe/testdata/indexers/proto/enum.proto
@@ -0,0 +1,16 @@
+syntax = "proto3";
+
+package pkg.proto;
+
+option java_package = "pkg.proto";
+
+//- @Wrapper defines/binding WrapperMessageProto
+message Wrapper {
+  //- @EnumerationType defines/binding EnumerationTypeProto
+  enum EnumerationType {
+    //- @ENUMERATOR defines/binding EnumeratorProto
+    ENUMERATOR = 0;
+  }
+  //- @EnumerationType ref EnumerationTypeProto
+  EnumerationType field = 1;
+}
diff --git a/tools/build_rules/verifier_test/cc_indexer_test.bzl b/tools/build_rules/verifier_test/cc_indexer_test.bzl
index ed5cd70be..9dca7adee 100644
--- a/tools/build_rules/verifier_test/cc_indexer_test.bzl
+++ b/tools/build_rules/verifier_test/cc_indexer_test.bzl
@@ -521,6 +521,12 @@ _bazel_extract_kzip = rule(
     implementation = _bazel_extract_kzip_impl,
 )
 
+def _expand_as_rootpath(ctx, option):
+    # Replace $(location X) and $(execpath X) with $(rootpath X).
+    return ctx.expand_location(
+        option.replace("$(location ", "$(rootpath ").replace("$(execpath ", "$(rootpath "),
+    )
+
 def _cc_index_source(ctx, src, test_runners):
     entries = ctx.actions.declare_file(
         ctx.label.name + "/" + src.basename + ".entries",
@@ -544,12 +550,12 @@ def _cc_index_source(ctx, src, test_runners):
         ctx,
         src,
         _INDEXER_LOGGING_ENV,
-        arguments = [ctx.expand_location(o) for o in ctx.attr.opts] + [
+        arguments = [_expand_as_rootpath(ctx, o) for o in ctx.attr.opts] + [
             "-i",
             src.short_path,
             "--",
             "-c",
-        ] + [ctx.expand_location(o) for o in ctx.attr.copts],
+        ] + [_expand_as_rootpath(ctx, o) for o in ctx.attr.copts],
     ))
     return entries
 
@@ -562,7 +568,7 @@ def _cc_index_compilation(ctx, compilation, test_runners):
     ctx.actions.run(
         mnemonic = "CcIndexCompilation",
         outputs = [entries],
-        inputs = [compilation],
+        inputs = [compilation] + ctx.files.deps,
         tools = [ctx.executable.indexer],
         executable = ctx.executable.indexer,
         arguments = [ctx.expand_location(o) for o in ctx.attr.opts] + [
@@ -575,7 +581,7 @@ def _cc_index_compilation(ctx, compilation, test_runners):
         ctx,
         compilation,
         _INDEXER_LOGGING_ENV,
-        arguments = [ctx.expand_location(o) for o in ctx.attr.opts] + [
+        arguments = [_expand_as_rootpath(ctx, o) for o in ctx.attr.opts] + [
             compilation.short_path,
         ],
     ))
@@ -683,6 +689,7 @@ cc_index = rule(
             allow_files = [
                 ".h",
                 ".meta",  # Cross language metadata files.
+                ".claim",  # Static claim files.
             ],
         ),
         "indexer": attr.label(