Pass s::CS to all usage of lower for dispatch

[Teo-ShaoWei]

I would like to propose for us to pass s::CommonSerialization to all usage of lower. This is so our custom serialization can also dispatch on the catch-all lower(x).

For example, the catch-all lower will become:

function lower(s::CS, a)
    if nfields(a) > 0
        JSON.Writer.CompositeTypeWrapper(a)
    else
        error("Cannot serialize type $(typeof(a))")
    end
end

Context

I am trying to use JSON.jl to create a JSON logger, and I am met with the problem that modules and no-field objects (especially Ptr{Nothing} cannot be logged. This resulted in a fallback to the console logger. Because our office enforce standardization of all logs in JSON format, these console logs get ignored, and so we are not alerted by errors!

I started out by defining lower(::Ptr{Nothing}), but this doesn't solve the bigger picture problem: There might one day be error logs that is not captured as it is not in JSON.

To resolve this, I could quite easily subtype CommonSerialization as the behavior is mainly identical. I just need something like:

function lower(a)
    if nfields(a) > 0
        JSON.Writer.CompositeTypeWrapper(a)
    else
        sprint(show, a)
    end
end

But this is type piracy and resulted in a nasty warning everything we run our code, not a good habit imo. The way I ended up doing it without type piracy warning needed me to duplicate most of lower and show_json within my package.

If lower takes in the serialization and show_json calls it as such, then I can define my own serialization S and have

function lower(s::S, a)
    if nfields(a) > 0
        JSON.Writer.CompositeTypeWrapper(a)
    else
        sprint(show, a)
    end
end

I think this is aligned with the rationale of creating the serialization type in the first place.

additional benefit

The code currently need a hack to handle enum printing as string. This is needed because user cannot define their own lower(x::Enum) without incurring a type piracy warning.

So with this change we can do away with that, and have:

show_json(io::SC, s::CS, x::Enum) = show_json(io, s, lower(s, x))

Then the show_json for strings will be clean and efficient, while that for enum fully demonstrate the necessary detour.

call to action

Let me know if we like this suggestion? Needs be I can perform the code change, just let me know that we are interested to have this 🙏

[fredrikekre]

I am trying to use JSON.jl to create a JSON logger

Unrelated to the issue here in JSON.jl, but have you seen https://github.com/JuliaLogging/LoggingFormats.jl#json-output-log-events-as-json ? I don't think the implementation there has the problem you are describing.

[Teo-ShaoWei]

Thanks, @fredrikekre for the suggestion! 🙏 I had some issue with JSON3.jl's pretty-printing sometime back, so we haven't considered moving to it. I would be interested to give it a good look again when there's ROI, e.g. for the next project, or if our current JSON logger is proven inadequate.