Skip to content

Commit

Permalink
Legacy support for v1
Browse files Browse the repository at this point in the history
  • Loading branch information
dsnet committed Dec 21, 2024
1 parent 764075c commit ed5197f
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 18 deletions.
28 changes: 21 additions & 7 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 @@ -1463,7 +1475,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 Down
37 changes: 37 additions & 0 deletions v1/breaking_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package json

// List of breaking v1 changes.
// TODO: Some of these might need to be a v1 compatibility option
// or alternatively a GODEBUG option (see https://go.dev/doc/godebug).
const (
// avoidNilPointerMethods means that implementation always avoids
// calling MarshalJSON and MarshalText methods on nil pointers.
// This is a break from historical v1 behavior.
// In particular, v1 would call the method only if the type is an interface,
// and the interface implements the method. However, if the type is a
// concrete pointer type or an interface (without the method) but contains
// a nil pointer, then the method is not called. This is inconsistent.
//
// See https://go.dev/play/p/WaTuxCmqDGL
// See https://go.dev/play/p/9WeUhxXYiMH
avoidNilPointerMethods = true

// namedByteInSliceAsJSONArray means that a named byte in a slice
// (e.g., []MyByte) is treated as JSON array of MyByte,
// rather than as a JSON string that contains base64-encoded binary data.
// The v1 behavior has always been relying on a bug in reflect,
// where reflect.Value.Bytes can be called on such types even though
// the Go language specifies that []MyByte cannot be directly converted
// into a []byte (i.e., the two types are invariant).
//
// See https://go.dev/issue/24746
namedByteInSliceAsJSONArray = true
)

func ternary[T any](boolVal bool, trueVal, falseVal T) T {
if boolVal {
return trueVal
} else {
return falseVal
}
}
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
3 changes: 3 additions & 0 deletions v1/diff.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/bin/bash

for X in $(ls *_test.go); do diff -u ../../go/src/encoding/json/$X $X; done
1 change: 0 additions & 1 deletion v1/failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ TestUnmarshal/#109
TestUnmarshal/#111
TestUnmarshal/#113
TestUnmarshal/#138
TestNullString
TestInterfaceSet
TestInterfaceSet/#01
TestInterfaceSet/#02
Expand Down
42 changes: 33 additions & 9 deletions v1/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,6 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// Package json implements legacy support for v1 [encoding/json].
//
// The entirety of the v1 API will eventually live in this package.
// For the time being, it only contains options needed to configure the v2 API
// to operate under v1 semantics.
package json

import (
Expand Down Expand Up @@ -38,9 +33,11 @@ type Options = jsonopts.Options
// - [FormatTimeDurationAsNanosecond]
// - [IgnoreStructErrors]
// - [MatchCaseSensitiveDelimiter]
// - [MergeWithLegacySemantics]
// - [OmitEmptyWithLegacyDefinition]
// - [RejectFloatOverflow]
// - [ReportLegacyErrorValues]
// - [SkipUnaddressableMethods]
// - [StringifyWithLegacySemantics]
// - [UnmarshalArrayFromAnyLength]
// - [jsonv2.Deterministic]
Expand Down Expand Up @@ -146,6 +143,17 @@ func MatchCaseSensitiveDelimiter(v bool) Options {
}
}

// TODO: null being ignored instead of clear.
// TODO: other odd behavior?
// 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 Expand Up @@ -199,12 +207,28 @@ func ReportLegacyErrorValues(v bool) Options {
}
}

// SkipUnaddressableMethods specifies that [Marshaler] and [Unmarshaler]
// methods declared on a pointer receiver are not called on values that
// are not addressable (e.g., values derived from a map or interface value).
// This option exists to maintain compatibility for behavior that
// has been widely regarded as a bug. Unfortunately, the effect of this option
// is not consistent and more heavily applies to marshaling than unmarshaling.
//
// The v1 default is true.
func SkipUnaddressableMethods(v bool) Options {
if v {
return jsonflags.SkipUnaddressableMethods | 1
} else {
return jsonflags.SkipUnaddressableMethods | 0
}
}

// StringifyWithLegacySemantics specifies that the `string` tag option
// may stringify bools and string values. It only takes effect on fields
// where the top-level type is a bool, string, numeric kind, or a pointer to
// such a kind. Specifically, `string` will not stringify bool, string,
// or numeric kinds within a composite data type
// (e.g., array, slice, struct, map, or interface).
// where the top-level type is a bool, string, or numeric type.
// Specifically, `string` will not stringify bool, string, or numeric types
// within a composite data type (e.g., array, slice, struct, map, or interface).
// It treats a quoted "null" as if it were a JSON null. TODO: keep this behavior?
//
// This affects either marshaling or unmarshaling.
// The v1 default is true.
Expand Down

0 comments on commit ed5197f

Please sign in to comment.