Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
giedriusbruzas opened this issue Oct 5, 2016 · 9 comments
Open

Comments

@giedriusbruzas
Copy link

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
Copy link
Member

This also causes trouble when trying to do lensing.

@wallymathieu
Copy link
Member

JNumber s implies that s is JsonPrimitive ...

@wallymathieu
Copy link
Member

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
Copy link
Member

@gusty
Copy link
Member

gusty commented Sep 22, 2018

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
Copy link
Member

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

@gusty
Copy link
Member

gusty commented Sep 22, 2018

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
Copy link
Member

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

This was referenced Sep 25, 2018
@wallymathieu
Copy link
Member

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

gusty pushed a commit that referenced this issue Sep 26, 2018
As mentioned in #17 (comment) , though it is a separate issue
gusty pushed a commit that referenced this issue Sep 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants