Closed czechboy0 closed 1 month ago
Hunch: Agree that this is a nice QOL improvement for this one case.
Thought: we initially tried to have the generator work without too much "look-ahead", i.e. that it wouldn't need to reach into other parts to work out whether it could or couldn't provide a nicety. We've now got a few places where we do that, but each of these will increase the runtime of the generation. I wonder how common this is, because if we have to pay the price of working out that we can't do it for almost every operation, it may not be worth it?
Question: should we consider (instead, or as well as) having the headers parameter Operations.addPollAnswer.Output.NoContent.init
be defaulted to .init()
if there are no required headers.
Discussed offline: let's do this only when there are no headers or body, but do not do it if there are any headers (even if they're all optional). Should be mostly isolated to changes in this method: https://github.com/apple/swift-openapi-generator/blob/58a276ac955bd50e3d252a9e79ebabdfe0d54b03/Sources/_OpenAPIGeneratorCore/Translator/Responses/translateResponse.swift#L25
Okay @arthurcro, feel free to pick this up
Hey @czechboy0! I made some progress on the implementation and I have a draft version working. However, before I open a pull request, I'd like to discuss some specific details with you to ensure I've correctly interpreted the requirements.
Adding the static property to this function won't work because it needs to be generated outside the struct declaration.
Instead, I was thinking we could add it to: https://github.com/apple/swift-openapi-generator/blob/58a276ac955bd50e3d252a9e79ebabdfe0d54b03/Sources/_OpenAPIGeneratorCore/Translator/Responses/translateResponseOutcome.swift#L31 instead.
It would mean updating the signature to something like:
func translateResponseOutcomeInTypes(
_ outcome: OpenAPI.Operation.ResponseOutcome,
operation: OperationDescription,
operationJSONPath: String
) throws -> (
payloadStruct: Declaration?,
enumCase: Declaration,
staticProperty: Declaration?,
throwingGetter: Declaration
) { ... }
What do you think? :slightly_smiling_face:
Then, I realised we also need to check for responseKind.wantsStatusCode
before adding this static property because if that's the case, it adds an extra associated value to the generated enum case which we can't default in the expression assigned to the static property.
Please let me know if you have any concerns!
Agreed with all of the above, and let's only generate the convenience static property for cases that don't require the explicit status code as an associated value.
Motivation
Today, some responses have no headers nor body (for example, 204 No Content).
In generated code it looks like this:
And when writing a server handler, adopters have to spell it as:
The
(.init())
bit is unnecessary, and we should make this common case prettier.Proposed solution
One possible way to fix this (I welcome other ideas, though) is by generating a
static var
for any response cases that have no required values in the initializer. In practice, since response bodies are always required, this would only apply to responses with no body and with only optional response headers.The generated code would look closer to the following:
which would allow the server handler to just have:
Alternatives considered
No response
Additional information
No response