Search code, repositories, users, issues, pull requests...

Copy link

@kenjitayama kenjitayama commented Mar 18, 2016

long should be mapped to Int64 instead of Int, because long will have a range of signed 64 bits.

Following the recommendation of the Swift Doc, fixing only the mapping for long is necessary and sufficient.

Int
In most cases, you don’t need to pick a specific size of integer to use in your code. Swift provides an additional integer type, Int, which has the same size as the current platform’s native word size:
On a 32-bit platform, Int is the same size as Int32.
On a 64-bit platform, Int is the same size as Int64.
Unless you need to work with a specific size of integer, always use Int for integer values in your code. This aids code consistency and interoperability. Even on 32-bit platforms, Int can store any value between -2,147,483,648 and 2,147,483,647, and is large enough for many integer ranges.

@kenjitayama kenjitayama mentioned this pull request

[Swift] generating Int32/Int64 when those are required in format #2133

Closed

@wing328

Copy link

Contributor

wing328 commented Mar 18, 2016

@kenjitayama thanks for the PR. Please update the languageSpecificPrimitives at https://github.com/kenjitayama/swagger-codegen/blob/fixSwiftCodegenTypeMapping/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/SwiftCodegen.java#L76 to include Int64

@wing328 wing328 added the Client: Swift label

@wing328 wing328 added this to the v2.1.6 milestone

@wing328 wing328 added Enhancement: Feature Enhancement: General and removed Enhancement: Feature labels


 SwiftCodegen : change typeMapping for 'long' (Int64 instead of Int).

929ea5e

@kenjitayama kenjitayama force-pushed the fixSwiftCodegenTypeMapping branch from adf63de to 929ea5e Compare

March 18, 2016 06:31

Copy link

Author

kenjitayama commented Mar 18, 2016

@wing328 Didn't notice that part... Added now.

Copy link

Contributor

cjolif commented Mar 18, 2016

I think this (unfortunately) won't be enough. First as I commented on the issue, I think 32 version is also needed. But also, the code generated now is not functional. Indeed while Int can be automatically put in a AnyObject this is not the case of Int64 (see: http://stackoverflow.com/questions/28920232/why-do-integers-not-conform-to-the-anyobject-protocol for why, basically there is some auto-casting Swift is doing to NSNumber for Int but not bit-exact versions). So in the encodeToJSON method this case must be taken into account. I was actually working on a PR myself and faced that when testing the generated code :(

@wing328 wing328 modified the milestones: Future, v2.1.6

Copy link

Author

kenjitayama commented Mar 18, 2016

@wing328 Thanks for pointing out.
I was using my custom template, and Int64 was fine for that, and didn't notice that.
Looks like there is more work to do for a complete fix...

Copy link

Author

kenjitayama commented Mar 18, 2016

Sorry, intended to @ mention @cjolif in my previous comment.

Copy link

Author

kenjitayama commented Mar 18, 2016

I'm not sure I can come up with a complete fix, so maybe this PR can be closed for now.

Copy link

Contributor

cjolif commented Mar 18, 2016

I'll try to find time to propose a PR that does take that into account. But no promise on when.

Copy link

Contributor

cjolif commented Mar 19, 2016

Looks like I was not as far as I thought. At least a tentative here: #2417

Copy link

Author

kenjitayama commented Mar 19, 2016

@cjolif Awesome! Everyone will have a safer Swift client :)
I see why you needed to use NSNumber, mentioned in your previous comment.
It seems to me that this is why you said tentative, but looks best for now.
A fix without this kind of workaround would need a bigger fix, with a different approach.