Implement structured field and rule paths, add field and rule values to ValidationErrors

Makefile

@@ -16,7 +16,7 @@ GOLANGCI_LINT_VERSION ?= v1.60.1
 # Set to use a different version of protovalidate-conformance.
 # Should be kept in sync with the version referenced in proto/buf.lock and
 # 'buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go' in go.mod.
-CONFORMANCE_VERSION ?= v0.8.2
+CONFORMANCE_VERSION ?= v0.9.0
 
 .PHONY: help
 help: ## Describe useful make targets

go.mod

@@ -3,7 +3,7 @@ module github.com/bufbuild/protovalidate-go
 go 1.21.1
 
 require (
-	buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.35.2-20240920164238-5a7b106cbb87.1
+	buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.35.2-20241127180247-a33202765966.1
 	github.com/envoyproxy/protoc-gen-validate v1.1.0
 	github.com/google/cel-go v0.22.1
 	github.com/stretchr/testify v1.10.0

go.sum

@@ -1,5 +1,5 @@
-buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.35.2-20240920164238-5a7b106cbb87.1 h1:7QIeAuTdLp173vC/9JojRMDFcpmqtoYrxPmvdHAOynw=
-buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.35.2-20240920164238-5a7b106cbb87.1/go.mod h1:mnHCFccv4HwuIAOHNGdiIc5ZYbBCvbTWZcodLN5wITI=
+buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.35.2-20241127180247-a33202765966.1 h1:jLd96rDDNJ+zIJxvV/L855VEtrjR0G4aePVDlCpf6kw=
+buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.35.2-20241127180247-a33202765966.1/go.mod h1:mnHCFccv4HwuIAOHNGdiIc5ZYbBCvbTWZcodLN5wITI=
 cel.dev/expr v0.18.0 h1:CJ6drgk+Hf96lkLikr4rFf19WrU0BOWEihyZnI2TAzo=
 cel.dev/expr v0.18.0/go.mod h1:MrpN08Q+lEBs+bGYdLxxHkZoUSsCp0nSKTs0nTymJgw=
 github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI=

internal/constraints/cache.go

@@ -49,7 +49,7 @@ func (c *Cache) Build(
 	allowUnknownFields bool,
 	forItems bool,
 ) (set expression.ProgramSet, err error) {
-	constraints, done, err := c.resolveConstraints(
+	constraints, setOneof, done, err := c.resolveConstraints(
 		fieldDesc,
 		fieldConstraints,
 		forItems,
@@ -83,11 +83,12 @@ func (c *Cache) Build(
 			err = compileErr
 			return false
 		}
-		precomputedASTs, compileErr := c.loadOrCompileStandardConstraint(fieldEnv, desc)
+		precomputedASTs, compileErr := c.loadOrCompileStandardConstraint(fieldEnv, setOneof, desc)
 		if compileErr != nil {
 			err = compileErr
 			return false
 		}
+		precomputedASTs.SetRuleValue(rule, desc)
 		asts = asts.Merge(precomputedASTs)
 		return true
 	})
@@ -108,26 +109,26 @@ func (c *Cache) resolveConstraints(
 	fieldDesc protoreflect.FieldDescriptor,
 	fieldConstraints *validate.FieldConstraints,
 	forItems bool,
-) (rules protoreflect.Message, done bool, err error) {
+) (rules protoreflect.Message, fieldRule protoreflect.FieldDescriptor, done bool, err error) {
 	constraints := fieldConstraints.ProtoReflect()
 	setOneof := constraints.WhichOneof(fieldConstraintsOneofDesc)
 	if setOneof == nil {
-		return nil, true, nil
+		return nil, nil, true, nil
 	}
 	expected, ok := c.getExpectedConstraintDescriptor(fieldDesc, forItems)
 	if ok && setOneof.FullName() != expected.FullName() {
-		return nil, true, errors.NewCompilationErrorf(
+		return nil, nil, true, errors.NewCompilationErrorf(
 			"expected constraint %q, got %q on field %q",
 			expected.FullName(),
 			setOneof.FullName(),
 			fieldDesc.FullName(),
 		)
 	}
 	if !ok || !constraints.Has(setOneof) {
-		return nil, true, nil
+		return nil, nil, true, nil
 	}
 	rules = constraints.Get(setOneof).Message()
-	return rules, false, nil
+	return rules, setOneof, false, nil
 }
 
 // prepareEnvironment prepares the environment for compiling standard constraint
@@ -157,16 +158,23 @@ func (c *Cache) prepareEnvironment(
 // CEL expressions.
 func (c *Cache) loadOrCompileStandardConstraint(
 	env *cel.Env,
+	setOneOf protoreflect.FieldDescriptor,
 	constraintFieldDesc protoreflect.FieldDescriptor,
 ) (set expression.ASTSet, err error) {
 	if cachedConstraint, ok := c.cache[constraintFieldDesc]; ok {
 		return cachedConstraint, nil
 	}
-	exprs := extensions.Resolve[*validate.PredefinedConstraints](
-		constraintFieldDesc.Options(),
-		validate.E_Predefined,
-	)
-	set, err = expression.CompileASTs(exprs.GetCel(), env)
+	exprs := expression.Expressions{
+		Constraints: extensions.Resolve[*validate.PredefinedConstraints](
+			constraintFieldDesc.Options(),
+			validate.E_Predefined,
+		).GetCel(),
+		RulePath: []*validate.FieldPathElement{
+			errors.FieldPathElement(setOneOf),
+			errors.FieldPathElement(constraintFieldDesc),
+		},
+	}
+	set, err = expression.CompileASTs(exprs, env)
 	if err != nil {
 		return set, errors.NewCompilationErrorf(
 			"failed to compile standard constraint %q: %w",

internal/constraints/cache_test.go

@@ -118,6 +118,8 @@ func TestCache_LoadOrCompileStandardConstraint(t *testing.T) {
 	env, err := celext.DefaultEnv(false)
 	require.NoError(t, err)
 
+	constraints := &validate.FieldConstraints{}
+	oneOfDesc := constraints.ProtoReflect().Descriptor().Oneofs().ByName("type").Fields().ByName("float")
 	msg := &cases.FloatIn{}
 	desc := getFieldDesc(t, msg, "val")
 	require.NotNil(t, desc)
@@ -126,15 +128,15 @@ func TestCache_LoadOrCompileStandardConstraint(t *testing.T) {
 	_, ok := cache.cache[desc]
 	assert.False(t, ok)
 
-	asts, err := cache.loadOrCompileStandardConstraint(env, desc)
+	asts, err := cache.loadOrCompileStandardConstraint(env, oneOfDesc, desc)
 	require.NoError(t, err)
 	assert.NotNil(t, asts)
 
 	cached, ok := cache.cache[desc]
 	assert.True(t, ok)
 	assert.Equal(t, cached, asts)
 
-	asts, err = cache.loadOrCompileStandardConstraint(env, desc)
+	asts, err = cache.loadOrCompileStandardConstraint(env, oneOfDesc, desc)
 	require.NoError(t, err)
 	assert.Equal(t, cached, asts)
 }

internal/errors/utils.go

@@ -16,8 +16,14 @@ package errors
 
 import (
 	"errors"
+	"slices"
+	"strconv"
+	"strings"
 
+	"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
 	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/reflect/protoreflect"
+	"google.golang.org/protobuf/types/descriptorpb"
 )
 
 // Merge is a utility to resolve and combine errors resulting from
@@ -49,20 +55,110 @@ func Merge(dst, src error, failFast bool) (ok bool, err error) {
 	return !(failFast && len(dstValErrs.Violations) > 0), dst
 }
 
-// PrefixErrorPaths prepends the formatted prefix to the violations of a
-// ValidationError.
-func PrefixErrorPaths(err error, format string, args ...any) {
+func FieldPathElement(field protoreflect.FieldDescriptor) *validate.FieldPathElement {
+	if field == nil {
+		return nil
+	}
+	return &validate.FieldPathElement{
+		FieldNumber: proto.Int32(int32(field.Number())),
+		FieldName:   proto.String(field.TextName()),
+		FieldType:   descriptorpb.FieldDescriptorProto_Type(field.Kind()).Enum(),
+	}
+}
+
+func FieldPath(field protoreflect.FieldDescriptor) *validate.FieldPath {
+	if field == nil {
+		return nil
+	}
+	return &validate.FieldPath{
+		Elements: []*validate.FieldPathElement{
+			FieldPathElement(field),
+		},
+	}
+}
+
+// UpdatePaths modifies the field and rule paths of an error, appending an
+// element to the end of each field path (if provided) and prepending a list of
+// elements to the beginning of each rule path (if provided.)
+//
+// Note that this function is ordinarily used to append field paths in reverse
+// order, as the stack bubbles up through the evaluators. Then, at the end, the
+// path is reversed. Rule paths are generally static, so this optimization isn't
+// applied for rule paths.
+func UpdatePaths(err error, fieldSuffix *validate.FieldPathElement, rulePrefix []*validate.FieldPathElement) {
+	if fieldSuffix == nil && len(rulePrefix) == 0 {
+		return
+	}
 	var valErr *ValidationError
 	if errors.As(err, &valErr) {
-		PrefixFieldPaths(valErr, format, args...)
+		for _, violation := range valErr.Violations {
+			if fieldSuffix != nil {
+				if violation.Proto.GetField() == nil {
+					violation.Proto.Field = &validate.FieldPath{}
+				}
+				violation.Proto.Field.Elements = append(violation.Proto.Field.Elements, fieldSuffix)
+			}
+			if len(rulePrefix) != 0 {
+				violation.Proto.Rule.Elements = append(
+					append([]*validate.FieldPathElement{}, rulePrefix...),
+					violation.Proto.GetRule().GetElements()...,
+				)
+			}
+		}
+	}
+}
+
+// FinalizePaths reverses all field paths in the error and populates the
+// deprecated string-based field path.
+func FinalizePaths(err error) {
+	var valErr *ValidationError
+	if errors.As(err, &valErr) {
+		for _, violation := range valErr.Violations {
+			if violation.Proto.GetField() != nil {
+				slices.Reverse(violation.Proto.GetField().GetElements())
+				//nolint:staticcheck // Intentional use of deprecated field
+				violation.Proto.FieldPath = proto.String(FieldPathString(violation.Proto.GetField().GetElements()))
+			}
+		}
+	}
+}
+
+// FieldPathString takes a FieldPath and encodes it to a string-based dotted
+// field path.
+func FieldPathString(path []*validate.FieldPathElement) string {
+	var result strings.Builder
+	for i, element := range path {
+		if i > 0 {
+			result.WriteByte('.')
+		}
+		result.WriteString(element.GetFieldName())
+		subscript := element.GetSubscript()
+		if subscript == nil {
+			continue
+		}
+		result.WriteByte('[')
+		switch value := subscript.(type) {
+		case *validate.FieldPathElement_Index:
+			result.WriteString(strconv.FormatUint(value.Index, 10))
+		case *validate.FieldPathElement_BoolKey:
+			result.WriteString(strconv.FormatBool(value.BoolKey))
+		case *validate.FieldPathElement_IntKey:
+			result.WriteString(strconv.FormatInt(value.IntKey, 10))
+		case *validate.FieldPathElement_UintKey:
+			result.WriteString(strconv.FormatUint(value.UintKey, 10))
+		case *validate.FieldPathElement_StringKey:
+			result.WriteString(strconv.Quote(value.StringKey))
+		}
+		result.WriteByte(']')
 	}
+	return result.String()
 }
 
 func MarkForKey(err error) {
 	var valErr *ValidationError
 	if errors.As(err, &valErr) {
 		for _, violation := range valErr.Violations {
-			violation.ForKey = proto.Bool(true)
+			violation.Proto.ForKey = proto.Bool(true)
 		}
 	}
 }

internal/errors/utils_test.go

@@ -53,7 +53,7 @@ func TestMerge(t *testing.T) {
 
 		t.Run("validation", func(t *testing.T) {
 			t.Parallel()
-			exErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}}
+			exErr := &ValidationError{Violations: []*Violation{{Proto: &validate.Violation{ConstraintId: proto.String("foo")}}}}
 			ok, err := Merge(nil, exErr, true)
 			var valErr *ValidationError
 			require.ErrorAs(t, err, &valErr)
@@ -72,7 +72,7 @@ func TestMerge(t *testing.T) {
 		t.Run("non-validation dst", func(t *testing.T) {
 			t.Parallel()
 			dstErr := errors.New("some error")
-			srcErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}}
+			srcErr := &ValidationError{Violations: []*Violation{{Proto: &validate.Violation{ConstraintId: proto.String("foo")}}}}
 			ok, err := Merge(dstErr, srcErr, true)
 			assert.Equal(t, dstErr, err)
 			assert.False(t, ok)
@@ -83,7 +83,7 @@ func TestMerge(t *testing.T) {
 
 		t.Run("non-validation src", func(t *testing.T) {
 			t.Parallel()
-			dstErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}}
+			dstErr := &ValidationError{Violations: []*Violation{{Proto: &validate.Violation{ConstraintId: proto.String("foo")}}}}
 			srcErr := errors.New("some error")
 			ok, err := Merge(dstErr, srcErr, true)
 			assert.Equal(t, srcErr, err)
@@ -96,18 +96,18 @@ func TestMerge(t *testing.T) {
 		t.Run("validation", func(t *testing.T) {
 			t.Parallel()
 
-			dstErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}}
-			srcErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("bar")}}}
-			exErr := &ValidationError{Violations: []*validate.Violation{
-				{ConstraintId: proto.String("foo")},
-				{ConstraintId: proto.String("bar")},
+			dstErr := &ValidationError{Violations: []*Violation{{Proto: &validate.Violation{ConstraintId: proto.String("foo")}}}}
+			srcErr := &ValidationError{Violations: []*Violation{{Proto: &validate.Violation{ConstraintId: proto.String("bar")}}}}
+			exErr := &ValidationError{Violations: []*Violation{
+				{Proto: &validate.Violation{ConstraintId: proto.String("foo")}},
+				{Proto: &validate.Violation{ConstraintId: proto.String("bar")}},
 			}}
 			ok, err := Merge(dstErr, srcErr, true)
 			var valErr *ValidationError
 			require.ErrorAs(t, err, &valErr)
 			assert.True(t, proto.Equal(exErr.ToProto(), valErr.ToProto()))
 			assert.False(t, ok)
-			dstErr = &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}}
+			dstErr = &ValidationError{Violations: []*Violation{{Proto: &validate.Violation{ConstraintId: proto.String("foo")}}}}
 			ok, err = Merge(dstErr, srcErr, false)
 			require.ErrorAs(t, err, &valErr)
 			assert.True(t, proto.Equal(exErr.ToProto(), valErr.ToProto()))