Skip empty values handling when fields cannot be nothing

[NeroBlackstone]

Pull request checklist

• [x] Did you bump the project version?
• [x] Did you add a description to the Pull Request?
• [x] Did you add new tests?
• [x] Did you add reviewers?

Related issue

Hi! I read the source code and it's easy to understand. I realized that we don't need to apply empty values ​​handling specifically for specified fields, just skip those non-nullable fields during empty values ​​handling.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 71.83%. Comparing base (85e37b4) to head (839e035).

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
+ Coverage   71.78%   71.83%   +0.05%     
==========================================
  Files          24       24              
  Lines        1056     1058       +2     
==========================================
+ Hits          758      760       +2     
  Misses        298      298              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[dmitrii-doronin]

Hello, again! nulltype does not necessary mean nothing or missing. You can overload this function to achieve behaviour like 0.00001 (very small float) ->(after deser)-> 0. Does it still support that case?

[dmitrii-doronin]

Hi, @NeroBlackstone! I've been very busy lately but here's a sample I frequently see in line of my work. Does your code cover this case?

using Serde

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

struct Test 
    b::Maybe{String}
end

json = """{"b": ""}"""

Serde.isempty(::Type{Test}, x::String) = isempty(x)

Serde.deser_json(Test, json)

Maybe{String} <: Nothing # false

[gryumov]

@NeroBlackstone, hi, i expect that such a case should not be deserialized if we perform a check using ((Nothing <: fieldtype) || (Missing <: fieldtype)) && isempty(T, value), we won't encounter an error and the value will simply be NA.

using Serde

struct Foo 
    b::String
end

json = """{"b": "NA"}"""

Serde.isempty(::Type{Foo}, x::String) = x == "NA"

julia> Serde.deser_json(Foo, json)
ERROR: ParamError: parameter 'b::String' was not passed or has the value 'nothing'

But I understand your problem and we need to solve it differently.

[gryumov]

Hey @NeroBlackstone, what do you think about this?☝️

[gryumov]

@NeroBlackstone If you add such logic, isempty could potentially allow a nullvalue to pass through, which might not be safe.

using Serde

function Serde.deser(
    ::Serde.CustomType,
    ::Type{D},
    data::AbstractDict{K,V},
)::D where {D<:Any,K<:Union{AbstractString,Symbol},V<:Any}
    vals = Any[]

    for (type, name) in zip(Serde._field_types(D), fieldnames(D))
        key = Serde.custom_name(D, Val(name))
        val = get(data, K(key), Serde.default_value(D, Val(name)))
        val = if isnothing(val) ||
             ismissing(val) ||
             (((Nothing <: type) || (Missing <: type)) && isempty(T, value))
            Serde.nulltype(type)
        else
            val
        end
        push!(vals, Serde.eldeser(D, type, key, val))
    end

    return D(vals...)
end

struct Foo 
    b::String
end

json = """{"b": "NA"}"""

Serde.isempty(::Type{Foo}, x::String) = x == "NA"

julia> Serde.deser_json(Foo, json)
Foo("NA")

[gryumov]

BoundTypes essentially serves as an extended configuration of isempty. @NeroBlackstone what do you think?

using Serde
using BoundTypes

struct Foo
    a::StringMinLength{String,1}
    b::String
end

julia> Serde.deser(Foo, Dict("a" => "dsds", "b" => ""))
Foo("dsds", "")

julia> Serde.deser(Foo, Dict("a" => "", "b" => ""))
ERROR: ConstraintError: constraints of type StringMinLength violated.
Wrong value: length of the "" must be at least 1 characters (0).

[gryumov]

I still think that isempty is a property of the type, not the field

using Serde

struct Foo
    nonull_field1::Int
    nonull_field2::Int
    could_null::Union{Nothing,Int}

    function Foo(
        nonull_field1::Int, 
        nonull_field2::Int, 
        could_null::Union{Nothing,Int}
    )
        return new(nonull_field1, nonull_field2, could_null == 0 ? nothing : could_null)
    end
end

julia> Serde.deser(Foo, Dict("nonull_field1" => 0, "nonull_field2" => 0, "could_null" => 0))
Foo(0, 0, nothing)

[gryumov]

@NeroBlackstone, I think we should close the PR for now since there's no clear solution yet

[gryumov]

@NeroBlackstone, I think we should close the PR for now since there's no clear solution yet

🫡Agree, feel free to close this PR. Thank you for your review and times.

If you have any real-life cases to discuss in the future, I’m always ready. Thank you.