[SR-10596] Decodable does not fail to decode Float as Int for whole numbers

[swift-ci]

Previous ID	SR-10596
Radar	rdar://problem/50371749
Original Reporter	jozen (JIRA User)
Type	Bug
Status	In Progress
Resolution

Attachment: Download

Additional Detail from JIRA

Votes	0
Component/s	Foundation
Labels	Bug, Codable
Assignee	hungtruong (JIRA)
Priority	Medium

md5: bc6d6a7e328908f8a562ab76fbb1e965

Issue 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

import Foundation

struct Swifter: Decodable {
  let floatParsedAsInt: Int
}

let json = """
{
 "floatParsedAsInt": 1.0,
}
""".data(using: .utf8)!
let myStruct = try JSONDecoder().decode(Swifter.self, from: json)
print(myStruct.floatParsedAsInt) //output 1

By comparison the swift compiler would reject the follow code:

var intValue: Int = 1.0 // Error: cannot convert value of type 'Double' to specified type 'Int'

or

struct JSONStructureWithInt: Decodable {
  let intAsString: Int
}

let json = """
{
 "intAsString": "1",
}
""".data(using: .utf8)!

let myStruct1 = try JSONDecoder().decode(JSONStructureWithString.self, from: json) // Error: Expected to decode Int but found a string/data instead
print(myStruct1.intAsString) 

[theblixguy]

cc @itaiferber

[millenomi]

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.

[swift-ci]

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.

[itaiferber]

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 from 1.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 an Int, and we'd need a very compelling reason to break the behavior on them.

[swift-ci]

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

[swift-ci]

Comment by Julian (JIRA)

Obviously I’ve fixed my code and updated my tests to include some non-whole numbers since that incident

[swift-ci]

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.

[swift-ci]

Comment by Julian (JIRA)

Just wanted to follow up here and thank you guys for taking a look at my ticket!