`Parsed` fixes and documentation

[pitdicker]

trafficstars

This is the first part of #1418 and applies to the branch for 0.4.x.

We have two small bugs in our implementation of Parsed:

• If there is a timestamp and an offset field, the offset is added to the timestamp. But the definition of a Unix timestamp is that the value is in UTC, so this is not correct.
• We were returning OUT_OF_RANGE if the year value didn't match with the values of year_div_100 or year_mod_100. It should return IMPOSSIBLE instead.

I extended the documentation of Parsed to describe why it exists (the resolution algorithm) and to give an example (fixes #55). Partly taken from the blog post before chrono 0.2: https://lifthrasiir.github.io/rustlog/worklog-2015-02-19.html

All the methods now have more accurate documentation that describes their error causes.

The documentation got a couple of rounds of self-review.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 96.66667% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 92.14%. Comparing base (02c68d6) to head (3ea3af6). Report is 3 commits behind head on main.

Files	Patch %	Lines
src/format/parsed.rs	96.66%	2 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1439      +/-   ##
==========================================
+ Coverage   92.11%   92.14%   +0.03%     
==========================================
  Files          40       40              
  Lines       18026    18079      +53     
==========================================
+ Hits        16604    16659      +55     
+ Misses       1422     1420       -2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[pitdicker]

Found more oddity to document for the to_naive_datetime_with_offset method:

The offset is assumed to have a given value. It is not compared against the offset field set in the Parsed type, so it is allowed to be inconsistent.

Actually that whole method is just weird to work with. The logic should be inverted to return a NaiveDateTime, and an optional FixedOffset if there is enough information available. In September I planned to deprecate that method and replace it with something more sensible but never made the PR.

[pitdicker]

• If there is a timestamp and an offset field, the offset is added to the timestamp. But the definition of a Unix timestamp is that the value is in UTC, so this is not correct.

I have to admit I read the current code wrong. It is working exactly like it should. The timestamp is assumed to be in UTC, it adds an offset, and then checks the rest of the values as local date and time. Dropped the first commit.

[pitdicker]

Found one more missing error case in Parsed::to_fixed_offset. The use of Options covered it up :smile:. If there is no offset field set, it should return NOT_ENOUGH instead of OUT_OF_RANGE.

[djc]

This is shaping up nicely!

[pitdicker]

Found one more missing error case in Parsed::to_fixed_offset.

Wow, apparently I found and fixed it last year also in #1042 😆.