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

@@ -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

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 \

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

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
-	}
-
-	swd, err := time.LoadLocation("Europe/Stockholm")
-	if !assert.NoError(t, err) {
-		return
-	}
+	require.NoError(t, err)
 
-	// 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)
-
-	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) {
-			continue
-		}
-
-		swd2016str := swd2016.Format(format.format)
-		if !assert.NotEmpty(t, swd2016str) {
+	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 i, format := range timeFormats {
+		format := format
+		if format.typ == timeFormatShort {
 			continue
 		}
 
-		// 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())
-			}
-		}
-
-		// 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())
-			}
-		}
-
-		// 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())
-			}
-		}
-
-		// 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())
-			}
-		}
-
-		// 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())
-			}
-		}
+		nameBase := fmt.Sprintf("%d;timeFormatType=%d;%s", i, format.typ, format.format)
+
+		t.Run(path.Join(nameBase), func(t *testing.T) {
+			est2016str := est2016.Format(format.format)
+			loc2016str := loc2016.Format(format.format)
+
+			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())
+				}
+			})
+
+			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())
+				}
+			})
+
+			t.Run("nil default location", func(t *testing.T) {
+				assert := require.New(t)
+
+				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

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 {