go: multi-error validation with ValidateAll()

[danielhochman]

Updated version of #455 using two methods Validate() and ValidateAll() instead of moving to a single method Validate(all bool). This retains compatibility with the existing interface checks in other modules such as:

type validator interface {
  Validate() error
}

if v, ok := message.(validator); ok {
  v.Validate()
}

Fixes #47

Co-authored-by: Mateusz Czapliński mateusz.czaplinski@wpengine.com

[brandoncole]

For nested scenarios, I wonder if it's worth adding a message that returns all errors recursively as well to completely flatten a tree of errors? If you feel like that complexity might be too much to implement here I'm more than happy to agree that it should be implemented in clients b/c this is already a great step.

[akavel]

Thing is, such a func is quite complex, and yet basically 100% operates on interfaces (doesn't need to be specialized per-error-type), so IMO it doesn't need to be a method, and potentially it could be put as a "helper" func in an external package, e.g. a subpackage of PGV for being imported separately. Still, I think it could make quite a lot of sense to add such a func (or even method if really insisted), maybe with a "walker/visitor"-like signature, but I'd personally vote for working on one in a separate PR. And @brandoncole if you're really interested in this topic, maybe you'd like to try your hand at writing such a PR yourself? 🙂

[brandoncole]

I like that suggestion, it would cut down on the boilerplate and I agree it could probably just be a centrally defined function.

[akonradi]

I found this much easier to review relative to 5ef93ae, right before reverting take 1 of this. Even containing all the changes from dd3f943, the diff is tiny! danielhochman/protoc-gen-validate@5ef93ae...multierror-validation

[akonradi]

This looks good to me, but I've also been wrong very recently. @akavel you're obviously an interested party, please feel free to chime in. I'm happy to merge this but holding off for a few days so anyone else following #47 has a chance to comment.

[brandoncole]

Just wanted to say thank you once again to all the folks involved in this great work 🙏

[akavel]

Thing is, such a func is quite complex, and yet basically 100% operates on interfaces (doesn't need to be specialized per-error-type), so IMO it doesn't need to be a method, and potentially it could be put as a "helper" func in an external package, e.g. a subpackage of PGV for being imported separately. Still, I think it could make quite a lot of sense to add such a func (or even method if really insisted), maybe with a "walker/visitor"-like signature, but I'd personally vote for working on one in a separate PR. And @brandoncole if you're really interested in this topic, maybe you'd like to try your hand at writing such a PR yourself? 🙂

[akavel]

LGTM, thank you so much @danielhochman ! ❤️ 🚀

[danielhochman]

@akonradi ready to go!

[akonradi]

Thanks, all!

[akavel]

Huge thanks @danielhochman for stepping in, following up with a new version and taking it up to the end! ❤️