encoding/json: decompose legacy options

WARNING: This commit contains breaking changes
for those already using GOEXPERIMENT=jsonv2.

This decomposes FormatBytesWithLegacySemantics as:
* FormatBytesWithLegacySemantics
* FormatByteArrayAsArray
* ParseBytesWithLooseRFC4648

This decomposes FormatTimeWithLegacySemantics as:
* FormatDurationAsNano
* ParseTimeWithLooseRFC3339

In particular, it splits out specific behaviors from the option
that may need to be specified on a finer-grain level.

FormatByteArrayAsArray and FormatDurationAsNano are targeted
to just the default representation of a [N]byte or time.Duration type.
Both of these are not necessary if the `format` tag is explicitly specified.
However, we want to isolate their behavior from other behaviors that used to
be part of FormatBytesWithLegacySemantics and FormatTimeWithLegacySemantics.

ParseBytesWithLooseRFC4648 and ParseTimeWithLooseRFC3339 are targeted
to just historically buggy parsing according to the relevant RFCs,
which may need to be enabled by some services for backwards compatibility.

While FormatTimeWithLegacySemantics is deleted, we still need
FormatBytesWithLegacySemantics to configure highly esoteric
aspects of how v1 used to handle byte slices.

We rename OmitEmptyWithLegacyDefinition as OmitEmptyWithLegacySemantics
to be consistent with other options with the WithLegacySemantics suffix.

Updates #71497

Change-Id: Ic660515fb086fe3af237135f195736de99c2bd33
Reviewed-on: https://go-review.googlesource.com/c/go/+/685395
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>

3 files changed, 21 insertions, 19 deletions

diff --git a/src/encoding/json/v2/arshal_default.go b/src/encoding/json/v2/arshal_default.go
index 5ca51c6635..0b30ac4fb7 100644
--- a/src/encoding/json/v2/arshal_default.go
+++ b/src/encoding/json/v2/arshal_default.go
@@ -329,8 +329,9 @@ func makeBytesArshaler(t reflect.Type, fncs *arshaler) *arshaler {
 			default:
 				return newInvalidFormatError(enc, t, mo)
 			}
-		} else if mo.Flags.Get(jsonflags.FormatBytesWithLegacySemantics) &&
-			(va.Kind() == reflect.Array || hasMarshaler) {
+		} else if mo.Flags.Get(jsonflags.FormatByteArrayAsArray) && va.Kind() == reflect.Array {
+			return marshalArray(enc, va, mo)
+		} else if mo.Flags.Get(jsonflags.FormatBytesWithLegacySemantics) && hasMarshaler {
 			return marshalArray(enc, va, mo)
 		}
 		if mo.Flags.Get(jsonflags.FormatNilSliceAsNull) && va.Kind() == reflect.Slice && va.IsNil() {
@@ -366,8 +367,9 @@ func makeBytesArshaler(t reflect.Type, fncs *arshaler) *arshaler {
 			default:
 				return newInvalidFormatError(dec, t, uo)
 			}
-		} else if uo.Flags.Get(jsonflags.FormatBytesWithLegacySemantics) &&
-			(va.Kind() == reflect.Array || dec.PeekKind() == '[') {
+		} else if uo.Flags.Get(jsonflags.FormatByteArrayAsArray) && va.Kind() == reflect.Array {
+			return unmarshalArray(dec, va, uo)
+		} else if uo.Flags.Get(jsonflags.FormatBytesWithLegacySemantics) && dec.PeekKind() == '[' {
 			return unmarshalArray(dec, va, uo)
 		}
 		var flags jsonwire.ValueFlags
