Allow developers to implement and process their own json.Options types

[mikeschinkel]

UPDATE

See updated description in the comment below.

---

Here is a simple proof of concept for allowing both Structs and Variadic Funcs for option configuration.

See https://github.com/golang/go/discussions/63397#discussioncomment-7333534 for entire discussion.

---

However, below is the content I wrote in the comment:

@dsnet — 

Thank you for the question.

What I proposed would keep the Marshal function signature exactly as you have it in the Git repo for the experimental v2 code, e.g.:

func Marshal(in any, opts ...Options) (out []byte, err error) 

I did not answer immediately as I wanted to implement a proof-of-concept as a pull request so you can see it in action. Fortunately by doing this I discovered circular dependencies in what I proposed above, but by tweaking the concept I was able to get it working in less code than I was originally envisioning.

The approach I came up with uses the following two (2) interfaces to get around a json->jsonopts->json import cycle:

type optionsArshaler interface {
    OptionsArshal(*jsonopts.Struct, internal.NotForPublicUse)
}

type optionsCoder interface {
    OptionsCode(*jsonopts.Struct, internal.NotForPublicUse)
}

They get applied to the MarshalOptions struct and allow jsonopts.Join() to call those functions back into json and thus allow MarshalOptions.OptionsArshal() and MarshalOptions.OptionsCode() to set the values of *jsonopts.Struct:

The PR is relatively small and self-contained and only implements a subset of options and only for MarshalOptions; I did not implement the Unmarshal and Coder options to keep the scope of the PR small so it is easier to review for concept.

I did not try to implement setting v1 options (yet) because I don't want to go down the rabbit hole until I know that it will not abruptly lead to a dead-end. 🙂

I think what I came up can drop the need for the get<OptProp>() functions to be called dynamically via type assertions, but I'm not 100% sure as my brain was getting twisted trying to avoid the import cycle. If you see that there will still be issues with evolving this in the future I can add those back in. I did leave commented-out code showing what it would look like if we used get<OptProp>() functions and dynamic calls, but I think we don't actually need them; you tell me?

How to try it

To test it look for the file marshal_test.go in the root so you can run the code and see it work, for a simple case, albeit. I did not write exhaustive tests at this point.

I'm going to make this comment so I can get a link to it, use that link in the PR, and then come back here and post a link to the PR. My branch will be named best-of-both-options.

[mikeschinkel]

Have updated this PR to be simply what is needed to allow developers to create their own Hybrid API, if they desire.

There is only a tiny change needed and a few things made visible. This PR is ready to merge from my perspective.

Background.

Also, this PR was tested using a package that can be found here in my fork.

[dsnet]

I'm fairly open to the idea of expanding options to support user-defined options, but I don't think this is quite the way we want to go. The jsonopts.Struct is an implementation detail and should not be exported as we want the freedom to change its representation as necessary.

I don't think we're ready to think about how to open up the json.Options interface for the initial release of v2, so locking down what can implement it for the time being gives us the most flexibility in the futue.

For the time being, I recommend doing something like:

package jsonopts

type Marshal struct {
    StringifyNumbers          bool
    Deterministic             bool
    FormatNilSliceAsNull      bool
    FormatNilMapAsNull        bool
    MatchCaseInsensitiveNames bool
    DiscardUnknownMembers     bool
    Marshalers                *json.Marshalers
}

func (m Marshal) Options() json.Options {
    return json.JoinOptions(
        json.StringifyNumbers(m.StringifyNumbers),
        json.Deterministic(m.Deterministic),
        json.FormatNilSliceAsNull(m.FormatNilSliceAsNull),
        json.FormatNilMapAsNull(m.FormatNilMapAsNull),
        json.MatchCaseInsensitiveNames(m.MatchCaseInsensitiveNames),
        json.DiscardUnknownMembers(m.DiscardUnknownMembers),
        json.WithMarshalers(m.Marshalers),
    )
}

Thus, usage of it would be like:

json.Marshal(v, jsonopts.Marshal{Deterministic: true}.Options())

There's an additional call to the Options method, but we can eventually aim to eliminate that when we open up json.Options to support external implementations.

[mikeschinkel]

@dsnet — Fair enough.

BTW, wouldn’t that be this instead?

json.Marshal(v, jsonopts.Marshal{Deterministic: true}.Options()...)

[dsnet]

Since json.Options can represent a set of options, I don't think you'll need the ....

I thought a little more about this, and I think I might want to see how https://github.com/golang/go/issues/61405 shakes out. For your use case, we would want the json.Options interface to be able to present the fact that you can iterate over the individual option members. Thus, the "json" package would check whether you can self-iterate and then call that iteration.

[mikeschinkel]

@dsnet — 👌