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

Omit throws on json computed var when possible #710

Open
liambutler-lawrence opened this issue Dec 24, 2024 · 2 comments
Open

Omit throws on json computed var when possible #710

liambutler-lawrence opened this issue Dec 24, 2024 · 2 comments
Labels
kind/feature New feature. status/triage Collecting information required to triage the issue.

Comments

@liambutler-lawrence
Copy link

Motivation

Currently, a computed var json is generated for every REST response body. Unfortunately, this var is marked as throws even if application/json is the only content type defined for that response body. This is the case for 100% of our REST API methods, and I imagine is the most common case across all users of this library.

/// The associated value of the enum case if `self` is `.json`.
                    ///
                    /// - Throws: An error if `self` is not `.json`.
                    /// - SeeAlso: `.json`.
                    public var json: Operations.postLinkedAccounts.Output.Conflict.Body.jsonPayload {
                        get throws {
                            switch self {
                            case let .json(body):
                                return body
                            }
                        }
                    }

This means that at the call site, I have to use try! to do something that is in reality perfectly safe:

case .conflict(let error): throw .conflict(try! error.body.json)

Proposed solution

The generator should only add the throws annotation to the computed var json if in fact there are multiple cases/content types for that response body.

Alternatives considered

No response

Additional information

No response

@liambutler-lawrence liambutler-lawrence added kind/feature New feature. status/triage Collecting information required to triage the issue. labels Dec 24, 2024
@czechboy0
Copy link
Contributor

Additional info to aid the discussion here - this is the proposal introducing the shorthand syntax: https://swiftpackageindex.com/apple/swift-openapi-generator/1.6.0/documentation/swift-openapi-generator/soar-0007

@simonjbeaumont
Copy link
Collaborator

Thanks for your interest in the project, and for taking the time to file this issue.

The shorthand API provides an alternative to the defensive, non-throwing, switch-based API.

However, we don't want to remove keywords and sacrifice local reasoning.

The response is an enum, regardless of whether there is only one documented response, and using the assumption-based, shorthand API is throwing.

If the server returns an undocumented response it will throw.

Bear in mind that this allows the evolution of APIs. It's possible that, with the version of the OpenAPI document the client has, there is only one documented response, but the server might grow another response, and until the client updates their doc, there's an asymmetry regarding the valid responses.

I'm pretty disinclined to remove throwing from these getters.

Happy holidays :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature. status/triage Collecting information required to triage the issue.
Projects
None yet
Development

No branches or pull requests

3 participants