From f31dc0aaab5a2feeca5c41783abbc347731fd08e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Thu, 30 May 2019 11:21:02 +0200 Subject: [PATCH] Adjust timezone logic This commit adjusts the previous commit re. the new `ToTimeInDefaultLocationE` and related functions. The functional change is that we don't default to any location if not provided from the caller. This is in line with how `ToTime` worked before we started this, and even if the default behaviour may look weird in some cases, it will not break anything. Most applications will want to use the new *InDefaultLocation functions and decide which default location to use: ```go loc := time.Local if config.Location != "" { loc = time.LoadLocation(config.Location) } t, err := StringToDateInDefaultLocation("2019-01-01", loc) ``` This commit also configure Travis to test on OSX and Windows in addition to Linux. --- .travis.yml | 7 +- Makefile | 12 +-- cast.go | 3 + cast_test.go | 216 +++++++++++++++++++++++---------------------------- caste.go | 121 +++++++++++++++++------------ 5 files changed, 181 insertions(+), 178 deletions(-) diff --git a/.travis.yml b/.travis.yml index 833a487..c251807 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,9 +8,14 @@ go: - tip os: - linux + - osx + - windows matrix: allow_failures: - go: tip + exclude: + - os: windows + go: tip fast_finish: true script: - - make check + - if [[ "$TRAVIS_OS_NAME" == "windows" ]]; then go test -v -race ./...; else make check; fi diff --git a/Makefile b/Makefile index f01a5db..74629c2 100644 --- a/Makefile +++ b/Makefile @@ -1,15 +1,15 @@ GOVERSION := $(shell go version | cut -d ' ' -f 3 | cut -d '.' -f 2) -.PHONY: check fmt lint test test-race vet test-cover-html help +.PHONY: check fmt test test-race vet test-cover-html help .DEFAULT_GOAL := help -check: test-race fmt vet lint ## Run tests and linters +check: test-race fmt vet ## Run tests and linters test: ## Run tests go test ./... test-race: ## Run tests with race detector - go test -race ./... + go test -v -race ./... fmt: ## Run gofmt linter ifeq "$(GOVERSION)" "12" @@ -20,12 +20,6 @@ ifeq "$(GOVERSION)" "12" done endif -lint: ## Run golint linter - @for d in `go list` ; do \ - if [ "`golint $$d | tee /dev/stderr`" ]; then \ - echo "^ golint errors!" && echo && exit 1; \ - fi \ - done vet: ## Run go vet linter @if [ "`go vet | tee /dev/stderr`" ]; then \ diff --git a/cast.go b/cast.go index 0cfe941..58f674f 100644 --- a/cast.go +++ b/cast.go @@ -20,6 +20,9 @@ func ToTime(i interface{}) time.Time { return v } +// ToTimeInDefaultLocationE casts an empty interface to time.Time, +// interpreting inputs without a timezone to be in the given location. +// To fall back to the local timezone, use time.Local as the last argument. func ToTimeInDefaultLocation(i interface{}, location *time.Location) time.Time { v, _ := ToTimeInDefaultLocationE(i, location) return v diff --git a/cast_test.go b/cast_test.go index 828eb05..2c83fdb 100644 --- a/cast_test.go +++ b/cast_test.go @@ -8,10 +8,12 @@ package cast import ( "fmt" "html/template" + "path" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestToUintE(t *testing.T) { @@ -1173,7 +1175,7 @@ func TestIndirectPointers(t *testing.T) { assert.Equal(t, ToInt(z), 13) } -func TestToTimeEE(t *testing.T) { +func TestToTime(t *testing.T) { tests := []struct { input interface{} expect time.Time @@ -1286,147 +1288,123 @@ func TestToDurationE(t *testing.T) { } } -func TestToTime(t *testing.T) { +func TestToTimeWithTimezones(t *testing.T) { + est, err := time.LoadLocation("EST") - if !assert.NoError(t, err) { - return - } + require.NoError(t, err) irn, err := time.LoadLocation("Iran") - if !assert.NoError(t, err) { - return - } + require.NoError(t, err) - swd, err := time.LoadLocation("Europe/Stockholm") - if !assert.NoError(t, err) { - return - } - - // time.Parse*() fns handle the target & local timezones being the same - // differently, so make sure we use one of the timezones as local by - // temporarily change it. - if !locationEqual(time.Local, swd) { - var originalLocation *time.Location - originalLocation, time.Local = time.Local, swd - defer func() { - time.Local = originalLocation - }() - } + require.NoError(t, err) // Test same local time in different timezones - utc2016 := time.Date(2016, time.January, 1, 0, 0, 0, 0, time.UTC) - est2016 := time.Date(2016, time.January, 1, 0, 0, 0, 0, est) - irn2016 := time.Date(2016, time.January, 1, 0, 0, 0, 0, irn) - swd2016 := time.Date(2016, time.January, 1, 0, 0, 0, 0, swd) + utc2016 := time.Date(2016, time.January, 1, 3, 1, 0, 0, time.UTC) + est2016 := time.Date(2016, time.January, 1, 3, 1, 0, 0, est) + irn2016 := time.Date(2016, time.January, 1, 3, 1, 0, 0, irn) + loc2016 := time.Date(2016, time.January, 1, 3, 1, 0, 0, time.Local) - for _, format := range timeFormats { - t.Logf("Checking time format '%s', has timezone: %v", format.format, format.hasTimezone) - - est2016str := est2016.Format(format.format) - if !assert.NotEmpty(t, est2016str) { + for i, format := range timeFormats { + format := format + if format.typ == timeFormatShort { continue } - swd2016str := swd2016.Format(format.format) - if !assert.NotEmpty(t, swd2016str) { - continue - } + nameBase := fmt.Sprintf("%d;timeFormatType=%d;%s", i, format.typ, format.format) - // Test conversion without a default location - converted, err := ToTimeE(est2016str) - if assert.NoError(t, err) { - if format.hasTimezone { - // Converting inputs with a timezone should preserve it - assertTimeEqual(t, est2016, converted) - assertLocationEqual(t, est, converted.Location()) - } else { - // Converting inputs without a timezone should be interpreted - // as a local time in UTC. - assertTimeEqual(t, utc2016, converted) - assertLocationEqual(t, time.UTC, converted.Location()) - } - } + t.Run(path.Join(nameBase), func(t *testing.T) { + est2016str := est2016.Format(format.format) + loc2016str := loc2016.Format(format.format) - // Test conversion of a time in the local timezone without a default - // location - converted, err = ToTimeE(swd2016str) - if assert.NoError(t, err) { - if format.hasTimezone { - // Converting inputs with a timezone should preserve it - assertTimeEqual(t, swd2016, converted) - assertLocationEqual(t, swd, converted.Location()) - } else { - // Converting inputs without a timezone should be interpreted - // as a local time in UTC. - assertTimeEqual(t, utc2016, converted) - assertLocationEqual(t, time.UTC, converted.Location()) - } - } + t.Run("without default location", func(t *testing.T) { + assert := require.New(t) + converted, err := ToTimeE(est2016str) + assert.NoError(err) + if format.hasNumericTimezone() { + assertTimeEqual(t, est2016, converted) + assertLocationEqual(t, est, converted.Location()) + } else { + assertTimeEqual(t, utc2016, converted) + assertLocationEqual(t, time.UTC, converted.Location()) + } + }) - // Conversion with a nil default location sould have same behavior - converted, err = ToTimeInDefaultLocationE(est2016str, nil) - if assert.NoError(t, err) { - if format.hasTimezone { - // Converting inputs with a timezone should preserve it - assertTimeEqual(t, est2016, converted) - assertLocationEqual(t, est, converted.Location()) - } else { - // Converting inputs without a timezone should be interpreted - // as a local time in the local timezone. - assertTimeEqual(t, swd2016, converted) - assertLocationEqual(t, swd, converted.Location()) - } - } + t.Run("local timezone without a default location", func(t *testing.T) { + assert := require.New(t) + converted, err := ToTimeE(loc2016str) + assert.NoError(err) + if format.hasAnyTimezone() { + // Local timezone strings can be either named or numeric and + // time.Parse connects the dots. + assertTimeEqual(t, loc2016, converted) + assertLocationEqual(t, time.Local, converted.Location()) + } else { + assertTimeEqual(t, utc2016, converted) + assertLocationEqual(t, time.UTC, converted.Location()) + } + }) - // Test conversion with a default location that isn't UTC - converted, err = ToTimeInDefaultLocationE(est2016str, irn) - if assert.NoError(t, err) { - if format.hasTimezone { - // Converting inputs with a timezone should preserve it - assertTimeEqual(t, est2016, converted) - assertLocationEqual(t, est, converted.Location()) - } else { - // Converting inputs without a timezone should be interpreted - // as a local time in the given location. - assertTimeEqual(t, irn2016, converted) - assertLocationEqual(t, irn, converted.Location()) - } - } + t.Run("nil default location", func(t *testing.T) { + assert := require.New(t) - // Test conversion of a time in the local timezone with a default - // location that isn't UTC - converted, err = ToTimeInDefaultLocationE(swd2016str, irn) - if assert.NoError(t, err) { - if format.hasTimezone { - // Converting inputs with a timezone should preserve it - assertTimeEqual(t, swd2016, converted) - assertLocationEqual(t, swd, converted.Location()) - } else { - // Converting inputs without a timezone should be interpreted - // as a local time in the given location. - assertTimeEqual(t, irn2016, converted) - assertLocationEqual(t, irn, converted.Location()) - } - } + converted, err := ToTimeInDefaultLocationE(est2016str, nil) + assert.NoError(err) + if format.hasNumericTimezone() { + assertTimeEqual(t, est2016, converted) + assertLocationEqual(t, est, converted.Location()) + } else { + assertTimeEqual(t, utc2016, converted) + assertLocationEqual(t, time.UTC, converted.Location()) + } + + }) + + t.Run("default location not UTC", func(t *testing.T) { + assert := require.New(t) + + converted, err := ToTimeInDefaultLocationE(est2016str, irn) + assert.NoError(err) + if format.hasNumericTimezone() { + assertTimeEqual(t, est2016, converted) + assertLocationEqual(t, est, converted.Location()) + } else { + assertTimeEqual(t, irn2016, converted) + assertLocationEqual(t, irn, converted.Location()) + } + }) + + t.Run("time in the local timezone default location not UTC", func(t *testing.T) { + assert := require.New(t) + + converted, err := ToTimeInDefaultLocationE(loc2016str, irn) + assert.NoError(err) + + if format.hasNumericTimezone() { + assertTimeEqual(t, loc2016, converted) + assertLocationEqual(t, time.Local, converted.Location()) + } else { + assertTimeEqual(t, irn2016, converted) + assertLocationEqual(t, irn, converted.Location()) + } + }) + }) } } -func assertTimeEqual(t *testing.T, expected, actual time.Time, msgAndArgs ...interface{}) bool { - if !expected.Equal(actual) { - return assert.Fail(t, fmt.Sprintf("Expected time '%s', got '%s'", expected, actual), msgAndArgs...) - } - return true +func assertTimeEqual(t *testing.T, expected, actual time.Time) { + t.Helper() + require.True(t, expected.Equal(actual), fmt.Sprintf("expected\n%s\ngot\n%s", expected, actual)) + format := "2006-01-02 15:04:05.999999999 -0700" + require.Equal(t, expected.Format(format), actual.Format(format)) } -func assertLocationEqual(t *testing.T, expected, actual *time.Location, msgAndArgs ...interface{}) bool { - if !locationEqual(expected, actual) { - return assert.Fail(t, fmt.Sprintf("Expected location '%s', got '%s'", expected, actual), msgAndArgs...) - } - return true +func assertLocationEqual(t *testing.T, expected, actual *time.Location) { + t.Helper() + require.True(t, locationEqual(expected, actual), fmt.Sprintf("Expected location '%s', got '%s'", expected, actual)) } func locationEqual(a, b *time.Location) bool { - // A note about comparring time.Locations: + // A note about comparing time.Locations: // - can't only compare pointers // - can't compare loc.String() because locations with the same // name can have different offsets diff --git a/caste.go b/caste.go index c3e4203..3ccbc7f 100644 --- a/caste.go +++ b/caste.go @@ -20,12 +20,12 @@ var errNegativeNotAllowed = errors.New("unable to cast negative value") // ToTimeE casts an interface to a time.Time type. func ToTimeE(i interface{}) (tim time.Time, err error) { - return ToTimeInDefaultLocationE(i, time.UTC) + return ToTimeInDefaultLocationE(i, nil) } // ToTimeInDefaultLocationE casts an empty interface to time.Time, -// interpreting inputs without a timezone to be in the given location, -// or the local timezone if nil. +// interpreting inputs without a timezone to be in the given location. +// To fall back to the local timezone, use time.Local as the last argument. func ToTimeInDefaultLocationE(i interface{}, location *time.Location) (tim time.Time, err error) { i = indirect(i) @@ -1211,66 +1211,31 @@ func ToDurationSliceE(i interface{}) ([]time.Duration, error) { // predefined list of formats. If no suitable format is found, an error is // returned. func StringToDate(s string) (time.Time, error) { - return StringToDateInDefaultLocation(s, time.UTC) + return parseDateWith(s, nil, timeFormats) } -// StringToDateInDefaultLocation casts an empty interface to a time.Time, -// interpreting inputs without a timezone to be in the given location, -// or the local timezone if nil. +// StringToDateInDefaultLocation to parse a string into a time.Time type using a +// predefined list of formats, interpreting inputs without a timezone to be in +// the given location. +// To fall back to the local timezone, use time.Local as the last argument. func StringToDateInDefaultLocation(s string, location *time.Location) (time.Time, error) { - if location == nil { - location = time.Local - } return parseDateWith(s, location, timeFormats) } -type timeFormat struct { - format string - hasTimezone bool -} - -var ( - timeFormats = []timeFormat{ - timeFormat{time.RFC3339, true}, - timeFormat{"2006-01-02T15:04:05", false}, // iso8601 without timezone - timeFormat{time.RFC1123Z, true}, - timeFormat{time.RFC1123, false}, - timeFormat{time.RFC822Z, true}, - timeFormat{time.RFC822, false}, - - timeFormat{time.RFC850, true}, - timeFormat{"2006-01-02 15:04:05.999999999 -0700 MST", true}, // Time.String() - timeFormat{"2006-01-02T15:04:05-0700", true}, // RFC3339 without timezone hh:mm colon - timeFormat{"2006-01-02 15:04:05Z0700", true}, // RFC3339 without T or timezone hh:mm colon - timeFormat{"2006-01-02 15:04:05", false}, - - timeFormat{time.ANSIC, false}, - timeFormat{time.UnixDate, false}, - timeFormat{time.RubyDate, true}, - timeFormat{"2006-01-02 15:04:05Z07:00", true}, - timeFormat{"2006-01-02", false}, - timeFormat{"02 Jan 2006", false}, - timeFormat{"2006-01-02 15:04:05 -07:00", true}, - timeFormat{"2006-01-02 15:04:05 -0700", true}, - timeFormat{time.Kitchen, false}, - timeFormat{time.Stamp, false}, - timeFormat{time.StampMilli, false}, - timeFormat{time.StampMicro, false}, - timeFormat{time.StampNano, false}, - } -) - -func parseDateWith(s string, defaultLocation *time.Location, formats []timeFormat) (d time.Time, e error) { +func parseDateWith(s string, location *time.Location, formats []timeFormat) (d time.Time, e error) { for _, format := range formats { if d, e = time.Parse(format.format, s); e == nil { // Some time formats have a zone name, but no offset, so it gets // put in that zone name (not the default one passed in to us), but // without that zone's offset. So set the location manually. - if !format.hasTimezone && defaultLocation != nil { + // Note that we only do this when we get a location in the new *InDefaultLocation + // variants to avoid breaking existing behaviour in ToTime, however + // weird that existing behaviour may be. + if location != nil && !format.hasNumericTimezone() { year, month, day := d.Date() hour, min, sec := d.Clock() - d = time.Date(year, month, day, hour, min, sec, d.Nanosecond(), defaultLocation) + d = time.Date(year, month, day, hour, min, sec, d.Nanosecond(), location) } return @@ -1279,6 +1244,64 @@ func parseDateWith(s string, defaultLocation *time.Location, formats []timeForma return d, fmt.Errorf("unable to parse date: %s", s) } +type timeFormatType int + +const ( + timeFormatShort timeFormatType = iota // time or date only, no timezone + timeFormatNoTimezone + + // All below have some kind of timezone information, a name and/or offset. + timeFormatNamedTimezone + + // All below have what we consider to be solid timezone information. + timeFormatNumericAndNamedTimezone + timeFormatNumericTimezone +) + +type timeFormat struct { + format string + typ timeFormatType +} + +func (f timeFormat) hasNumericTimezone() bool { + return f.typ >= timeFormatNumericAndNamedTimezone +} + +func (f timeFormat) hasAnyTimezone() bool { + return f.typ >= timeFormatNamedTimezone +} + +var ( + timeFormats = []timeFormat{ + {time.RFC3339, timeFormatNumericTimezone}, + {"2006-01-02T15:04:05", timeFormatNoTimezone}, // iso8601 without timezone + {time.RFC1123Z, timeFormatNumericTimezone}, + {time.RFC1123, timeFormatNamedTimezone}, + {time.RFC822Z, timeFormatNumericTimezone}, + {time.RFC822, timeFormatNamedTimezone}, + {time.RFC850, timeFormatNamedTimezone}, + {"2006-01-02 15:04:05.999999999 -0700 MST", timeFormatNumericAndNamedTimezone}, // Time.String() + {"2006-01-02T15:04:05-0700", timeFormatNumericTimezone}, // RFC3339 without timezone hh:mm colon + {"2006-01-02 15:04:05Z0700", timeFormatNumericTimezone}, // RFC3339 without T or timezone hh:mm colon + {"2006-01-02 15:04:05", timeFormatNoTimezone}, + {time.ANSIC, timeFormatNoTimezone}, + // Must try RubyDate before UnixDate, see: + // https://github.com/golang/go/issues/32358 + {time.RubyDate, timeFormatNumericTimezone}, + {time.UnixDate, timeFormatNamedTimezone}, + {"2006-01-02 15:04:05Z07:00", timeFormatNumericTimezone}, + {"2006-01-02", timeFormatShort}, + {"02 Jan 2006", timeFormatShort}, + {"2006-01-02 15:04:05 -07:00", timeFormatNumericTimezone}, + {"2006-01-02 15:04:05 -0700", timeFormatNumericTimezone}, + {time.Kitchen, timeFormatShort}, + {time.Stamp, timeFormatShort}, + {time.StampMilli, timeFormatShort}, + {time.StampMicro, timeFormatShort}, + {time.StampNano, timeFormatShort}, + } +) + // jsonStringToObject attempts to unmarshall a string as JSON into // the object passed as pointer. func jsonStringToObject(s string, v interface{}) error {