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

go.mod

@@ -25,3 +25,6 @@ require (
 	gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
+
+// DO NOT MERGE!
+replace buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go => buf.build/gen/go/jchadwick-buf/protovalidate/protocolbuffers/go v1.35.1-20241107000346-6ecee89ee0c9.1

go.sum

@@ -1,5 +1,5 @@
-buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.35.1-20240920164238-5a7b106cbb87.1 h1:9wP6ZZYWnF2Z0TxmII7m3XNykxnP4/w8oXeth6ekcRI=
-buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.35.1-20240920164238-5a7b106cbb87.1/go.mod h1:Duw/9JoXkXIydyASnLYIiufkzySThoqavOsF+IihqvM=
+buf.build/gen/go/jchadwick-buf/protovalidate/protocolbuffers/go v1.35.1-20241107000346-6ecee89ee0c9.1 h1:AG/SqE0AfoVJpTJeiOIZ1HnfpR+nYWdbW/vDB3qAKlk=
+buf.build/gen/go/jchadwick-buf/protovalidate/protocolbuffers/go v1.35.1-20241107000346-6ecee89ee0c9.1/go.mod h1:Cy8tc4XPqN8l0D+F/Jf4Hy9X9a3somHIH9BTkmJJTmo=
 github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI=
 github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=

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)
 		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,13 +16,19 @@ 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
+// Merge is a utility to resolve and combine Errors resulting from
 // evaluation. If ok is false, execution of validation should stop (either due
-// to failFast or the result is not a ValidationError).
+// to failFast or the result is not a ValidationErrors).
 //
 //nolint:errorlint
 func Merge(dst, src error, failFast bool) (ok bool, err error) {
@@ -49,20 +55,112 @@ 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 {
+	return &validate.FieldPathElement{
+		FieldNumber: proto.Int32(int32(field.Number())),
+		FieldName:   proto.String(field.TextName()),
+		FieldType:   descriptorpb.FieldDescriptorProto_Type(field.Kind()).Enum(),
+	}
+}
+
+// AppendFieldPath appends an element to the end of each field path in err.
+// As an exception, if skipSubscript is true, any field paths ending in a
+// subscript element will not have a suffix element appended to them.
+//
+// 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.
+func AppendFieldPath(err error, suffix *validate.FieldPathElement, skipSubscript bool) {
+	var valErr *ValidationError
+	if errors.As(err, &valErr) {
+		for _, violation := range valErr.Violations {
+			if violation, ok := violation.(*ViolationData); ok {
+				// Special case: Here we skip appending if the last element had a
+				// subscript. This is a weird special case that makes it
+				// significantly simpler to handle reverse-constructing paths with
+				// maps and slices.
+				if skipSubscript &&
+					len(violation.Field) > 0 &&
+					violation.Field[len(violation.Field)-1].Subscript != nil {
+					continue
+				}
+				violation.Field = append(violation.Field, suffix)
+			}
+		}
+	}
+}
+
+// PrependRulePath prepends some elements to the beginning of each rule path in
+// err. Note that unlike field paths, the rule path is not appended in reverse
+// order. This is because rule paths are very often fixed, simple paths, so it
+// is better to avoid the copy instead if possible. This prepend is only used in
+// the error case for nested rules (repeated.items, map.keys, map.values.)
+func PrependRulePath(err error, prefix []*validate.FieldPathElement) {
+	var valErr *ValidationError
+	if errors.As(err, &valErr) {
+		for _, violation := range valErr.Violations {
+			if violation, ok := violation.(*ViolationData); ok {
+				violation.Rule = append(
+					append([]*validate.FieldPathElement{}, prefix...),
+					violation.Rule...,
+				)
+			}
+		}
+	}
+}
+
+// ReverseFieldPaths reverses all field paths in the error.
+func ReverseFieldPaths(err error) {
 	var valErr *ValidationError
 	if errors.As(err, &valErr) {
-		PrefixFieldPaths(valErr, format, args...)
+		for _, violation := range valErr.Violations {
+			if violation, ok := violation.(*ViolationData); ok {
+				slices.Reverse(violation.Field)
+			}
+		}
+	}
+}
+
+// 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_SintKey:
+			result.WriteString(strconv.FormatInt(value.SintKey, 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)
+			if violation, ok := violation.(*ViolationData); ok {
+				violation.ForKey = true
+			}
 		}
 	}
 }

internal/errors/utils_test.go

@@ -18,7 +18,6 @@ import (
 	"errors"
 	"testing"
 
-	"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/proto"
@@ -53,7 +52,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{&ViolationData{ConstraintID: "foo"}}}
 			ok, err := Merge(nil, exErr, true)
 			var valErr *ValidationError
 			require.ErrorAs(t, err, &valErr)
@@ -72,7 +71,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{&ViolationData{ConstraintID: "foo"}}}
 			ok, err := Merge(dstErr, srcErr, true)
 			assert.Equal(t, dstErr, err)
 			assert.False(t, ok)
@@ -83,7 +82,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{&ViolationData{ConstraintID: "foo"}}}
 			srcErr := errors.New("some error")
 			ok, err := Merge(dstErr, srcErr, true)
 			assert.Equal(t, srcErr, err)
@@ -96,18 +95,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{&ViolationData{ConstraintID: "foo"}}}
+			srcErr := &ValidationError{Violations: []Violation{&ViolationData{ConstraintID: ("bar")}}}
+			exErr := &ValidationError{Violations: []Violation{
+				&ViolationData{ConstraintID: "foo"},
+				&ViolationData{ConstraintID: "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{&ViolationData{ConstraintID: "foo"}}}
 			ok, err = Merge(dstErr, srcErr, false)
 			require.ErrorAs(t, err, &valErr)
 			assert.True(t, proto.Equal(exErr.ToProto(), valErr.ToProto()))