Span-based email validation

[Finchiedev]

Hello folks, I've been working on a command-line library that uses validator to report errors in arguments, and have recently focused on improving the quality of reported error messages. For the invalid email test@"example.com, instead of:

Invalid email: test@"example.com

I would like to report something like:

Invalid email: invalid character in domain
test@"example.com
     ^

To facilitate this, I have created a new function, validate_email_span, that should behave exactly the same as validate_email except instead of returning a bool, it returns a Result with additional context; an error enum and span for the first error encountered in the email.

I understand such code may be out of scope for this library, but thought it would be worth sending a patch upstream before considering a different crate - this one is quite nice! This new function, and its supporting types, have been placed inside a span module. If such contributions are in scope I would like to explore adding more contextual validators, such as IP addresses.

If there's anything I can do to ease the review process please let me know :)

[Keats]

Have a look at https://github.com/Keats/validator/pull/262 This is going to be merged next week so that's probably not going to work cc @pintariching I don't know how i feel about having those kind of error messages, they seem way too detailed for 99% of the usecases imo?

[Finchiedev]

Thanks for letting me know. I am very happy to rebase my changes on top of #262 if they're considered in-scope for this repository.

I do agree this is too much context for most use cases, however however since validate_email still exists (as a wrapper around validate_email_span) users can decide for themselves the level of granularity they require. This is similar to how validate_ip is already implemented: if users want a boolean, they can use validate_ip. If they want a more detailed error, they can use the IpAddr::from_str it wraps. There is clearly value to both approaches, I'm just not sure if this library is interested in supporting the more detailed use case.

[pintariching]

This could be perhaps behind a feature so that you could select what kind of errors you want?

I think that most users use this crate to validate structs in a CRUD API, so having such detailed error messages is unneeded I think?

However for more complex usages I think this could work behind a feature flag, something like diagnostics perhaps?

The macro side would need some touch ups in case we accept this as currently it works with bools only.

[Finchiedev]

Thanks for your input. I am sorry for not explaining myself well; as I see it, there are 2 options:

1. Add span-based reporting inside a submodule
   • e.g. validator::span::validate_email_span to complement validator::validate_email
   • diagnostics does sound like a better name, this is just what I've called it so far
   • your idea for features is cool, but I am struggling to think of a use case for that? Users already need to handle unknown error cases due to non_exhaustive being set.
   • from a user perspective, very little should change. this just adds the option if they need it (like i did)
   • all the existing API is there. validate_email still returns a bool, the only difference is internally it just runs return validate_email_span().is_ok()
   • I have not looked into this crate's macros and was not planning on touching them, but happy to do that work if it'd be beneficial
2. Create my own crate
   • no idea what the scope of this crate is, and don't want to push for unwanted maintenance burden
   • if it doesn't fit this crate it's no trouble for me to publish this PR separately
   • the changes are mostly self-contained already, I'd just prefer to keep it in a single library if it suits everyone

Also, I didn't do a very good job of clarifying that these error messages are made using data from the new API, not by it. The EmailValidationError struct only contains a text range and error enum, and it's up to the user to make anything with it.

tl;dr this only adds a new API, I'm not sure if it belongs in this crate but am very willing to make any changes you'd like