encoding/json/v2: report error on time.Duration without explicit format

The default representation of a time.Duration is still undecided.
In order to keep the future open, report an error on a time.Duration
without an explicit format flag provided.

Updates #71631

Change-Id: I08248404ff6551723851417c8188a13f53c61937
Reviewed-on: https://go-review.googlesource.com/c/go/+/682455
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
This commit is contained in:
Joe Tsai
2025-06-17 12:34:22 -07:00
committed by Gopher Robot
parent f866958246
commit ed7815726d
4 changed files with 39 additions and 25 deletions

View File

@@ -365,7 +365,7 @@ type (
Interface any `json:",omitzero,format:invalid"` Interface any `json:",omitzero,format:invalid"`
} }
structDurationFormat struct { structDurationFormat struct {
D1 time.Duration D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
D2 time.Duration `json:",format:units"` D2 time.Duration `json:",format:units"`
D3 time.Duration `json:",format:sec"` D3 time.Duration `json:",format:sec"`
D4 time.Duration `json:",string,format:sec"` D4 time.Duration `json:",string,format:sec"`
@@ -4312,14 +4312,14 @@ func TestMarshal(t *testing.T) {
}, { }, {
name: jsontest.Name("Duration/Zero"), name: jsontest.Name("Duration/Zero"),
in: struct { in: struct {
D1 time.Duration D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
D2 time.Duration `json:",format:nano"` D2 time.Duration `json:",format:nano"`
}{0, 0}, }{0, 0},
want: `{"D1":"0s","D2":0}`, want: `{"D1":"0s","D2":0}`,
}, { }, {
name: jsontest.Name("Duration/Positive"), name: jsontest.Name("Duration/Positive"),
in: struct { in: struct {
D1 time.Duration D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
D2 time.Duration `json:",format:nano"` D2 time.Duration `json:",format:nano"`
}{ }{
123456789123456789, 123456789123456789,
@@ -4329,7 +4329,7 @@ func TestMarshal(t *testing.T) {
}, { }, {
name: jsontest.Name("Duration/Negative"), name: jsontest.Name("Duration/Negative"),
in: struct { in: struct {
D1 time.Duration D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
D2 time.Duration `json:",format:nano"` D2 time.Duration `json:",format:nano"`
}{ }{
-123456789123456789, -123456789123456789,
@@ -4356,11 +4356,12 @@ func TestMarshal(t *testing.T) {
want: `{"D"`, want: `{"D"`,
wantErr: EM(errInvalidFormatFlag).withPos(`{"D":`, "/D").withType(0, T[time.Duration]()), wantErr: EM(errInvalidFormatFlag).withPos(`{"D":`, "/D").withType(0, T[time.Duration]()),
}, { }, {
/* TODO(https://go.dev/issue/71631): Re-enable this test case.
name: jsontest.Name("Duration/IgnoreInvalidFormat"), name: jsontest.Name("Duration/IgnoreInvalidFormat"),
opts: []Options{invalidFormatOption}, opts: []Options{invalidFormatOption},
in: time.Duration(0), in: time.Duration(0),
want: `"0s"`, want: `"0s"`,
}, { }, { */
name: jsontest.Name("Duration/Format"), name: jsontest.Name("Duration/Format"),
opts: []Options{jsontext.Multiline(true)}, opts: []Options{jsontext.Multiline(true)},
in: structDurationFormat{ in: structDurationFormat{
@@ -4388,6 +4389,7 @@ func TestMarshal(t *testing.T) {
"D10": "45296078090012" "D10": "45296078090012"
}`, }`,
}, { }, {
/* TODO(https://go.dev/issue/71631): Re-enable this test case.
name: jsontest.Name("Duration/Format/Legacy"), name: jsontest.Name("Duration/Format/Legacy"),
opts: []Options{jsonflags.FormatTimeWithLegacySemantics | 1}, opts: []Options{jsonflags.FormatTimeWithLegacySemantics | 1},
in: structDurationFormat{ in: structDurationFormat{
@@ -4395,11 +4397,12 @@ func TestMarshal(t *testing.T) {
D2: 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, D2: 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond,
}, },
want: `{"D1":45296078090012,"D2":"12h34m56.078090012s","D3":0,"D4":"0","D5":0,"D6":"0","D7":0,"D8":"0","D9":0,"D10":"0"}`, want: `{"D1":45296078090012,"D2":"12h34m56.078090012s","D3":0,"D4":"0","D5":0,"D6":"0","D7":0,"D8":"0","D9":0,"D10":"0"}`,
}, { }, { */
/* TODO(https://go.dev/issue/71631): Re-enable this test case.
name: jsontest.Name("Duration/MapKey"), name: jsontest.Name("Duration/MapKey"),
in: map[time.Duration]string{time.Second: ""}, in: map[time.Duration]string{time.Second: ""},
want: `{"1s":""}`, want: `{"1s":""}`,
}, { }, { */
name: jsontest.Name("Duration/MapKey/Legacy"), name: jsontest.Name("Duration/MapKey/Legacy"),
opts: []Options{jsonflags.FormatTimeWithLegacySemantics | 1}, opts: []Options{jsonflags.FormatTimeWithLegacySemantics | 1},
in: map[time.Duration]string{time.Second: ""}, in: map[time.Duration]string{time.Second: ""},
@@ -8713,33 +8716,33 @@ func TestUnmarshal(t *testing.T) {
name: jsontest.Name("Duration/Null"), name: jsontest.Name("Duration/Null"),
inBuf: `{"D1":null,"D2":null}`, inBuf: `{"D1":null,"D2":null}`,
inVal: addr(struct { inVal: addr(struct {
D1 time.Duration D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
D2 time.Duration `json:",format:nano"` D2 time.Duration `json:",format:nano"`
}{1, 1}), }{1, 1}),
want: addr(struct { want: addr(struct {
D1 time.Duration D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
D2 time.Duration `json:",format:nano"` D2 time.Duration `json:",format:nano"`
}{0, 0}), }{0, 0}),
}, { }, {
name: jsontest.Name("Duration/Zero"), name: jsontest.Name("Duration/Zero"),
inBuf: `{"D1":"0s","D2":0}`, inBuf: `{"D1":"0s","D2":0}`,
inVal: addr(struct { inVal: addr(struct {
D1 time.Duration D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
D2 time.Duration `json:",format:nano"` D2 time.Duration `json:",format:nano"`
}{1, 1}), }{1, 1}),
want: addr(struct { want: addr(struct {
D1 time.Duration D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
D2 time.Duration `json:",format:nano"` D2 time.Duration `json:",format:nano"`
}{0, 0}), }{0, 0}),
}, { }, {
name: jsontest.Name("Duration/Positive"), name: jsontest.Name("Duration/Positive"),
inBuf: `{"D1":"34293h33m9.123456789s","D2":123456789123456789}`, inBuf: `{"D1":"34293h33m9.123456789s","D2":123456789123456789}`,
inVal: new(struct { inVal: new(struct {
D1 time.Duration D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
D2 time.Duration `json:",format:nano"` D2 time.Duration `json:",format:nano"`
}), }),
want: addr(struct { want: addr(struct {
D1 time.Duration D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
D2 time.Duration `json:",format:nano"` D2 time.Duration `json:",format:nano"`
}{ }{
123456789123456789, 123456789123456789,
@@ -8749,11 +8752,11 @@ func TestUnmarshal(t *testing.T) {
name: jsontest.Name("Duration/Negative"), name: jsontest.Name("Duration/Negative"),
inBuf: `{"D1":"-34293h33m9.123456789s","D2":-123456789123456789}`, inBuf: `{"D1":"-34293h33m9.123456789s","D2":-123456789123456789}`,
inVal: new(struct { inVal: new(struct {
D1 time.Duration D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
D2 time.Duration `json:",format:nano"` D2 time.Duration `json:",format:nano"`
}), }),
want: addr(struct { want: addr(struct {
D1 time.Duration D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
D2 time.Duration `json:",format:nano"` D2 time.Duration `json:",format:nano"`
}{ }{
-123456789123456789, -123456789123456789,
@@ -8801,20 +8804,20 @@ func TestUnmarshal(t *testing.T) {
name: jsontest.Name("Duration/String/Mismatch"), name: jsontest.Name("Duration/String/Mismatch"),
inBuf: `{"D":-123456789123456789}`, inBuf: `{"D":-123456789123456789}`,
inVal: addr(struct { inVal: addr(struct {
D time.Duration D time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
}{1}), }{1}),
want: addr(struct { want: addr(struct {
D time.Duration D time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
}{1}), }{1}),
wantErr: EU(nil).withPos(`{"D":`, "/D").withType('0', timeDurationType), wantErr: EU(nil).withPos(`{"D":`, "/D").withType('0', timeDurationType),
}, { }, {
name: jsontest.Name("Duration/String/Invalid"), name: jsontest.Name("Duration/String/Invalid"),
inBuf: `{"D":"5minkutes"}`, inBuf: `{"D":"5minkutes"}`,
inVal: addr(struct { inVal: addr(struct {
D time.Duration D time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
}{1}), }{1}),
want: addr(struct { want: addr(struct {
D time.Duration D time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
}{1}), }{1}),
wantErr: EU(func() error { wantErr: EU(func() error {
_, err := time.ParseDuration("5minkutes") _, err := time.ParseDuration("5minkutes")
@@ -8824,10 +8827,10 @@ func TestUnmarshal(t *testing.T) {
name: jsontest.Name("Duration/Syntax/Invalid"), name: jsontest.Name("Duration/Syntax/Invalid"),
inBuf: `{"D":x}`, inBuf: `{"D":x}`,
inVal: addr(struct { inVal: addr(struct {
D time.Duration D time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
}{1}), }{1}),
want: addr(struct { want: addr(struct {
D time.Duration D time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag.
}{1}), }{1}),
wantErr: newInvalidCharacterError("x", "at start of value", len64(`{"D":`), "/D"), wantErr: newInvalidCharacterError("x", "at start of value", len64(`{"D":`), "/D"),
}, { }, {
@@ -8841,6 +8844,7 @@ func TestUnmarshal(t *testing.T) {
}{1}), }{1}),
wantErr: EU(errInvalidFormatFlag).withPos(`{"D":`, "/D").withType(0, timeDurationType), wantErr: EU(errInvalidFormatFlag).withPos(`{"D":`, "/D").withType(0, timeDurationType),
}, { }, {
/* TODO(https://go.dev/issue/71631): Re-enable this test case.
name: jsontest.Name("Duration/Format/Legacy"), name: jsontest.Name("Duration/Format/Legacy"),
inBuf: `{"D1":45296078090012,"D2":"12h34m56.078090012s"}`, inBuf: `{"D1":45296078090012,"D2":"12h34m56.078090012s"}`,
opts: []Options{jsonflags.FormatTimeWithLegacySemantics | 1}, opts: []Options{jsonflags.FormatTimeWithLegacySemantics | 1},
@@ -8849,24 +8853,26 @@ func TestUnmarshal(t *testing.T) {
D1: 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, D1: 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond,
D2: 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, D2: 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond,
}), }),
}, { }, { */
/* TODO(https://go.dev/issue/71631): Re-enable this test case.
name: jsontest.Name("Duration/MapKey"), name: jsontest.Name("Duration/MapKey"),
inBuf: `{"1s":""}`, inBuf: `{"1s":""}`,
inVal: new(map[time.Duration]string), inVal: new(map[time.Duration]string),
want: addr(map[time.Duration]string{time.Second: ""}), want: addr(map[time.Duration]string{time.Second: ""}),
}, { }, { */
name: jsontest.Name("Duration/MapKey/Legacy"), name: jsontest.Name("Duration/MapKey/Legacy"),
opts: []Options{jsonflags.FormatTimeWithLegacySemantics | 1}, opts: []Options{jsonflags.FormatTimeWithLegacySemantics | 1},
inBuf: `{"1000000000":""}`, inBuf: `{"1000000000":""}`,
inVal: new(map[time.Duration]string), inVal: new(map[time.Duration]string),
want: addr(map[time.Duration]string{time.Second: ""}), want: addr(map[time.Duration]string{time.Second: ""}),
}, { }, {
/* TODO(https://go.dev/issue/71631): Re-enable this test case.
name: jsontest.Name("Duration/IgnoreInvalidFormat"), name: jsontest.Name("Duration/IgnoreInvalidFormat"),
opts: []Options{invalidFormatOption}, opts: []Options{invalidFormatOption},
inBuf: `"1s"`, inBuf: `"1s"`,
inVal: addr(time.Duration(0)), inVal: addr(time.Duration(0)),
want: addr(time.Second), want: addr(time.Second),
}, { }, { */
name: jsontest.Name("Time/Zero"), name: jsontest.Name("Time/Zero"),
inBuf: `{"T1":"0001-01-01T00:00:00Z","T2":"01 Jan 01 00:00 UTC","T3":"0001-01-01","T4":"0001-01-01T00:00:00Z","T5":"0001-01-01T00:00:00Z"}`, inBuf: `{"T1":"0001-01-01T00:00:00Z","T2":"01 Jan 01 00:00 UTC","T3":"0001-01-01","T4":"0001-01-01T00:00:00Z","T5":"0001-01-01T00:00:00Z"}`,
inVal: new(struct { inVal: new(struct {

View File

@@ -52,6 +52,9 @@ func makeTimeArshaler(fncs *arshaler, t reflect.Type) *arshaler {
} }
} else if mo.Flags.Get(jsonflags.FormatTimeWithLegacySemantics) { } else if mo.Flags.Get(jsonflags.FormatTimeWithLegacySemantics) {
return marshalNano(enc, va, mo) return marshalNano(enc, va, mo)
} else {
// TODO(https://go.dev/issue/71631): Decide on default duration representation.
return newMarshalErrorBefore(enc, t, errors.New("no default representation; specify an explicit format"))
} }
// TODO(https://go.dev/issue/62121): Use reflect.Value.AssertTo. // TODO(https://go.dev/issue/62121): Use reflect.Value.AssertTo.
@@ -75,6 +78,9 @@ func makeTimeArshaler(fncs *arshaler, t reflect.Type) *arshaler {
} }
} else if uo.Flags.Get(jsonflags.FormatTimeWithLegacySemantics) { } else if uo.Flags.Get(jsonflags.FormatTimeWithLegacySemantics) {
return unmarshalNano(dec, va, uo) return unmarshalNano(dec, va, uo)
} else {
// TODO(https://go.dev/issue/71631): Decide on default duration representation.
return newUnmarshalErrorBeforeWithSkipping(dec, uo, t, errors.New("no default representation; specify an explicit format"))
} }
stringify := !u.isNumeric() || xd.Tokens.Last.NeedObjectName() || uo.Flags.Get(jsonflags.StringifyNumbers) stringify := !u.isNumeric() || xd.Tokens.Last.NeedObjectName() || uo.Flags.Get(jsonflags.StringifyNumbers)

View File

@@ -267,12 +267,13 @@ var arshalTestdata = []struct {
new: func() any { return new(jsonArshalerV2) }, new: func() any { return new(jsonArshalerV2) },
skipV1: true, skipV1: true,
}, { }, {
/* TODO(https://go.dev/issue/71631): Re-enable this test case.
name: "Duration", name: "Duration",
raw: []byte(`"1h1m1s"`), raw: []byte(`"1h1m1s"`),
val: addr(time.Hour + time.Minute + time.Second), val: addr(time.Hour + time.Minute + time.Second),
new: func() any { return new(time.Duration) }, new: func() any { return new(time.Duration) },
skipV1: true, skipV1: true,
}, { }, { */
name: "Time", name: "Time",
raw: []byte(`"2006-01-02T22:04:05Z"`), raw: []byte(`"2006-01-02T22:04:05Z"`),
val: addr(time.Unix(1136239445, 0).UTC()), val: addr(time.Unix(1136239445, 0).UTC()),

View File

@@ -1038,6 +1038,7 @@ func TestMergeComposite(t *testing.T) {
// //
// https://go.dev/issue/10275 // https://go.dev/issue/10275
func TestTimeDurations(t *testing.T) { func TestTimeDurations(t *testing.T) {
t.SkipNow() // TODO(https://go.dev/issue/71631): The default representation of time.Duration is still undecided.
for _, json := range jsonPackages { for _, json := range jsonPackages {
t.Run(path.Join("Marshal", json.Version), func(t *testing.T) { t.Run(path.Join("Marshal", json.Version), func(t *testing.T) {
got, err := json.Marshal(time.Minute) got, err := json.Marshal(time.Minute)