Created the CodeGenerationRequest

[stefanadranca]

Created the CodeGenerationRequest which represents the services, dependencies and trivia from an IDL file, and the IDL itself through its specific serializer and deserializer.

Motivation:

The CodeGenerationRequest is part of the new code generator library for grpc-swift. It represents services described by IDL and is the first stage in the process of code generation.

Modifications:

Implemented the CodeGenerationRequest struct and added the new target and library in manifest.

Result:

The code generators will be able to use a general representation for the services described in different IDLs.

[glbrntt]

We shouldn't add a product because we don't want to make it API yet.

[glbrntt]

The name should match the module name: grpcCodeGen

[glbrntt]

We don't use Foundation here

[glbrntt]

Why is this file in a layers directory?

[stefanadranca]

I was thinking we can have all the representations in a directory (CodeGenerationRequest, StructuredSwiftSyntax and SourceFile). Would "Representations" be a better name, or should I not create the directory?

[glbrntt]

I tend to split them into directories based on the "area" when there are (or will be) many files. In this case I'm not sure there will be enough files so I probably wouldn't bother. One thing that can be useful though is to but internal things in an Internal directory.

[glbrntt]

This should be public.

[glbrntt]

Suggested change

	/// The swift imports that this file depends on.
	/// The Swift imports that the generated file should depends on.

[gjcairo]

You could maybe also use - SeeAlso: to link to Dependency?

[glbrntt]

I don't think the documentation for these really explains what they are for.

[glbrntt]

This needs to be a struct backed by an enum so we can evolve it. We also don't need it to be raw representable as a String.

[gjcairo]

FYI, you also don't need to explicitly define the String the case maps to if you want it to be the same as the case name (i.e. having case typelias without = "typealias" works).

[glbrntt]

nit: documentation

[glbrntt]

This needs documentation

[gjcairo]

Do we need this to be optional? An empty array can still represent no dependencies.

[stefanadranca]

You're right, it doesn't need to be optional

[gjcairo]

You could maybe also use - SeeAlso: to link to Dependency?

[gjcairo]

Consider linking to ServiceDescriptor

[gjcairo]

FYI, you also don't need to explicitly define the String the case maps to if you want it to be the same as the case name (i.e. having case typelias without = "typealias" works).

[gjcairo]

Some docs are missing for these props

[gjcairo]

Nit, docs are missing

[glbrntt]

Not all files have extensions:

Suggested change

	/// The name of the source file containing the IDL. It contains the file's extension.
	/// The name of the source file containing the IDL, including the extension if applicable.

[glbrntt]

Suggested change

	/// The Swift imports that the generated file should depend on.
	/// The Swift imports that the generated file depends on.

[glbrntt]

Can you clarify in the docs that gRPC imports aren't required because we'll add those?

[glbrntt]

They don't have to include these things, we're just stating examples of what they'll be.

Suggested change

	/// Any comments at the top of the file including documentation and copyright headers.
	/// Any comments at the top of the file such as documentation and copyright headers.

[glbrntt]

Can you also add a note here saying that we'll place these verbatim at the top of the generated file?

[glbrntt]

See "Label tuple members and name closure parameters" https://www.swift.org/documentation/api-design-guidelines/#special-instructions:

Suggested change

	var lookupSerializer: (String) -> String
	var lookupSerializer: (_ messageType: String) -> String

[glbrntt]

What want Kind to be is an enum, but we can't do that because adding a new case to an enum is an API breaking change. Instead we can have a struct backed by an enum; internally we can have access to the enum so we can switch over its values. However, publicly there needs to be API to create each enum case. The usual pattern is like this:

public struct Kind {
  internal enum Value {
    case foo
    case bar
  }

  internal var value: Value
  internal init(_ value: Value) { 
    self.value = value
  }

  public static var foo: Self { 
    Self(.foo) 
  }

  public static var bar: Self { 
    Self(.bar) 
  }
}

[glbrntt]

This doesn't tell me anything more than [ServiceDescriptor] tells me (i.e. what the thing is). If you say "A description of each service to generate." then it tells the user what it's used for.

[glbrntt]

Docs should start with a single sentence summary and add more detail in other paragraphs. Tools like Xcode and DocC show the first paragraph more prominently so it's quite important for it to be concise.

We can also be more specific about the requirements here: the serializer type must conform to MessageSerializer.

Suggested change

	/// Closure that receives the message type and returns a string representation of the
	/// serializer call for that message type. The result is inserted in the string representing
	/// the generated code, where clients or servers serialize their output.
	///
	/// For example: `lookupSerializer: {"ProtobufSerializer<\($0)>()"}`.
	/// Closure that receives a message type as a `String` and returns a code snippet to
	/// initialize a `MessageSerializer` for that type as a `String`.
	///
	/// The result is inserted in the generated code, where clients serialize RPC inputs and
	/// servers serialize RPC outputs.
	///
	/// For example, to serialize Protobuf messages you could specify a serializer as:
	/// ```swift
	/// request.lookupSerializer = { messageType in
	/// "ProtobufSerializer<\(messageType)>()"
	/// }
	/// ```

[glbrntt]

This needs a similar treatment to the one above, but the deserializer must conform to MessageDeserializer.

[glbrntt]

Generally parameters should be specified in this order:

1. required
2. optional
3. closures

In this case, I think we should probably just remove the defaults for dependencies and services. It's highly unlikely that there will be no dependencies or services.

We should also invert the order of dependencies and leadingTrivia, if you're reading a file top-to-bottom the leading trivial comes at the very top followed by the dependencies.

[glbrntt]

nit: leave a space between these

[glbrntt]

Suggested change

	/// The keyword that specifies the item's kind (e.g. `func`, `struct`).
	/// The keyword that specifies the item's kind (e.g. `func`, `struct`).

[glbrntt]

Suggested change

	/// The imported item's symbol / name.
	/// The name of the imported item.

[glbrntt]

Same comment as on services, this doesn't tell me anything more than [MethodDescriptor].

[glbrntt]

I don't think we should default this, It'd be very strange to generate a service with no methods!