More documentation for NaiveTime, and addition fixes.

While writing documentation tests for NaiveTime it was found that
the addition involving leap seconds is *still* slightly broken.
(A consequence of having less tests, well.) The addition routine
has been rewritten to be explicit about leap seconds while passing
all other tests, so the rewrite does not change the intention.

src/datetime.rs

@@ -11,7 +11,7 @@ use std::cmp::Ordering;
 use std::ops::{Add, Sub};
 
 use {Weekday, Timelike, Datelike};
-use offset::{TimeZone, Offset};
+use offset::{TimeZone, Offset, add_with_leapsecond};
 use offset::utc::UTC;
 use offset::local::Local;
 use offset::fixed::FixedOffset;
@@ -49,7 +49,7 @@ impl<Tz: TimeZone> DateTime<Tz> {
     /// Unlike `date`, this is not associated to the time zone.
     #[inline]
     pub fn time(&self) -> NaiveTime {
-        self.datetime.time() + self.offset.local_minus_utc()
+        add_with_leapsecond(&self.datetime.time(), &self.offset.local_minus_utc())
     }
 
     /// Returns the number of non-leap seconds since January 1, 1970 0:00:00 UTC
@@ -141,7 +141,7 @@ impl<Tz: TimeZone> DateTime<Tz> {
     /// Returns a view to the naive local datetime.
     #[inline]
     pub fn naive_local(&self) -> NaiveDateTime {
-        self.datetime + self.offset.local_minus_utc()
+        add_with_leapsecond(&self.datetime, &self.offset.local_minus_utc())
     }
 }
 

src/format/mod.rs

@@ -10,7 +10,7 @@ use std::error::Error;
 use {Datelike, Timelike};
 use div::{div_floor, mod_floor};
 use duration::Duration;
-use offset::Offset;
+use offset::{Offset, add_with_leapsecond};
 use naive::date::NaiveDate;
 use naive::time::NaiveTime;
 
@@ -303,7 +303,7 @@ pub fn format<'a, I>(w: &mut fmt::Formatter, date: Option<&NaiveDate>, time: Opt
                         (Some(d), Some(t), None) =>
                             Some(d.and_time(*t).timestamp()),
                         (Some(d), Some(t), Some(&(_, off))) =>
-                            Some((d.and_time(*t) - off).timestamp()),
+                            Some(add_with_leapsecond(&d.and_time(*t), &-off).timestamp()),
                         (_, _, _) => None
                     }),
                 };

src/naive/date.rs

