Update protovalidate-go to work with 0.6.1 protos (#101)

Update protovalidate-go to work with the latest protovalidate proto
rules, including the Ignore enum. Update to the latest golangci-lint and
fix lint warnings.

---------

Co-authored-by: Chris Roche <github@rodaine.com>

.golangci.yml

@@ -57,6 +57,7 @@ issues:
   exclude:
     # we will continue to support the deprecated field until it's removed
     - "IgnoreEmpty is deprecated"
+    - "Skipped is deprecated"
   exclude-rules:
     # Loosen requirements on conformance executor
     - path: internal/cmd/
@@ -84,4 +85,4 @@ issues:
     - path: resolver/resolver.go
       linters:
         # uses deprecated fields on protoimpl.ExtensionInfo but its the only way
-        - staticcheck
+        - staticcheck

Makefile

@@ -15,7 +15,7 @@ ARGS ?= --strict --strict_message --strict_error
 # 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.5.6
+CONFORMANCE_VERSION ?= v0.6.1
 
 .PHONY: help
 help: ## Describe useful make targets
@@ -94,7 +94,7 @@ $(BIN)/license-header: $(BIN) Makefile
 
 $(BIN)/golangci-lint: $(BIN) Makefile
 	GOBIN=$(abspath $(@D)) $(GO) install \
-		github.com/golangci/golangci-lint/cmd/golangci-lint@v1.52.2
+		github.com/golangci/golangci-lint/cmd/golangci-lint@v1.56.1
 
 $(BIN)/protovalidate-conformance: $(BIN) Makefile
 	GOBIN=$(abspath $(BIN)) $(GO) install \

go.mod

@@ -3,7 +3,7 @@ module github.com/bufbuild/protovalidate-go
 go 1.19
 
 require (
-	buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.32.0-20240212200630-3014d81c3a48.1
+	buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.32.0-20240221180331-f05a6f4403ce.1
 	github.com/envoyproxy/protoc-gen-validate v1.0.4
 	github.com/google/cel-go v0.20.0
 	github.com/stretchr/testify v1.8.4

go.sum

@@ -1,5 +1,5 @@
-buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.32.0-20240212200630-3014d81c3a48.1 h1:rOe/itdO7+9cWOnKqvpn1K7VmTDwp3vI4juPz6aNQbc=
-buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.32.0-20240212200630-3014d81c3a48.1/go.mod h1:tiTMKD8j6Pd/D2WzREoweufjzaJKHZg35f/VGcZ2v3I=
+buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.32.0-20240221180331-f05a6f4403ce.1 h1:AmmAwHbvaeOIxDKG2+aTn5C36HjmFIMkrdTp49rp80Q=
+buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.32.0-20240221180331-f05a6f4403ce.1/go.mod h1:tiTMKD8j6Pd/D2WzREoweufjzaJKHZg35f/VGcZ2v3I=
 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

@@ -57,7 +57,7 @@ func (c *Cache) Build(
 	}
 
 	var asts expression.ASTSet
-	constraints.Range(func(desc protoreflect.FieldDescriptor, val protoreflect.Value) bool {
+	constraints.Range(func(desc protoreflect.FieldDescriptor, _ protoreflect.Value) bool {
 		precomputedASTs, compileErr := c.loadOrCompileStandardConstraint(env, desc)
 		if compileErr != nil {
 			err = compileErr

internal/constraints/cache_test.go

@@ -104,7 +104,7 @@ func TestCache_BuildStandardConstraints(t *testing.T) {
 			if test.exErr {
 				assert.Error(t, err)
 			} else {
-				assert.NoError(t, err)
+				require.NoError(t, err)
 				assert.Len(t, set, test.exCt)
 			}
 		})
@@ -126,15 +126,15 @@ func TestCache_LoadOrCompileStandardConstraint(t *testing.T) {
 	assert.False(t, ok)
 
 	asts, err := cache.loadOrCompileStandardConstraint(env, desc)
-	assert.NoError(t, err)
+	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)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, cached, asts)
 }
 

internal/errors/utils_test.go

@@ -30,10 +30,10 @@ func TestMerge(t *testing.T) {
 	t.Run("no errors", func(t *testing.T) {
 		t.Parallel()
 		ok, err := Merge(nil, nil, true)
-		assert.NoError(t, err)
+		require.NoError(t, err)
 		assert.True(t, ok)
 		ok, err = Merge(nil, nil, false)
-		assert.NoError(t, err)
+		require.NoError(t, err)
 		assert.True(t, ok)
 	})
 

internal/errors/validation.go

@@ -30,13 +30,13 @@ func (err *ValidationError) Error() string {
 	bldr.WriteString("validation error:")
 	for _, violation := range err.Violations {
 		bldr.WriteString("\n - ")
-		if violation.FieldPath != "" {
-			bldr.WriteString(violation.FieldPath)
+		if fieldPath := violation.GetFieldPath(); fieldPath != "" {
+			bldr.WriteString(fieldPath)
 			bldr.WriteString(": ")
 		}
 		_, _ = fmt.Fprintf(bldr, "%s [%s]",
-			violation.Message,
-			violation.ConstraintId)
+			violation.GetMessage(),
+			violation.GetConstraintId())
 	}
 	return bldr.String()
 }
@@ -52,12 +52,12 @@ func PrefixFieldPaths(err *ValidationError, format string, args ...any) {
 	prefix := fmt.Sprintf(format, args...)
 	for _, violation := range err.Violations {
 		switch {
-		case violation.FieldPath == "": // no existing field path
+		case violation.GetFieldPath() == "": // no existing field path
 			violation.FieldPath = prefix
-		case violation.FieldPath[0] == '[': // field is a map/list
-			violation.FieldPath = prefix + violation.FieldPath
+		case violation.GetFieldPath()[0] == '[': // field is a map/list
+			violation.FieldPath = prefix + violation.GetFieldPath()
 		default: // any other field
-			violation.FieldPath = fmt.Sprintf("%s.%s", prefix, violation.FieldPath)
+			violation.FieldPath = fmt.Sprintf("%s.%s", prefix, violation.GetFieldPath())
 		}
 	}
 }

internal/errors/validation_test.go

@@ -66,7 +66,7 @@ func TestPrefixFieldPaths(t *testing.T) {
 			}}
 			PrefixFieldPaths(err, test.format, test.args...)
 			for _, v := range err.Violations {
-				assert.Equal(t, test.expected, v.FieldPath)
+				assert.Equal(t, test.expected, v.GetFieldPath())
 			}
 		})
 	}

internal/evaluator/builder.go

@@ -204,9 +204,21 @@ func (bldr *Builder) buildField(
 	cache MessageCache,
 ) (field, error) {
 	fld := field{
-		Descriptor:  fieldDescriptor,
-		Required:    fieldConstraints.GetRequired(),
-		IgnoreEmpty: fieldDescriptor.HasPresence() || fieldConstraints.GetIgnoreEmpty(),
+		Descriptor: fieldDescriptor,
+		Required:   fieldConstraints.GetRequired(),
+		IgnoreEmpty: fieldDescriptor.HasPresence() ||
+			fieldConstraints.GetIgnoreEmpty() ||
+			fieldConstraints.GetIgnore() == validate.Ignore_IGNORE_IF_UNPOPULATED ||
+			fieldConstraints.GetIgnore() == validate.Ignore_IGNORE_IF_DEFAULT_VALUE,
+		IgnoreDefault: fieldDescriptor.HasPresence() && fieldConstraints.GetIgnore() == validate.Ignore_IGNORE_IF_DEFAULT_VALUE,
+	}
+	if fld.IgnoreDefault {
+		if fieldDescriptor.Kind() == protoreflect.MessageKind && fieldDescriptor.Cardinality() != protoreflect.Repeated {
+			msg := dynamicpb.NewMessage(fieldDescriptor.Message())
+			fld.Zero = protoreflect.ValueOfMessage(msg)
+		} else {
+			fld.Zero = fieldDescriptor.Default()
+		}
 	}
 	err := bldr.buildValue(fieldDescriptor, fieldConstraints, false, &fld.Value, cache)
 	return fld, err
@@ -254,15 +266,21 @@ func (bldr *Builder) processIgnoreEmpty(
 ) error {
 	// the only time we need to ignore empty on a value is if it's evaluating a
 	// field item (repeated element or map key/value).
-	val.IgnoreEmpty = forItems && constraints.GetIgnoreEmpty()
-	if !val.IgnoreEmpty {
+	val.IgnoreEmpty = forItems && (constraints.GetIgnoreEmpty() ||
+		constraints.GetIgnore() == validate.Ignore_IGNORE_IF_UNPOPULATED ||
+		constraints.GetIgnore() == validate.Ignore_IGNORE_IF_DEFAULT_VALUE)
+	switch {
+	case !val.IgnoreEmpty:
 		// only need the zero value for checking ignore_empty constraint
 		return nil
-	}
-	val.Zero = fdesc.Default()
-	if forItems && fdesc.IsList() {
+	case fdesc.IsList():
 		msg := dynamicpb.NewMessage(fdesc.ContainingMessage())
 		val.Zero = msg.Get(fdesc).List().NewElement()
+	case fdesc.Kind() == protoreflect.MessageKind:
+		msg := dynamicpb.NewMessage(fdesc.Message())
+		val.Zero = protoreflect.ValueOfMessage(msg)
+	default:
+		val.Zero = fdesc.Default()
 	}
 	return nil
 }
@@ -303,6 +321,7 @@ func (bldr *Builder) processEmbeddedMessage(
 ) error {
 	if fdesc.Kind() != protoreflect.MessageKind ||
 		rules.GetSkipped() ||
+		rules.GetIgnore() == validate.Ignore_IGNORE_ALWAYS ||
 		fdesc.IsMap() || (fdesc.IsList() && !forItems) {
 		return nil
 	}
@@ -327,6 +346,7 @@ func (bldr *Builder) processWrapperConstraints(
 ) error {
 	if fdesc.Kind() != protoreflect.MessageKind ||
 		rules.GetSkipped() ||
+		rules.GetIgnore() == validate.Ignore_IGNORE_ALWAYS ||
 		fdesc.IsMap() || (fdesc.IsList() && !forItems) {
 		return nil
 	}

internal/evaluator/field.go

@@ -33,6 +33,11 @@ type field struct {
 	// This field is generally true for nullable fields or fields with the
 	// ignore_empty constraint explicitly set.
 	IgnoreEmpty bool
+	// IgnoreDefault indicates if a field should skip validation on its zero value,
+	// including for fields which have field presence and are set to the zero value.
+	IgnoreDefault bool
+	// Zero is the default or zero-value for this value's type
+	Zero protoreflect.Value
 }
 
 func (f field) Evaluate(val protoreflect.Value, failFast bool) error {
@@ -53,6 +58,9 @@ func (f field) EvaluateMessage(msg protoreflect.Message, failFast bool) (err err
 	}
 
 	val := msg.Get(f.Descriptor)
+	if f.IgnoreDefault && val.Equal(f.Zero) {
+		return nil
+	}
 	if err = f.Value.Evaluate(val, failFast); err != nil {
 		errors.PrefixErrorPaths(err, "%s", f.Descriptor.Name())
 	}

internal/expression/ast_test.go

@@ -71,7 +71,7 @@ func TestASTSet_ToProgramSet(t *testing.T) {
 	empty := ASTSet{}
 	set, err = empty.ToProgramSet()
 	assert.Empty(t, set)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 }
 
 func TestASTSet_ReduceResiduals(t *testing.T) {
@@ -86,6 +86,6 @@ func TestASTSet_ReduceResiduals(t *testing.T) {
 	require.NoError(t, err)
 	assert.Len(t, asts.asts, 1)
 	set, err := asts.ReduceResiduals(cel.Globals(&Variable{Name: "foo", Val: true}))
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Empty(t, set)
 }

internal/expression/compile_test.go

@@ -36,7 +36,7 @@ func TestCompile(t *testing.T) {
 		var exprs []*validate.Constraint
 		set, err := Compile(exprs, baseEnv)
 		assert.Nil(t, set)
-		assert.NoError(t, err)
+		require.NoError(t, err)
 	})
 
 	t.Run("success", func(t *testing.T) {
@@ -47,7 +47,7 @@ func TestCompile(t *testing.T) {
 		}
 		set, err := Compile(exprs, baseEnv, cel.Variable("this", cel.IntType))
 		assert.Len(t, set, len(exprs))
-		assert.NoError(t, err)
+		require.NoError(t, err)
 	})
 
 	t.Run("env extension err", func(t *testing.T) {
@@ -58,7 +58,7 @@ func TestCompile(t *testing.T) {
 		set, err := Compile(exprs, baseEnv, cel.Types(true))
 		assert.Nil(t, set)
 		var compErr *errors.CompilationError
-		assert.ErrorAs(t, err, &compErr)
+		require.ErrorAs(t, err, &compErr)
 	})
 
 	t.Run("bad syntax", func(t *testing.T) {
@@ -69,7 +69,7 @@ func TestCompile(t *testing.T) {
 		set, err := Compile(exprs, baseEnv)
 		assert.Nil(t, set)
 		var compErr *errors.CompilationError
-		assert.ErrorAs(t, err, &compErr)
+		require.ErrorAs(t, err, &compErr)
 	})
 
 	t.Run("invalid output type", func(t *testing.T) {
@@ -80,6 +80,6 @@ func TestCompile(t *testing.T) {
 		set, err := Compile(exprs, baseEnv)
 		assert.Nil(t, set)
 		var compErr *errors.CompilationError
-		assert.ErrorAs(t, err, &compErr)
+		require.ErrorAs(t, err, &compErr)
 	})
 }