Refactor validation errors

internal/cmd/protovalidate-conformance-go/main.go

@@ -21,7 +21,9 @@ import (
 	"os"
 	"strings"
 
+	"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
 	"github.com/bufbuild/protovalidate-go"
+	"github.com/bufbuild/protovalidate-go/internal/errors"
 	"github.com/bufbuild/protovalidate-go/internal/gen/buf/validate/conformance/harness"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protodesc"
@@ -113,7 +115,7 @@ func TestCase(val *protovalidate.Validator, files *protoregistry.Files, testCase
 	case *protovalidate.ValidationError:
 		return &harness.TestResult{
 			Result: &harness.TestResult_ValidationError{
-				ValidationError: res.ToProto(),
+				ValidationError: violationsToProto(res.Violations),
 			},
 		}
 	case *protovalidate.RuntimeError:
@@ -133,6 +135,19 @@ func TestCase(val *protovalidate.Validator, files *protoregistry.Files, testCase
 	}
 }
 
+func violationsToProto(violations []errors.Violation) *validate.Violations {
+	result := make([]*validate.Violation, len(violations))
+	for i := range violations {
+		result[i] = &validate.Violation{
+			FieldPath:    &violations[i].FieldPath,
+			ConstraintId: &violations[i].ConstraintID,
+			Message:      &violations[i].Message,
+			ForKey:       &violations[i].ForKey,
+		}
+	}
+	return &validate.Violations{Violations: result}
+}
+
 func unexpectedErrorResult(format string, args ...any) *harness.TestResult {
 	return &harness.TestResult{
 		Result: &harness.TestResult_UnexpectedError{

internal/constraints/cache.go

@@ -88,6 +88,7 @@ func (c *Cache) Build(
 			err = compileErr
 			return false
 		}
+		precomputedASTs.SetRuleValue(rule)
 		asts = asts.Merge(precomputedASTs)
 		return true
 	})

internal/errors/utils.go

@@ -16,13 +16,12 @@ package errors
 
 import (
 	"errors"
-
-	"google.golang.org/protobuf/proto"
+	"slices"
 )
 
-// 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) {
@@ -50,7 +49,7 @@ func Merge(dst, src error, failFast bool) (ok bool, err error) {
 }
 
 // PrefixErrorPaths prepends the formatted prefix to the violations of a
-// ValidationError.
+// ValidationErrors.
 func PrefixErrorPaths(err error, format string, args ...any) {
 	var valErr *ValidationError
 	if errors.As(err, &valErr) {
@@ -61,8 +60,18 @@ func PrefixErrorPaths(err error, format string, args ...any) {
 func MarkForKey(err error) {
 	var valErr *ValidationError
 	if errors.As(err, &valErr) {
-		for _, violation := range valErr.Violations {
-			violation.ForKey = proto.Bool(true)
+		for i := range valErr.Violations {
+			valErr.Violations[i].ForKey = true
 		}
 	}
 }
+
+// EqualViolations returns true if the underlying violations are equal.
+func EqualViolations(a, b []Violation) bool {
+	return slices.EqualFunc(a, b, EqualViolation)
+}
+
+// EqualViolation returns true if the underlying violations are equal.
+func EqualViolation(a, b Violation) bool {
+	return a.FieldPath == b.FieldPath && a.ConstraintID == b.ConstraintID && a.Message == b.Message && a.ForKey == b.ForKey
+}

internal/errors/utils_test.go

@@ -18,10 +18,8 @@ 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"
 )
 
 func TestMerge(t *testing.T) {
@@ -53,15 +51,15 @@ 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{{ConstraintID: "foo"}}}
 			ok, err := Merge(nil, exErr, true)
 			var valErr *ValidationError
 			require.ErrorAs(t, err, &valErr)
-			assert.True(t, proto.Equal(exErr.ToProto(), valErr.ToProto()))
+			assert.True(t, EqualViolations(exErr.Violations, valErr.Violations))
 			assert.False(t, ok)
 			ok, err = Merge(nil, exErr, false)
 			require.ErrorAs(t, err, &valErr)
-			assert.True(t, proto.Equal(exErr.ToProto(), valErr.ToProto()))
+			assert.True(t, EqualViolations(exErr.Violations, valErr.Violations))
 			assert.True(t, ok)
 		})
 	})
@@ -72,7 +70,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{{ConstraintID: "foo"}}}
 			ok, err := Merge(dstErr, srcErr, true)
 			assert.Equal(t, dstErr, err)
 			assert.False(t, ok)
@@ -83,7 +81,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{{ConstraintID: "foo"}}}
 			srcErr := errors.New("some error")
 			ok, err := Merge(dstErr, srcErr, true)
 			assert.Equal(t, srcErr, err)
