Skip to content

Commit

Permalink
Drop Options argument from user-declared methods and functions
Browse files Browse the repository at this point in the history 
WARNING: This commit contains breaking changes.

This drops Options as an argument from
the MarshalerTo and UnmarshalerFrom interfaces and
the MarshalToFunc and UnmarshalFromFunc functions.

Instead, the options is stored within the
jsontext.Encoder or jsontext.Decoder and
can be retrieved through the Options method.

This simplifies the API for custom marshalers and unmarshalers and
makes it impossible to accidentally drop the options
when recursively calling json.MarshalEncode or json.UnmarshalDecode
from within a custom marshaler or unmarshaler implementation.

Fixes golang/go#71611
  • Loading branch information
@dsnet
dsnet committed Feb 13, 2025
1 parent 60a0516 commit 0cb66e1
Show file tree
Hide file tree
Showing 11 changed files with 204 additions and 189 deletions.
60 changes: 15 additions & 45 deletions arshal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,35 +33,6 @@ var (
// export exposes internal functionality of the "jsontext" package.
var export = jsontext.Internal.Export(&internal.AllowInternalUse)

// mayReuseOpt reuses coderOpts if joining opts with the coderOpts
// would produce the equivalent set of options.
func mayReuseOpt(coderOpts *jsonopts.Struct, opts []Options) *jsonopts.Struct {
switch len(opts) {
// In the common case, the caller plumbs down options from the caller's caller,
// which is usually the [jsonopts.Struct] constructed by the top-level arshal call.
case 1:
o, _ := opts[0].(*jsonopts.Struct)
if o == coderOpts {
return coderOpts
}
// If the caller provides no options, then just reuse the coder's options,
// which may contain both marshaling/unmarshaling and encoding/decoding flags.
case 0:
return coderOpts
}
return nil
}

var structOptionsPool = &sync.Pool{New: func() any { return new(jsonopts.Struct) }}

func getStructOptions() *jsonopts.Struct {
return structOptionsPool.Get().(*jsonopts.Struct)
}
func putStructOptions(o *jsonopts.Struct) {
*o = jsonopts.Struct{}
structOptionsPool.Put(o)
}

// Marshal serializes a Go value as a []byte according to the provided
// marshal and encode options (while ignoring unmarshal or decode options).
// It does not terminate the output with a newline.
Expand Down Expand Up @@ -231,15 +202,13 @@ func MarshalWrite(out io.Writer, in any, opts ...Options) (err error) {
// See [Marshal] for details about the conversion of a Go value into JSON.
func MarshalEncode(out *jsontext.Encoder, in any, opts ...Options) (err error) {
xe := export.Encoder(out)
mo := mayReuseOpt(&xe.Struct, opts)
if mo == nil {
mo = getStructOptions()
defer putStructOptions(mo)
*mo = xe.Struct // initialize with encoder options before joining
mo.JoinWithoutCoderOptions(opts...)
if len(opts) > 0 {
optsOriginal := xe.Struct
defer func() { xe.Struct = optsOriginal }()
xe.Struct.JoinWithoutCoderOptions(opts...)
}
err = marshalEncode(out, in, mo)
if err != nil && mo.Flags.Get(jsonflags.ReportErrorsWithLegacySemantics) {
err = marshalEncode(out, in, &xe.Struct)
if err != nil && xe.Flags.Get(jsonflags.ReportErrorsWithLegacySemantics) {
return internal.TransformMarshalError(in, err)
}
return err
Expand Down Expand Up @@ -480,15 +449,13 @@ func unmarshalFull(in *jsontext.Decoder, out any, uo *jsonopts.Struct) error {
// See [Unmarshal] for details about the conversion of JSON into a Go value.
func UnmarshalDecode(in *jsontext.Decoder, out any, opts ...Options) (err error) {
xd := export.Decoder(in)
uo := mayReuseOpt(&xd.Struct, opts)
if uo == nil {
uo = getStructOptions()
defer putStructOptions(uo)
*uo = xd.Struct // initialize with decoder options before joining
uo.JoinWithoutCoderOptions(opts...)
if len(opts) > 0 {
optsOriginal := xd.Struct
defer func() { xd.Struct = optsOriginal }()
xd.Struct.JoinWithoutCoderOptions(opts...)
}
err = unmarshalDecode(in, out, uo)
if err != nil && uo.Flags.Get(jsonflags.ReportErrorsWithLegacySemantics) {
err = unmarshalDecode(in, out, &xd.Struct)
if err != nil && xd.Flags.Get(jsonflags.ReportErrorsWithLegacySemantics) {
return internal.TransformUnmarshalError(out, err)
}
return err
Expand Down Expand Up @@ -544,6 +511,9 @@ func newAddressableValue(t reflect.Type) addressableValue {
return addressableValue{reflect.New(t).Elem(), true}
}

// TODO: Remove *jsonopts.Struct argument from [marshaler] and [unmarshaler].
// This can be directly accessed on the encoder or decoder.

// All marshal and unmarshal behavior is implemented using these signatures.
// The *jsonopts.Struct argument is guaranteed to identical to or at least
// a strict super-set of the options in Encoder.Struct or Decoder.Struct.
Expand Down
12 changes: 6 additions & 6 deletions arshal_funcs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,9 @@ func MarshalFunc[T any](fn func(T) ([]byte, error)) *Marshalers {
// on the provided encoder. It may return [SkipFunc] such that marshaling can
// move on to the next marshal function. However, no mutable method calls may
// be called on the encoder if [SkipFunc] is returned.
// The pointer to [jsontext.Encoder], the value of T, and the [Options] value
// The pointer to [jsontext.Encoder] and the value of T
// must not be retained outside the function call.
func MarshalToFunc[T any](fn func(*jsontext.Encoder, T, Options) error) *Marshalers {
func MarshalToFunc[T any](fn func(*jsontext.Encoder, T) error) *Marshalers {
t := reflect.TypeFor[T]()
assertCastableTo(t, true)
typFnc := typedMarshaler{
Expand All @@ -220,7 +220,7 @@ func MarshalToFunc[T any](fn func(*jsontext.Encoder, T, Options) error) *Marshal
xe := export.Encoder(enc)
prevDepth, prevLength := xe.Tokens.DepthLength()
xe.Flags.Set(jsonflags.WithinArshalCall | 1)
err := fn(enc, va.castTo(t).Interface().(T), mo)
err := fn(enc, va.castTo(t).Interface().(T))
xe.Flags.Set(jsonflags.WithinArshalCall | 0)
currDepth, currLength := xe.Tokens.DepthLength()
if err == nil && (prevDepth != currDepth || prevLength+1 != currLength) {
Expand Down Expand Up @@ -291,9 +291,9 @@ func UnmarshalFunc[T any](fn func([]byte, T) error) *Unmarshalers {
// on the provided decoder. It may return [SkipFunc] such that unmarshaling can
// move on to the next unmarshal function. However, no mutable method calls may
// be called on the decoder if [SkipFunc] is returned.
// The pointer to [jsontext.Decoder], the value of T, and [Options] value
// The pointer to [jsontext.Decoder] and the value of T
// must not be retained outside the function call.
func UnmarshalFromFunc[T any](fn func(*jsontext.Decoder, T, Options) error) *Unmarshalers {
func UnmarshalFromFunc[T any](fn func(*jsontext.Decoder, T) error) *Unmarshalers {
t := reflect.TypeFor[T]()
assertCastableTo(t, false)
typFnc := typedUnmarshaler{
Expand All @@ -302,7 +302,7 @@ func UnmarshalFromFunc[T any](fn func(*jsontext.Decoder, T, Options) error) *Unm
xd := export.Decoder(dec)
prevDepth, prevLength := xd.Tokens.DepthLength()
xd.Flags.Set(jsonflags.WithinArshalCall | 1)
err := fn(dec, va.castTo(t).Interface().(T), uo)
err := fn(dec, va.castTo(t).Interface().(T))
xd.Flags.Set(jsonflags.WithinArshalCall | 0)
currDepth, currLength := xd.Tokens.DepthLength()
if err == nil && (prevDepth != currDepth || prevLength+1 != currLength) {
Expand Down
13 changes: 6 additions & 7 deletions arshal_methods.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ type Marshaler interface {
// should aim to have equivalent behavior for the default marshal options.
//
// The implementation must write only one JSON value to the Encoder and
// must not retain the pointer to [jsontext.Encoder] or the [Options] value.
// must not retain the pointer to [jsontext.Encoder].
type MarshalerTo interface {
MarshalJSONTo(*jsontext.Encoder, Options) error
MarshalJSONTo(*jsontext.Encoder) error

// TODO: Should users call the MarshalEncode function or
// should/can they call this method directly? Does it matter?
Expand Down Expand Up @@ -85,10 +85,9 @@ type Unmarshaler interface {
// It is recommended that UnmarshalJSONFrom implement merge semantics when
// unmarshaling into a pre-populated value.
//
// Implementations must not retain the pointer to [jsontext.Decoder] or
// the [Options] value.
// Implementations must not retain the pointer to [jsontext.Decoder].
type UnmarshalerFrom interface {
UnmarshalJSONFrom(*jsontext.Decoder, Options) error
UnmarshalJSONFrom(*jsontext.Decoder) error

// TODO: Should users call the UnmarshalDecode function or
// should/can they call this method directly? Does it matter?
Expand Down Expand Up @@ -193,7 +192,7 @@ func makeMethodArshaler(fncs *arshaler, t reflect.Type) *arshaler {
xe := export.Encoder(enc)
prevDepth, prevLength := xe.Tokens.DepthLength()
xe.Flags.Set(jsonflags.WithinArshalCall | 1)
err := va.Addr().Interface().(MarshalerTo).MarshalJSONTo(enc, mo)
err := va.Addr().Interface().(MarshalerTo).MarshalJSONTo(enc)
xe.Flags.Set(jsonflags.WithinArshalCall | 0)
currDepth, currLength := xe.Tokens.DepthLength()
if (prevDepth != currDepth || prevLength+1 != currLength) && err == nil {
Expand Down Expand Up @@ -283,7 +282,7 @@ func makeMethodArshaler(fncs *arshaler, t reflect.Type) *arshaler {
xd := export.Decoder(dec)
prevDepth, prevLength := xd.Tokens.DepthLength()
xd.Flags.Set(jsonflags.WithinArshalCall | 1)
err := va.Addr().Interface().(UnmarshalerFrom).UnmarshalJSONFrom(dec, uo)
err := va.Addr().Interface().(UnmarshalerFrom).UnmarshalJSONFrom(dec)
xd.Flags.Set(jsonflags.WithinArshalCall | 0)
currDepth, currLength := xd.Tokens.DepthLength()
if (prevDepth != currDepth || prevLength+1 != currLength) && err == nil {
Expand Down
Loading

0 comments on commit 0cb66e1

Please sign in to comment.