Skip to content

Commit

Permalink
time: quote original value in errors returned by ParseDuration
Browse files Browse the repository at this point in the history 
Quote original values passed as substring of ParseError.Message.
Improves the user experience of ParseDuration by making it
quote its original argument, for example:

   _, err := time.ParseDuration("for breakfast")
 will now produce an error, which when printed out is:

  time: invalid duration "for breakfast"
 instead of:

  time: invalid duration for breakfast

Adapt test cases for format.Parse and format.ParseDuration.

Fixes #38295

Change-Id: Ife322c8f3c859e1e4e8dd546d4cf0d519b4bfa81
Reviewed-on: https://go-review.googlesource.com/c/go/+/227878
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
  • Loading branch information
@obeyda @ianlancetaylor
obeyda authored and ianlancetaylor committed Apr 14, 2020
1 parent 5a706d1 commit 201cb04
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 73 deletions.
2 changes: 1 addition & 1 deletion src/time/example_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ func ExampleParse() {
// 2013-02-03 00:00:00 +0000 UTC
// 2006-01-02 15:04:05 +0000 UTC
// 2006-01-02 15:04:05 +0700 +0700
// error parsing time "2006-01-02T15:04:05Z07:00": extra text: 07:00
// error parsing time "2006-01-02T15:04:05Z07:00": extra text: "07:00"
}

