Skip to content

Commit

Permalink
Implement legacy semantics for merging
Browse files Browse the repository at this point in the history
  • Loading branch information
dsnet committed Dec 23, 2024
1 parent 5815faa commit 98c1d8f
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 52 deletions.
55 changes: 44 additions & 11 deletions arshal_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions arshal_methods.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
8 changes: 6 additions & 2 deletions arshal_time.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion v1/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
66 changes: 44 additions & 22 deletions v1/decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
16 changes: 0 additions & 16 deletions v1/failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
13 changes: 13 additions & 0 deletions v1/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type Options = jsonopts.Options
// - [FormatTimeDurationAsNanosecond]
// - [IgnoreStructErrors]
// - [MatchCaseSensitiveDelimiter]
// - [MergeWithLegacySemantics]
// - [OmitEmptyWithLegacyDefinition]
// - [RejectFloatOverflow]
// - [ReportLegacyErrorValues]
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 98c1d8f

Please sign in to comment.