[CodeGenLib] Translator for Client Code #1748
Conversation
| genericType: String? = nil, | ||
| conformances: [String] = [] |
There was a problem hiding this comment.
We need these to be able to represent generic functions
There was a problem hiding this comment.
This isn't quite right: the "conformance" here applies to the generic type, not the function. Also functions can have multiple generic types.
It looks like the best way to represent this would be by modifying the FunctionSignatureDescription to add: a generics: [ExistingTypeDescription] property and an optional var whereClause: WhereClause?.
That way we can express the Sendable requirement as a where clause:
func foo<R>() -> R where R: Sendable {
// ...
}
| /// | ||
| /// The parameter doesn't have a type, but is a closure. | ||
| /// For example: `(String) async throws -> Int`. | ||
| var closureDescription: ClosureSignatureDescription? = nil |
There was a problem hiding this comment.
I have decided to have a different variable that signals if the parameter is a closure (and also its description) because the ExistingTypeDescription rendering doesn't allow more complex types.
There was a problem hiding this comment.
I don't think this is the right decision, ExistingTypeDescription should be able to describe a closure so should probably just have a closure case which holds a ClosureSignatureDescription
| ) | ||
| let expectedSwift = | ||
| """ | ||
| protocol namespaceA_ServiceAClientProtocol: Sendable { |
| let expectedSwift = | ||
| """ | ||
| protocol namespaceA_ServiceAClientProtocol: Sendable { | ||
| func methodA<R: Sendable>( |
| request, | ||
| namespaceA.ServiceA.Methods.methodA.descriptor, | ||
| serializer, | ||
| deserializer, | ||
| body |
There was a problem hiding this comment.
The labels are missing here
| ) | ||
| } | ||
| } | ||
| struct namespaceA_ServiceAClient: namespaceA.ServiceA.ClientProtocol { |
| init(client: GRPCCore.GRPCClient) { | ||
| self.client = client | ||
| } | ||
| func methodA<R: Sendable>( |
| func methodA<R: Sendable>( | ||
| request: ClientRequest.Single<namespaceA.ServiceA.Methods.methodA.Input>, | ||
| _ body: @Sendable @escaping (ClientResponse.Single<namespaceA.ServiceA.Methods.methodA.Output>) async throws -> R | ||
| ) async rethrows -> R { |
There was a problem hiding this comment.
This should just be throws (the client API changed slightly so we can't use rethrows anymore)
| genericType: String? = nil, | ||
| conformances: [String] = [] |
There was a problem hiding this comment.
This isn't quite right: the "conformance" here applies to the generic type, not the function. Also functions can have multiple generic types.
It looks like the best way to represent this would be by modifying the FunctionSignatureDescription to add: a generics: [ExistingTypeDescription] property and an optional var whereClause: WhereClause?.
That way we can express the Sendable requirement as a where clause:
func foo<R>() -> R where R: Sendable {
// ...
}
| */ | ||
| /// Creates a representation for the client code that will be generated based on the ``CodeGenerationRequest`` object |
There was a problem hiding this comment.
Leave a blank line between these
| /// ) async rethrows -> R { | ||
| /// try await self.client.clientStreaming( | ||
| /// request, | ||
| /// namespaceA.ServiceA.Methods.methodA.descriptor, | ||
| /// serializer, | ||
| /// deserializer, | ||
| /// body | ||
| /// ) | ||
| /// } | ||
| /// } |
There was a problem hiding this comment.
The indentation looks off here
| for service: CodeGenerationRequest.ServiceDescriptor, | ||
| in codeGenerationRequest: CodeGenerationRequest | ||
| ) -> Declaration { | ||
| let methods = service.methods.compactMap { |
There was a problem hiding this comment.
This should just be a map as makeClientProtocolMethod doesn't return an optional
| for service: CodeGenerationRequest.ServiceDescriptor, | ||
| in codeGenerationRequest: CodeGenerationRequest | ||
| ) -> Declaration { | ||
| let methods = service.methods.compactMap { |
There was a problem hiding this comment.
nit: use map here instead
| for method: CodeGenerationRequest.ServiceDescriptor.MethodDescriptor, | ||
| in service: CodeGenerationRequest.ServiceDescriptor, | ||
| from codeGenerationRequest: CodeGenerationRequest, | ||
| serializerDeserializer: Bool |
There was a problem hiding this comment.
don't love this naming, serializerAndDeserializer / generateSerializerAndDeserializer are clearer
| if !serializerDeserializer { | ||
| let body = self.makeSerializerDeserializerCall( | ||
| for: method, | ||
| in: service, | ||
| from: codeGenerationRequest | ||
| ) | ||
| return .function(signature: functionSignature, body: body) | ||
| } | ||
| return .function(signature: functionSignature) |
There was a problem hiding this comment.
Bit of a subjective style thing, but in my opinion if the condition being checked is an if-else then we should spell out both branches (i.e. if serializerDeserializer { x } else { y }). If the condition is being used to determine whether we can continue then it's okay to just have an if/guard (e.g. if !inputIsValid { return }).
| method.isInputStreaming | ||
| ? ExistingTypeDescription.member(["ClientRequest", "Stream"]) | ||
| : ExistingTypeDescription.member(["ClientRequest", "Single"]) |
There was a problem hiding this comment.
nit: minimise branching
let requestType = method.isInputStreaming ? "Stream" : "Single"
let clientRequestType = ExistingTypeDescription.member(["ClientRequest", requestType])| if method.isInputStreaming && method.isOutputStreaming { | ||
| return "bidirectionalStreaming" | ||
| } | ||
| if method.isInputStreaming && !method.isOutputStreaming { | ||
| return "clientStreaming" | ||
| } | ||
| if !method.isInputStreaming && method.isOutputStreaming { | ||
| return "serverStreaming" | ||
| } | ||
| return "unary" |
There was a problem hiding this comment.
There's a neater way of doing this which ensures you cover all possibilities:
switch (method.isInputStreaming, method.isOutputStreaming) {
case (true, true):
return "bidi..."
case (true, false):
return "client..."
case (false, true):
return "server..."
case (false, false):
return "unary"
}| ) | ||
| } | ||
|
|
||
| private func getGRPCMethodName( |
There was a problem hiding this comment.
"grpc method name" is misleading here, it sounds like the name of the RPC, clientMethod is a bit clearer. Also we should just pass in two parameters here: input and output streaming
| } | ||
| writer.writeLine(")") | ||
|
|
||
| do { |
There was a problem hiding this comment.
This do looks unnecessary, there's no catch
|
|
||
| do { | ||
| let keywords = signature.keywords | ||
| if !keywords.isEmpty { |
There was a problem hiding this comment.
This looks unnecessary too, we'll catch the is empty catch in the loop over keywords
| writer.writeLine(renderedFunctionKind(signature.kind) + "(") | ||
| let generics = signature.generics | ||
| let genericsString = | ||
| "\(generics.map{renderedExistingTypeDescription($0)}.joined(separator: ", "))" |
There was a problem hiding this comment.
| "\(generics.map{renderedExistingTypeDescription($0)}.joined(separator: ", "))" | |
| "\(generics.map {renderedExistingTypeDescription($0) }.joined(separator: ", "))" |
| /// | ||
| /// The parameter doesn't have a type, but is a closure. | ||
| /// For example: `(String) async throws -> Int`. | ||
| var closureDescription: ClosureSignatureDescription? = nil |
There was a problem hiding this comment.
I don't think this is the right decision, ExistingTypeDescription should be able to describe a closure so should probably just have a closure case which holds a ClosureSignatureDescription
| let serverCodeTranslator = ServerCodeTranslator() | ||
| let clientCodeTranslator = ClientCodeTranslator() |
There was a problem hiding this comment.
We should only create these if we use them, i.e. in the if server/if client blocks
…tingTypes and updated the methods depending on it
Motivation: In the generated client code for a service described in a Source IDL file we want to have: - a protocol declaring the methods for a client, with the serializer and deserializer as parameters. - an extension for the protocol defining the methods of the client, without the serializer and deserializer as parameters, that are calling the already declared methods, providing the default Protobuf serializer and deserializer. - a struct representing the generated client that has a GRPCClient as a property and conforms to the Client Protocol. Modifications: - Implemented the ClientCodeTranslator struct that contains all the methods for creating the `StructuredSwiftRepresentation` for the client code. - Added a new struct in the `StructuredSwiftRepresentation` that represents closure signatures, and a new `ParameterDescription` property that is set only when the parameter is a closure signature. - Added and modified render functions for the `StructuredSwiftRepresentation` additions. - Implemented snippet tests. Result: Client code can now be generated for a CodeGenerationRequest object.
2 participants
Motivation:
In the generated client code for a service described in a Source IDL file we want to have:
that are calling the already declared methods, providing the default Protobuf serializer and deserializer.
Modifications:
StructuredSwiftRepresentationfor the client code.StructuredSwiftRepresentationthat represents closure signatures, and a newParameterDescriptionproperty that is set only when the parameter is a closure signature.StructuredSwiftRepresentationadditions.Result:
Client code can now be generated for a CodeGenerationRequest object.