func ExampleParseInLocation() {
Expand Down
20 changes: 10 additions & 10 deletions src/time/format.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -856,7 +856,7 @@ func parse(layout, value string, defaultLocation, local *Location) (Time, error)
}
if std == 0 {
if len(value) != 0 {
return Time{}, &ParseError{alayout, avalue, "", value, ": extra text: " + value}
return Time{}, &ParseError{alayout, avalue, "", value, ": extra text: " + quote(value)}
}
break
}
Expand Down Expand Up @@ -1390,7 +1390,7 @@ func ParseDuration(s string) (Duration, error) {
return 0, nil
}
if s == "" {
return 0, errors.New("time: invalid duration " + orig)
return 0, errors.New("time: invalid duration " + quote(orig))
}
for s != "" {
var (
Expand All @@ -1402,13 +1402,13 @@ func ParseDuration(s string) (Duration, error) {

// The next character must be [0-9.]
if !(s[0] == '.' || '0' <= s[0] && s[0] <= '9') {
return 0, errors.New("time: invalid duration " + orig)
return 0, errors.New("time: invalid duration " + quote(orig))
}
// Consume [0-9]*
pl := len(s)
v, s, err = leadingInt(s)
if err != nil {
return 0, errors.New("time: invalid duration " + orig)
return 0, errors.New("time: invalid duration " + quote(orig))
}
pre := pl != len(s) // whether we consumed anything before a period

Expand All @@ -1422,7 +1422,7 @@ func ParseDuration(s string) (Duration, error) {
}
if !pre && !post {
// no digits (e.g. ".s" or "-.s")
return 0, errors.New("time: invalid duration " + orig)
return 0, errors.New("time: invalid duration " + quote(orig))
}

// Consume unit.
Expand All @@ -1434,17 +1434,17 @@ func ParseDuration(s string) (Duration, error) {
}
}
if i == 0 {
return 0, errors.New("time: missing unit in duration " + orig)
return 0, errors.New("time: missing unit in duration " + quote(orig))
}
u := s[:i]
s = s[i:]
unit, ok := unitMap[u]
if !ok {
return 0, errors.New("time: unknown unit " + u + " in duration " + orig)
return 0, errors.New("time: unknown unit " + quote(u) + " in duration " + quote(orig))
}
if v > (1<<63-1)/unit {
// overflow
return 0, errors.New("time: invalid duration " + orig)
return 0, errors.New("time: invalid duration " + quote(orig))
}
v *= unit
if f > 0 {
Expand All @@ -1453,13 +1453,13 @@ func ParseDuration(s string) (Duration, error) {
v += int64(float64(f) * (float64(unit) / scale))
if v < 0 {
// overflow
return 0, errors.New("time: invalid duration " + orig)
return 0, errors.New("time: invalid duration " + quote(orig))
}
}
d += v
if d < 0 {
// overflow
return 0, errors.New("time: invalid duration " + orig)
return 0, errors.New("time: invalid duration " + quote(orig))
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/time/format_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -536,12 +536,12 @@ var parseErrorTests = []ParseErrorTest{
{"Mon Jan _2 15:04:05.000 2006", "Thu Feb 4 23:00:59.-123 2010", "fractional second out of range"},
// issue 4502. StampNano requires exactly 9 digits of precision.
{StampNano, "Dec 7 11:22:01.000000", `cannot parse ".000000" as ".000000000"`},
{StampNano, "Dec 7 11:22:01.0000000000", "extra text: 0"},
{StampNano, "Dec 7 11:22:01.0000000000", `extra text: "0"`},
// issue 4493. Helpful errors.
{RFC3339, "2006-01-02T15:04:05Z07:00", `parsing time "2006-01-02T15:04:05Z07:00": extra text: 07:00`},
{RFC3339, "2006-01-02T15:04:05Z07:00", `parsing time "2006-01-02T15:04:05Z07:00": extra text: "07:00"`},
{RFC3339, "2006-01-02T15:04_abc", `parsing time "2006-01-02T15:04_abc" as "2006-01-02T15:04:05Z07:00": cannot parse "_abc" as ":"`},
{RFC3339, "2006-01-02T15:04:05_abc", `parsing time "2006-01-02T15:04:05_abc" as "2006-01-02T15:04:05Z07:00": cannot parse "_abc" as "Z07:00"`},
{RFC3339, "2006-01-02T15:04:05Z_abc", `parsing time "2006-01-02T15:04:05Z_abc": extra text: _abc`},
{RFC3339, "2006-01-02T15:04:05Z_abc", `parsing time "2006-01-02T15:04:05Z_abc": extra text: "_abc"`},
// invalid second followed by optional fractional seconds
{RFC3339, "2010-02-04T21:00:67.012345678-08:00", "second out of range"},
// issue 21113
Expand Down
135 changes: 76 additions & 59 deletions src/time/time_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -810,86 +810,103 @@ func TestNotJSONEncodableTime(t *testing.T) {

var parseDurationTests = []struct {
in string
ok bool
want Duration
}{
// simple
{"0", true, 0},
{"5s", true, 5 * Second},
{"30s", true, 30 * Second},
{"1478s", true, 1478 * Second},
{"0", 0},
{"5s", 5 * Second},
{"30s", 30 * Second},
{"1478s", 1478 * Second},
// sign
{"-5s", true, -5 * Second},
{"+5s", true, 5 * Second},
{"-0", true, 0},
{"+0", true, 0},
{"-5s", -5 * Second},
{"+5s", 5 * Second},
{"-0", 0},
{"+0", 0},
// decimal
{"5.0s", true, 5 * Second},
{"5.6s", true, 5*Second + 600*Millisecond},
{"5.s", true, 5 * Second},
{".5s", true, 500 * Millisecond},
{"1.0s", true, 1 * Second},
{"1.00s", true, 1 * Second},
{"1.004s", true, 1*Second + 4*Millisecond},
{"1.0040s", true, 1*Second + 4*Millisecond},
{"100.00100s", true, 100*Second + 1*Millisecond},
{"5.0s", 5 * Second},
{"5.6s", 5*Second + 600*Millisecond},
{"5.s", 5 * Second},
{".5s", 500 * Millisecond},
{"1.0s", 1 * Second},
{"1.00s", 1 * Second},
{"1.004s", 1*Second + 4*Millisecond},
{"1.0040s", 1*Second + 4*Millisecond},
{"100.00100s", 100*Second + 1*Millisecond},
// different units
{"10ns", true, 10 * Nanosecond},
{"11us", true, 11 * Microsecond},
{"12µs", true, 12 * Microsecond}, // U+00B5
{"12μs", true, 12 * Microsecond}, // U+03BC
{"13ms", true, 13 * Millisecond},
{"14s", true, 14 * Second},
{"15m", true, 15 * Minute},
{"16h", true, 16 * Hour},
{"10ns", 10 * Nanosecond},
{"11us", 11 * Microsecond},
{"12µs", 12 * Microsecond}, // U+00B5
{"12μs", 12 * Microsecond}, // U+03BC
{"13ms", 13 * Millisecond},
{"14s", 14 * Second},
{"15m", 15 * Minute},
{"16h", 16 * Hour},
// composite durations
{"3h30m", true, 3*Hour + 30*Minute},
{"10.5s4m", true, 4*Minute + 10*Second + 500*Millisecond},
{"-2m3.4s", true, -(2*Minute + 3*Second + 400*Millisecond)},
{"1h2m3s4ms5us6ns", true, 1*Hour + 2*Minute + 3*Second + 4*Millisecond + 5*Microsecond + 6*Nanosecond},
{"39h9m14.425s", true, 39*Hour + 9*Minute + 14*Second + 425*Millisecond},
{"3h30m", 3*Hour + 30*Minute},
{"10.5s4m", 4*Minute + 10*Second + 500*Millisecond},
{"-2m3.4s", -(2*Minute + 3*Second + 400*Millisecond)},
{"1h2m3s4ms5us6ns", 1*Hour + 2*Minute + 3*Second + 4*Millisecond + 5*Microsecond + 6*Nanosecond},
{"39h9m14.425s", 39*Hour + 9*Minute + 14*Second + 425*Millisecond},
// large value
{"52763797000ns", true, 52763797000 * Nanosecond},
{"52763797000ns", 52763797000 * Nanosecond},
// more than 9 digits after decimal point, see https://golang.org/issue/6617
{"0.3333333333333333333h", true, 20 * Minute},
{"0.3333333333333333333h", 20 * Minute},
// 9007199254740993 = 1<<53+1 cannot be stored precisely in a float64
{"9007199254740993ns", true, (1<<53 + 1) * Nanosecond},
{"9007199254740993ns", (1<<53 + 1) * Nanosecond},
// largest duration that can be represented by int64 in nanoseconds
{"9223372036854775807ns", true, (1<<63 - 1) * Nanosecond},
{"9223372036854775.807us", true, (1<<63 - 1) * Nanosecond},
{"9223372036s854ms775us807ns", true, (1<<63 - 1) * Nanosecond},
{"9223372036854775807ns", (1<<63 - 1) * Nanosecond},
{"9223372036854775.807us", (1<<63 - 1) * Nanosecond},
{"9223372036s854ms775us807ns", (1<<63 - 1) * Nanosecond},
// large negative value
{"-9223372036854775807ns", true, -1<<63 + 1*Nanosecond},
{"-9223372036854775807ns", -1<<63 + 1*Nanosecond},
// huge string; issue 15011.
{"0.100000000000000000000h", true, 6 * Minute},
{"0.100000000000000000000h", 6 * Minute},
// This value tests the first overflow check in leadingFraction.
{"0.830103483285477580700h", true, 49*Minute + 48*Second + 372539827*Nanosecond},

// errors
{"", false, 0},
{"3", false, 0},
{"-", false, 0},
{"s", false, 0},
{".", false, 0},
{"-.", false, 0},
{".s", false, 0},
{"+.s", false, 0},
{"3000000h", false, 0}, // overflow
{"9223372036854775808ns", false, 0}, // overflow
{"9223372036854775.808us", false, 0}, // overflow
{"9223372036854ms775us808ns", false, 0}, // overflow
// largest negative value of type int64 in nanoseconds should fail
// see https://go-review.googlesource.com/#/c/2461/
{"-9223372036854775808ns", false, 0},
{"0.830103483285477580700h", 49*Minute + 48*Second + 372539827*Nanosecond},
}

func TestParseDuration(t *testing.T) {
for _, tc := range parseDurationTests {
d, err := ParseDuration(tc.in)
if tc.ok && (err != nil || d != tc.want) {
if err != nil || d != tc.want {
t.Errorf("ParseDuration(%q) = %v, %v, want %v, nil", tc.in, d, err, tc.want)
} else if !tc.ok && err == nil {
}
}
}

var parseDurationErrorTests = []struct {
in string
expect string
}{
// invalid
{"", `""`},
{"3", `"3"`},
{"-", `"-"`},
{"s", `"s"`},
{".", `"."`},
{"-.", `"-."`},
{".s", `".s"`},
{"+.s", `"+.s"`},
{"1d", `"1d"`},
// overflow
{"9223372036854775810ns", `"9223372036854775810ns"`},
{"9223372036854775808ns", `"9223372036854775808ns"`},
// largest negative value of type int64 in nanoseconds should fail
// see https://go-review.googlesource.com/#/c/2461/
{"-9223372036854775808ns", `"-9223372036854775808ns"`},
{"9223372036854776us", `"9223372036854776us"`},
{"3000000h", `"3000000h"`},
{"9223372036854775.808us", `"9223372036854775.808us"`},
{"9223372036854ms775us808ns", `"9223372036854ms775us808ns"`},
}

func TestParseDurationErrors(t *testing.T) {
for _, tc := range parseDurationErrorTests {
_, err := ParseDuration(tc.in)
if err == nil {
t.Errorf("ParseDuration(%q) = _, nil, want _, non-nil", tc.in)
} else if !strings.Contains(err.Error(), tc.expect) {
t.Errorf("ParseDuration(%q) = _, %q, error does not contain %q", tc.in, err, tc.expect)
}
}
}
Expand Down

0 comments on commit 201cb04

Please sign in to comment.