From 98c1d8f27f39c0bf96439515995dc64809cf657d Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Sat, 21 Dec 2024 09:24:40 -0800 Subject: [PATCH] Implement legacy semantics for merging --- arshal_default.go | 55 +++++++++++++++++++++++++++++++-------- arshal_methods.go | 6 +++++ arshal_time.go | 8 ++++-- v1/decode.go | 4 ++- v1/decode_test.go | 66 +++++++++++++++++++++++++++++++---------------- v1/failing.txt | 16 ------------ v1/options.go | 13 ++++++++++ 7 files changed, 116 insertions(+), 52 deletions(-) diff --git a/arshal_default.go b/arshal_default.go index c6c5c96..a55f1ce 100644 --- a/arshal_default.go +++ b/arshal_default.go @@ -157,7 +157,9 @@ func makeBoolArshaler(t reflect.Type) *arshaler { k := tok.Kind() switch k { case 'n': - va.SetBool(false) + if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) { + va.SetBool(false) + } return nil case 't', 'f': if !uo.Flags.Get(jsonflags.StringifyBoolsAndStrings) { @@ -229,7 +231,9 @@ func makeStringArshaler(t reflect.Type) *arshaler { k := val.Kind() switch k { case 'n': - va.SetString("") + if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) { + va.SetString("") + } return nil case '"': val = jsonwire.UnquoteMayCopy(val, flags.IsVerbatim()) @@ -351,7 +355,9 @@ func makeBytesArshaler(t reflect.Type, fncs *arshaler) *arshaler { k := val.Kind() switch k { case 'n': - va.SetZero() + if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) || va.Kind() == reflect.Slice { + va.SetZero() + } return nil case '"': val = jsonwire.UnquoteMayCopy(val, flags.IsVerbatim()) @@ -437,7 +443,9 @@ func makeIntArshaler(t reflect.Type) *arshaler { k := val.Kind() switch k { case 'n': - va.SetInt(0) + if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) { + va.SetInt(0) + } return nil case '"': if !uo.Flags.Get(jsonflags.StringifyNumbers) { @@ -515,7 +523,9 @@ func makeUintArshaler(t reflect.Type) *arshaler { k := val.Kind() switch k { case 'n': - va.SetUint(0) + if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) { + va.SetUint(0) + } return nil case '"': if !uo.Flags.Get(jsonflags.StringifyNumbers) { @@ -603,7 +613,9 @@ func makeFloatArshaler(t reflect.Type) *arshaler { k := val.Kind() switch k { case 'n': - va.SetFloat(0) + if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) { + va.SetFloat(0) + } return nil case '"': val = jsonwire.UnquoteMayCopy(val, flags.IsVerbatim()) @@ -883,7 +895,8 @@ func makeMapArshaler(t reflect.Type) *arshaler { return newUnmarshalErrorAfter(dec, t, err) } - if v2 := va.MapIndex(k.Value); v2.IsValid() { + v2 := va.MapIndex(k.Value) + if v2.IsValid() && !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) { if !xd.Flags.Get(jsonflags.AllowDuplicateNames) && (!seen.IsValid() || seen.MapIndex(k.Value).IsValid()) { // TODO: Unread the object name. name := xd.PreviousTokenOrValue() @@ -1123,7 +1136,9 @@ func makeStructArshaler(t reflect.Type) *arshaler { k := tok.Kind() switch k { case 'n': - va.SetZero() + if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) { + va.SetZero() + } return nil case '{': once.Do(init) @@ -1391,7 +1406,7 @@ func makeSliceArshaler(t reflect.Type) *arshaler { } v := addressableValue{va.Index(i)} // indexed slice element is always addressable i++ - if mustZero { + if mustZero && !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) { v.SetZero() } if err := unmarshal(dec, v, uo); err != nil { @@ -1463,7 +1478,9 @@ func makeArrayArshaler(t reflect.Type) *arshaler { k := tok.Kind() switch k { case 'n': - va.SetZero() + if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) { + va.SetZero() + } return nil case '[': once.Do(init) @@ -1481,7 +1498,9 @@ func makeArrayArshaler(t reflect.Type) *arshaler { continue } v := addressableValue{va.Index(i)} // indexed array element is addressable if array is addressable - v.SetZero() + if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) { + v.SetZero() + } if err := unmarshal(dec, v, uo); err != nil { return err } @@ -1593,6 +1612,20 @@ func makeInterfaceArshaler(t reflect.Type) *arshaler { if uo.Format != "" && uo.FormatDepth == xd.Tokens.Depth() { return newInvalidFormatError(dec, t, uo.Format) } + if uo.Flags.Get(jsonflags.MergeWithLegacySemantics) && !va.IsNil() { + e := va.Elem() + if e.Kind() == reflect.Pointer && !e.IsNil() { + if dec.PeekKind() == 'n' && e.Elem().Kind() == reflect.Pointer { + if _, err := dec.ReadToken(); err != nil { + return err + } + va.Elem().Elem().SetZero() + return nil + } + } else { + va.SetZero() + } + } if dec.PeekKind() == 'n' { if _, err := dec.ReadToken(); err != nil { return err diff --git a/arshal_methods.go b/arshal_methods.go index b97ac22..a6fa9bd 100644 --- a/arshal_methods.go +++ b/arshal_methods.go @@ -266,6 +266,12 @@ func makeMethodArshaler(fncs *arshaler, t reflect.Type) *arshaler { if err != nil { return err // must be a syntactic or I/O error } + if val.Kind() == 'n' { + if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) { + va.SetZero() + } + return nil + } if val.Kind() != '"' { return newUnmarshalErrorAfter(dec, t, errNonStringValue) } diff --git a/arshal_time.go b/arshal_time.go index 5a58bfb..cc4e2c5 100644 --- a/arshal_time.go +++ b/arshal_time.go @@ -82,7 +82,9 @@ func makeTimeArshaler(fncs *arshaler, t reflect.Type) *arshaler { } switch k := val.Kind(); k { case 'n': - *td = time.Duration(0) + if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) { + *td = time.Duration(0) + } return nil case '"': if u.isNumeric() && !uo.Flags.Get(jsonflags.StringifyNumbers) { @@ -145,7 +147,9 @@ func makeTimeArshaler(fncs *arshaler, t reflect.Type) *arshaler { } switch k := val.Kind(); k { case 'n': - *tt = time.Time{} + if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) { + *tt = time.Time{} + } return nil case '"': if u.isNumeric() && !uo.Flags.Get(jsonflags.StringifyNumbers) { diff --git a/v1/decode.go b/v1/decode.go index 659e17f..b5267c0 100644 --- a/v1/decode.go +++ b/v1/decode.go @@ -223,7 +223,9 @@ func (n *Number) UnmarshalJSONV2(dec *jsontext.Decoder, opts jsonv2.Options) err k := val.Kind() switch k { case 'n': - *n = "" // TODO: Should we merge with legacy semantics? + if legacy, _ := jsonv2.GetOption(opts, MergeWithLegacySemantics); !legacy { + *n = "" + } return nil case '"': if !stringify { diff --git a/v1/decode_test.go b/v1/decode_test.go index 1f9b6e1..471b054 100644 --- a/v1/decode_test.go +++ b/v1/decode_test.go @@ -1805,19 +1805,8 @@ func TestNullString(t *testing.T) { } } -func intp(x int) *int { - p := new(int) - *p = x - return p -} - -func intpp(x *int) **int { - pp := new(*int) - *pp = x - return pp -} - func TestInterfaceSet(t *testing.T) { + errUnmarshal := &UnmarshalTypeError{Value: "object", Offset: 5, Type: reflect.TypeFor[int](), Field: "X"} tests := []struct { CaseName pre any @@ -1828,22 +1817,55 @@ func TestInterfaceSet(t *testing.T) { {Name(""), "foo", `2`, 2.0}, {Name(""), "foo", `true`, true}, {Name(""), "foo", `null`, nil}, - - {Name(""), nil, `null`, nil}, - {Name(""), new(int), `null`, nil}, - {Name(""), (*int)(nil), `null`, nil}, - {Name(""), new(*int), `null`, new(*int)}, - {Name(""), (**int)(nil), `null`, nil}, - {Name(""), intp(1), `null`, nil}, - {Name(""), intpp(nil), `null`, intpp(nil)}, - {Name(""), intpp(intp(1)), `null`, intpp(nil)}, + {Name(""), map[string]any{}, `true`, true}, + {Name(""), []string{}, `true`, true}, + + {Name(""), any(nil), `null`, any(nil)}, + {Name(""), (*int)(nil), `null`, any(nil)}, + {Name(""), (*int)(addr(0)), `null`, any(nil)}, + {Name(""), (*int)(addr(1)), `null`, any(nil)}, + {Name(""), (**int)(nil), `null`, any(nil)}, + {Name(""), (**int)(addr[*int](nil)), `null`, (**int)(addr[*int](nil))}, + {Name(""), (**int)(addr(addr(1))), `null`, (**int)(addr[*int](nil))}, + {Name(""), (***int)(nil), `null`, any(nil)}, + {Name(""), (***int)(addr[**int](nil)), `null`, (***int)(addr[**int](nil))}, + {Name(""), (***int)(addr(addr[*int](nil))), `null`, (***int)(addr[**int](nil))}, + {Name(""), (***int)(addr(addr(addr(1)))), `null`, (***int)(addr[**int](nil))}, + + {Name(""), any(nil), `2`, float64(2)}, + {Name(""), (int)(1), `2`, float64(2)}, + {Name(""), (*int)(nil), `2`, float64(2)}, + {Name(""), (*int)(addr(0)), `2`, (*int)(addr(2))}, + {Name(""), (*int)(addr(1)), `2`, (*int)(addr(2))}, + {Name(""), (**int)(nil), `2`, float64(2)}, + {Name(""), (**int)(addr[*int](nil)), `2`, (**int)(addr(addr(2)))}, + {Name(""), (**int)(addr(addr(1))), `2`, (**int)(addr(addr(2)))}, + {Name(""), (***int)(nil), `2`, float64(2)}, + {Name(""), (***int)(addr[**int](nil)), `2`, (***int)(addr(addr(addr(2))))}, + {Name(""), (***int)(addr(addr[*int](nil))), `2`, (***int)(addr(addr(addr(2))))}, + {Name(""), (***int)(addr(addr(addr(1)))), `2`, (***int)(addr(addr(addr(2))))}, + + {Name(""), any(nil), `{}`, map[string]any{}}, + {Name(""), (int)(1), `{}`, map[string]any{}}, + {Name(""), (*int)(nil), `{}`, map[string]any{}}, + {Name(""), (*int)(addr(0)), `{}`, errUnmarshal}, + {Name(""), (*int)(addr(1)), `{}`, errUnmarshal}, + {Name(""), (**int)(nil), `{}`, map[string]any{}}, + {Name(""), (**int)(addr[*int](nil)), `{}`, errUnmarshal}, + {Name(""), (**int)(addr(addr(1))), `{}`, errUnmarshal}, + {Name(""), (***int)(nil), `{}`, map[string]any{}}, + {Name(""), (***int)(addr[**int](nil)), `{}`, errUnmarshal}, + {Name(""), (***int)(addr(addr[*int](nil))), `{}`, errUnmarshal}, + {Name(""), (***int)(addr(addr(addr(1)))), `{}`, errUnmarshal}, } for _, tt := range tests { t.Run(tt.Name, func(t *testing.T) { - skipKnownFailure(t) b := struct{ X any }{tt.pre} blob := `{"X":` + tt.json + `}` if err := Unmarshal([]byte(blob), &b); err != nil { + if reflect.DeepEqual(err, tt.post) { + return + } t.Fatalf("%s: Unmarshal(%#q) error: %v", tt.Where, blob, err) } if !reflect.DeepEqual(b.X, tt.post) { diff --git a/v1/failing.txt b/v1/failing.txt index 623a34f..73d31b7 100644 --- a/v1/failing.txt +++ b/v1/failing.txt @@ -5,17 +5,6 @@ TestUnmarshal/#109 TestUnmarshal/#111 TestUnmarshal/#113 TestUnmarshal/#138 -TestNullString -TestInterfaceSet -TestInterfaceSet/#01 -TestInterfaceSet/#02 -TestInterfaceSet/#07 -TestInterfaceSet/#10 -TestInterfaceSet/#11 -TestUnmarshalNulls -TestPrefilled -TestPrefilled/#00 -TestPrefilled/#01 TestEncodeRenamedByteSlice TestNilMarshal TestNilMarshal/#08 @@ -29,8 +18,3 @@ TestStringOption/Unmarshal/Null/v1 TestStringOption/Unmarshal/Deep/v1 TestPointerReceiver TestPointerReceiver/Marshal/v1 -TestPointerReceiver/Unmarshal/v1 -TestMergeNull -TestMergeNull/Unmarshal/v1 -TestMergeComposite -TestMergeComposite/Unmarshal/v1 diff --git a/v1/options.go b/v1/options.go index a626723..4a66338 100644 --- a/v1/options.go +++ b/v1/options.go @@ -38,6 +38,7 @@ type Options = jsonopts.Options // - [FormatTimeDurationAsNanosecond] // - [IgnoreStructErrors] // - [MatchCaseSensitiveDelimiter] +// - [MergeWithLegacySemantics] // - [OmitEmptyWithLegacyDefinition] // - [RejectFloatOverflow] // - [ReportLegacyErrorValues] @@ -146,6 +147,18 @@ func MatchCaseSensitiveDelimiter(v bool) Options { } } +// MergeWithLegacySemantics MUSTDO +// +// This only affects unmarshaling and is ignored when marshaling. +// The v1 default is true. +func MergeWithLegacySemantics(v bool) Options { + if v { + return jsonflags.MergeWithLegacySemantics | 1 + } else { + return jsonflags.MergeWithLegacySemantics | 0 + } +} + // OmitEmptyWithLegacyDefinition specifies that the `omitempty` tag option // follows a definition of empty where a field is omitted if the Go value is // false, 0, a nil pointer, a nil interface value,