Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The generator is boxing properties of recursive types that do not need boxing #682

Open
acecilia opened this issue Nov 27, 2024 · 2 comments
Labels
area/generator Affects: plugin, CLI, config file. kind/bug Feature doesn't work as expected.

Comments

@acecilia
Copy link

Description

👋 I am observing that the generator is boxing properties that do not need boxing

Reproduction

This OpenAPI spec:

components:
  schemas:
    MyStruct:
      type: object
      properties:
        myProperty:
          type: array
          items:
            $ref: '#/components/schemas/MyStruct'

Generates boxed code:

internal struct MyStruct: Codable, Hashable, Sendable {
    /// - Remark: Generated from `#/components/schemas/MyStruct/myProperty`.
    internal var myProperty: [Components.Schemas.MyStruct]? {
        get  {
            storage.value.myProperty
        }
        _modify {
            yield &storage.value.myProperty
        }
    }
    /// Creates a new `MyStruct`.
    ///
    /// - Parameters:
    ///   - myProperty:
    internal init(myProperty: [Components.Schemas.MyStruct]? = nil) {
        storage = .init(value: .init(myProperty: myProperty))
    }
    internal enum CodingKeys: String, CodingKey {
        case myProperty
    }
    internal init(from decoder: any Decoder) throws {
        storage = try .init(from: decoder)
    }
    internal func encode(to encoder: any Encoder) throws {
        try storage.encode(to: encoder)
    }
    /// Internal reference storage to allow type recursion.
    private var storage: OpenAPIRuntime.CopyOnWriteBox<Storage>
    private struct Storage: Codable, Hashable, Sendable {
        /// - Remark: Generated from `#/components/schemas/MyStruct/myProperty`.
        var myProperty: [Components.Schemas.MyStruct]?
        init(myProperty: [Components.Schemas.MyStruct]? = nil) {
            self.myProperty = myProperty
        }
        typealias CodingKeys = Components.Schemas.MyStruct.CodingKeys
    }
}

Package version(s)

├── swift-openapi-generator<https://github.com/apple/[email protected]>
│   ├── swift-algorithms<https://github.com/apple/[email protected]>
│   │   └── swift-numerics<https://github.com/apple/[email protected]>
│   ├── swift-collections<https://github.com/apple/[email protected]>
│   ├── openapikit<https://github.com/mattpolzin/[email protected]>
│   │   └── yams<https://github.com/jpsim/[email protected]>
│   ├── yams<https://github.com/jpsim/[email protected]>
│   └── swift-argument-parser<https://github.com/apple/[email protected]>
├── swift-openapi-runtime<https://github.com/apple/[email protected]>
│   └── swift-http-types<https://github.com/apple/[email protected]>
├── swift-openapi-urlsession<https://github.com/apple/[email protected]>
│   ├── swift-openapi-runtime<https://github.com/apple/[email protected]>
│   │   └── swift-http-types<https://github.com/apple/[email protected]>
│   ├── swift-http-types<https://github.com/apple/[email protected]>
│   └── swift-collections<https://github.com/apple/[email protected]>
└── swift-argument-parser<https://github.com/apple/[email protected]>

Expected behavior

This swift code is valid and compiles:

struct MYStruct {
    let myProperty: [MYStruct]
}

So I would expect that the generated code does not have any boxing.

Environment

swift-driver version: 1.90.11.1 Apple Swift version 5.10 (swiftlang-5.10.0.13 clang-1500.3.9.4)
Target: arm64-apple-macosx14.0

Additional information

Originally raised in #70 (comment)

@acecilia acecilia added kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue. labels Nov 27, 2024
@czechboy0 czechboy0 added area/generator Affects: plugin, CLI, config file. and removed status/triage Collecting information required to triage the issue. labels Nov 27, 2024
@czechboy0
Copy link
Contributor

Yes this looks like a bug. The Array is enough to break the cycle, so we shouldn't need to box it again.

The way to fix this would be to first write a failing snippet test with this case (look for the tests with the word "recursive" in the name for inspiration), and then update the boxing logic.

@acecilia, would you consider contributing a fix here, with our guidance?

@acecilia
Copy link
Author

Thanks for guidance 👌 Will try yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/generator Affects: plugin, CLI, config file. kind/bug Feature doesn't work as expected.
Projects
None yet
Development

No branches or pull requests

2 participants