Add a swift4 client generator #6010

ehyche · 2017-07-07T12:56:04Z

PR checklist

[x ] Read the contribution guidelines.
[ x] Ran the shell/batch script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates)
[x ] Filed the PR against the correct branch: master for non-breaking changes and 2.3.0 branch for breaking (non-backward compatible) changes.

Description of the PR

This PR implements the issue referenced here: #6002

This adds a new "swift4" generator. Swift 4 added the "Codable" protocol, which is just a combination of the "Encodable" and "Decodable" protocols. These protocols greatly simplify serialization to JSON and deserialization from JSON in Swift. For example, the Pet model object now just looks like:

open class Pet: Codable {

    public enum Status: String, Codable { 
        case available = "available"
        case pending = "pending"
        case sold = "sold"
    }
    public var id: Int64?
    public var category: Category?
    public var name: String?
    public var photoUrls: [String]?
    public var tags: [Tag]?
    /** pet status in the store */
    public var status: Status?

    public init() {}

}

open class Category: Codable {

    public var id: Int64?
    public var name: String?

    public init() {}

}

open class Tag: Codable {

    public var id: Int64?
    public var name: String?

    public init() {}

}

As compared to the "swift" and "swift3" generators, there is now no need for all of the generated Decoders in Models.mustache, and there is now no need for the JSONEncodable protocol encodeToJSON() method that previously each model object needed to implement. So this greatly simplified the model objects and reduced the size of the generated code.

NOTE: As of this writing, Swift 4 is only present in the Xcode 9 betas. The swift4 generator samples will not compile in Xcode 8, as it does not have support for Codable.

With this PR, I have:

Created a new Swift4Codegen generator which is based on the existing Swift3Codegen generator
Created new templates for swift4 which were based on existing swift3 templates but simplified significantly.
Included swift4 samples which are based on the swift3 samples with some slight changes
Made sure that all of the current swift4 unit tests pass

Future work:

Add more unit tests to test edge cases like:
- Missing required parameters
- Invalid date and data in the JSON
- Invalid JSON

jaz-ah · 2017-07-07T14:50:12Z

@ehyche this is super cool - i'll have time a little later today to take a look - thx!

ehyche · 2017-07-07T14:56:26Z

Thanks, @jaz-ah ! If you decide to pull down the branch and try it out running the unit tests yourself in Xcode, keep in mind you'll need to do that with the Xcode 9 beta.

jaz-ah · 2017-07-07T15:19:58Z

understood - i'm using Xcode9 Beta2 right now - is that ok?

jaz-ah · 2017-07-07T18:25:19Z

modules/swagger-codegen/src/main/resources/swift4/Podspec.mustache

