yonaskolb / SwagGen

OpenAPI/Swagger 3.0 Parser and Swift code generator
MIT License
627 stars 148 forks source link

Don't generate inline schemas when `allOf` only contains a single schema #278

Closed liamnichols closed 3 years ago

liamnichols commented 3 years ago

This relates to the patch applied in #269, which in turn relates to #267. cc @nicholascross, @ymhuang0808. Firstly please excuse me if i missed some important historical context but I'm just trying to solve a slightly different regression that I noticed in some of my own examples.

In #269, we reintroduced inline generation for allOf types however in turn this introduced this diff: https://github.com/yonaskolb/SwagGen/pull/269/files#diff-6582636d47b022b95a7a5dfd79ff1e767f5f862e5eba00c00eb46208c5e7aab1

I believe that this is separate to the original issue however since Zoo.AllOfDog and Zoo.InlineAnimal go unused and this is related to the issue that I am seeing with my schema.

If we take the following:

UserSubclass:
  allOf:
    - $ref: "#/components/schemas/User"
    - type: object
      properties:
        age:
          type: integer
        last_error:
          allOf:
            - $ref: "#/components/schemas/Error"
          nullable: true
          description: last error reported to user object, or null if they have not seen an error.

The intention of the last_error property is to reference an existing schema (Error) but to override its nullability and/or description when used within UserSubclass since its likely that these two properties on the schema (and possibly more) will vary. My understanding is that this approach is commonly used and i've seen other generators working with it.

In fact, SwagGen was already properly resolving the property types to Animal and Dog as expected (hence why the inline types went unused) thanks to this:

https://github.com/yonaskolb/SwagGen/blob/76d42a66befce35a83cd2220dec3a654eebe08e0/Sources/SwagGenKit/SwiftFormatter.swift#L156-L162

As a result, my change ensures that inlineSchema is only returned if group.schemas.count > 1 and this means that the unused LastError inline type would be omitted again:

Before ```diff diff --git a/Specs/TestSpec/generated/Swift/Sources/Models/UserSubclass.swift b/Specs/TestSpec/generated/Swift/Sources/Models/UserSubclass.swift index d11ebc7..a6414ef 100644 --- a/Specs/TestSpec/generated/Swift/Sources/Models/UserSubclass.swift +++ b/Specs/TestSpec/generated/Swift/Sources/Models/UserSubclass.swift @@ -9,8 +9,33 @@ public class UserSubclass: User { public var age: Int? - public init(id: Int? = nil, name: String? = nil, age: Int? = nil) { + /** last error reported to user object, or null if they have not seen an error. */ + public var lastError: ErrorType? + + /** last error reported to user object, or null if they have not seen an error. */ + public class LastError: ErrorType { + + public override init(code: Int, message: String) { + super.init(code: code, message: message) + } + + public required init(from decoder: Decoder) throws { + try super.init(from: decoder) + } + + public override func encode(to encoder: Encoder) throws { + try super.encode(to: encoder) + } + + override public func isEqual(to object: Any?) -> Bool { + guard object is LastError else { return false } + return super.isEqual(to: object) + } + } + + public init(id: Int? = nil, name: String? = nil, age: Int? = nil, lastError: ErrorType? = nil) { self.age = age + self.lastError = lastError super.init(id: id, name: name) } @@ -18,6 +43,7 @@ public class UserSubclass: User { let container = try decoder.container(keyedBy: StringCodingKey.self) age = try container.decodeIfPresent("age") + lastError = try container.decodeIfPresent("last_error") try super.init(from: decoder) } @@ -25,12 +51,14 @@ public class UserSubclass: User { var container = encoder.container(keyedBy: StringCodingKey.self) try container.encodeIfPresent(age, forKey: "age") + try container.encodeIfPresent(lastError, forKey: "last_error") try super.encode(to: encoder) } override public func isEqual(to object: Any?) -> Bool { guard let object = object as? UserSubclass else { return false } guard self.age == object.age else { return false } + guard self.lastError == object.lastError else { return false } return super.isEqual(to: object) } } ```
After ```diff diff --git a/Specs/TestSpec/generated/Swift/Sources/Models/UserSubclass.swift b/Specs/TestSpec/generated/Swift/Sources/Models/UserSubclass.swift index d11ebc7..d34e692 100644 --- a/Specs/TestSpec/generated/Swift/Sources/Models/UserSubclass.swift +++ b/Specs/TestSpec/generated/Swift/Sources/Models/UserSubclass.swift @@ -9,8 +9,12 @@ public class UserSubclass: User { public var age: Int? - public init(id: Int? = nil, name: String? = nil, age: Int? = nil) { + /** last error reported to user object, or null if they have not seen an error. */ + public var lastError: ErrorType? + + public init(id: Int? = nil, name: String? = nil, age: Int? = nil, lastError: ErrorType? = nil) { self.age = age + self.lastError = lastError super.init(id: id, name: name) } @@ -18,6 +22,7 @@ public class UserSubclass: User { let container = try decoder.container(keyedBy: StringCodingKey.self) age = try container.decodeIfPresent("age") + lastError = try container.decodeIfPresent("last_error") try super.init(from: decoder) } @@ -25,12 +30,14 @@ public class UserSubclass: User { var container = encoder.container(keyedBy: StringCodingKey.self) try container.encodeIfPresent(age, forKey: "age") + try container.encodeIfPresent(lastError, forKey: "last_error") try super.encode(to: encoder) } override public func isEqual(to object: Any?) -> Bool { guard let object = object as? UserSubclass else { return false } guard self.age == object.age else { return false } + guard self.lastError == object.lastError else { return false } return super.isEqual(to: object) } } ```

As for the original issue reported in #267, I updated TestSpec in c10a5e693df9a24fe5b2ce3131ac777202da5b29 to include the expected results described by @ymhuang0808 (at least I think I did 😅) so hopefully the updated fixtures describe fixes to both issues... Please confirm though!

Thanks 🙇