json2: replace encoder with new implementation

[Larsimusrex]

Replaces the json2 encoder with my new implement. Adds supports for aliases. Improve prettify, compatibility with json and stability.

fix: #25115
fix: #24165
partial: #24903

DEPRECATED:

• Encodable -> JsonEncoder
• json_str -> to_json
• encode_pretty -> encode(..., prettify: true)

BREAKING:

• Enum are encoded as strings by default (like json)
• none is encoded as null, unless @[omitempty] is specified when @[required] is specified
• solidus (/) is never escaped
• utf-8 output by default
• access to internal Encoder struct and encode_value function removed
• prettify uses 4 spaces as default indent instead of 2

[huly-for-github]

Connected to Huly®: V_0.6-24624

[enghitalo]

"none is encoded as null, unless @[omitempty] is specified"

Why?

[Larsimusrex]

Yeah, on second thought, that is probably not a good idea. The only benefit would be being more explicit about the fields you provide. But it would increase the output a lot for structs that have a lot of option fields. It would also be different from json.

A more reasonable approach would be to always omit none, unless @[required] is specified, given that required fields always need to be initialized.

[JalonSolov]

Problem with outputting none is that other languages will think that's a valid value, not "something was uninitialized".

I think you were correct at first - it should be output as null (unless omitted), and should be read in as none for options.

[jorgeluismireles]

What about the problem of encoding/decoding objects with duplicated keys?

struct Base {
    key string
}
struct Other {
    Base
    key string
}

[Larsimusrex]

What about the problem of encoding/decoding objects with duplicated keys?

Not implemented in this PR, but the encoder should always use the embedded field (same as in json).

[Larsimusrex]

Here is a quick benchmark I threw together:

// import x.json2 as json
import json

struct Values {
	id int = 99
	name string = 'john doe'
	value f64 = 999.123
	availible bool = true
	
	sometimes ?int
	capital_letters string = 'HI' @[json: 'CAPITAL_LETTERS']
	ignore bool @[skip]
}


fn main() {
	mut map_data := map[string][]Values{}
	for character in `a`..`l` {
		map_data[character.str()] = []Values{len: 10}
	}
	
	
	for _ in 0..10_000 {
		json.encode(map_data)
	}
}

oldold is json's encoder, old is json2's previous encoder.

Do note that the new encoder caches field attribute/name data, so repeatably encoding the same struct is somewhat rigged (but also not to far fetched).

[enghitalo]

@JalonSolov What do you mean by outputting none?

[enghitalo]

I still think that encoding none as null is a huge waste of processing and memory. Furthermore, the use of x @[omitempty]is mainly to omit the initial values of variables like 0 or empty string. While @[required] is made exactly to force the declaration of values as null, for example.

[spytheman]

Can you please expand a bit about the motivation for the deprecations/changing the name of the interface and the method @Larsimusrex ?
(Encodable -> JsonEncoder, json_str -> to_json)

[Larsimusrex]

This would be closer to decoder2, which has from_json_<type> and <type>Decoder.

I would be open to change, but then we should probably come up with a common scheme for all en/decoders.

Also searching for interfaces in vlib most use a Somthinger name: Reader, Hash64er, Canceler...

[spytheman]

@enghitalo perhaps I do not understand something, but imho the current behavior in this PR is fine, and compatible with json (i.e. not producing values in the string for the none fields):

Details

import json

struct Person {
        id       int = 99
        username string
        surname  ?string
        email    ?string
        age      ?int
        //      fvalue ?f64 = 999.123 // cgen error
        //      availible ?bool = true  // cgen error
        //      ignore bool @[skip] // cgen error
}

p := Person{}
dump(p)
res := json.encode(p)
dump(res)

produces:

#0 15:50:11 ^ replace_encoder_this_time_frfr ~/v>v run old_json_encode.v
[old_json_encode.v:15] p: Person{
    id: 99
    username: ''
    surname: Option(none)
    email: Option(none)
    age: Option(none)
}
[old_json_encode.v:17] res: {"id":99,"username":""}

and

import x.json2 as json

struct Person {
	id       int = 99
	username string
	surname  ?string
	email    ?string
	age      ?int
	// fvalue    ?f64  = 999.123 // ok, just commented for parity with old json
	// availible ?bool = true    // ok, just commented for parity with old json
	// ignore    bool @[skip] // ok, just commented for parity with old json
}

