Fix nonzero validator to not validate nil array or slices. (#148)

* Fix issue where a nil array or slice was failing nonzero validation. Fix issue where required or nonzero didn't work on maps.

* Add changelog.

* Cleanup error messaging on validation.

* Adjust nonemptry allowNil conditional branch.

* Add specific error for empty string and regex.

CHANGELOG.md

@@ -13,6 +13,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ### Removed
 
 ### Fixed
+- Fixed nonzero validator to not fail on nil array or slice. #147
+- Fixed nonzero validator to validate maps.
+- Fixed required validator to validate maps.
 
 ## [0.8.1]
 

error.go

@@ -98,6 +98,14 @@ var (
 	ErrRequired = errors.New("missing required field")
 
 	ErrEmpty = errors.New("empty field")
+
+	ErrArrayEmpty = errors.New("empty array")
+
+	ErrMapEmpty = errors.New("empty map")
+
+	ErrRegexEmpty = errors.New("regex value is not set")
+
+	ErrStringEmpty = errors.New("string value is not set")
 )
 
 // Error Classes

reify.go

@@ -167,7 +167,7 @@ func reifyInto(opts *options, to reflect.Value, from *Config) Error {
 
 	switch k {
 	case reflect.Map:
-		return reifyMap(opts, to, from)
+		return reifyMap(opts, to, from, nil)
 	case reflect.Struct:
 		return reifyStruct(opts, to, from)
 	case reflect.Slice, reflect.Array:
@@ -183,7 +183,7 @@ func reifyInto(opts *options, to reflect.Value, from *Config) Error {
 	return raiseInvalidTopLevelType(to.Interface(), opts.meta)
 }
 
-func reifyMap(opts *options, to reflect.Value, from *Config) Error {
+func reifyMap(opts *options, to reflect.Value, from *Config, validators []validatorTag) Error {
 	parentFields := opts.activeFields
 	defer func() { opts.activeFields = parentFields }()
 
@@ -198,7 +198,7 @@ func reifyMap(opts *options, to reflect.Value, from *Config) Error {
 
 	fields := from.fields.dict()
 	if len(fields) == 0 {
-		if err := tryRecursiveValidate(to, opts, nil); err != nil {
+		if err := tryRecursiveValidate(to, opts, validators); err != nil {
 			return raiseValidation(from.ctx, from.metadata, "", err)
 		}
 		return nil
@@ -224,6 +224,9 @@ func reifyMap(opts *options, to reflect.Value, from *Config) Error {
 		to.SetMapIndex(key, v)
 	}
 
+	if err := runValidators(to.Interface(), validators); err != nil {
+		return raiseValidation(from.ctx, from.metadata, "", err)
+	}
 	if err := tryValidate(to); err != nil {
 		return raiseValidation(from.ctx, from.metadata, "", err)
 	}
@@ -321,7 +324,7 @@ func reifyGetField(
 		}
 
 		// Primitive types return early when it doesn't implement the Initializer interface.
-		if fieldType.Kind() != reflect.Map && fieldType.Kind() != reflect.Struct && !hasInitDefaults(fieldType) {
+		if fieldType.Kind() != reflect.Struct && !hasInitDefaults(fieldType) {
 			if err := tryRecursiveValidate(to, opts.opts, opts.validators); err != nil {
 				return raiseValidation(cfg.ctx, cfg.metadata, name, err)
 			}
@@ -479,7 +482,7 @@ func reifyMergeValue(
 		if err != nil {
 			return reflect.Value{}, raiseExpectedObject(opts.opts, val)
 		}
-		return old, reifyMap(opts.opts, old, sub)
+		return old, reifyMap(opts.opts, old, sub, opts.validators)
 
 	case reflect.Struct:
 		sub, err := val.toConfig(opts.opts)

validator.go

@@ -392,31 +392,53 @@ func validateRequired(v interface{}, name string) error {
 		}
 		return nil
 	}
-	if err := validateNonEmpty(v, name); err != nil {
-		return ErrRequired
+	if err := validateNonEmptyWithAllowNil(v, name, false); err != nil {
+		return err
 	}
 	return nil
 }
 
-func validateNonEmpty(v interface{}, _ string) error {
+func validateNonEmpty(v interface{}, name string) error {
+	return validateNonEmptyWithAllowNil(v, name, true)
+}
+
+func validateNonEmptyWithAllowNil(v interface{}, _ string, allowNil bool) error {
 	if s, ok := v.(string); ok {
 		if s == "" {
-			return ErrEmpty
+			return ErrStringEmpty
 		}
 		return nil
 	}
 
 	if r, ok := v.(regexp.Regexp); ok {
 		if r.String() == "" {
-			return ErrEmpty
+			return ErrRegexEmpty
 		}
 		return nil
 	}
 
 	val := reflect.ValueOf(v)
 	if val.Kind() == reflect.Array || val.Kind() == reflect.Slice {
+		if val.IsNil() {
+			if allowNil {
+				return nil
+			}
+			return ErrRequired
+		}
+		if val.Len() == 0 {
+			return ErrArrayEmpty
+		}
+		return nil
+	}
+	if val.Kind() == reflect.Map {
+		if val.IsNil() {
+			if allowNil {
+				return nil
+			}
+			return ErrRequired
+		}
 		if val.Len() == 0 {
-			return ErrEmpty
+			return ErrMapEmpty
 		}
 		return nil
 	}

validator_test.go

@@ -299,6 +299,12 @@ func TestValidationPass(t *testing.T) {
 		}{
 			X: &nestedNestedPtrStructValidator{},
 		},
+		&struct {
+			X []string `validate:"nonzero"` // field not present in config, nonzero (no error)
+		}{},
+		&struct {
+			X map[string]string `validate:"nonzero"` // field not present in config, nonzero (no error)
+		}{},
 	}
 
 	for i, test := range tests {
@@ -539,6 +545,7 @@ func TestValidateRequiredFailing(t *testing.T) {
 		"b": "",
 		"c": nil,
 		"d": []string{},
+		"f": map[string]string{},
 	})
 
 	tests := []struct {
@@ -552,7 +559,7 @@ func TestValidateRequiredFailing(t *testing.T) {
 		{ErrRequired, &struct {
 			A int `validate:"required"`
 		}{}},
-		{ErrRequired, &struct {
+		{ErrStringEmpty, &struct {
 			A string `validate:"required"`
 		}{}},
 		{ErrRequired, &struct {
@@ -563,13 +570,13 @@ func TestValidateRequiredFailing(t *testing.T) {
 		}{}},
 
 		// Access empty string field "b"
-		{ErrRequired, &struct {
+		{ErrStringEmpty, &struct {
 			B string `validate:"required"`
 		}{}},
-		{ErrRequired, &struct {
+		{ErrStringEmpty, &struct {
 			B *string `validate:"required"`
 		}{}},
-		{ErrRequired, &struct {
+		{ErrRegexEmpty, &struct {
 			B *regexp.Regexp `validate:"required"`
 		}{}},
 
@@ -580,7 +587,7 @@ func TestValidateRequiredFailing(t *testing.T) {
 		{ErrRequired, &struct {
 			C int `validate:"required"`
 		}{}},
-		{ErrRequired, &struct {
+		{ErrStringEmpty, &struct {
 			C string `validate:"required"`
 		}{}},
 		{ErrRequired, &struct {
@@ -590,10 +597,29 @@ func TestValidateRequiredFailing(t *testing.T) {
 			C time.Duration `validate:"required"`
 		}{}},
 
-		// Check empty []string field 'd'
-		{ErrRequired, &struct {
+		// Check required []string field 'd'
+		{ErrArrayEmpty, &struct {
 			D []string `validate:"required"`
 		}{}},
+
+		// Check required []string filed 'e' not in config (pre-initialized)
+		{ErrArrayEmpty, &struct {
+			E []string `validate:"required"`
+		}{
+			E: []string{},
+		}},
+
+		// Check empty map[string]string field 'f'
+		{ErrMapEmpty, &struct {
+			F map[string]string `validate:"required"`
+		}{}},
+
+		// Check required map[string] 'g' not in config (pre-initialized)
+		{ErrMapEmpty, &struct {
+			G map[string]string `validate:"required"`
+		}{
+			G: map[string]string{},
+		}},
 	}
 
 	for i, test := range tests {
@@ -616,6 +642,7 @@ func TestValidateNonzeroFailing(t *testing.T) {
 		"i": 0,
 		"s": "",
 		"a": []int{},
+		"c": map[string]string{},
 	})
 
 	tests := []struct {
@@ -663,23 +690,55 @@ func TestValidateNonzeroFailing(t *testing.T) {
 		}{}},
 
 		// test string types accessing 's'
-		{ErrEmpty, &struct {
+		{ErrStringEmpty, &struct {
 			S string `validate:"nonzero"`
 		}{}},
-		{ErrEmpty, &struct {
+		{ErrStringEmpty, &struct {
 			S *string `validate:"nonzero"`
 		}{}},
-		{ErrEmpty, &struct {
+		{ErrRegexEmpty, &struct {
 			S *regexp.Regexp `validate:"nonzero"`
 		}{}},
 
 		// test array type accessing 'a'
-		{ErrEmpty, &struct {
+		{ErrArrayEmpty, &struct {
 			A []int `validate:"nonzero"`
 		}{}},
-		{ErrEmpty, &struct {
+		{ErrArrayEmpty, &struct {
 			A []uint8 `validate:"nonzero"`
 		}{}},
+
+		// test array type accessing 'b' (pre-initialized)
+		{ErrArrayEmpty, &struct {
+			B []int `validate:"nonzero"`
+		}{
+			B: []int{},
+		}},
+		{ErrArrayEmpty, &struct {
+			B []uint8 `validate:"nonzero"`
+		}{
+			B: []uint8{},
+		}},
+
+		// test array type accessing 'c'
+		{ErrMapEmpty, &struct {
+			C map[string]string `validate:"nonzero"`
+		}{}},
+		{ErrMapEmpty, &struct {
+			C map[string]string `validate:"nonzero"`
+		}{}},
+
+		// test array type accessing 'd' (pre-initialized)
+		{ErrMapEmpty, &struct {
+			D map[string]string `validate:"nonzero"`
+		}{
+			D: map[string]string{},
+		}},
+		{ErrMapEmpty, &struct {
+			D map[string]string `validate:"nonzero"`
+		}{
+			D: map[string]string{},
+		}},
 	}
 
 	for i, test := range tests {