quinnj / JSON3.jl

Other
214 stars 47 forks source link

Distinguishing between different null types #143

Open kskyten opened 3 years ago

kskyten commented 3 years ago

Currently, these is only one type of null in the code generator. The following code

json = """
[
    {"a": null, "b": 1},
    {"a": null, "b": null}
]
"""

JSON3.generate_exprs(JSON3.generate_type(JSON3.read(json)), mutable=false)

produces this struct

struct Root
    a::Nothing
    b::Union{Nothing, Int64}
end

This is only because the field a does not have enough samples to produce a meaningful type. On the other hand, the values in b are typical in JSON and would be nice to represent with "the software engineer's null" (JuliaLang/julia#22682). I would like the code generator to output something like

const Maybe{T} = Union{Some{T}, Nothing}

struct Root
    a::Missing
    b::Maybe{Int64}
end

or


const Maybe{T} = Union{Some{T}, Nothing}

struct Root
    a::Any
    b::Maybe{Int64}
end
mcmcgrath13 commented 3 years ago

null is currently represented in JSON3 as nothing, which is why the current handling of missingness shows those as the types. There are a couple of places where this is a bit murkier as Nothing is used as a stand-in for missing and perhaps that could be more distinct. However, neither of those use cases would show up in the example you provided as the samples show that null is the only provided option. More samples is the safer answer for that one. Any as the top type for only nothing could more easily be done at the generate_exprs phase - let me think on if there's anything I'm not thinking of there (or maybe that should be opt in/out).

I'm a bit hesitant to introduce too many edge cases/constructs here as these are intended as more of a "starter type" where maybe it's usable out of the box, maybe it needs a bit of editing to be used efficiently. @quinnj what do you think?

Relatedly, Some doesn't seem to currently be supported by StructTypes out of the box:

using JSON3
using StructTypes

struct MyType
    a::Some{Int}
end

StructTypes.StructType(::Type{MyType}) = StructTypes.Struct()

json = """{"a": null}"""

JSON3.read(json, MyType)
# ^ ERROR: ArgumentError: Some{Int64} doesn't have a defined `StructTypes.StructType`
quinnj commented 3 years ago

Hmmm, yeah, I think I agree with @mcmcgrath13 that the type generating machinery is really about getting a good "start" to generating types and not really getting it perfect. The produced files are easily editable.

Yeah, maybe we need to add support for Some somehow; it's a bit late though and my brain is running itself in circles trying to think of what that looks like between StructTypes/JSON3. I'll try to wrap my head around it again in the morning 😴

quinnj commented 3 years ago

Ok, support for Some added in the StructTypes.jl package: https://github.com/JuliaData/StructTypes.jl/pull/45