@@ -96,21 +94,21 @@ 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{{ConstraintID: "foo"}}}
+			srcErr := &ValidationError{Violations: []Violation{{ConstraintID: ("bar")}}}
+			exErr := &ValidationError{Violations: []Violation{
+				{ConstraintID: "foo"},
+				{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.True(t, EqualViolations(exErr.Violations, valErr.Violations))
 			assert.False(t, ok)
-			dstErr = &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}}
+			dstErr = &ValidationError{Violations: []Violation{{ConstraintID: "foo"}}}
 			ok, err = Merge(dstErr, srcErr, false)
 			require.ErrorAs(t, err, &valErr)
-			assert.True(t, proto.Equal(exErr.ToProto(), valErr.ToProto()))
+			assert.True(t, EqualViolations(exErr.Violations, valErr.Violations))
 			assert.True(t, ok)
 		})
 	})

internal/errors/validation.go

@@ -18,47 +18,78 @@ import (
 	"fmt"
 	"strings"
 
-	"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
-	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/reflect/protoreflect"
 )
 
+// Violation represents a single instance where a validation rule was not met.
+// It provides information about the field that caused the violation, the
+// specific unfulfilled constraint, and a human-readable error message.
+type Violation struct {
+	// FieldPath is a machine-readable identifier that points to the specific
+	// field that failed the validation.  This could be a nested field, in which
+	// case the path will include all the parent fields leading to the actual
+	// field that caused the violation.
+	FieldPath string
+
+	// FieldValue is the value of the specific field that failed validation.
+	FieldValue protoreflect.Value
+
+	// ForKey` indicates whether the violation was caused by a map key, rather
+	// than a value.
+	ForKey bool
+
+	// ConstraintID is the unique identifier of the constraint that was not
+	// fulfilled.
+	ConstraintID string
+
+	// RuleValue is the value of the rule that specified the failed constraint.
+	// Only constraints that have a corresponding rule are set (i.e.: standard
+	// constraints and predefined constraints). In other cases, such as custom
+	// constraints, this will be an invalid value.
+	RuleValue protoreflect.Value
+
+	// Message is a human-readable error message that describes the nature of
+	// the violation. This can be the default error message from the violated
+	// constraint, or it can be a custom message that gives more context about
+	// the violation.
+	Message string
+}
+
 // A ValidationError is returned if one or more constraint violations were
 // detected.
-type ValidationError validate.Violations
+type ValidationError struct {
+	Violations []Violation
+}
 
 func (err *ValidationError) Error() string {
 	bldr := &strings.Builder{}
 	bldr.WriteString("validation error:")
 	for _, violation := range err.Violations {
 		bldr.WriteString("\n - ")
-		if fieldPath := violation.GetFieldPath(); fieldPath != "" {
+		if fieldPath := violation.FieldPath; fieldPath != "" {
 			bldr.WriteString(fieldPath)
 			bldr.WriteString(": ")
 		}
 		_, _ = fmt.Fprintf(bldr, "%s [%s]",
-			violation.GetMessage(),
-			violation.GetConstraintId())
+			violation.Message,
+			violation.ConstraintID)
 	}
 	return bldr.String()
 }
 
