go-ozzo / ozzo-validation

An idiomatic Go (golang) validation package. Supports configurable and extensible validation rules (validators) using normal language constructs instead of error-prone struct tags.
MIT License
3.78k stars 224 forks source link

Should the IsEmpty check be bypassed for numbers #52

Closed jesse-c closed 6 years ago

jesse-c commented 6 years ago
minAllowed := 1

return validation.Validate(0,
        validation.Min(minAllowed),
        validation.Max(maxAllowed),
    )

I would expect an error from this, as 0 is less than the min value (1), but instead nil is returned. The issue is because (r *ThresholdRule) Validate does a if isNil || IsEmpty(value) { check. Same for Max (what if I want to check if my value 0 satisfies the rule of the min value being 10).

I think that in this context, that check shouldn't be performed. It could be conditionally performed based on reflection (as it does for other reasons).

I'm happy to make a MR to do this.

Edit: I'll add in too while I'm here, thanks for the library!

qiangxue commented 6 years ago

This is by design and the same applies to most rules. The trick is to add a Required rule if you want to make sure a non-empty value is provided. See https://github.com/go-ozzo/ozzo-validation#required-vs-not-nil

jesse-c commented 6 years ago

Makes sense. I think the error is just slightly misleading (cannot be blank).

Thanks!