p := Person{}
dump(p)
res := json.encode(p)
dump(res)

with the new encoder, produces the same:

#0 15:50:56 ^ replace_encoder_this_time_frfr ~/v>v run new_json2_encode.v
[new_json2_encode.v:15] p: Person{
    id: 99
    username: ''
    surname: Option(none)
    email: Option(none)
    age: Option(none)
}
[new_json2_encode.v:17] res: {"id":99,"username":""}
#0 15:51:13 ^ replace_encoder_this_time_frfr ~/v>

Can you please provide runnable code, not just descriptions, about what you do not agree about, and how it should behave in your opinion instead?

(otherwise we may be arguing about the same thing, just using different words, and the PR will be needlessly blocked...)

[spytheman]

I would be open to change

No, I do not want to change it, or defend the existing naming.

I was just curious about the motivation, since it does represent a small breaking change, but your explanation is reasonable and logical 👍🏻 .

[enghitalo]

Ok @spytheman . I'll do my best to try to come up with something tonight. If I can't, please proceed as you see fit

[spytheman]

To use @[required]:

import x.json2 as json
struct Person {
    id       int = 99 @[required]
    username string @[required]
    surname  ?string @[required]
    email    ?string @[required]
    age      ?int @[required]
}
p := Person{
    id: 20
    username: 'bilbo'
    surname: none
    email: none
    age: none
}
res := json.encode(p)
dump(res)

This program has a difference between json and x.json2:

#0 16:03:25 ^ replace_encoder_this_time_frfr ~/v>v run old_json_encode.v
[old_json_encode.v:17] res: {"id":20,"username":"bilbo"}
#0 16:03:26 ^ replace_encoder_this_time_frfr ~/v>v run new_json2_encode.v
[new_json2_encode.v:17] res: {"id":20,"username":"bilbo","surname":null,"email":null,"age":null}

Do you think, that the output of the json version is preferable?

[JalonSolov]

@JalonSolov What do you mean by outputting none?

JSON has null, V has none. none should never be output in JSON, as only V would understand that JSON.

Also in JSON, all fields are output unless omitempty is used, and uninitialized things should then be output as null, not none.

@[required] only applies to initializing a struct, not to to anything related to JSON.

[JalonSolov]

The json2 output is preferred, as that is how other languages handle things. Go uses omitempty to signal not outputting null values.

Again, @[required] only applies to initializing struct in V, not to anything JSON related.

[spytheman]

The @[omitempty] tag, is intended to affect string values, not ?string ones.

(keep in mind, that the support for option field values evolved after most of the json related support in the compiler).

[spytheman]

import json
struct Person {
    id       int = 99 @[required]
    emptystring string @[omitempty]
    username string @[required]
    surname  ?string @[required]
    email    ?string @[required]
    age      ?int @[required]
}
p := Person{
    id: 20
    username: 'bilbo'
    surname: none
    email: none
    age: none
}
res := json.encode(p)
dump(res)

this produces: [old_json_encode.v:18] res: {"id":20,"username":"bilbo"}

but with the new encoder, currently:

[new_json2_encode.v:18] res: {"id":20,"emptystring":"","username":"bilbo"}

[spytheman]

i.e. the new x.json2 encoder currently:
a) lacks support for the omitempty tag for string fields that are '' on the V side.
b) produces useless null values for ?string fields, that are none on the V side.

In my opinion, a should be fixed 100%.
As for b, I tend to think, that it should be fixed too, since there is no point in encoding, transferring, and storing values that are none, when the field can be simply omitted.

I do not think that coupling to @[required] is a good idea - it is something, that has meaning on the V side, but not something that the json encoder should care about.

I do think that encoding null for none values should be opt in per field, but controlled with a json specific tag, perhaps @[json_keep_null] ?

[spytheman]

The json2 output is preferred, as that is how other languages handle things. Go uses omitempty to signal not outputting null values.

Go has no concept of option values though, while we do, and the default for them could be different than that of pure value fields.

That said supporting @[omitempty] for option fields may be indeed more consistent and mnemonic.

[Larsimusrex]

@[required] only applies to initializing a struct, not to anything related to JSON.

