Implement structured field and rule paths, add field and rule values …

…to ValidationErrors (#154)

This implements the structured field and rule paths, passing the
conformance test changes in
bufbuild/protovalidate#265.

In addition, it also adds the ability to get the field and rule values.
These are not present in the protobuf form of the violation.

The API is still heavily centered around the protobuf form of
violations, but the `ValidationError` type is no longer an alias to the
protobuf `buf.validate.Violations` message. The intention is that users
needing access to the error information will still use the proto
violations message, only using the "native" interface methods to access
in-memory data that is not feasible to transport over the wire. (e.g.
`protoreflect.Value` pointers.)

DO NOT MERGE until the following is done:
- [x] bufbuild/protovalidate#265 is merged
- [x] Dependencies updated to stable version release

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,117 @@ 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) {
+// FieldPathElement returns a buf.validate.FieldPathElement that corresponds to
+// a provided FieldDescriptor. If the provided FieldDescriptor is nil, nil is
+// returned.
+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(),
+	}
+}
+
+// FieldPath returns a single-element buf.validate.FieldPath corresponding to
+// the provided FieldDescriptor, or nil if the provided FieldDescriptor is nil.
+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()
 }
 
+// MarkForKey marks the provided error as being for a map key, by setting the
+// `for_key` flag on each violation within the validation error.
 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()))