Repository: kythe/kythe
Pull request #4883 (2021-04-09 15:10:46)

feat(common): allow named captures in vnames.json rules
Allow the use of named captures and replacements in vnames.json.  Go already had this, but the 3 languages should be consistent.

shahms: /gcbrun
shahms: Also included a C++ test.

Patch:
diff --git a/external.bzl b/external.bzl
index 389dfcc75..a5bb09f8e 100644
--- a/external.bzl
+++ b/external.bzl
@@ -290,7 +290,7 @@ def _java_dependencies():
             "com.google.errorprone:error_prone_annotations:2.3.1",
             "com.google.guava:guava:26.0-jre",
             "com.google.jimfs:jimfs:1.1",
-            "com.google.re2j:re2j:1.2",
+            "com.google.re2j:re2j:1.6",
             "com.google.truth:truth:1.0",
             "com.googlecode.java-diff-utils:diffutils:1.3.0",
             "org.apache.tomcat:tomcat-annotations-api:9.0.34",
diff --git a/kythe/cxx/common/BUILD b/kythe/cxx/common/BUILD
index 1d2d93a99..7906ecc5d 100644
--- a/kythe/cxx/common/BUILD
+++ b/kythe/cxx/common/BUILD
@@ -124,12 +124,10 @@ cc_library(
 cc_library(
     name = "lib",
     srcs = [
-        "file_vname_generator.cc",
         "kythe_metadata_file.cc",
         "protobuf_metadata_file.cc",
     ],
     hdrs = [
-        "file_vname_generator.h",
         "kythe_metadata_file.h",
         "protobuf_metadata_file.h",
         "vname_ordering.h",
@@ -141,6 +139,7 @@ cc_library(
     ],
     visibility = [PUBLIC_VISIBILITY],
     deps = [
+        ":file_vname_generator",
         ":json_proto",
         "//external:zlib",
         "//kythe/cxx/common/schema:edges",
@@ -187,20 +186,17 @@ cc_library(
 )
 
 cc_library(
-    name = "file_vname_generator_testlib",
-    testonly = 1,
-    srcs = [
-        "file_vname_generator_test.cc",
-    ],
-    copts = [
-        "-Wno-non-virtual-dtor",
-        "-Wno-unused-variable",
-        "-Wno-implicit-fallthrough",
-    ],
+    name = "file_vname_generator",
+    srcs = ["file_vname_generator.cc"],
+    hdrs = ["file_vname_generator.h"],
     deps = [
-        ":lib",
-        "//third_party:gtest",
-        "@com_google_protobuf//:protobuf",
+        "//kythe/proto:storage_cc_proto",
+        "@com_github_google_glog//:glog",
+        "@com_github_tencent_rapidjson//:rapidjson",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
+        "@com_google_absl//absl/strings",
+        "@com_google_absl//absl/types:optional",
         "@com_googlesource_code_re2//:re2",
     ],
 )
@@ -208,8 +204,14 @@ cc_library(
 cc_test(
     name = "file_vname_generator_test",
     size = "small",
+    srcs = [
+        "file_vname_generator_test.cc",
+    ],
     deps = [
-        ":file_vname_generator_testlib",
+        ":file_vname_generator",
+        "//third_party:gtest",
+        "@com_google_protobuf//:protobuf",
+        "@com_googlesource_code_re2//:re2",
     ],
 )
 
diff --git a/kythe/cxx/common/file_vname_generator.cc b/kythe/cxx/common/file_vname_generator.cc
index 5babdbae0..5b2863a4d 100644
--- a/kythe/cxx/common/file_vname_generator.cc
+++ b/kythe/cxx/common/file_vname_generator.cc
@@ -16,97 +16,99 @@
 
 #include "file_vname_generator.h"
 
+#include "absl/strings/numbers.h"
+#include "absl/strings/str_cat.h"
+#include "absl/strings/str_replace.h"
+#include "absl/types/optional.h"
 #include "glog/logging.h"
 #include "rapidjson/document.h"
 #include "rapidjson/error/en.h"
 
 namespace kythe {
+namespace {
 
-// FullMatchN supports "3..16 args"
-static constexpr int kMaxRegexArgs = 16;
+const LazyRE2 kSubstitutionsPattern = {R"(@\w+@)"};
 
-FileVNameGenerator::FileVNameGenerator() {
-  CHECK_EQ(RE2::NoError, substitution_matcher_.error_code());
+std::string EscapeBackslashes(absl::string_view value) {
+  return absl::StrReplaceAll(value, {{R"(\)", R"(\\)"}});
 }
 
-std::string FileVNameGenerator::ApplyRule(const StringConsRule& rule,
-                                          const re2::StringPiece* argv,
-                                          int argc) const {
-  std::string ret;
-  for (const auto& node : rule) {
-    switch (node.kind) {
-      case StringConsNode::Kind::kEmitText:
-        ret.append(node.raw_text);
-        break;
-      case StringConsNode::Kind::kUseSubstitution:
-        // Invariant: 0 <= node.capture_index < argc
-        ret.append(std::string(argv[node.capture_index]));
-        break;
-    }
+absl::optional<absl::string_view> FindMatch(absl::string_view text,
+                                            const RE2& pattern) {
+  re2::StringPiece match;
+  if (pattern.Match(text, 0, text.size(), RE2::UNANCHORED, &match, 1)) {
+    return absl::string_view(match.data(), match.size());
   }
-  return ret;
+  return absl::nullopt;
 }
 
-bool FileVNameGenerator::ParseRule(const std::string& rule, int max_capture,
-                                   StringConsRule* result,
-                                   std::string* error_text) {
-  CHECK(result != nullptr);
-  CHECK(error_text != nullptr);
-  re2::StringPiece rule_left(rule);
-  while (!rule_left.empty()) {
-    re2::StringPiece prefix, substitution;
-    if (!RE2::Consume(&rule_left, substitution_matcher_, &prefix,
-                      &substitution)) {
-      prefix = rule_left;
-      rule_left = re2::StringPiece();
-    }
-    if (!prefix.empty()) {
-      StringConsNode node;
-      node.kind = StringConsNode::Kind::kEmitText;
-      node.raw_text = std::string(prefix);
-      result->push_back(node);
-    }
-    if (!substitution.empty()) {
-      StringConsNode node;
-      node.kind = StringConsNode::Kind::kUseSubstitution;
-      node.capture_index = std::stoi(std::string(substitution));
-      if (node.capture_index == 0 || node.capture_index > max_capture) {
-        *error_text =
-            "Capture index out of range: " + std::to_string(node.capture_index);
-        return false;
+absl::StatusOr<std::string> ParseTemplate(const RE2& pattern,
+                                          absl::string_view input) {
+  std::string result;
+  while (absl::optional<absl::string_view> match =
+             FindMatch(input, *kSubstitutionsPattern)) {
+    absl::string_view group = match->substr(1, match->size() - 2);
+
+    int index = 0;
+    if (!absl::SimpleAtoi(group, &index)) {
+      auto iter = pattern.NamedCapturingGroups().find(std::string(group));
+      if (iter == pattern.NamedCapturingGroups().end()) {
+        return absl::InvalidArgumentError(
+            absl::StrCat("Unknown named capture: ", group));
       }
-      // Make these zero-based.
-      --node.capture_index;
-      result->push_back(node);
+      index = iter->second;
     }
+    if (index == 0 || index > pattern.NumberOfCapturingGroups()) {
+      return absl::InvalidArgumentError(
+          absl::StrCat("Capture index out of range: ", index));
+    }
+    absl::string_view prefix = input.substr(0, match->begin() - input.begin());
+    absl::StrAppend(&result, EscapeBackslashes(prefix), "\\", index);
+    input.remove_prefix(prefix.size() + match->size());
   }
-  return true;
+  // Include the unmatched tail.
+  absl::StrAppend(&result, EscapeBackslashes(input));
+  return result;
 }
 
-kythe::proto::VName FileVNameGenerator::LookupBaseVName(
-    const std::string& path) const {
-  re2::StringPiece argv[kMaxRegexArgs];
-  RE2::Arg args[kMaxRegexArgs];
-  RE2::Arg* arg_pointers[kMaxRegexArgs];
-  for (size_t n = 0; n < kMaxRegexArgs; ++n) {
-    args[n] = &argv[n];
-    arg_pointers[n] = &args[n];
+absl::StatusOr<std::string> ParseTemplateMember(const RE2& pattern,
+                                                const rapidjson::Value& parent,
+                                                absl::string_view name) {
+  const auto member =
+      parent.FindMember(rapidjson::Value(name.data(), name.size()));
+  if (member == parent.MemberEnd()) {
+    return "";
+  }
+  if (!member->value.IsString()) {
+    return absl::InvalidArgumentError(
+        absl::StrCat("VName template ", name, " is not a string."));
   }
+  return ParseTemplate(pattern, member->value.GetString());
+}
+
+}  // namespace
+
+kythe::proto::VName FileVNameGenerator::LookupBaseVName(
+    absl::string_view path) const {
   for (const auto& rule : rules_) {
-    // Invariant: capture_groups <= kMaxRegexArgs
-    // RE2 will fail to match if we provide more args than there are captures
-    // for a given regex.
-    int capture_groups = rule.pattern->NumberOfCapturingGroups();
-    if (RE2::FullMatchN(path, *rule.pattern, arg_pointers, capture_groups)) {
+    std::vector<re2::StringPiece> captures(
+        1 +
+        std::max({RE2::MaxSubmatch(rule.corpus), RE2::MaxSubmatch(rule.root),
+                  RE2::MaxSubmatch(rule.path)}));
+    if (rule.pattern->Match(path, 0, path.size(), RE2::ANCHOR_BOTH,
+                            captures.data(), captures.size())) {
       kythe::proto::VName result;
       if (!rule.corpus.empty()) {
-        result.set_corpus(ApplyRule(rule.corpus, argv, capture_groups));
+        rule.pattern->Rewrite(result.mutable_corpus(), rule.corpus,
+                              captures.data(), captures.size());
       }
       if (!rule.root.empty()) {
-        result.set_root(ApplyRule(rule.root, argv, capture_groups));
+        rule.pattern->Rewrite(result.mutable_root(), rule.root, captures.data(),
+                              captures.size());
       }
       if (!rule.path.empty()) {
-        result.set_path(ApplyRule(rule.path, argv, capture_groups));
+        rule.pattern->Rewrite(result.mutable_path(), rule.path, captures.data(),
+                              captures.size());
       }
       return result;
     }
@@ -115,92 +117,78 @@ kythe::proto::VName FileVNameGenerator::LookupBaseVName(
 }
 
 kythe::proto::VName FileVNameGenerator::LookupVName(
-    const std::string& path) const {
+    absl::string_view path) const {
   kythe::proto::VName vname = LookupBaseVName(path);
   if (vname.path().empty()) {
-    vname.set_path(path);
+    vname.set_path(path.data(), path.size());
   }
   return vname;
 }
 
-bool FileVNameGenerator::LoadJsonString(const std::string& data,
+bool FileVNameGenerator::LoadJsonString(absl::string_view data,
                                         std::string* error_text) {
-  CHECK(error_text != nullptr);
+  absl::Status status = LoadJsonString(data);
+  if (!status.ok() && error_text != nullptr) {
+    *error_text = status.ToString();
+  }
+  return status.ok();
+}
+
+absl::Status FileVNameGenerator::LoadJsonString(absl::string_view data) {
   using Value = rapidjson::Value;
   rapidjson::Document document;
-  document.Parse(data.c_str());
+  document.Parse(data.data(), data.size());
   if (document.HasParseError()) {
-    if (error_text) {
-      *error_text = rapidjson::GetParseError_En(document.GetParseError());
-      error_text->append(" near offset ");
-      error_text->append(std::to_string(document.GetErrorOffset()));
-    }
-    return false;
+    return absl::InvalidArgumentError(
+        absl::StrCat(rapidjson::GetParseError_En(document.GetParseError()),
+                     " near offset ", document.GetErrorOffset()));
   }
   if (!document.IsArray()) {
-    *error_text = "Root element in JSON was not an array.";
-    return false;
+    return absl::InvalidArgumentError("Root element in JSON was not an array.");
   }
   for (Value::ConstValueIterator rule = document.Begin();
        rule != document.End(); ++rule) {
     if (!rule->IsObject()) {
-      *error_text = "Expected an array of objects.";
-      return false;
+      return absl::InvalidArgumentError("Expected an array of objects.");
     }
     const auto regex = rule->FindMember("pattern");
     if (regex == rule->MemberEnd() || !regex->value.IsString()) {
-      *error_text = "VName rule is missing its pattern.";
-      return false;
+      return absl::InvalidArgumentError("VName rule is missing its pattern.");
     }
     VNameRule next_rule;
     // RE2s don't act like values. Just box them.
     next_rule.pattern = std::make_shared<RE2>(regex->value.GetString());
     if (next_rule.pattern->error_code() != RE2::NoError) {
-      *error_text = next_rule.pattern->error();
-      return false;
+      return absl::InvalidArgumentError(next_rule.pattern->error());
     }
-    int num_captures = next_rule.pattern->NumberOfCapturingGroups();
-    CHECK(num_captures <= kMaxRegexArgs);
     const auto vname = rule->FindMember("vname");
     if (vname == rule->MemberEnd() || !vname->value.IsObject()) {
-      *error_text = "VName rule is missing its template.";
-      return false;
+      return absl::InvalidArgumentError("VName rule is missing its template.");
     }
-    const auto root = vname->value.FindMember("root");
-    if (root != vname->value.MemberEnd()) {
-      if (!root->value.IsString()) {
-        *error_text = "VName template root is not a string.";
-        return false;
-      }
-      if (!ParseRule(root->value.GetString(), num_captures, &next_rule.root,
-                     error_text)) {
-        return false;
-      }
+
+    absl::StatusOr<std::string> root =
+        ParseTemplateMember(*next_rule.pattern, vname->value, "root");
+    if (!root.ok()) {
+      return root.status();
     }
-    const auto corpus = vname->value.FindMember("corpus");
-    if (corpus != vname->value.MemberEnd()) {
-      if (!corpus->value.IsString()) {
-        *error_text = "VName template corpus is not a string.";
-        return false;
-      }
-      if (!ParseRule(corpus->value.GetString(), num_captures, &next_rule.corpus,
-                     error_text)) {
-        return false;
-      }
+    next_rule.root = *std::move(root);
+
+    absl::StatusOr<std::string> corpus =
+        ParseTemplateMember(*next_rule.pattern, vname->value, "corpus");
+    if (!corpus.ok()) {
+      return corpus.status();
     }
-    const auto path = vname->value.FindMember("path");
-    if (path != vname->value.MemberEnd()) {
-      if (!path->value.IsString()) {
-        *error_text = "VName template path is not a string.";
-        return false;
-      }
-      if (!ParseRule(path->value.GetString(), num_captures, &next_rule.path,
-                     error_text)) {
-        return false;
-      }
+    next_rule.corpus = *std::move(corpus);
+
+    absl::StatusOr<std::string> path =
+        ParseTemplateMember(*next_rule.pattern, vname->value, "path");
+    if (!path.ok()) {
+      return path.status();
     }
+    next_rule.path = *std::move(path);
+
     rules_.push_back(next_rule);
   }
-  return true;
+  return absl::OkStatus();
 }
 }  // namespace kythe
diff --git a/kythe/cxx/common/file_vname_generator.h b/kythe/cxx/common/file_vname_generator.h
index b07254881..46164a65d 100644
--- a/kythe/cxx/common/file_vname_generator.h
+++ b/kythe/cxx/common/file_vname_generator.h
@@ -21,6 +21,9 @@
 #include <string>
 #include <vector>
 
+#include "absl/status/status.h"
+#include "absl/status/statusor.h"
+#include "absl/strings/string_view.h"
 #include "kythe/proto/storage.pb.h"
 #include "re2/re2.h"
 
@@ -29,20 +32,21 @@ namespace kythe {
 /// \brief Generates file VNames based on user-configurable paths and templates.
 class FileVNameGenerator {
  public:
-  FileVNameGenerator();
+  explicit FileVNameGenerator() = default;
 
   /// \brief Adds configuration entries from the json-encoded `json_string`.
   /// \param json_string The string containing the configuration to add.
   /// \param error_text Non-null. Will be set to text describing any errors.
   /// \return false if the string could not be parsed.
-  bool LoadJsonString(const std::string& data, std::string* error_text);
+  bool LoadJsonString(absl::string_view data, std::string* error_text);
+  absl::Status LoadJsonString(absl::string_view data);
 
   /// \brief Returns a base VName for a given file path (or an empty VName if
   /// no configuration rule matches the path).
-  kythe::proto::VName LookupBaseVName(const std::string& path) const;
+  kythe::proto::VName LookupBaseVName(absl::string_view path) const;
 
   /// \brief Returns a VName for the given file path.
-  kythe::proto::VName LookupVName(const std::string& path) const;
+  kythe::proto::VName LookupVName(absl::string_view path) const;
 
   /// \brief Sets the default base VName to use when no rules match.
   void set_default_base_vname(const kythe::proto::VName& default_vname) {
@@ -50,47 +54,19 @@ class FileVNameGenerator {
   }
 
  private:
-  /// \brief A command to use when building a result string.
-  struct StringConsNode {
-    enum class Kind {
-      kEmitText,        ///< Emit text verbatim.
-      kUseSubstitution  ///< Copy from a regex capture.
-    } kind;
-    /// Text to emit if `kind` is `kEmitText`.
-    std::string raw_text;
-    /// Substitution to use if `kind` is `kUseSubstitution`.
-    int capture_index;
-  };
-  using StringConsRule = std::vector<StringConsNode>;
-  /// \brief Applies a `StringConsRule` using the result of a regex match.
-  /// \param rule The rule to apply.
-  /// \param argv The `RE2::Arg` results from the regex match.
-  /// \param argc The length of the array `argv`.
-  std::string ApplyRule(const StringConsRule& rule,
-                        const re2::StringPiece* argv, int argc) const;
-  /// \brief Parses `rule` into a `StringConsRule`.
-  /// \param rule The rule to parse.
-  /// \param max_capture_index The maximum utterable capture index.
-  /// \param result The StringConsRule to parse into.
-  /// \param error_text Set to a descriptive message if parsing fails.
-  /// \return false if we could not parse a rule.
-  bool ParseRule(const std::string& rule, int max_capture_index,
-                 StringConsRule* result, std::string* error_text);
   /// \brief A rule to apply to certain paths.
   struct VNameRule {
     /// The pattern used to match against a path.
     std::shared_ptr<re2::RE2> pattern;
     /// Substitution pattern used to construct the corpus.
-    StringConsRule corpus;
+    std::string corpus;
     /// Substitution pattern used to construct the root.
-    StringConsRule root;
+    std::string root;
     /// Substitution pattern used to construct the path.
-    StringConsRule path;
+    std::string path;
   };
   /// The rules to apply to incoming paths. The first one to match is used.
   std::vector<VNameRule> rules_;
-  /// Used internally to find substitution markers when compiling rules.
-  RE2 substitution_matcher_{"([^@]*)@([^@]+)@"};
   /// The default base VName to use when no rules match.
   kythe::proto::VName default_vname_;
 };
diff --git a/kythe/cxx/extractor/proto/BUILD b/kythe/cxx/extractor/proto/BUILD
index 4c498a3c9..a84a3e802 100644
--- a/kythe/cxx/extractor/proto/BUILD
+++ b/kythe/cxx/extractor/proto/BUILD
@@ -28,6 +28,7 @@ cc_library(
     visibility = ["//kythe/cxx/extractor/textproto:__subpackages__"],
     deps = [
         "//kythe/cxx/common:file_utils",
+        "//kythe/cxx/common:file_vname_generator",
         "//kythe/cxx/common:index_writer",
         "//kythe/cxx/common:lib",
         "//kythe/cxx/common:path_utils",
diff --git a/kythe/cxx/indexer/proto/BUILD b/kythe/cxx/indexer/proto/BUILD
index d6a86c868..750da8e97 100644
--- a/kythe/cxx/indexer/proto/BUILD
+++ b/kythe/cxx/indexer/proto/BUILD
@@ -42,6 +42,7 @@ cc_library(
         ":proto_graph_builder",
         ":search_path",
         ":source_tree",
+        "//kythe/cxx/common:file_vname_generator",
         "//kythe/cxx/common:kythe_uri",
         "//kythe/cxx/common:lib",
         "//kythe/cxx/common:path_utils",
@@ -115,6 +116,7 @@ cc_binary(
     visibility = ["//visibility:public"],
     deps = [
         ":proto_analyzer",
+        "//kythe/cxx/common:file_vname_generator",
         "//kythe/cxx/common:init",
         "//kythe/cxx/common:json_proto",
         "//kythe/cxx/common:kzip_reader",
diff --git a/kythe/java/com/google/devtools/kythe/extractors/shared/FileVNames.java b/kythe/java/com/google/devtools/kythe/extractors/shared/FileVNames.java
index dddc14066..029fc43db 100644
--- a/kythe/java/com/google/devtools/kythe/extractors/shared/FileVNames.java
+++ b/kythe/java/com/google/devtools/kythe/extractors/shared/FileVNames.java
@@ -187,7 +187,7 @@ public class FileVNames {
     }
   }
 
-  /** Subset of a {@link VName} with built-in templating '@<num>@' markers. */
+  /** Subset of a {@link VName} with built-in templating '@<group>@' markers. */
   private static class VNameTemplate {
     private final String corpus;
     private final String root;
@@ -232,20 +232,19 @@ public class FileVNames {
       return fillInWith(m, () -> defaultCorpus);
     }
 
-    private static final Pattern replacerMatcher = Pattern.compile("@(\\d+)@");
+    private static final Pattern replacerPattern = Pattern.compile("@(\\w+)@");
 
     private static String fillIn(String tmpl, Matcher m) {
-      Matcher replacers = replacerMatcher.matcher(tmpl);
+      Matcher replacers = replacerPattern.matcher(tmpl);
       Deque<ReplacementMarker> matches = new ArrayDeque<>();
       while (replacers.find()) {
         matches.addFirst(
-            new ReplacementMarker(
-                replacers.start(), replacers.end(), Integer.parseInt(replacers.group(1))));
+            new ReplacementMarker(replacers.start(), replacers.end(), replacers.group(1)));
       }
       StringBuilder builder = new StringBuilder(tmpl);
       while (!matches.isEmpty()) {
         ReplacementMarker res = matches.removeFirst();
-        builder.replace(res.start, res.end, m.group(res.grp));
+        builder.replace(res.start, res.end, res.groupText(m));
       }
       return builder.toString();
     }
@@ -260,13 +259,22 @@ public class FileVNames {
   private static class ReplacementMarker {
     final int start;
     final int end;
-    final int grp;
+    final String grp;
 
-    ReplacementMarker(int start, int end, int grp) {
+    ReplacementMarker(int start, int end, String grp) {
       this.start = start;
       this.end = end;
       this.grp = grp;
     }
+
+    String groupText(Matcher m) {
+      try {
+        int idx = Integer.parseInt(grp);
+        return m.group(idx);
+      } catch (NumberFormatException unused) {
+        return m.group(grp);
+      }
+    }
   }
 
   private static class PatternDeserializer implements JsonDeserializer<Pattern> {


Test patch:
diff --git a/kythe/cxx/common/file_vname_generator_test.cc b/kythe/cxx/common/file_vname_generator_test.cc
index ec8410b05..c65e7dce1 100644
--- a/kythe/cxx/common/file_vname_generator_test.cc
+++ b/kythe/cxx/common/file_vname_generator_test.cc
@@ -86,7 +86,7 @@ TEST(FileVNameGenerator, LookupGroups) {
   EXPECT_EQ(corpus_vname.DebugString(),
             generator.LookupBaseVName("corpus/some/path/here").DebugString());
   kythe::proto::VName grp1_vname;
-  grp1_vname.set_corpus("grp1/endingGroup");
+  grp1_vname.set_corpus("grp1/grp1/endingGroup");
   grp1_vname.set_root("12345");
   EXPECT_EQ(grp1_vname.DebugString(),
             generator.LookupBaseVName("grp1/12345/endingGroup").DebugString());
@@ -249,10 +249,10 @@ const char kSharedTestFile[] = R"d([
     }
   },
   {
-    "pattern": "(grp1)/(\\d+)/(.*)",
+    "pattern": "(?P<CORPUS>grp1)/(\\d+)/(.*)",
     "vname": {
       "root": "@2@",
-      "corpus": "@1@/@3@"
+      "corpus": "@1@/@CORPUS@/@3@"
     }
   },
   {
diff --git a/kythe/javatests/com/google/devtools/kythe/extractors/shared/FileVNamesTest.java b/kythe/javatests/com/google/devtools/kythe/extractors/shared/FileVNamesTest.java
index 6a4d1b0d3..eeda75be1 100644
--- a/kythe/javatests/com/google/devtools/kythe/extractors/shared/FileVNamesTest.java
+++ b/kythe/javatests/com/google/devtools/kythe/extractors/shared/FileVNamesTest.java
@@ -53,10 +53,10 @@ public class FileVNamesTest extends TestCase {
                 "    }",
                 "  },",
                 "  {",
-                "    \"pattern\": \"(grp1)/(\\\\d+)/(.*)\",",
+                "    \"pattern\": \"(?P<corpus>grp1)/(\\\\d+)/(.*)\",",
                 "    \"vname\": {",
                 "      \"root\": \"@2@\",",
-                "      \"corpus\": \"@1@/@3@\"",
+                "      \"corpus\": \"@1@/@corpus@/@3@\"",
                 "    }",
                 "  },",
                 "  {",
@@ -95,7 +95,7 @@ public class FileVNamesTest extends TestCase {
           .addRule(rule("static/path", v("static", "root", "")))
           .addRule(rule("dup/path", v("first", "", "")))
           .addRule(rule("dup/path2", v("second", "", "")))
-          .addRule(rule("(grp1)/(\\d+)/(.*)", v("@1@/@3@", "@2@", "")))
+          .addRule(rule("(?P<corpus>grp1)/(\\d+)/(.*)", v("@1@/@corpus@/@3@", "@2@", "")))
           .addRule(rule("bazel-bin/([^/]+)/java/.*[.]jar!/.*", v("@1@", "java", "")))
           .addRule(rule("third_party/([^/]+)/.*[.]jar!/.*", v("third_party", "@1@", "")))
           .addRule(rule("([^/]+)/java/.*", v("@1@", "java", "")))
@@ -137,7 +137,7 @@ public class FileVNamesTest extends TestCase {
     assertThat(f.lookupBaseVName("corpus/some/path/here"))
         .isEqualTo(VName.newBuilder().setCorpus("corpus").build());
     assertThat(f.lookupBaseVName("grp1/12345/endingGroup"))
-        .isEqualTo(VName.newBuilder().setCorpus("grp1/endingGroup").setRoot("12345").build());
+        .isEqualTo(VName.newBuilder().setCorpus("grp1/grp1/endingGroup").setRoot("12345").build());
 
     assertThat(f.lookupBaseVName("bazel-bin/kythe/java/some/path/A.jar!/some/path/A.class"))
         .isEqualTo(VName.newBuilder().setCorpus("kythe").setRoot("java").build());