Open swift-ci opened 5 years ago
cc @itaiferber
I don't have the JSON spec in front of me, but IIRC in JavaScript all numbers are Double and they are just treated as integer in specific instances. I would not be surprised if JSON mandates this be the case.
Comment by Hung Truong (JIRA)
@millenomi ah interesting, perhaps I should write some more test cases with an "Int" in the JSON to see how it behaves. Though I think there are already a few tests in that suite that serialize and deserialize some Ints.
Just to copy my comments from hungtruong (JIRA User)'s PR:
Fundamentally, JSON doesn't have multiple data types beyond "number", and the value
1
is no different from1.0
as far as JSON is concerned. If a number can be exactly represented as an integer, I don't see a reason to prevent it from decoding as such.We have to consider backwards compatibility, too — it's entirely possible there are folks relying on being able to decode
1.0
as anInt
, and we'd need a very compelling reason to break the behavior on them.
Comment by Julian (JIRA)
So some background here. I filed this bug after I shipped a pretty big bug into production. That was totally my fault for screwing up the data parsing, but I was pretty surprised to find that the API didn’t fail on runtime for an incorrect parse when I had specified the parse incorrectly
Comment by Julian (JIRA)
Obviously I’ve fixed my code and updated my tests to include some non-whole numbers since that incident
Comment by Hung Truong (JIRA)
jozen (JIRA User) I typically prefer to fail early on cases like these where an edge case in the implementation could cause unexpected behavior like the one you describe. I think the challenge here is to take a really weakly typed language like Javascript and convert it to a strongly typed language like Swift. And I think @itaiferber does bring up a good point about backwards compatibility, though hopefully devs would be able to test the new version of Swift and make updates accordingly before any bugs made it into production. I'm still sort of on the fence on this but I can see why we'd want a really compelling reason to make a change like this.
Comment by Julian (JIRA)
Just wanted to follow up here and thank you guys for taking a look at my ticket!
Attachment: Download
Additional Detail from JIRA
| | | |------------------|-----------------| |Votes | 0 | |Component/s | Foundation | |Labels | Bug, Codable | |Assignee | hungtruong (JIRA) | |Priority | Medium | md5: bc6d6a7e328908f8a562ab76fbb1e965Issue Description:
Decodable should cause run time errors when attempting to parse Floats as Integers. However, this does not occur when the Float value is a whole number. Because Decodable will only fail at runtime, it is prudent that it correctly fails on these simpler cases as it can mask issues
By comparison the swift compiler would reject the follow code:
or