Repository: kythe/kythe
Pull request #5909 (2023-10-24 21:14:56)

feat(cxx_extractor): add ability to canonicalize VName paths per-file


Patch:
diff --git a/kythe/cxx/common/BUILD b/kythe/cxx/common/BUILD
index 993b22833..986558022 100644
--- a/kythe/cxx/common/BUILD
+++ b/kythe/cxx/common/BUILD
@@ -86,6 +86,8 @@ cc_library(
     visibility = [PUBLIC_VISIBILITY],
     deps = [
         ":common_status",
+        ":regex",
+        "@com_google_absl//absl/algorithm:container",
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/log",
         "@com_google_absl//absl/memory",
@@ -93,6 +95,7 @@ cc_library(
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/synchronization",
+        "@com_google_absl//absl/types:span",
     ],
 )
 
diff --git a/kythe/cxx/common/path_utils.cc b/kythe/cxx/common/path_utils.cc
index 8919ada0b..02f11c430 100644
--- a/kythe/cxx/common/path_utils.cc
+++ b/kythe/cxx/common/path_utils.cc
@@ -28,6 +28,7 @@
 #include <utility>
 #include <vector>
 
+#include "absl/algorithm/container.h"
 #include "absl/log/log.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/match.h"
@@ -37,6 +38,8 @@
 #include "absl/strings/string_view.h"
 #include "absl/strings/strip.h"
 #include "absl/synchronization/mutex.h"
+#include "absl/types/span.h"
+#include "kythe/cxx/common/regex.h"
 #include "kythe/cxx/common/status.h"
 
 namespace kythe {
@@ -123,6 +126,15 @@ PathParts SplitPath(absl::string_view path) {
   return {path.substr(0, pos), absl::ClippedSubstr(path, pos + 1)};
 }
 
+constexpr struct VisitPattern {
+  absl::string_view operator()(absl::string_view pattern) const {
+    return pattern;
+  }
+  absl::string_view operator()(const Regex& pattern) const {
+    return pattern.pattern();
+  }
+} kVisitPattern;
+
 }  // namespace
 
 absl::StatusOr<PathCleaner> PathCleaner::Create(absl::string_view root) {
@@ -185,7 +197,8 @@ absl::StatusOr<std::string> PathRealizer::Relativize(
 }
 
 absl::StatusOr<PathCanonicalizer> PathCanonicalizer::Create(
-    absl::string_view root, Policy policy) {
+    absl::string_view root, Policy policy,
+    absl::Span<const PathEntry> path_map) {
   absl::StatusOr<PathCleaner> cleaner = PathCleaner::Create(root);
   if (!cleaner.ok()) {
     return cleaner.status();
@@ -195,12 +208,34 @@ absl::StatusOr<PathCanonicalizer> PathCanonicalizer::Create(
   if (!realizer.ok()) {
     return realizer.status();
   }
-  return PathCanonicalizer(policy, *std::move(cleaner), *std::move(realizer));
+
+  std::vector<Policy> override_policies;
+  std::vector<absl::string_view> override_paths;
+  for (const auto& [path, policy] : path_map) {
+    if (!realizer->has_value()) {
+      realizer = MaybeMakeRealizer(policy, root);
+      if (!realizer.ok()) {
+        return realizer.status();
+      }
+    }
+    override_policies.push_back(policy);
+    override_paths.push_back(std::visit(kVisitPattern, path));
+  }
+  absl::StatusOr<RegexSet> override_set = RegexSet::Build(override_paths);
+  if (!override_set.ok()) {
+    return override_set.status();
+  }
+  return PathCanonicalizer(policy, *std::move(cleaner), *std::move(realizer),
+                           *std::move(override_set),
+                           std::move(override_policies));
 }
 
 absl::StatusOr<std::string> PathCanonicalizer::Relativize(
     absl::string_view path) const {
-  switch (policy_) {
+  absl::StatusOr<Policy> policy = PolicyFor(path);
+  if (!policy.ok()) return policy.status();
+
+  switch (*policy) {
     case Policy::kPreferRelative:
       if (auto resolved = MaybeRealPath(realizer_, path)) {
         if (!IsAbsolutePath(*resolved)) {
@@ -216,10 +251,22 @@ absl::StatusOr<std::string> PathCanonicalizer::Relativize(
     case Policy::kCleanOnly:
       return cleaner_.Relativize(path);
   }
-  LOG(FATAL) << "Unknown policy: " << static_cast<int>(policy_);
+  LOG(FATAL) << "Unknown policy: " << static_cast<int>(*policy);
   return std::string(path);
 }
 
+absl::StatusOr<PathCanonicalizer::Policy> PathCanonicalizer::PolicyFor(
+    absl::string_view path) const {
+  absl::StatusOr<std::vector<int>> match = override_set_.ExplainMatch(path);
+  if (!match.ok()) {
+    return match.status();
+  }
+  if (match->empty()) {
+    return policy_;
+  }
+  return override_policy_[*absl::c_min_element(*match)];
+}
+
 std::optional<PathCanonicalizer::Policy> ParseCanonicalizationPolicy(
     absl::string_view policy) {
   using Policy = PathCanonicalizer::Policy;
@@ -264,6 +311,46 @@ std::string JoinPath(absl::string_view a, absl::string_view b) {
                       absl::StripPrefix(b, "/"));
 }
 
+bool AbslParseFlag(absl::string_view text, PathCanonicalizer::PathEntry* entry,
+                   std::string* error) {
+  size_t pos = text.find('@');
+  if (pos == text.npos) {
+    *error = "missing @ delimiter between path and policy";
+    return false;
+  }
+  absl::StatusOr<Regex> path = Regex::Compile(text.substr(0, pos));
+  if (!path.ok()) {
+    *error = path.status().message();
+    return false;
+  }
+  entry->path = *std::move(path);
+  return AbslParseFlag(text.substr(pos + 1), &entry->policy, error);
+}
+
+std::string AbslUnparseFlag(const PathCanonicalizer::PathEntry& entry) {
+  return absl::StrCat(std::visit(kVisitPattern, entry.path), "@",
+                      AbslUnparseFlag(entry.policy));
+}
+
+bool AbslParseFlag(absl::string_view text,
+                   std::vector<PathCanonicalizer::PathEntry>* entries,
+                   std::string* error) {
+  for (const auto& entry : absl::StrSplit(text, ' ')) {
+    if (!AbslParseFlag(entry, &entries->emplace_back(), error)) {
+      entries->pop_back();
+      return false;
+    }
+  }
+  return true;
+}
+
+std::string AbslUnparseFlag(
+    const std::vector<PathCanonicalizer::PathEntry>& entries) {
+  return absl::StrJoin(entries, " ", [](std::string* out, const auto& entry) {
+    absl::StrAppend(out, AbslUnparseFlag(entry));
+  });
+}
+
 std::string CleanPath(absl::string_view input) {
   const bool is_absolute_path = absl::StartsWith(input, "/");
   std::vector<absl::string_view> parts;
diff --git a/kythe/cxx/common/path_utils.h b/kythe/cxx/common/path_utils.h
index 8b0a4dc2a..533be40ef 100644
--- a/kythe/cxx/common/path_utils.h
+++ b/kythe/cxx/common/path_utils.h
@@ -21,11 +21,15 @@
 #include <optional>
 #include <string>
 #include <utility>
+#include <variant>
+#include <vector>
 
 #include "absl/container/flat_hash_map.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
 #include "absl/synchronization/mutex.h"
+#include "absl/types/span.h"
+#include "kythe/cxx/common/regex.h"
 
 namespace kythe {
 
@@ -97,9 +101,21 @@ class PathCanonicalizer {
     kPreferReal = 2,      ///< Use real path, except if errors.
   };
 
+  /// \brief An entry to use when overriding the default policy.
+  struct PathEntry {
+    /// \brief The path pattern to override.
+    std::variant<Regex, std::string> path;
+
+    /// \brief The policy to apply to matching paths.
+    Policy policy = Policy::kCleanOnly;
+  };
+
   /// \brief Creates a new PathCanonicalizer with the given root and policy.
+  /// \param root The root directory to use when relativizing paths.
+  /// \param policy The default policy to apply.
   static absl::StatusOr<PathCanonicalizer> Create(
-      absl::string_view root, Policy policy = Policy::kCleanOnly);
+      absl::string_view root, Policy policy = Policy::kCleanOnly,
+      absl::Span<const PathEntry> path_map = {});
 
   /// \brief Transforms the provided path into a relative path depending on
   ///        configured policy.
@@ -107,12 +123,22 @@ class PathCanonicalizer {
 
  private:
   explicit PathCanonicalizer(Policy policy, PathCleaner cleaner,
-                             std::optional<PathRealizer> realizer)
-      : policy_(policy), cleaner_(cleaner), realizer_(realizer) {}
+                             std::optional<PathRealizer> realizer,
+                             RegexSet override_set,
+                             std::vector<Policy> override_policy)
+      : policy_(policy),
+        cleaner_(std::move(cleaner)),
+        realizer_(std::move(realizer)),
+        override_set_(std::move(override_set)),
+        override_policy_(std::move(override_policy)) {}
+
+  absl::StatusOr<Policy> PolicyFor(absl::string_view path) const;
 
   Policy policy_;
   PathCleaner cleaner_;
   std::optional<PathRealizer> realizer_;
+  RegexSet override_set_;
+  std::vector<Policy> override_policy_;
 };
 
 /// \brief Parses a flag string as a PathCanonicalizer::Policy.
@@ -135,6 +161,24 @@ std::string AbslUnparseFlag(PathCanonicalizer::Policy policy);
 std::optional<PathCanonicalizer::Policy> ParseCanonicalizationPolicy(
     absl::string_view policy);
 
+/// \brief Parses a single PathEntry flag value as `<pattern>@<policy>`
+bool AbslParseFlag(absl::string_view text, PathCanonicalizer::PathEntry* entry,
+                   std::string* error);
+
+/// \brief Returns the flag string representation of
+/// PathCanonicalizer::PathEntry.
+std::string AbslUnparseFlag(const PathCanonicalizer::PathEntry& entry);
+
+/// \brief Parses a space separated list of PathEntry flag values.
+bool AbslParseFlag(absl::string_view text,
+                   std::vector<PathCanonicalizer::PathEntry>* entries,
+                   std::string* error);
+
+/// \brief Returns the flag string representation of
+/// PathCanonicalizer::PathEntry.
+std::string AbslUnparseFlag(
+    const std::vector<PathCanonicalizer::PathEntry>& entry);
+
 /// \brief Append path `b` to path `a`, cleaning and returning the result.
 std::string JoinPath(absl::string_view a, absl::string_view b);
 
diff --git a/kythe/cxx/common/regex.h b/kythe/cxx/common/regex.h
index 1b44444ec..5ab49b315 100644
--- a/kythe/cxx/common/regex.h
+++ b/kythe/cxx/common/regex.h
@@ -13,6 +13,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+#ifndef KYTHE_CXX_COMMON_REGEX_H_
+#define KYTHE_CXX_COMMON_REGEX_H_
 
 #include <memory>
 
@@ -22,9 +24,6 @@
 #include "re2/re2.h"
 #include "re2/set.h"
 
-#ifndef KYTHE_CXX_COMMON_REGEX_H_
-#define KYTHE_CXX_COMMON_REGEX_H_
-
 namespace kythe {
 
 /// \brief Regex is a Regular value type implemented on top of RE2.
@@ -53,7 +52,10 @@ class Regex {
 
   /// \brief Retrieves the underlying RE2 object to be compatible with the RE2
   /// non-member functions.
-  operator const RE2&() const { return *re_; }
+  operator const RE2&() const { return *re_; }  // NOLINT
+
+  /// \brief Returns the string specification of the regular expression.
+  absl::string_view pattern() const { return re_->pattern(); }
 
  private:
   explicit Regex(std::shared_ptr<const RE2> re) : re_(std::move(re)) {}
diff --git a/kythe/cxx/extractor/cxx_extractor.cc b/kythe/cxx/extractor/cxx_extractor.cc
index 402efd19e..875ad7b05 100644
--- a/kythe/cxx/extractor/cxx_extractor.cc
+++ b/kythe/cxx/extractor/cxx_extractor.cc
@@ -1140,7 +1140,8 @@ CompilationWriter::RootPath CompilationWriter::RootRelativePath(
 
   if (!canonicalizer_.has_value()) {
     if (absl::StatusOr<PathCanonicalizer> canonicalizer =
-            PathCanonicalizer::Create(root_directory_, path_policy_);
+            PathCanonicalizer::Create(root_directory_, path_policy_,
+                                      path_policy_overrides_);
         canonicalizer.ok()) {
       canonicalizer_ = *std::move(canonicalizer);
     } else {
diff --git a/kythe/cxx/extractor/cxx_extractor.h b/kythe/cxx/extractor/cxx_extractor.h
index 620675f41..e6ab604b9 100644
--- a/kythe/cxx/extractor/cxx_extractor.h
+++ b/kythe/cxx/extractor/cxx_extractor.h
@@ -146,6 +146,10 @@ class KzipWriterSink : public CompilationWriterSink {
 /// writes it to an index file.
 class CompilationWriter {
  public:
+  CompilationWriter() = default;
+  CompilationWriter(const CompilationWriter&) = delete;
+  CompilationWriter& operator=(const CompilationWriter&) = delete;
+
   /// \brief Set the arguments to be used for this compilation.
   ///
   /// `args` should be the `argv` (without terminating null) that would be
@@ -185,6 +189,13 @@ class CompilationWriter {
     canonicalizer_.reset();
     path_policy_ = policy;
   }
+  /// \brief Configure the path canonicalization configuration.
+  void set_path_canonicalization_policy_overrides(
+      std::vector<PathCanonicalizer::PathEntry> entries) {
+    canonicalizer_.reset();
+    path_policy_overrides_ = std::move(entries);
+  }
+  /// \brief Configure per-path canonicalization overrides.
   /// \brief Don't include empty directories.
   void set_exclude_empty_dirs(bool exclude) { exclude_empty_dirs_ = exclude; }
   /// \brief Don't include files read during autoconfiguration.
@@ -254,6 +265,8 @@ class CompilationWriter {
   /// The policy to use when generating relative paths.
   PathCanonicalizer::Policy path_policy_ =
       PathCanonicalizer::Policy::kCleanOnly;
+  /// The per-path policy to use when generating relative paths.
+  std::vector<PathCanonicalizer::PathEntry> path_policy_overrides_;
   /// If nonempty, the name of the target that generated this compilation.
   std::string target_name_;
   /// If nonempty, the rule type that generated this compilation.
@@ -317,8 +330,12 @@ class ExtractorConfiguration {
     compilation_output_path_ = path;
   }
   /// \brief Sets the canonicalization policy to use for VName paths.
-  void SetPathCanonizalizationPolicy(PathCanonicalizer::Policy policy) {
+  void SetPathCanonizalizationPolicy(
+      PathCanonicalizer::Policy policy,
+      std::vector<PathCanonicalizer::PathEntry> overrides = {}) {
     index_writer_.set_path_canonicalization_policy(policy);
+    index_writer_.set_path_canonicalization_policy_overrides(
+        std::move(overrides));
   }
   /// \brief Executes the extractor with this configuration, returning true on
   /// success.
diff --git a/kythe/cxx/extractor/cxx_extractor_bazel_main.cc b/kythe/cxx/extractor/cxx_extractor_bazel_main.cc
index 1e35cac55..32a83729e 100644
--- a/kythe/cxx/extractor/cxx_extractor_bazel_main.cc
+++ b/kythe/cxx/extractor/cxx_extractor_bazel_main.cc
@@ -48,6 +48,10 @@ ABSL_FLAG(kythe::PathCanonicalizer::Policy, canonicalize_vname_paths,
           kythe::PathCanonicalizer::Policy::kCleanOnly,
           "Policy to use when canonicalization VName paths: "
           "clean-only (default), prefer-relative, prefer-real.");
+ABSL_FLAG(std::vector<kythe::PathCanonicalizer::PathEntry>,
+          per_file_canonicalization_policy, {},
+          "Space-separated list of <pattern>@<policy> to use when "
+          "canonicalizing paths");
 
 static void LoadExtraAction(const std::string& path,
                             blaze::ExtraActionInfo* info,
@@ -108,7 +112,8 @@ int main(int argc, char* argv[]) {
   config.SetBuildConfig(absl::GetFlag(FLAGS_build_config));
   config.SetCompilationOutputPath(cpp_info.output_file());
   config.SetPathCanonizalizationPolicy(
-      absl::GetFlag(FLAGS_canonicalize_vname_paths));
+      absl::GetFlag(FLAGS_canonicalize_vname_paths),
+      absl::GetFlag(FLAGS_per_file_canonicalization_policy));
   config.Extract(kythe::supported_language::Language::kCpp);
   google::protobuf::ShutdownProtobufLibrary();
   return 0;


Test patch:
diff --git a/kythe/cxx/common/path_utils_test.cc b/kythe/cxx/common/path_utils_test.cc
index 293784b5c..4d98bd675 100644
--- a/kythe/cxx/common/path_utils_test.cc
+++ b/kythe/cxx/common/path_utils_test.cc
@@ -29,10 +29,15 @@
 #include "absl/log/check.h"
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace kythe {
 namespace {
+using ::testing::ElementsAre;
+using ::testing::FieldsAre;
+using ::testing::Property;
+using ::testing::VariantWith;
 
 std::error_code Symlink(const std::string& target,
                         const std::string& linkpath) {
@@ -152,6 +157,27 @@ TEST(PathUtilsTest, CanonicalizerPolicyParseTest) {
   EXPECT_NE("", error);
 }
 
+TEST(PathUtilsTest, CanonicalizerPathEntryParseTest) {
+  std::string error;
+  std::vector<PathCanonicalizer::PathEntry> entries;
+  ASSERT_TRUE(
+      AbslParseFlag("a@clean-only b@prefer-relative", &entries, &error));
+  EXPECT_THAT(
+      entries,
+      ElementsAre(FieldsAre(VariantWith<Regex>(Property(&Regex::pattern, "a")),
+                            PathCanonicalizer::Policy::kCleanOnly),
+                  FieldsAre(VariantWith<Regex>(Property(&Regex::pattern, "b")),
+                            PathCanonicalizer::Policy::kPreferRelative)));
+  EXPECT_EQ(AbslUnparseFlag(entries), "a@clean-only b@prefer-relative");
+}
+
+TEST(PathUtilsTest, CanonicalizerPathEntryParseFailuresTest) {
+  std::string error;
+  std::vector<PathCanonicalizer::PathEntry> entries;
+  ASSERT_FALSE(AbslParseFlag("clean-only b@prefer-relative", &entries, &error));
+  EXPECT_THAT(error, "missing @ delimiter between path and policy");
+}
+
 class CanonicalizerTest : public ::testing::Test {
  public:
   void SetUp() override {
@@ -256,5 +282,41 @@ TEST_F(CanonicalizerTest, CanonicalizerPreferReal) {
                 .value_or(""));
 }
 
+TEST_F(CanonicalizerTest, CanonicalizerRealOverrideClean) {
+  AddDirectory("concrete");
+  AddDirectory("concrete/subdir");
+  AddDirectory("concrete/file");
+  AddSymlink("concrete", "link");
+
+  AddDirectory("base");
+  AddDirectory("elsewhere");
+  AddDirectory("elsewhere/subdir");
+  AddDirectory("elsewhere/file");
+  AddSymlink("../elsewhere", "base/link");
+
+  const std::string base = JoinPath(root(), "base");
+  PathCanonicalizer canonicalizer =
+      PathCanonicalizer::Create(
+          root(), PathCanonicalizer::Policy::kCleanOnly,
+          {{.path = ".*/base/link",
+            .policy = PathCanonicalizer::Policy::kPreferReal}})
+          .value();
+
+  // Use the default clean-only policy for links/paths which don't match.
+  EXPECT_EQ("link/file",
+            canonicalizer.Relativize(JoinPath(root(), "link/subdir/../file"))
+                .value_or(""));
+  EXPECT_EQ("link/file",
+            canonicalizer.Relativize(JoinPath(root(), "./link/subdir/../file"))
+                .value_or(""));
+  // And the prefer-real policy for those which do.
+  EXPECT_EQ("elsewhere/file",
+            canonicalizer.Relativize(JoinPath(base, "link/subdir/../file"))
+                .value_or(""));
+  EXPECT_EQ("base/bad/file",
+            canonicalizer.Relativize(JoinPath(base, "bad/subdir/../file"))
+                .value_or(""));
+}
+
 }  // namespace
 }  // namespace kythe