Breaking change wish-list

[jhpratt]

Note: There is no breaking change current planned. This issue is to keep track of things that may happen when there is a breaking change at some undetermined point in the future. Items are in no particular order.

---

Potential changes

These are mostly things that I have thought about at some point, with varying levels of certainty.

• Eliminate large-dates feature flag. Support the ±999,999 range of years unconditionally, adding a modifier to the [year] component to solve the issue of ambiguity.
• Remove the serde-well-known feature flag, which is already deprecated in favor of using the relevant flags (serde, formatting, and/or parsing) directly.
• Simplify error handling. Anonymous enums would be wonderful but are not required. In particular, the large number of conversions should be cut down, as many of them exist when they can already be performed transitively. The amount of information exposed should be restricted to as little as necessary, as this has caused issues in the past.
• Remove anything deprecated. This is currently minimal.
• Make FormatItem opaque and require FormatItem::Literal to be valid UTF-8. Likewise for OwnedFormatItem.
• Remove Copy implementation for Parsed.
• Use ranged integers in public APIs. This would currently require adding countless new methods.
• Rename Duration to SignedDuration or something else.
• Make Duration::seconds generic over i64, f32, and f64. Similarly for saturaturating_seconds_* and checked_seconds_*. Alternatively, have a seconds_float method that is generic while keeping the integer case separate.
• Remove Duration::time_fn. This is a carry-over from time 0.1 and was presumably meant as a poor man's benchmarking tool. Instant is not meant to be used in that manner.
• Remove time::Instant in favor of an extension trait on std::time::Instant. This would reduce the number of trait implementations needed. The extension trait was added in v0.3.35, with time::Instant being deprecated at the same time.

[jeff-hiner]

Separate feature flags for serde and the provided impl Serialize/Deserialize for T would be very helpful (and address #672)

[jhpratt]

@jeff-hiner I'll be honest, it's not likely to happen. The clippy issue you linked would solve your request while being more general purpose and would avoid the overhead of an additional feature flag. I'm also not aware of any widely-used crates that gate serde support and Serialize/Deserialize impls separately.

[nox]

I'm also not aware of any widely-used crates that gate serde support and Serialize/Deserialize impls separately.

The url crate (167M downloads) implement the serde traits with a sensible representation (the string representation one expects), and then provides serialize_internal and deserialize_internal for when you want to use a more performant serialization format.

To me the situation here is similar to Path not implementing Display, but still providing the Path::display method that returns something that implements Display, because providing the implementation directly would be a footgun.

[jhpratt]

Note that I was responding to the use of separate feature flags for Serialize/Deserialize impls, which is not present in url. Regardless, time also uses a sensible representation for implementing serde traits — the same format as is used for Display. Just as serialize_internal and deserialize_internal necessitate the use of serde helper attributes, you can already use helper attributes provided by time on the relevant fields. The only concern I have seen raised is relating to the default not being a well-known format, but that can be worked around if and when that clippy issue is resolved. If it's that much of an issue for you, I suggest you bring it up with your (common) employer to find some time to work on it.

Path not implementing Display is not because it's a footgun but because it's a lossy conversion. That is not the case for any type in time.

[nox]

Regardless, time also uses a sensible representation for implementing serde traits — the same format as is used for Display.

That's false. You find in the string printed by the Display the same amount of information as in the tuple the serde impl serializes to, but the serde impl doesn't serializes as a string. With this logic, you could argue that url's serialize_internal uses the same format as Display, and that would be wrong.

Oh with the human-readable serializer you mean? That's fair.

[jeff-hiner]

@jeff-hiner I'll be honest, it's not likely to happen. The clippy issue you linked would solve your request while being more general purpose and would avoid the overhead of an additional feature flag.

Looks like the clippy issue has been parked for 2 years. It was claimed, but I don't see any progress over the last year. I'd try to PR in a fix myself but as it involves digging into clippy's parser I honestly have no idea where to even start. Not optimistic that's going to be fixed soon.

I'm guessing here, but it looks like the current default is intended for human-readable serialization like serde_yaml. I suppose the similarity with Display is convenient for that use case. Unfortunately this default is unsuitable for those of us using serde_json and time to write REST endpoints or API clients, because it doesn't appear to follow any RFC or ISO convention.

I don't mind having to explicitly annotate serde(with = "..."). But the compiler can't help me if I miss one, because there's a valid but incompatible impl in scope. chrono::DateTime<Utc> defaults to RFC3339, which is why I'm only noticing it on my migration.

Since there's no one "right" way to impl Serialize for OffsetDateTime, maybe we simply shouldn't provide one. Removing the default Serialize/Deserialize entirely and making users opt into a specific serde impl set seems like the only sensible compromise. This would be a breaking change, thus why I'm bringing it up here.

[jhpratt]

It was claimed, but I don't see any progress over the last year.

Are we looking at the same thing? There was an update just a couple days ago on a PR that implements it. It may have stalled, but it is at least on the person's backlog.

it looks like the current default is intended for human-readable serialization

The default is intended to be something human-readable (when that flag is enabled) that can also be deserialized. There was no reasoning beyond that.

Unfortunately this default is unsuitable for those of us using serde_json and time to write REST endpoints or API clients, because it doesn't appear to follow any RFC or ISO convention.

chrono::DateTime<Utc> defaults to RFC3339

A trivial check shows that chrono does not use RFC3339, as that specification does not support years that are negative or contain more than four digits.

Since there's no one "right" way to impl Serialize for OffsetDateTime, maybe we simply shouldn't provide one.

That's an awfully large leap. As I said, the intent was to have something that is human-readable and can be round-tripped. Just as with formatting being present because Display is not everyone's cup of soup, there is a way to override the default. That there is not currently a way to prevent you from using the default does not mean we should take something reasonable away from those who want it. It's an ecosystem problem, not a time problem imo.

[jeff-hiner]

It was claimed, but I don't see any progress over the last year.

Are we looking at the same thing? There was an update just a couple days ago on a PR that implements it. It may have stalled, but it is at least on the person's backlog.

I'm looking at rust-lang/rust-clippy#8581 as linked in my original comment. That issue was opened 24 Mar 2022 (over two years ago), then claimed in June of the same year. A separate author opened a PR that was last touched at the beginning of the year, in Jan 2024. That PR received feedback in Feb, but no commits have been made since. As of right now the PR has conflicts. It appears the work required to have clippy support the appropriate parsing is non-trivial, or at the very least the PR implementation is controversial.

A trivial check shows that chrono does not use RFC3339, as that specification does not support years that are negative or contain more than four digits.

A trivial check on docs.rs shows that chrono does in fact use RFC3339 for DateTime<Tz>. It's very possible the implementation isn't complete or correct, but RFC3339 appears to be the intent of the default serialization.

[jhpratt]

Things aren't fast in Rust given that it's entirely driven by volunteers. The PR author commented just this week that they intend to continue work on it. Given that you presumably are using this as part of your employment, that is why I suggested asking for some time to work on the issue.

It's very possible the implementation isn't complete or correct, but RFC3339 appears to be the intent of the default serialization.

It definitely isn't correct for the reason that I stated. It's not RFC3339 if it doesn't strictly follow the specification.