@@ -852,6 +852,17 @@ impl NaiveDate {
     /// assert_eq!(d.format_with_items(fmt.clone()).to_string(), "2015-09-05");
     /// assert_eq!(d.format("%Y-%m-%d").to_string(), "2015-09-05");
     /// ~~~~
+    ///
+    /// The resulting `DelayedFormat` can be formatted directly via the `Display` trait.
+    ///
+    /// ~~~~
+    /// use chrono::NaiveDate;
+    /// use chrono::format::strftime::StrftimeItems;
+    ///
+    /// let fmt = StrftimeItems::new("%Y-%m-%d");
+    /// let d = NaiveDate::from_ymd(2015, 9, 5);
+    /// assert_eq!(format!("{}", d.format_with_items(fmt.clone())), "2015-09-05");
+    /// ~~~~
     #[inline]
     pub fn format_with_items<'a, I>(&self, items: I) -> DelayedFormat<I>
             where I: Iterator<Item=Item<'a>> + Clone {
@@ -881,6 +892,16 @@ impl NaiveDate {
     /// assert_eq!(d.format("%Y-%m-%d").to_string(), "2015-09-05");
     /// assert_eq!(d.format("%A, %-d %B, %C%y").to_string(), "Saturday, 5 September, 2015");
     /// ~~~~
+    ///
+    /// The resulting `DelayedFormat` can be formatted directly via the `Display` trait.
+    ///
+    /// ~~~~
+    /// use chrono::NaiveDate;
+    ///
+    /// let d = NaiveDate::from_ymd(2015, 9, 5);
+    /// assert_eq!(format!("{}", d.format("%Y-%m-%d")), "2015-09-05");
+    /// assert_eq!(format!("{}", d.format("%A, %-d %B, %C%y")), "Saturday, 5 September, 2015");
+    /// ~~~~
     #[inline]
     pub fn format<'a>(&self, fmt: &'a str) -> DelayedFormat<StrftimeItems<'a>> {
         self.format_with_items(StrftimeItems::new(fmt))

src/naive/time.rs

@@ -406,6 +406,18 @@ impl NaiveTime {
     /// assert_eq!(t.format_with_items(fmt.clone()).to_string(), "23:56:04");
     /// assert_eq!(t.format("%H:%M:%S").to_string(), "23:56:04");
     /// ~~~~
+    ///
+    /// The resulting `DelayedFormat` can be formatted directly via the `Display` trait.
+    ///
+    /// ~~~~
+    /// use chrono::NaiveTime;
+    /// use chrono::format::strftime::StrftimeItems;
+    ///
+    /// let fmt = StrftimeItems::new("%H:%M:%S");
+    /// let t = NaiveTime::from_hms(23, 56, 4);
+    /// assert_eq!(format!("{}", t.format_with_items(fmt.clone())), "23:56:04");
+    /// assert_eq!(format!("{}", t.format("%H:%M:%S")), "23:56:04");
+    /// ~~~~
     #[inline]
     pub fn format_with_items<'a, I>(&self, items: I) -> DelayedFormat<I>
             where I: Iterator<Item=Item<'a>> + Clone {
@@ -436,6 +448,17 @@ impl NaiveTime {
     /// assert_eq!(t.format("%H:%M:%S%.6f").to_string(), "23:56:04.012345");
     /// assert_eq!(t.format("%-I:%M %p").to_string(), "11:56 PM");
     /// ~~~~
+    ///
+    /// The resulting `DelayedFormat` can be formatted directly via the `Display` trait.
+    ///
+    /// ~~~~
+    /// use chrono::NaiveTime;
+    ///
+    /// let t = NaiveTime::from_hms_nano(23, 56, 4, 12_345_678);
+    /// assert_eq!(format!("{}", t.format("%H:%M:%S")), "23:56:04");
+    /// assert_eq!(format!("{}", t.format("%H:%M:%S%.6f")), "23:56:04.012345");
+    /// assert_eq!(format!("{}", t.format("%-I:%M %p")), "11:56 PM");
+    /// ~~~~
     #[inline]
     pub fn format<'a>(&self, fmt: &'a str) -> DelayedFormat<StrftimeItems<'a>> {
         self.format_with_items(StrftimeItems::new(fmt))
@@ -661,30 +684,125 @@ impl hash::Hash for NaiveTime {
     }
 }
 
+/// An addition of `Duration` to `NaiveTime` wraps around and never overflows or underflows.
+/// In particular the addition ignores integral number of days.
+///
+/// As a part of Chrono's [leap second handling](./index.html#leap-second-handling),
+/// the addition assumes that **there is no leap second ever**,
+/// except when the `NaiveTime` itself represents a leap second
+/// in which case the assumption becomes that **there is exactly single leap second ever**.
+///
+/// Therefore, assuming that `03:00:60` is a leap second:
+///
+/// - `03:00:00 + 1s = 03:00:01`.
+/// - `03:00:59 + 1s = 03:01:00`.
+/// - `03:00:59 + 60s = 03:02:00`.
+/// - `03:00:60 + 1s = 03:01:00`.
+///   Note that the sum is identical to the previous; the associativity would no longer hold.
+/// - `03:00:60 + 60s = 03:01:59`.
+/// - `03:00:60 + 61s = 03:02:00`.
+/// - `03:00:60.1 + 0.8s = 03:00:60.9`.
+///
+/// It can be thought that the clock *forgets* a leap second once it ticked any direction.
+///
+/// # Example
+///
+/// ~~~~
+/// use chrono::{NaiveTime, Duration};
+///
+/// let hmsm = |h,m,s,milli| NaiveTime::from_hms_milli(h, m, s, milli);
+/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::zero(),                  hmsm(3, 5, 7, 0));
+/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::seconds(1),              hmsm(3, 5, 8, 0));
+/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::seconds(-1),             hmsm(3, 5, 6, 0));
+/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::seconds(60 + 4),         hmsm(3, 6, 11, 0));
+/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::seconds(7*60*60 - 6*60), hmsm(9, 59, 7, 0));
+/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::milliseconds(80),        hmsm(3, 5, 7, 80));
+/// assert_eq!(hmsm(3, 5, 7, 950) + Duration::milliseconds(280),     hmsm(3, 5, 8, 230));
+/// assert_eq!(hmsm(3, 5, 7, 950) + Duration::milliseconds(-980),    hmsm(3, 5, 6, 970));
+/// ~~~~
+///
+/// The addition wraps around.
+///
+/// ~~~~
+/// # use chrono::{NaiveTime, Duration};
+/// # let hmsm = |h,m,s,milli| NaiveTime::from_hms_milli(h, m, s, milli);
+/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::seconds(22*60*60), hmsm(1, 5, 7, 0));
+/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::seconds(-8*60*60), hmsm(19, 5, 7, 0));
+/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::days(800),         hmsm(3, 5, 7, 0));
+/// ~~~~
+///
+/// Leap seconds are handled, but the addition assumes that it is the only leap second happened.
+///
+/// ~~~~
+/// # use chrono::{NaiveTime, Duration};
+/// # let hmsm = |h,m,s,milli| NaiveTime::from_hms_milli(h, m, s, milli);
+/// assert_eq!(hmsm(3, 5, 59, 1_300) + Duration::zero(),             hmsm(3, 5, 59, 1_300));
+/// assert_eq!(hmsm(3, 5, 59, 1_300) + Duration::milliseconds(-500), hmsm(3, 5, 59, 800));
+/// assert_eq!(hmsm(3, 5, 59, 1_300) + Duration::milliseconds(500),  hmsm(3, 5, 59, 1_800));
+/// assert_eq!(hmsm(3, 5, 59, 1_300) + Duration::milliseconds(800),  hmsm(3, 6, 0, 100));
+/// assert_eq!(hmsm(3, 5, 59, 1_300) + Duration::seconds(10),        hmsm(3, 6, 9, 300));
+/// assert_eq!(hmsm(3, 5, 59, 1_300) + Duration::seconds(-10),       hmsm(3, 5, 50, 300));
+/// assert_eq!(hmsm(3, 5, 59, 1_300) + Duration::days(1),            hmsm(3, 5, 59, 300));
+/// ~~~~
 impl Add<Duration> for NaiveTime {
     type Output = NaiveTime;
 
-    fn add(self, rhs: Duration) -> NaiveTime {
+    fn add(self, mut rhs: Duration) -> NaiveTime {
-        // there is no direct interface in `Duration` to get only the nanosecond part,
+        let mut secs = self.secs;
-        // so we need to do the additional calculation here.
+        let mut frac = self.frac;
-        let mut rhssecs = rhs.num_seconds();
+
-        let mut rhs2 = rhs - Duration::seconds(rhssecs);
+        if frac >= 1_000_000_000 {
-        if rhs2 < Duration::zero() { // possible when rhs < 0
+            let rfrac = 2_000_000_000 - frac;
-            rhssecs -= 1;
+            if rhs >= Duration::nanoseconds(rfrac as i64) {
-            rhs2 = rhs2 + Duration::seconds(1);
+                rhs = rhs - Duration::nanoseconds(rfrac as i64);
+                secs += 1;
+                frac = 0;
+            } else if rhs < Duration::nanoseconds(-(frac as i64)) {
+                rhs = rhs + Duration::nanoseconds(frac as i64);
+                frac = 0;
+            } else {
+                frac = (frac as i64 + rhs.num_nanoseconds().unwrap()) as u32;
+                debug_assert!(frac < 2_000_000_000);
+                return NaiveTime { secs: secs, frac: frac };
+            }
         }
-        debug_assert!(rhs2 >= Duration::zero());
+        debug_assert!(secs <= 86400);
-        let mut secs = self.secs + (rhssecs % 86400 + 86400) as u32;
+        debug_assert!(frac < 1_000_000_000);
-        let mut nanos = self.frac + rhs2.num_nanoseconds().unwrap() as u32;
 
-        // always ignore leap seconds after the current whole second
+        let rhssecs = rhs.num_seconds();
-        let maxnanos = if self.frac >= 1_000_000_000 {2_000_000_000} else {1_000_000_000};
+        let rhsfrac = (rhs - Duration::seconds(rhssecs)).num_nanoseconds().unwrap();
+        debug_assert!(Duration::seconds(rhssecs) + Duration::nanoseconds(rhsfrac) == rhs);
+        let rhssecs = (rhssecs % 86400) as i32;
+        let rhsfrac = rhsfrac as i32;
+        debug_assert!(-86400 < rhssecs && rhssecs < 86400);
+        debug_assert!(-1_000_000_000 < rhsfrac && rhsfrac < 1_000_000_000);
 
-        if nanos >= maxnanos {
+        let mut secs = secs as i32 + rhssecs;
-            nanos -= maxnanos;
+        let mut frac = frac as i32 + rhsfrac;
+        debug_assert!(-86400 < secs && secs <= 2 * 86400 + 1);
+        debug_assert!(-1_000_000_000 < frac && frac < 2_000_000_000);
+
+        if frac < 0 {
+            frac += 1_000_000_000;
+            secs -= 1;
+        } else if frac >= 1_000_000_000 {
+            frac -= 1_000_000_000;
             secs += 1;
         }
-        NaiveTime { secs: secs % 86400, frac: nanos }
+        debug_assert!(-86400 <= secs && secs <= 2 * 86400 + 1);
+        debug_assert!(0 <= frac && frac < 1_000_000_000);
+
+        if secs < 0 {
+            secs += 86400 * 2;
+        }
+        if secs >= 86400 * 2 {
+            secs -= 86400 * 2;
+        } else if secs >= 86400 {
+            secs -= 86400;
+        }
+        debug_assert!(0 <= secs && secs < 86400);
+
+        NaiveTime { secs: secs as u32, frac: frac as u32 }
     }
 }
 
@@ -1031,6 +1149,7 @@ mod tests {
         check(hmsm(3, 5, 7, 900), Duration::zero(), hmsm(3, 5, 7, 900));
         check(hmsm(3, 5, 7, 900), Duration::milliseconds(100), hmsm(3, 5, 8, 0));
         check(hmsm(3, 5, 7, 1_300), Duration::milliseconds(800), hmsm(3, 5, 8, 100));
+        check(hmsm(3, 5, 7, 1_300), Duration::milliseconds(1800), hmsm(3, 5, 9, 100));
         check(hmsm(3, 5, 7, 900), Duration::seconds(86399), hmsm(3, 5, 6, 900)); // overwrap
         check(hmsm(3, 5, 7, 900), Duration::seconds(-86399), hmsm(3, 5, 8, 900));
         check(hmsm(3, 5, 7, 900), Duration::days(12345), hmsm(3, 5, 7, 900));

src/offset/mod.rs

@@ -21,8 +21,10 @@
  */
 
 use std::fmt;
+use std::ops::Add;
 
 use Weekday;
+use Timelike;
 use duration::Duration;
 use naive::date::NaiveDate;
 use naive::time::NaiveTime;
@@ -31,6 +33,18 @@ use date::Date;
 use datetime::DateTime;
 use format::{parse, Parsed, ParseResult, StrftimeItems};
 
+/// Same to `*lhs + *rhs`, but keeps the leap second information.
+/// `rhs` should *not* have a fractional second.
+// TODO this should be replaced by the addition with FixedOffset in 0.3!
+pub fn add_with_leapsecond<T: Timelike + Add<Duration, Output=T>>(lhs: &T, rhs: &Duration) -> T {
+    debug_assert!(*rhs == Duration::seconds(rhs.num_seconds()));
+
+    // extract and temporarily remove the fractional part and later recover it
+    let nanos = lhs.nanosecond();
+    let lhs = lhs.with_nanosecond(0).unwrap();
+    (lhs + *rhs).with_nanosecond(nanos).unwrap()
+}
+
 /// The conversion result from the local time to the timezone-aware datetime types.
 #[derive(Clone, PartialEq, Debug)]
 pub enum LocalResult<T> {
@@ -305,7 +319,8 @@ pub trait TimeZone: Sized + Clone {
     /// Converts the local `NaiveDateTime` to the timezone-aware `DateTime` if possible.
     fn from_local_datetime(&self, local: &NaiveDateTime) -> LocalResult<DateTime<Self>> {
         self.offset_from_local_datetime(local).map(|offset| {
-            DateTime::from_utc(*local - offset.local_minus_utc(), offset)
+            let utc = add_with_leapsecond(local, &-offset.local_minus_utc());
+            DateTime::from_utc(utc, offset)
         })
     }