aaubry / YamlDotNet

YamlDotNet is a .NET library for YAML
MIT License
2.48k stars 466 forks source link

FSharp deserialization of records with optional record fields #913

Closed tymokvo closed 2 weeks ago

tymokvo commented 4 months ago

I'm trying to figure out the canonical way to deserialize a record with an optional field that is not a scalar value.

Given the script:

[<CLIMutable>]
type SubThing = { value: string }

[<CLIMutable>]
type Thing = { subThing: SubThing option }

open YamlDotNet.Serialization

let d = Deserializer()

"""
subThing:
  value: "a"
"""
|> d.Deserialize<Thing>
|> printfn "%A"

Running this fails with the cryptic error:

(Line: 3, Col: 3, Idx: 13) - (Line: 3, Col: 3, Idx: 13): Exception during deserialization
Stopped due to error

I would expect this to result in { subThing = Some { value = "a" } }.

A simple omission of the field succeeds in the way I would expect:

"""
{}
"""
|> d.Deserialize<Thing>
|> printfn "%A"
{ subThing = None }

However, if the type of the subThing field is a scalar (e.g. string), the behavior is as I expect; omitting it results in None and supplying it results in Some <value>.

Is this behavior configurable in some way?

EdwardCooke commented 4 months ago

The Deserializer isn't meant to be instantiated directly. You should use the DeserializerBuilder so you can set all the options for it. It also adds all of the necessary objects and other required classes so it will work correctly.

I'm not an F# developer and don't know what the implications of removing option from the definition will do with other code.

Using the DeserializerBuilder and a couple of options and removing option from the subThing property made your code work:

[<CLIMutable>]
type SubThing = { value: string }

[<CLIMutable>]
type Thing = { subThing: SubThing }

open YamlDotNet.Serialization

let d = DeserializerBuilder().EnablePrivateConstructors().IncludeNonPublicProperties().Build()

"""
subThing:
  value: "a"
"""
|> d.Deserialize<Thing>
|> printfn "%A"

Results:

{ subThing = { value = "a" } }
tymokvo commented 4 months ago

I'm not an F# developer and don't know what the implications of removing option from the definition will do with other code.

It's basically equivalent to Nullable<T> in C# land. So, removing it makes the field required.

I don't know enough about how YamlDotNet works to know what's going on, but serializing the values of F# types also results in somewhat unexpected behavior which might hint at what's happening? It may be related to the implementation of Option.

Using this script, for example, the second value doesn't successfully round-trip to and from YAML.

#r "nuget: YamlDotNet"

[<CLIMutable>]
type SubThing = { value: string }

[<CLIMutable>]
type Thing = { subThing: SubThing option }

open YamlDotNet.Serialization

let d = DeserializerBuilder().EnablePrivateConstructors().IncludeNonPublicProperties().Build()

let s = SerializerBuilder().Build()

{
    subThing = None
}
|> s.Serialize
|> (fun v ->
    printfn "serialize None:\n%A" v
    v
)
|> d.Deserialize<Thing>
|> printfn "deserialize None:\n%A"

{
    subThing = Some { value = "a" }
}
|> s.Serialize
|> (fun v ->
    printfn "serialize Some:\n%A" v
    v
)
|> d.Deserialize<Thing>
|> printfn "deserialize Some:\n%A"
serialize None:
"subThing@: 
subThing: 
"
deserialize None:
{ subThing = None }
serialize Some:
"subThing@: &o0
  Value:
    value@: a
    value: a
subThing: *o0
"
(Line: 2, Col: 3, Idx: 17) - (Line: 2, Col: 8, Idx: 22): Property 'Value' not found on type 'Microsoft.FSharp.Core.FSharpOption`1[[FSI_0002+SubThing, FSI-ASSEMBLY2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null]]'.
Stopped due to error
EdwardCooke commented 1 month ago

YamlDotNet uses reflection by default to set and get values, there's some special handling for the nullable type because of the value field. We may be able to do the same thing for the FSharpOption generic. I just don't have time to work on that right now.

If you search the codebase for Nullable you can see where we do the checks for it. It would be a bit tricky with FSharp since that base generic class isn't included in the csharp and I don't want to include any dependencies, like fsharp. We currently don't impose non .net default libraries to use yamldotnet and I'd like to keep it that way.

To accomplish this we'd probably have to do type checking using string comparisons and more reflection to get/set the correct fields.