That is exactly why it must output the option even when it's none. Otherwise, the decoder will throw an error that the field was not provided, meaning that some structs could be encoded but not decoded back.

[Larsimusrex]

a) lacks support for the omitempty tag for string fields that are '' on the V side.
b) produces useless null values for ?string fields, that are none on the V side.

I will fix both.

[JalonSolov]

I think omitempty should apply to Option fields that are none as well as strings that are '', since it is essentially the same idea - no valid value set.

If omitempty is not set on an Option field, it should output null. null on input to an Option field should set it to none.

[Larsimusrex]

null on input to an Option field should set it to none.

It already does with decoder2.

If omitempty is not set on an Option field, it should output null.

I disagree. JSON should be as portable as possible. This assumes that other programming languages have an equivalent for options/none, definitely not a given.

It also only clarifies that a field exists, not what it's "normal" type would be. The real solution is just to document what output can be expected from your application.

[JalonSolov]

In Go, if a field is set to nil, and it is encoded to JSON, it encodes to null unless omitempty is on the field. Example:

package main

import (
        "encoding/json"
        "fmt"
)

type Struct1 struct {
        Foo int
        Bar []string
}

type Struct2 struct {
        Foo int
        Bar []string `json:"Bar,omitempty"`
}

func main() {
        s1 := &Struct1{Foo: 1}
        b1, _ := json.Marshal(s1)
        fmt.Println(string(b1))

        s2 := &Struct2{Foo: 2}
        b2, _ := json.Marshal(s2)
        fmt.Println(string(b2))
}

Output is:

{"Foo":1,"Bar":null}
{"Foo":2}

[Larsimusrex]

@JalonSolov
Here's a compromise:

We would add additional encoder options for the omitempty and keep_null attributes. One could define their preferred options and pass them to every encode call.

const my_options = json.EncoderOptions {
    prettify: true
    indent_string: '\t'

    keep_null: true
}

//...

json.encode(data, my_options)

[JalonSolov]

Too many options. In general, Alex says "make it same as Go" when talking about features like this.

[spytheman]

Too many options. In general, Alex says "make it same as Go" when talking about features like this.

But Go does not support option fields ...

i.e V has one more degree of freedom, and imho given that different people prefer different things in different situations (as evidenced by this very PR), having a way to configure what the output should be, is reasonable.

[JalonSolov]

V is also supposed to be "Simple". Having extra options are rarely a bad thing, and some would likely welcome them, but this seems like a simple enough default...

[jorgeluismireles]

Advantage of json.EncoderOptions is that can manage additions for future improvements as far is backward compatible. Appart options, Golang don't have enums nor sum types, that eventually can raise new problems that can be sorted by adding flags to EncoderOptions.

[jorgeluismireles]

Golang can't marshal (encode) private fields (starting with lowecase). I wonder if V encode/decode takes into account pub fields only to be parsing or not or even if could be good idea to make it configurable in EncoderOptions.

[Larsimusrex]

a) lacks support for the omitempty tag for string fields that are '' on the V side.
b) produces useless null values for ?string fields, that are none on the V side.

@spytheman
I fixed the second one. Not sure what you mean with a) though. Empty strings are omitted when @[omitempty] is specified (did you mean something else?).

Also it is still unclear what the default behavior for option fields should be and I don't think this discussion is going anywhere. It also raised a bigger question for options in general. Currently they are always omitted (see below), I guess that would change too?

import json

fn main() {
	a := [?int(0), 1, none, 3, none]
	b := {'a': ?int(0), 'b': ?int(1), 'c': ?int(none), 'd': ?int(3), 'e': ?int(none)}
	
	println(json.encode(a))
	println(json.encode(b))
}

// [0,1,3]
// {"a":0,"b":1,"d":3}

[jorgeluismireles]

For this array

a := [?int(0), 1, none, 3, none]

this json would have the same length and data in current positions:

[0,1,null,3,null]

[spytheman]

Not sure what you mean with a) though. Empty strings are omitted when @[omitempty] is specified (did you mean something else?).

@Larsimusrex Yes, you are right. Yesterday I got: [new_json2_encode.v:18] res: {"id":20,"emptystring":"","username":"bilbo"}, so I assumed that @[omitempty] for string in the new encoder had a problem, but now I can not reproduce it, so I think that I was wrong, and tested with a version of the code without the attribute. Sorry for the confusion.

