From 98289021f3444a2f80c8f7630f5e519d9c338963 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Fri, 8 Sep 2023 07:46:19 +0000 Subject: [PATCH] time: clarify docs to avoid date calculation pitfalls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I recently reviewed some code that did time calculations using `time.UnixMicro(0).UTC()`. I commented that because time calculations are independent of the location, they should drop the `.UTC()`, and they replied that it made their tests fail. I looked into it and eventually discovered it was because they were using AddDate. Dramatically simplified, their code did something like:     orig := time.Date(2013, time.March, 23, 12, 00, 0, 0, time.UTC)     want := time.Date(2013, time.March, 23, 0, 0, 0, 0, time.UTC)     epoch := time.UnixMicro(0)     days := int(orig.Sub(epoch).Hours() / 24)     got := epoch.AddDate(0, 0, days)     if !got.Equal(want) {         t.Errorf("ay caramba: %v vs %v", got.UTC(), want.UTC())     } The issue is that their tests run in Pacific time, which is currently PST (UTC-8) but was PDT (UTC-7) in January 1970. It turns out they were implementing some business policy that really cares abut calendar days so AddDate is correct, but it's certainly a bit confusing! The idea with this change is to remove the risk that readers make a false shortcut in their mind: "Locations do not affect time calculations". To do this we remove some text from the core time.Time doc and shift it to the areas of the library that deal with these intrinsically confusing operations. Change-Id: I8200e9edef7d1cdd8516719e34814eb4b78d30a2 Reviewed-on: https://go-review.googlesource.com/c/go/+/526676 Reviewed-by: Ian Lance Taylor LUCI-TryBot-Result: Go LUCI Auto-Submit: Ian Lance Taylor Reviewed-by: Rob Pike Reviewed-by: Ian Lance Taylor --- src/time/example_test.go | 22 ++++++++++++++++++---- src/time/time.go | 20 ++++++++++++++------ src/time/zoneinfo.go | 4 ++++ 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/time/example_test.go b/src/time/example_test.go index 059c6310a6..cfdee8f4d7 100644 --- a/src/time/example_test.go +++ b/src/time/example_test.go @@ -591,19 +591,33 @@ func ExampleTime_Add() { } func ExampleTime_AddDate() { - start := time.Date(2009, 1, 1, 0, 0, 0, 0, time.UTC) + start := time.Date(2023, 03, 25, 12, 0, 0, 0, time.UTC) oneDayLater := start.AddDate(0, 0, 1) + dayDuration := oneDayLater.Sub(start) oneMonthLater := start.AddDate(0, 1, 0) oneYearLater := start.AddDate(1, 0, 0) + zurich, err := time.LoadLocation("Europe/Zurich") + if err != nil { + panic(err) + } + // This was the day before a daylight saving time transition in Zürich. + startZurich := time.Date(2023, 03, 25, 12, 0, 0, 0, zurich) + oneDayLaterZurich := startZurich.AddDate(0, 0, 1) + dayDurationZurich := oneDayLaterZurich.Sub(startZurich) + fmt.Printf("oneDayLater: start.AddDate(0, 0, 1) = %v\n", oneDayLater) fmt.Printf("oneMonthLater: start.AddDate(0, 1, 0) = %v\n", oneMonthLater) fmt.Printf("oneYearLater: start.AddDate(1, 0, 0) = %v\n", oneYearLater) + fmt.Printf("oneDayLaterZurich: startZurich.AddDate(0, 0, 1) = %v\n", oneDayLaterZurich) + fmt.Printf("Day duration in UTC: %v | Day duration in Zürich: %v\n", dayDuration, dayDurationZurich) // Output: - // oneDayLater: start.AddDate(0, 0, 1) = 2009-01-02 00:00:00 +0000 UTC - // oneMonthLater: start.AddDate(0, 1, 0) = 2009-02-01 00:00:00 +0000 UTC - // oneYearLater: start.AddDate(1, 0, 0) = 2010-01-01 00:00:00 +0000 UTC + // oneDayLater: start.AddDate(0, 0, 1) = 2023-03-26 12:00:00 +0000 UTC + // oneMonthLater: start.AddDate(0, 1, 0) = 2023-04-25 12:00:00 +0000 UTC + // oneYearLater: start.AddDate(1, 0, 0) = 2024-03-25 12:00:00 +0000 UTC + // oneDayLaterZurich: startZurich.AddDate(0, 0, 1) = 2023-03-26 12:00:00 +0200 CEST + // Day duration in UTC: 24h0m0s | Day duration in Zürich: 23h0m0s } func ExampleTime_After() { diff --git a/src/time/time.go b/src/time/time.go index 3d4416e76b..1db9d3768e 100644 --- a/src/time/time.go +++ b/src/time/time.go @@ -109,12 +109,11 @@ import ( // As this time is unlikely to come up in practice, the IsZero method gives // a simple way of detecting a time that has not been initialized explicitly. // -// Each Time has associated with it a Location, consulted when computing the -// presentation form of the time, such as in the Format, Hour, and Year methods. -// The methods Local, UTC, and In return a Time with a specific location. -// Changing the location in this way changes only the presentation; it does not -// change the instant in time being denoted and therefore does not affect the -// computations described in earlier paragraphs. +// Each time has an associated Location. The methods Local, UTC, and In return a +// Time with a specific Location. Changing the Location of a Time value with +// these methods does not change the actual instant it represents, only the time +// zone in which to interpret it. + // // Representations of a Time value saved by the GobEncode, MarshalBinary, // MarshalJSON, and MarshalText methods store the Time.Location's offset, but not @@ -951,6 +950,15 @@ func Until(t Time) Duration { // For example, AddDate(-1, 2, 3) applied to January 1, 2011 // returns March 4, 2010. // +// Note that dates are fundamentally coupled to timezones, and calendrical +// periods like days don't have fixed durations. AddDate uses the Location of +// the Time value to determine these durations. That means that the same +// AddDate arguments can produce a different shift in absolute time depending on +// the base Time value and its Location. For example, AddDate(0, 0, 1) applied +// to 12:00 on March 27 always returns 12:00 on March 28. At some locations and +// in some years this is a 24 hour shift. In others it's a 23 hour shift due to +// daylight savings time transitions. +// // AddDate normalizes its result in the same way that Date does, // so, for example, adding one month to October 31 yields // December 1, the normalized form for November 31. diff --git a/src/time/zoneinfo.go b/src/time/zoneinfo.go index 42d40d584e..c8d1762302 100644 --- a/src/time/zoneinfo.go +++ b/src/time/zoneinfo.go @@ -16,6 +16,10 @@ import ( // Typically, the Location represents the collection of time offsets // in use in a geographical area. For many Locations the time offset varies // depending on whether daylight savings time is in use at the time instant. +// +// Location is used to provide a time zone in a printed Time value and for +// calculations involving intervals that may cross daylight savings time +// boundaries. type Location struct { name string zone []zone