fsprojects / Fleece

Json mapper for F#
http://fsprojects.github.io/Fleece
Apache License 2.0
199 stars 31 forks source link

Parsing an integer value when JSON contains a decimal point #17

Open giedriusbruzas opened 8 years ago

giedriusbruzas commented 8 years ago
let result : int ParseResult = parseJSON "1.1"
let (Choice1Of2 value) = result
//val value : int = 1

I feel this is incorrect and should instead result in a failure. There's possibly other similar edge cases like this one.

wallymathieu commented 6 years ago

This also causes trouble when trying to do lensing.

wallymathieu commented 6 years ago

JNumber s implies that s is JsonPrimitive ...

wallymathieu commented 6 years ago

Seems like F# Data us fixing this as well: https://www.nuget.org/packages/FSharp.Data/3.0.0-beta4 :

(Breaking Change) Don't silently convert decimals and floats to integers in JsonProvider.

wallymathieu commented 6 years ago

https://github.com/fsharp/FSharp.Data/pull/1185

gusty commented 6 years ago

I think this behavior is not correct at all. Anyway, I also think this should be corrected in the original library (as FSharp.Data just did). We should forward this issue to Newtonsoft and System.Json Also we should note that Fleece is a Json mapper for different libraries, since different libraries have different behavior (there is no agreed standard for Json <-> .NET) it's not up to Fleece to try to fix them.

wallymathieu commented 6 years ago

I agree. It only makes sense to align with the libraries. Right now JNumber active pattern has some interesting behavior.

gusty commented 6 years ago

I don't know if this is related:

1 |> toJson |> string;;
val it : string = "1"

but

(2,1) |> toJson |> string;;
val it : string = "[
  2.0,
  1.0
]"
wallymathieu commented 6 years ago

We also see this kind of behavior: https://github.com/JamesNK/Newtonsoft.Json/issues/1400 So I guess it's a tradeoff what Json parser you use.

wallymathieu commented 6 years ago

I've added a PR that documents the behavior. Note that Fsharp.Data works the way you want.