[spytheman]

Also it is still unclear what the default behavior for option fields should be and I don't think this discussion is going anywhere. It also raised a bigger question for options in general. Currently they are always omitted (see below), I guess that would change too?

To me it is now very clear, that option fields with none values should be omitted by default, and there could be a parameter to the encoder to keep them as null in the output, just like in your proposal. That will keep the struct declarations more readable, and the serialized/transferred data minimal. Go does not have the concept of option values, so its behavior is irrelevant here. Perhaps using another language's encoder, that does have option values could be a better example to analyze for nuance.

The array and map examples, are very good points though, since they do demonstrate that omitting none is not suitable in all cases (it does not just save space, but can also change the form of the data/lose information and introduce bugs/mismatches with the current json encoder).
=> I do think that option values should be kept in arrays/maps, without needing a parameter to change that.

It also shows that the new encoder, currently does not handle:

import x.json2
a := [?int(0), 1, none, 3, none]
println(json2.encode(a))

currently produces: [0,0,-134701072,32767,0] on my i3 linux box.

The map one, leads to a checker error:

import x.json2
b := {'a': ?int(0), 'b': ?int(1), 'c': ?int(none), 'd': ?int(3), 'e': ?int(none)}
println(json2.encode(b))

[Larsimusrex]

The previous implementation also could not encode arrays/maps of options. This is because generics do not support options.

fn do_generic[T](t T) {

}

fn main() {
        a := ?int(null)

        do_generic(a)

}

It works for struct fields because they is a separate is_option field in FieldData. Maybe you could find similar workarounds for arrays and maps, but they wouldn't be pretty.

[enghitalo]

Imho keep_null parameter could follow the same logic for either struct option fields and array of option elements

[JalonSolov]

Or keep it consistent, and output null for options with value of none unless omitempty is used. Yes, this will make the output "messier", but it does make it more consistent with other types.

If this isn't done, then Options act opposite all other types, where you have to ADD an attribute to get output.

[enghitalo]

Well, imho different types exist for one reason, each one with its own particularity

[JalonSolov]

Yes, that is obviously true. However, it also shouldn't require so much "cognitive space" to figure out which things work which way for something as simple as JSON output.

Think of it this way...

You've created a struct with no option fields, and you JSON encode it. You see all the fields in the output. Now you add an option field, and JSON encode it again, not paying any attention... until the code reading the JSON fails, and you track it down to that option field not being output.

Now you have to research why it wasn't output, and find out you have to add a special attribute just to get it to show up in the JSON.

To me, that's needless complexity, when it is much simpler the other way... it's output unless you add an attribute to tell it to be skipped, EXACTLY THE SAME as you would do for any other field.

[spytheman]

@JalonSolov I am willing to bet, that the debugging scenario, that you describe, will happen much less often, compared to the case, where people using x.json2, will have useless null values in the output, that they wish did not have, and then they will have to spend time, to research how to remove them.

Defaults matter a lot. The defaults for option fields, should be sensible for them, because that increases the clarity not just for the writer, but for all the readers of the code too.

The omitempty mechanism makes sense for non option fields, because they do not have a "safe" value, that will not be present in the input, while you still may need to accommodate the json output that will go to different consumers (which may not be implemented in V, and that may expect the sensible default of not emitting MBs of stuff that can be just inferred).

For option fields though (which Go lacks), since they do have the ability to have a "safe" none value, that will be different than any other valid value of theirs, the default of just skipping them makes a lot more sense, for minimizing the amount of data that will be transferred/processed.

I do think, that researching JSON encoders for languages, that do have option values, can be beneficial to bring more light on the problem.

[spytheman]

Since I do doubt that we will reach a consensus, I think that @medvednikov should decide what is preferable, in order to unblock the PR, which in all other aspects looks fine to me, and is a big improvement.

[medvednikov]

As I understand, the options are

1. ?Foo(none) not included in the json output. Gives us minimal and clean json.

2. ?Foo(none) as null by default, like Go's nil in json.

[medvednikov]

I think Optionals won't be used a lot in json
so it's better to make them explicit (option 2)

on the other hand, that would require changes in json2 and json and make things complex

we can go for the minimal way and keep it as is