-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add support to tag isvalid #1363
base: master
Are you sure you want to change the base?
Conversation
How does this
|
This seems more like a utility validator, and I'm all for it. It allows custom validators without ever touching reflection yourself, and instead having the library do the heavy lifting. The developer using isvalid just returns an error if they notice the struct is bad, and thats it. Benefit would also be that this may work with other validators if they adapt it. |
The |
I agree I don't love the tag name especially because I could foresee a future similar validation added where the function signature is WDYT about |
validateFn sounds good to me |
Sorry guys I just lost this entire discussion, I have one clear motivation: if I have a custom validator, lets say also, if I have several enumerations, I end up with several custom validators. Or I can use a generic one. That is my motivation last the oneof tag is useful for string types, not for enumerations based on integers/ uint8 (like the ones generated by some tool like enumer) so I can rename it to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the PR, the only thing I would add is a negative test that it doesn't panic when the type doesn't implement the validate function.
It's not required, but would help to ensure that interface conversions are not throwing panics.
fix doc Co-authored-by: nodivbyzero <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved.
It would be beneficial to add a new example to showcase the functionality. While unit tests are valuable, having both a unit test and an example is even better ;)
Fixes Or Enhances
This Pull Requests adds a new tag called
isvalid
If the field is marked with the validator tag
isvalid
, the type must implement the interfaceValidate() error
and the return must be nil to be consideredvalid
A possible use case is: when dealing with Enumerations, the type can support a method
Validate() error
to check if the value is in specific the range defined. If we use enumer it generates aIsA<Type>() bool
method that can be used to verify if the enumeration is valid or not, instead force theoneof
tag (that needs to be always updated when we add one new value.I wrote a pull request to add a Validate method on enumerations here and the interface
Validate() error
seems pretty common.It may clash with existing tags that people may register, this is something that I don't know how to solve.
Make sure that you've checked the boxes below before you submit PR:
@go-playground/validator-maintainers