+  s.source_files = '{{projectName}}/Classes/Swaggers/**/*.swift'{{#usePromiseKit}}
+  s.dependency 'PromiseKit', '~> 4.2.2'{{/usePromiseKit}}{{#useRxSwift}}
+  s.dependency 'RxSwift', '~> 3.4.1'{{/useRxSwift}}
+  s.dependency 'Alamofire', '~> 4.0'


I would make this 4.5 since that is when Alamofire fixed Xcode9 related issues.

ehyche · 2017-07-07T18:35:18Z

@jaz-ah : Yes, Xcode 9 beta 2 works - that is what I used to develop this.

jaz-ah · 2017-07-07T18:44:18Z

modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/Swift4Codegen.java

+                    "ErrorResponse", "Response",
+
+                    // swift keywords
+                    "Int", "Int32", "Int64", "Int64", "Float", "Double", "Bool", "Void", "String", "Character", "AnyObject", "Any", "Error", "URL",


let's add Codable as a reserved word here

jaz-ah · 2017-07-07T19:12:31Z

@ehyche just a couple minor comments if you want to add those I think we're good to merge!

ehyche · 2017-07-07T19:18:36Z

@jaz-ah : thanks for the review! I'm updating and re-generating the samples now. Will push up changes shortly.

- Changed Alamofire dependency from 4.0 to 4.5 - Added "Codable", "Encodable", and "Decodable" to list of reserved words in generator - Ran "pod update" in default, promisekit, and rxswift samples test projects

ehyche · 2017-07-07T19:37:00Z

@jaz-ah : I have made updates per your review comments and pushed changes up.

jaz-ah · 2017-07-07T19:41:22Z

great - looks like we're good to go @wing328 ... @ehyche we'll have to keep an eye on bug fixes in swift3 to make sure they make their way into swift4 for a while, it's the only disadvantage to the extra generator.

wing328 · 2017-07-08T17:33:39Z

@jaz-ah thanks for reviewing the change.

@ehyche thanks for the contribution 👍

I'll try to setup CI to cover the Swift4 Petstore clients later and let you guys know if I need any help.

cc @jgavris @Edubits

wing328 · 2017-07-08T18:05:58Z

FYI. Added @ehyche as the template creator of swift4 in the project README and sent out the following tweet to help promote the new generator:

https://twitter.com/wing328/status/883748380718977024

(please help retweet)

* Add a swift4 client generator * Updates per review comments: - Changed Alamofire dependency from 4.0 to 4.5 - Added "Codable", "Encodable", and "Decodable" to list of reserved words in generator - Ran "pod update" in default, promisekit, and rxswift samples test projects

jgavris · 2017-07-10T14:38:44Z

This is great work, thanks. Let's make sure to keep cherry-picking the patches to the Swift 3 generator up to Swift 4 as we make them.

amelnychuck · 2017-07-11T08:52:51Z

Hi all,

First off let me just say great work so far @ehyche — glad to see someone jumping on this after the WWDC announcement.

A couple of quick comments:

It looks like there are still quite a few references to the JSONEncodable previous way of handling things that I imagine can be removed or replaced now. (Please correct me if I'm wrong.)
After running one of my APIs through the swift4 generator, I'm receiving a lot of errors similar to the following for most all of my API methods:

A lot of these seem to be related to the fact that while the Codable protocol is referenced, it's not conformed to properly, and both Encodable and Decodable must be conformed to correctly. This is mostly a problem when you have custom objects in your Model rather than basic types like strings and bools.

Anyways, great work again and I look forward to seeing the generator further improved/fixed before the GM hits!

ehyche · 2017-07-11T14:51:43Z

@amelnychuck : Thanks! Yes, there is only one place that JSONEncodable is still used, and it is on my ToDo list to remove that last reference. Thanks for the reminder.

Regarding the errors:

There is no problem referencing non-basic types in your models, like this:

open class PriorityList: Codable {
...
public var selectedCommand: RelayCommand?
}

as long as RelayCommand also conforms to Codable. Is RelayCommand also a generated model object? If so, it should also conform to Codable.

In the request builder, the fact that I'm seeing RequestBuilder means that something unexpected happened in the generation. Since getBuilder<T:Codable> always assumes that the type parameter conforms to Codable. So I would never expect T to be Any. This probably indicates a corner case I haven't yet considered.

Is your schema publicly accessible? If so, I'd be glad to take a look at it, and find and fix the issues in the current swift4 generator.

amelnychuck · 2017-07-11T16:38:59Z

@ehyche
Regarding point 1. Yes exactly, RelayCommand does not conform to Codable, but it is generated.

RelayCommand.swift (in the Models folder of the swift 4 output) contains the following:

//
// RelayCommand.swift
//
// Generated by swagger-codegen
// https://github.com/swagger-api/swagger-codegen
//

import Foundation


public typealias RelayCommand = Any

Looks like the generator only makes it a type alias of Any. (Possibly this is an issue with my swagger specification?) I'm guessing this is also related to point 2.

Unfortunately my schema isn't publicly accessible (I can see if I can create a sample that reproduces the issue though), but I'll keep looking into it and if I can give you any more information to help diagnose the issue with the generation, let me know what I can do to help.

ehyche · 2017-07-11T17:14:59Z

@amelnychuck : Looks like RelayCommand was generated by this part of model.mustache:

import Foundation

{{#description}}

/** {{description}} */{{/description}}
{{#isArrayModel}}
public typealias {{classname}} = [{{arrayModelType}}]
{{/isArrayModel}}

So I would expect {{arrayModelType}} to be something other than "Any" - something like another generated model, or a primitive Swift type.

amelnychuck · 2017-07-11T17:29:39Z

Still looking into the issue, but I agree.

On further examination, it looks like removing all references to RelayCommand in the swift3 version of the SDK solved that issue and I am able to build a test project containing the swift3 sdk and Alamofire successfully. Looks like that's something I'll need to solve as to why the generator is making that type Any...

That being said, when I remove all the references to RelayCommand in my Swift4SDK and sample project (at which point swift3 worked correctly), errors related to that seem to go away, but I still get the list of errors for every instance of let requestBuilder: RequestBuilder<Any>.Type = SwaggerClientAPI.requestBuilderFactory.getBuilder()
in the project, which does not fail for the swift3 generated version.

cstreel · 2017-08-29T03:48:22Z

I've got a question, I need to populate the CodingKeys enum in swift4 classes, to map response keys to variables. I can see it in the mustache template (line 75), but I can't find documentation on how to define my yaml to generate it.

wing328 · 2017-08-29T04:01:22Z

https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/test/resources/2_0/petstore-with-fake-endpoints-models-for-testing.yaml contains various edge cases (enum models, enum properties, etc)

You may want to use it to generate the Swift 4 client to see how different mustache tags are populated.

cstreel · 2017-08-29T04:12:16Z

Found my issue, i need to use the 2.3.0 Generator not the 2.2.3 generator

Add a swift4 client generator

e68767b

wing328 added Client: Swift Enhancement: New generator labels Jul 7, 2017

wing328 added this to the v2.2.3 milestone Jul 7, 2017

jaz-ah reviewed Jul 7, 2017

View reviewed changes

Updates per review comments:

4892094

- Changed Alamofire dependency from 4.0 to 4.5 - Added "Codable", "Encodable", and "Decodable" to list of reserved words in generator - Ran "pod update" in default, promisekit, and rxswift samples test projects

wing328 merged commit 2ce3365 into swagger-api:master Jul 8, 2017

wing328 mentioned this pull request Jul 9, 2017

[Swift4] Setup Travis CI to cover Swift4 Petstore #6015

Open

ehyche deleted the ehyche-swift4-integration branch October 9, 2017 02:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a swift4 client generator #6010

Add a swift4 client generator #6010

ehyche commented Jul 7, 2017

jaz-ah commented Jul 7, 2017

ehyche commented Jul 7, 2017

jaz-ah commented Jul 7, 2017

jaz-ah Jul 7, 2017

ehyche commented Jul 7, 2017

jaz-ah Jul 7, 2017

jaz-ah commented Jul 7, 2017

ehyche commented Jul 7, 2017

ehyche commented Jul 7, 2017

jaz-ah commented Jul 7, 2017

wing328 commented Jul 8, 2017

wing328 commented Jul 8, 2017

jgavris commented Jul 10, 2017

amelnychuck commented Jul 11, 2017

ehyche commented Jul 11, 2017

amelnychuck commented Jul 11, 2017

ehyche commented Jul 11, 2017

amelnychuck commented Jul 11, 2017 •

edited

Loading

cstreel commented Aug 29, 2017

wing328 commented Aug 29, 2017

cstreel commented Aug 29, 2017

Add a swift4 client generator #6010

Add a swift4 client generator #6010

Conversation

ehyche commented Jul 7, 2017

PR checklist

Description of the PR

jaz-ah commented Jul 7, 2017

ehyche commented Jul 7, 2017

jaz-ah commented Jul 7, 2017

jaz-ah Jul 7, 2017

Choose a reason for hiding this comment

ehyche commented Jul 7, 2017

jaz-ah Jul 7, 2017

Choose a reason for hiding this comment

jaz-ah commented Jul 7, 2017

ehyche commented Jul 7, 2017

ehyche commented Jul 7, 2017

jaz-ah commented Jul 7, 2017

wing328 commented Jul 8, 2017

wing328 commented Jul 8, 2017

jgavris commented Jul 10, 2017

amelnychuck commented Jul 11, 2017

ehyche commented Jul 11, 2017

amelnychuck commented Jul 11, 2017

ehyche commented Jul 11, 2017

amelnychuck commented Jul 11, 2017 • edited Loading

cstreel commented Aug 29, 2017

wing328 commented Aug 29, 2017

cstreel commented Aug 29, 2017

amelnychuck commented Jul 11, 2017 •

edited

Loading