Implement structured field and rule paths for violations

Makefile

@@ -11,7 +11,7 @@ COPYRIGHT_YEARS := 2023
 LICENSE_IGNORE := -e internal/testdata/
 LICENSE_HEADER_VERSION := 0294fdbe1ce8649ebaf5e87e8cdd588e33730bbb
 # NOTE: Keep this version in sync with the version in `/bazel/deps.bzl`.
-PROTOVALIDATE_VERSION ?= v0.8.1
+PROTOVALIDATE_VERSION ?= v0.9.0
 
 # Set to use a different compiler. For example, `GO=go1.18rc1 make test`.
 GO ?= go

bazel/deps.bzl

@@ -65,10 +65,10 @@ _dependencies = {
     },
     # NOTE: Keep Version in sync with `/Makefile`.
     "com_github_bufbuild_protovalidate": {
-        "sha256": "c637c8cbaf71b6dc38171e47c2c736581b4cfef385984083561480367659d14f",
-        "strip_prefix": "protovalidate-0.8.1",
+        "sha256": "fca6143d820e9575f3aec328918fa25acc8eeb6e706050127d3a36cfdede4610",
+        "strip_prefix": "protovalidate-0.9.0",
         "urls": [
-            "https://github.com/bufbuild/protovalidate/archive/v0.8.1.tar.gz",
+            "https://github.com/bufbuild/protovalidate/archive/v0.9.0.tar.gz",
         ],
     },
 }

buf/validate/internal/cel_constraint_rules.cc