@@ -395,7 +397,7 @@ func makeBytesArshaler(t reflect.Type, fncs *arshaler) *arshaler {
 			if err != nil {
 				return newUnmarshalErrorAfter(dec, t, err)
 			}
-			if len(val) != encodedLen(len(b)) && !uo.Flags.Get(jsonflags.FormatBytesWithLegacySemantics) {
+			if len(val) != encodedLen(len(b)) && !uo.Flags.Get(jsonflags.ParseBytesWithLooseRFC4648) {
 				// TODO(https://go.dev/issue/53845): RFC 4648, section 3.3,
 				// specifies that non-alphabet characters must be rejected.
 				// Unfortunately, the "base32" and "base64" packages allow
@@ -1065,7 +1067,7 @@ func makeStructArshaler(t reflect.Type) *arshaler {
 			}
 
 			// Check for the legacy definition of omitempty.
-			if f.omitempty && mo.Flags.Get(jsonflags.OmitEmptyWithLegacyDefinition) && isLegacyEmpty(v) {
+			if f.omitempty && mo.Flags.Get(jsonflags.OmitEmptyWithLegacySemantics) && isLegacyEmpty(v) {
 				continue
 			}
 
@@ -1080,7 +1082,7 @@ func makeStructArshaler(t reflect.Type) *arshaler {
 			// OmitEmpty skips the field if the marshaled JSON value is empty,
 			// which we can know up front if there are no custom marshalers,
 			// otherwise we must marshal the value and unwrite it if empty.
-			if f.omitempty && !mo.Flags.Get(jsonflags.OmitEmptyWithLegacyDefinition) &&
+			if f.omitempty && !mo.Flags.Get(jsonflags.OmitEmptyWithLegacySemantics) &&
 				!nonDefault && f.isEmpty != nil && f.isEmpty(v) {
 				continue // fast path for omitempty
 			}
@@ -1145,7 +1147,7 @@ func makeStructArshaler(t reflect.Type) *arshaler {
 			}
 
 			// Try unwriting the member if empty (slow path for omitempty).
-			if f.omitempty && !mo.Flags.Get(jsonflags.OmitEmptyWithLegacyDefinition) {
+			if f.omitempty && !mo.Flags.Get(jsonflags.OmitEmptyWithLegacySemantics) {
 				var prevName *string
 				if prevIdx >= 0 {
 					prevName = &fields.flattened[prevIdx].name
diff --git a/src/encoding/json/v2/arshal_test.go b/src/encoding/json/v2/arshal_test.go
index 879a2f3e0d..88887e1b00 100644
--- a/src/encoding/json/v2/arshal_test.go
+++ b/src/encoding/json/v2/arshal_test.go
@@ -1924,12 +1924,12 @@ func TestMarshal(t *testing.T) {
 }`,
 	}, {
 		name: jsontest.Name("Structs/OmitEmpty/Legacy/Zero"),
-		opts: []Options{jsonflags.OmitEmptyWithLegacyDefinition | 1},
+		opts: []Options{jsonflags.OmitEmptyWithLegacySemantics | 1},
 		in:   structOmitEmptyAll{},
 		want: `{}`,
 	}, {
 		name: jsontest.Name("Structs/OmitEmpty/Legacy/NonEmpty"),
-		opts: []Options{jsontext.Multiline(true), jsonflags.OmitEmptyWithLegacyDefinition | 1},
+		opts: []Options{jsontext.Multiline(true), jsonflags.OmitEmptyWithLegacySemantics | 1},
 		in: structOmitEmptyAll{
 			Bool:                  true,
 			PointerBool:           addr(true),
@@ -2144,7 +2144,7 @@ func TestMarshal(t *testing.T) {
 	"Default": "AQIDBA=="
 }`}, {
 		name: jsontest.Name("Structs/Format/ArrayBytes/Legacy"),
-		opts: []Options{jsontext.Multiline(true), jsonflags.FormatBytesWithLegacySemantics | 1},
+		opts: []Options{jsontext.Multiline(true), jsonflags.FormatByteArrayAsArray | jsonflags.FormatBytesWithLegacySemantics | 1},
 		in: structFormatArrayBytes{
 			Base16:    [4]byte{1, 2, 3, 4},
 			Base32:    [4]byte{1, 2, 3, 4},
@@ -4394,7 +4394,7 @@ func TestMarshal(t *testing.T) {
 	}, {
 		/* TODO(https://go.dev/issue/71631): Re-enable this test case.
 		name: jsontest.Name("Duration/Format/Legacy"),
-		opts: []Options{jsonflags.FormatTimeWithLegacySemantics | 1},
+		opts: []Options{jsonflags.FormatDurationAsNano | 1},
 		in: structDurationFormat{
 			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,
@@ -4407,7 +4407,7 @@ func TestMarshal(t *testing.T) {
 		want: `{"1s":""}`,
 		}, { */
 		name: jsontest.Name("Duration/MapKey/Legacy"),
-		opts: []Options{jsonflags.FormatTimeWithLegacySemantics | 1},
+		opts: []Options{jsonflags.FormatDurationAsNano | 1},
 		in:   map[time.Duration]string{time.Second: ""},
 		want: `{"1000000000":""}`,
 	}, {
@@ -6399,7 +6399,7 @@ func TestUnmarshal(t *testing.T) {
 		wantErr: EU(errors.New("illegal character '\\r' at offset 3")).withPos(`{"Base64": `, "/Base64").withType('"', T[[]byte]()),
 	}, {
 		name:  jsontest.Name("Structs/Format/Bytes/Base64/NonAlphabet/Ignored"),
-		opts:  []Options{jsonflags.FormatBytesWithLegacySemantics | 1},
+		opts:  []Options{jsonflags.ParseBytesWithLooseRFC4648 | 1},
 		inBuf: `{"Base64": "aa=\r\n="}`,
 		inVal: new(structFormatBytes),
 		want:  &structFormatBytes{Base64: []byte{105}},
@@ -8885,7 +8885,7 @@ func TestUnmarshal(t *testing.T) {
 		/* TODO(https://go.dev/issue/71631): Re-enable this test case.
 		name:  jsontest.Name("Duration/Format/Legacy"),
 		inBuf: `{"D1":45296078090012,"D2":"12h34m56.078090012s"}`,
-		opts:  []Options{jsonflags.FormatTimeWithLegacySemantics | 1},
+		opts:  []Options{jsonflags.FormatDurationAsNano | 1},
 		inVal: new(structDurationFormat),
 		want: addr(structDurationFormat{
 			D1: 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond,
@@ -8899,7 +8899,7 @@ func TestUnmarshal(t *testing.T) {
 		want:  addr(map[time.Duration]string{time.Second: ""}),
 		}, { */
 		name:  jsontest.Name("Duration/MapKey/Legacy"),
-		opts:  []Options{jsonflags.FormatTimeWithLegacySemantics | 1},
+		opts:  []Options{jsonflags.FormatDurationAsNano | 1},
 		inBuf: `{"1000000000":""}`,
 		inVal: new(map[time.Duration]string),
 		want:  addr(map[time.Duration]string{time.Second: ""}),
diff --git a/src/encoding/json/v2/arshal_time.go b/src/encoding/json/v2/arshal_time.go
index fefa50ff5f..06fed03e05 100644
--- a/src/encoding/json/v2/arshal_time.go
+++ b/src/encoding/json/v2/arshal_time.go
@@ -50,7 +50,7 @@ func makeTimeArshaler(fncs *arshaler, t reflect.Type) *arshaler {
 				if !m.initFormat(mo.Format) {
 					return newInvalidFormatError(enc, t, mo)
 				}
-			} else if mo.Flags.Get(jsonflags.FormatTimeWithLegacySemantics) {
+			} else if mo.Flags.Get(jsonflags.FormatDurationAsNano) {
 				return marshalNano(enc, va, mo)
 			} else {
 				// TODO(https://go.dev/issue/71631): Decide on default duration representation.
@@ -76,7 +76,7 @@ func makeTimeArshaler(fncs *arshaler, t reflect.Type) *arshaler {
 				if !u.initFormat(uo.Format) {
 					return newInvalidFormatError(dec, t, uo)
 				}
-			} else if uo.Flags.Get(jsonflags.FormatTimeWithLegacySemantics) {
+			} else if uo.Flags.Get(jsonflags.FormatDurationAsNano) {
 				return unmarshalNano(dec, va, uo)
 			} else {
 				// TODO(https://go.dev/issue/71631): Decide on default duration representation.
@@ -150,7 +150,7 @@ func makeTimeArshaler(fncs *arshaler, t reflect.Type) *arshaler {
 				if !u.initFormat(uo.Format) {
 					return newInvalidFormatError(dec, t, uo)
 				}
-			} else if uo.Flags.Get(jsonflags.FormatTimeWithLegacySemantics) {
+			} else if uo.Flags.Get(jsonflags.ParseTimeWithLooseRFC3339) {
 				u.looseRFC3339 = true
 			}