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

Generate convenience initializers that flatten some nested scopes #22

Closed
czechboy0 opened this issue May 25, 2023 · 4 comments · Fixed by #308
Closed

Generate convenience initializers that flatten some nested scopes #22

czechboy0 opened this issue May 25, 2023 · 4 comments · Fixed by #308
Labels
area/generator Affects: plugin, CLI, config file. kind/usability Usability of generated code, ergonomics. 🆕 semver/minor Adds new public API. size/L Large task. (A couple of weeks of work.) status/needs-design Needs further discussion and a concrete proposal.
Milestone

Comments

@czechboy0
Copy link
Contributor

czechboy0 commented May 25, 2023

Creating input types is quite verbose because they contain multiple nested enums. (e.g. .ok(.init(headers: .init(), body: .json(...)))).

It may be useful to generate static helper functions on the output type so it's possible to write .ok(body: .json(...)) instead, or even .ok(...) directly.

We have to be careful here, though, as it's possible to go very far and generate a ton of conveniences, that end up making the API more difficult to understand. So will require a well thought-out proposal first, describing which high-value conveniences we should generate, and how we evaluate further requests.

We also need to keep in mind that the generator complexity shouldn't be significantly increased with this work, as per https://github.com/apple/swift-openapi-generator/blob/main/Sources/swift-openapi-generator/Documentation.docc/Articles/Project-scope-and-goals.md#principle-reduce-complexity-of-the-generator-implementation

@czechboy0 czechboy0 added 🆕 semver/minor Adds new public API. area/generator Affects: plugin, CLI, config file. status/needs-design Needs further discussion and a concrete proposal. size/M Medium task. (A couple of days of work.) labels May 25, 2023
@czechboy0
Copy link
Contributor Author

For unwrapping, let's see if this could work: case let .ok(.json(response)):.

@simonjbeaumont
Copy link
Collaborator

@czechboy0 this would work in a guard/else. Is that what you hand in mind?

I think we still need some heuristics for what to name these and when to generate them.

I'm still not sure how much value that work will add when adopters can write the same in their code.

I think conveniences on for creating inputs and outputs will probably add more value. WDYT?

@czechboy0
Copy link
Contributor Author

@czechboy0 this would work in a guard/else. Is that what you hand in mind?

Yeah, in a guard, if, or a switch. Just wanted to note here that once can unwrap more than one level in one go.

I think we still need some heuristics for what to name these and when to generate them.

Agreed. Right now I'm just dumping thoughts in this issue, I don't yet have a good idea of how we approach improving these.

@czechboy0
Copy link
Contributor Author

Ah nevermind, that won't work as the response associate value us a struct, not the body enum itself.

@czechboy0 czechboy0 added size/L Large task. (A couple of weeks of work.) and removed size/M Medium task. (A couple of days of work.) labels Aug 9, 2023
@czechboy0 czechboy0 added this to the Post-1.0 milestone Aug 25, 2023
@czechboy0 czechboy0 added the kind/usability Usability of generated code, ergonomics. label Aug 31, 2023
simonjbeaumont added a commit that referenced this issue Oct 3, 2023
…tputs (#308)

### Motivation

The review period for SOAR-0007 (Shorthand APIs for inputs and outputs)
has now concluded. This pull request adds the required SPIs to the
runtime library to throw a runtime error when the response and/or body
does not match that of the shorthand API being used.

For further context, please review the proposal itself.[^1] 

[^1]: #291

### Modifications

- Add support for computed properties with effects (e.g. throwing
getters).
- Generate a protocol extension to `APIProtocol` with an overload for
each operation that lifts each of the parameters of `Input.init` as
function parameters.
- Generate a throwing computed property for each enum case related to a
documented outcome, which will return the associated value for the
expected case, or throw a runtime error if the value is a different enum
case.

### Result

Code that used to be written like this 

```swift
// before
switch try await client.getGreeting(.init()) {
case .ok(let response):
    switch response.body {
    case .json(let body):
        print(body.message)
    }
case .undocumented(statusCode: _, _):
    throw UnexpectedResponseError()
}

// after
print(try await client.getGreeting().ok.body.json.message)
//                     ^          ^  ^       ^
//                     |          |  |       `- (New) Throws if body did not conform to documented JSON.
//                     |          |  |
//                     |          |  `- (New) Throws if HTTP response is not 200 (OK).
//                     |          |
//                     |          `- (New) No need to wrap parameters in input value.
//                     |
//                     `- (Existing) Throws if there is an error making the API call.
```

### Test Plan

This PR includes updates to the various tests:
- `SnippetBasedReferenceTests`
- `FileBasedReferenceTests`
- `PetstoreConsumerTests`

### Related Issues

- Resolves #22 
- Resolves #104 
- Resolves #145

---------

Signed-off-by: Si Beaumont <[email protected]>
simonjbeaumont added a commit that referenced this issue Oct 3, 2023
### Motivation

We'd like to run a proposal for generating additional API to simplify
providing operation inputs and handling
operation outputs for simple API calls with just one successful outcome.

### Modifications

- Add SOAR-0007: Shorthand APIs for operation inputs and outputs

(See the proposal itself for details)[^1]

### Result

n/a

### Test Plan

n/a

### Related Issues

#22, #104, #105, #145

[^1]: https://github.com/apple/swift-openapi-generator/pull/291/files

---------

Signed-off-by: Si Beaumont <[email protected]>
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/usability Usability of generated code, ergonomics. 🆕 semver/minor Adds new public API. size/L Large task. (A couple of weeks of work.) status/needs-design Needs further discussion and a concrete proposal.
Projects
None yet
2 participants