Poor error message when attempting to format NaiveDate with unsupported specifiers

[friendlymatthew]

When attempting to format a NaiveDate with time-related/unsupported specifiers, the error message is not informative.

For example:

use chrono::NaiveDate;

fn main() {
    let d = NaiveDate::from_ymd_opt(2015, 9, 5).unwrap();
    let _ = d.format("%Y-%m-%dT%H:%M:%S%.3f").to_string(); // uses time specifiers (i.e. %H, %M, etc...)
    let _ = d.format("%Y-%m-%k").to_string(); // This is just hogwash, %k isn't a time or date specifier!
}

Both outputs:

  Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.01s
     Running `target/debug/fmtdate`
thread 'main' panicked at /Users/matthew/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/alloc/src/string.rs:2688:14:

a Display implementation returned an error unexpectedly: Error

The error message could be improved indicating time specifiers or unsupported specifiers cannot be used with NaiveDate.

[djc]

You might want to try StrftimeItems::parse() to flush out errors in the format string? This API can't be changed much without compatibility issues, but we can probably come up with some API that might provide better errors (there might even be open PRs for this already).

[friendlymatthew]

Thank you.

Would modifying the ParseError message be a breaking change? If someone provides a invalid format string, StrftimeItems returns the message: ParseError(BadFormat).

Although this error message gives us context that the format string is invalid, it would be great to specify which specifier is invalid, especially since StrftimeItems already iterates through every Item to check if it's invalid.

I'd be happy to post a PR for this change.

[djc]

I think we can add variants to ParseErrorKind, do you want to submit a PR for this?

[friendlymatthew]

I think we can add variants to ParseErrorKind, do you want to submit a PR for this?

I'm wondering if we want to first update the Item::Error variant to include a message like Item::Error(&'a str). This way, when we encounter an unrecognized specifier (or any of the _ => Item::Error arms above), we could keep track of which specifier is invalid. Of course, Item::Error is a general parsing error, but we could be more specific about why the error occured. For example, this line could have a message like Item::Error("premature end of string").

Then the message in Item::Error could turtle it's way up to ParseErrorKind with ParseErrorKind::BadFormat becoming ParseErrorKind::BadFormat(&'a str). I'm not sure if this would be a breaking change, but it'd definitely refactor a lot of code, e.g. BadFormat can't be const.

If capturing the specific unsupported specifier isn't needed, I could also refactor Item::Error to be Item::Error(ItemErrorKind) and list out all the possible parsing errors you encounter in StrftimeItems.

[djc]

I don't think semver allows us to change existing variants, only add new variants.