-// ToProto converts this error into its proto.Message form.
-func (err *ValidationError) ToProto() *validate.Violations {
-	return (*validate.Violations)(err)
-}
-
 // PrefixFieldPaths prepends to the provided prefix to the error's internal
 // field paths.
 func PrefixFieldPaths(err *ValidationError, format string, args ...any) {
 	prefix := fmt.Sprintf(format, args...)
-	for _, violation := range err.Violations {
+	for i := range err.Violations {
+		violation := &err.Violations[i]
 		switch {
-		case violation.GetFieldPath() == "": // no existing field path
-			violation.FieldPath = proto.String(prefix)
-		case violation.GetFieldPath()[0] == '[': // field is a map/list
-			violation.FieldPath = proto.String(prefix + violation.GetFieldPath())
+		case violation.FieldPath == "": // no existing field path
+			violation.FieldPath = prefix
+		case violation.FieldPath[0] == '[': // field is a map/list
+			violation.FieldPath = prefix + violation.FieldPath
 		default: // any other field
-			violation.FieldPath = proto.String(fmt.Sprintf("%s.%s", prefix, violation.GetFieldPath()))
+			violation.FieldPath = fmt.Sprintf("%s.%s", prefix, violation.FieldPath)
 		}
 	}
 }

internal/errors/validation_test.go

@@ -17,9 +17,7 @@ package errors
 import (
 	"testing"
 
-	"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
 	"github.com/stretchr/testify/assert"
-	"google.golang.org/protobuf/proto"
 )
 
 func TestPrefixFieldPaths(t *testing.T) {
@@ -61,13 +59,13 @@ func TestPrefixFieldPaths(t *testing.T) {
 		test := tc
 		t.Run(test.expected, func(t *testing.T) {
 			t.Parallel()
-			err := &ValidationError{Violations: []*validate.Violation{
-				{FieldPath: proto.String(test.fieldPath)},
-				{FieldPath: proto.String(test.fieldPath)},
+			err := &ValidationError{Violations: []Violation{
+				{FieldPath: test.fieldPath},
+				{FieldPath: test.fieldPath},
 			}}
 			PrefixFieldPaths(err, test.format, test.args...)
 			for _, v := range err.Violations {
-				assert.Equal(t, test.expected, v.GetFieldPath())
+				assert.Equal(t, test.expected, v.FieldPath)
 			}
 		})
 	}

internal/evaluator/any.go

@@ -15,9 +15,7 @@
 package evaluator
 
 import (
-	"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
 	"github.com/bufbuild/protovalidate-go/internal/errors"
-	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protoreflect"
 )
 
@@ -37,12 +35,12 @@ type anyPB struct {
 func (a anyPB) Evaluate(val protoreflect.Value, failFast bool) error {
 	typeURL := val.Message().Get(a.TypeURLDescriptor).String()
 
-	err := &errors.ValidationError{Violations: []*validate.Violation{}}
+	err := &errors.ValidationError{}
 	if len(a.In) > 0 {
 		if _, ok := a.In[typeURL]; !ok {
-			err.Violations = append(err.Violations, &validate.Violation{
-				ConstraintId: proto.String("any.in"),
-				Message:      proto.String("type URL must be in the allow list"),
+			err.Violations = append(err.Violations, errors.Violation{
+				ConstraintID: "any.in",
+				Message:      "type URL must be in the allow list",
 			})
 			if failFast {
 				return err
@@ -52,9 +50,9 @@ func (a anyPB) Evaluate(val protoreflect.Value, failFast bool) error {
 
 	if len(a.NotIn) > 0 {
 		if _, ok := a.NotIn[typeURL]; ok {
-			err.Violations = append(err.Violations, &validate.Violation{
-				ConstraintId: proto.String("any.not_in"),
-				Message:      proto.String("type URL must not be in the block list"),
+			err.Violations = append(err.Violations, errors.Violation{
+				ConstraintID: "any.not_in",
+				Message:      "type URL must not be in the block list",
 			})
 		}
 	}

internal/evaluator/cel.go

@@ -23,11 +23,11 @@ import (
 type celPrograms expression.ProgramSet
 
 func (c celPrograms) Evaluate(val protoreflect.Value, failFast bool) error {
-	return expression.ProgramSet(c).Eval(val.Interface(), failFast)
+	return expression.ProgramSet(c).Eval(val, failFast)
 }
 
 func (c celPrograms) EvaluateMessage(msg protoreflect.Message, failFast bool) error {
-	return expression.ProgramSet(c).Eval(msg.Interface(), failFast)
+	return expression.ProgramSet(c).Eval(protoreflect.ValueOfMessage(msg), failFast)
 }
 
 func (c celPrograms) Tautology() bool {

internal/evaluator/enum.go

@@ -15,9 +15,7 @@
 package evaluator
 
 import (
-	"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
 	"github.com/bufbuild/protovalidate-go/internal/errors"
-	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protoreflect"
 )
 
@@ -31,9 +29,9 @@ type definedEnum struct {
 
 func (d definedEnum) Evaluate(val protoreflect.Value, _ bool) error {
 	if d.ValueDescriptors.ByNumber(val.Enum()) == nil {
-		return &errors.ValidationError{Violations: []*validate.Violation{{
-			ConstraintId: proto.String("enum.defined_only"),
-			Message:      proto.String("value must be one of the defined enum values"),
+		return &errors.ValidationError{Violations: []errors.Violation{{
+			ConstraintID: "enum.defined_only",
+			Message:      "value must be one of the defined enum values",
 		}}}
 	}
 	return nil

internal/evaluator/field.go

@@ -15,9 +15,7 @@
 package evaluator
 
 import (
-	"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
 	"github.com/bufbuild/protovalidate-go/internal/errors"
-	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protoreflect"
 )
 
@@ -47,10 +45,10 @@ func (f field) Evaluate(val protoreflect.Value, failFast bool) error {
 
 func (f field) EvaluateMessage(msg protoreflect.Message, failFast bool) (err error) {
 	if f.Required && !msg.Has(f.Descriptor) {
-		return &errors.ValidationError{Violations: []*validate.Violation{{
-			FieldPath:    proto.String(string(f.Descriptor.Name())),
-			ConstraintId: proto.String("required"),
-			Message:      proto.String("value is required"),
+		return &errors.ValidationError{Violations: []errors.Violation{{
+			FieldPath:    string(f.Descriptor.Name()),
+			ConstraintID: "required",
+			Message:      "value is required",
 		}}}
 	}