@@ -28,7 +28,6 @@ namespace {
 
 absl::Status ProcessConstraint(
     ConstraintContext& ctx,
-    absl::string_view fieldPath,
     const google::api::expr::runtime::BaseActivation& activation,
     const CompiledConstraint& expr) {
   auto result_or = expr.expr->Evaluate(activation, ctx.arena);
@@ -40,17 +39,21 @@ absl::Status ProcessConstraint(
     if (!result.BoolOrDie()) {
       // Add violation with the constraint message.
       Violation& violation = *ctx.violations.add_violations();
-      *violation.mutable_field_path() = fieldPath;
       violation.set_message(expr.constraint.message());
       violation.set_constraint_id(expr.constraint.id());
+      if (expr.rulePath.has_value()) {
+        *violation.mutable_rule() = *expr.rulePath;
+      }
     }
   } else if (result.IsString()) {
     if (!result.StringOrDie().value().empty()) {
       // Add violation with custom message.
       Violation& violation = *ctx.violations.add_violations();
-      *violation.mutable_field_path() = fieldPath;
       violation.set_message(std::string(result.StringOrDie().value()));
       violation.set_constraint_id(expr.constraint.id());
+      if (expr.rulePath.has_value()) {
+        *violation.mutable_rule() = *expr.rulePath;
+      }
     }
   } else if (result.IsError()) {
     const cel::runtime::CelError& error = *result.ErrorOrDie();
@@ -85,6 +88,7 @@ cel::runtime::CelValue ProtoFieldToCelValue(
 absl::Status CelConstraintRules::Add(
     google::api::expr::runtime::CelExpressionBuilder& builder,
     Constraint constraint,
+    absl::optional<FieldPath> rulePath,
     const google::protobuf::FieldDescriptor* rule) {
   auto pexpr_or = cel::parser::Parse(constraint.expression());
   if (!pexpr_or.ok()) {
@@ -96,7 +100,7 @@ absl::Status CelConstraintRules::Add(
     return expr_or.status();
   }
   std::unique_ptr<cel::runtime::CelExpression> expr = std::move(expr_or).value();
-  exprs_.emplace_back(CompiledConstraint{std::move(constraint), std::move(expr), rule});
+  exprs_.emplace_back(CompiledConstraint{std::move(constraint), std::move(expr), std::move(rulePath), rule});
   return absl::OkStatus();
 }
 
@@ -105,17 +109,17 @@ absl::Status CelConstraintRules::Add(
     std::string_view id,
     std::string_view message,
     std::string_view expression,
+    absl::optional<FieldPath> rulePath,
     const google::protobuf::FieldDescriptor* rule) {
   Constraint constraint;
   *constraint.mutable_id() = id;
   *constraint.mutable_message() = message;
   *constraint.mutable_expression() = expression;
-  return Add(builder, constraint, rule);
+  return Add(builder, constraint, std::move(rulePath), rule);
 }
 
 absl::Status CelConstraintRules::ValidateCel(
     ConstraintContext& ctx,
-    std::string_view fieldName,
     google::api::expr::runtime::Activation& activation) const {
   activation.InsertValue("rules", rules_);
   activation.InsertValue("now", cel::runtime::CelValue::CreateTimestamp(absl::Now()));
@@ -126,7 +130,7 @@ absl::Status CelConstraintRules::ValidateCel(
       activation.InsertValue(
           "rule", ProtoFieldToCelValue(rules_.MessageOrDie(), expr.rule, ctx.arena));
     }
-    status = ProcessConstraint(ctx, fieldName, activation, expr);
+    status = ProcessConstraint(ctx, activation, expr);
     if (ctx.shouldReturn(status)) {
       break;
     }

buf/validate/internal/cel_constraint_rules.h

@@ -28,6 +28,7 @@ namespace buf::validate::internal {
 struct CompiledConstraint {
   buf::validate::Constraint constraint;
   std::unique_ptr<google::api::expr::runtime::CelExpression> expr;
+  const absl::optional<FieldPath> rulePath;
   const google::protobuf::FieldDescriptor* rule;
 };
 
@@ -41,25 +42,23 @@ class CelConstraintRules : public ConstraintRules {
   absl::Status Add(
       google::api::expr::runtime::CelExpressionBuilder& builder,
       Constraint constraint,
+      absl::optional<FieldPath> rulePath,
       const google::protobuf::FieldDescriptor* rule);
   absl::Status Add(
       google::api::expr::runtime::CelExpressionBuilder& builder,
       std::string_view id,
       std::string_view message,
       std::string_view expression,
+      absl::optional<FieldPath> rulePath,
       const google::protobuf::FieldDescriptor* rule);
-  [[nodiscard]] const std::vector<CompiledConstraint>& getExprs() const { return exprs_; }
 
   // Validate all the cel rules given the activation that already has 'this' bound.
   absl::Status ValidateCel(
       ConstraintContext& ctx,
-      std::string_view fieldName,
       google::api::expr::runtime::Activation& activation) const;
 
   void setRules(google::api::expr::runtime::CelValue rules) { rules_ = rules; }
   void setRules(const google::protobuf::Message* rules, google::protobuf::Arena* arena);
-  [[nodiscard]] const google::api::expr::runtime::CelValue& getRules() const { return rules_; }
-  [[nodiscard]] google::api::expr::runtime::CelValue& getRules() { return rules_; }
 
  protected:
   google::api::expr::runtime::CelValue rules_;

buf/validate/internal/cel_rules.h

@@ -16,13 +16,47 @@
 
 #include "absl/status/status.h"
 #include "buf/validate/internal/cel_constraint_rules.h"
+#include "buf/validate/internal/constraints.h"
 #include "buf/validate/internal/message_factory.h"
 #include "buf/validate/validate.pb.h"
 #include "google/protobuf/arena.h"
 #include "google/protobuf/descriptor.h"
 
 namespace buf::validate::internal {
 
+template <typename T>
+constexpr int ruleFieldNumber() = delete;
+
+#define MAP_RULES_TO_FIELD_NUMBER(R, fieldNumber) \
+  template <>                                     \
+  constexpr int ruleFieldNumber<R>() {            \
+    return fieldNumber;                           \
+  }
+
+MAP_RULES_TO_FIELD_NUMBER(FloatRules, FieldConstraints::kFloatFieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(DoubleRules, FieldConstraints::kDoubleFieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(Int32Rules, FieldConstraints::kInt32FieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(Int64Rules, FieldConstraints::kInt64FieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(UInt32Rules, FieldConstraints::kUint32FieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(UInt64Rules, FieldConstraints::kUint64FieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(SInt32Rules, FieldConstraints::kSint32FieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(SInt64Rules, FieldConstraints::kSint64FieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(Fixed32Rules, FieldConstraints::kFixed32FieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(Fixed64Rules, FieldConstraints::kFixed64FieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(SFixed32Rules, FieldConstraints::kSfixed32FieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(SFixed64Rules, FieldConstraints::kSfixed64FieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(BoolRules, FieldConstraints::kBoolFieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(StringRules, FieldConstraints::kStringFieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(BytesRules, FieldConstraints::kBytesFieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(EnumRules, FieldConstraints::kEnumFieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(RepeatedRules, FieldConstraints::kRepeatedFieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(MapRules, FieldConstraints::kMapFieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(AnyRules, FieldConstraints::kAnyFieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(DurationRules, FieldConstraints::kDurationFieldNumber)
+MAP_RULES_TO_FIELD_NUMBER(TimestampRules, FieldConstraints::kTimestampFieldNumber)
+
+#undef MAP_RULES_TO_FIELD_NUMBER
+
 template <typename R>
 absl::Status BuildCelRules(
     std::unique_ptr<MessageFactory>& messageFactory,
@@ -60,13 +94,17 @@ absl::Status BuildCelRules(
     R::GetReflection()->ListFields(rules, &fields);
   }
   for (const auto* field : fields) {
+    FieldPath rulePath;
+    *rulePath.mutable_elements()->Add() = fieldPathElement(field);
+    *rulePath.mutable_elements()->Add() =
+        staticFieldPathElement<FieldConstraints, ruleFieldNumber<R>()>();
     if (!field->options().HasExtension(buf::validate::predefined)) {
       continue;
     }
     const auto& fieldLvl = field->options().GetExtension(buf::validate::predefined);
     for (const auto& constraint : fieldLvl.cel()) {
       auto status = result.Add(
-          builder, constraint.id(), constraint.message(), constraint.expression(), field);
+          builder, constraint.id(), constraint.message(), constraint.expression(), rulePath, field);
       if (!status.ok()) {
         return status;
       }

buf/validate/internal/constraint_rules.h

@@ -34,14 +34,18 @@ struct ConstraintContext {
     return !status.ok() || (failFast && violations.violations_size() > 0);
   }
 
-  void prefixFieldPath(std::string_view prefix, int start) {
+  void appendFieldPathElement(const FieldPathElement &element, int start) {
     for (int i = start; i < violations.violations_size(); i++) {
       auto* violation = violations.mutable_violations(i);
-      if (violation->field_path().empty()) {
-        *violation->mutable_field_path() = prefix;
-      } else {
-        violation->set_field_path(absl::StrCat(prefix, ".", violation->field_path()));
-      }
+      *violation->mutable_field()->mutable_elements()->Add() = element;
+    }
+  }
+
+  void appendRulePathElement(std::initializer_list<FieldPathElement> suffix, int start) {
+    for (int i = start; i < violations.violations_size(); i++) {
+      auto* violation = violations.mutable_violations(i);
+      auto* elements = violation->mutable_rule()->mutable_elements();
+      std::copy(suffix.begin(), suffix.end(), RepeatedPtrFieldBackInserter(elements));
     }
   }
 };