go-json-experiment / json

Experimental implementation of a proposed v2 encoding/json package
BSD 3-Clause "New" or "Revised" License
341 stars 11 forks source link

marshalerV2 vs marshalFuncV2 #34

Open elv-gilles opened 2 months ago

elv-gilles commented 2 months ago

Say a struct config is defined with a function to initialise default values. With a large hierarchy, this is useful such that only relevant or mandatory fields have to be filled by users.

This is the code used with encoding/json

// InitDefaults initializes the default configuration.
func (c *AppConfig) InitDefaults() *AppConfig {
    c.QSpacesConfig.InitDefaults()
    return c
}

func (c *AppConfig) UnmarshalJSON(bts []byte) error {
    c.InitDefaults()

    type alias AppConfig
    return json.Unmarshal(bts, (*alias)(c))
}

With json/v2 (jsonexp), the following are possible:

func (c *AppConfig) UnmarshalJSONV2(dec *jsontext.Decoder, opts jsonexp.Options) error {
    c.InitDefaults()

    type alias AppConfig
    return jsonexp.UnmarshalDecode(dec, (*alias)(c), opts)
}

or

    rd io.Reader = ...
    c := &AppConfig{}
    dec := jsontext.NewDecoder(rd,
        jsonexp.WithUnmarshalers(
            jsonexp.UnmarshalFuncV2(func(dec *jsontext.Decoder, val *AppConfig, opts jsonexp.Options) error {
                val.InitDefaults()
                return jsonexp.SkipFunc
            })))
    err := jsonexp.UnmarshalDecode(dec, c)

but the below is not possible and returns an error unmarshal method cannot be skipped.

func (c *AppConfig) UnmarshalJSONV2(dec *jsontext.Decoder, opts jsonexp.Options) error {
    c.InitDefaults()
    return jsonexp.SkipFunc
}

I believe the error is correct for V1 un/marshalling methods or text un/marshalling, but I would think logical that V2 un/marshalling methods be functionally equivalent to UnmarshalFuncV2.

This commit 6510be0 of my json-experiment fork has changes showing this looks possible.

What do you think ?

dsnet commented 2 months ago

The challenge is that other code might call a T.UnmarshalJSONV2 method directly without going through json.Unmarshal. In such a situation, it would be strange for them to get a json.SkipFunc error as they would then need to write something like:

if err := v.UnmarshalJSONV2(dec, opts); err != nil {
    if err == json.SkipFunc {
        err = json.UnmarshalDecode(dec, &v, opts)
    }
    if err != nil {
        return err
    }
}

Now, you could ask why someone would directly call the UnmarshalJSON or UnmarshalJSONV2 methods (over calling the json.Unmarshal function). I don't have a great answer for that, but empirical evidence shows that it is decently common.

elv-gilles commented 2 months ago

Now, you could ask why

Yea, I would :-) ...

Thanks for your answer, looks reasonable.