Skip to content

Conversation

@stefanadranca
Copy link
Collaborator

Motivation:

The types are used by the code renderer that will be brought over from OpenAPI Generator to the new gRPC Swift CodeGenLib.

Modifications:

Copied the TypeName, TypeUsage and some TypeName extensions that will be used in the context of gRPC, and modified TypeName so it represents only the Swift path of a type.

Result:

We will be able to bring over the Code Renderer and we will have types representations that can be used by the service translator.

Motivation:

The types are used by the code renderer that will be brought over from OpenAPI Generator
to the new gRPC Swift CodeGenLib.

Modifications:

Copied the TypeName, TypeUsage and some TypeName extensions that will be used
in the context of gRPC, and modified TypeName so it represents only the Swift path
of a type.

Result:

We will be able to bring over the Code Renderer and we will have types representations that can be used
by the service translator.
}

/// A list of components that make up the type name.
private let components: [Component]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can also use Strings to represent the components (since the struct has only one property). I wanted to check if there is any advantage that I don't see in still having a struct.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we should just use Strings here which means we can simplify more of this type too.

Comment on lines 107 to 125
extension TypeName {
/// Returns the type name for the String type.
static var string: Self { .swift("String") }

/// Returns the type name for the Int type.
static var int: Self { .swift("Int") }

/// Returns a type name for a type with the specified name in the
/// Swift module.
/// - Parameter name: The name of the type.
/// - Returns: A TypeName representing the specified type within the Swift module.
static func swift(_ name: String) -> TypeName { TypeName(swiftKeyPath: ["Swift", name]) }

/// Returns a type name for a type with the specified name in the
/// Foundation module.
/// - Parameter name: The name of the type.
/// - Returns: A TypeName representing the specified type within the Foundation module.
static func foundation(_ name: String) -> TypeName {
TypeName(swiftKeyPath: ["Foundation", name])
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This extension is from another file - Builtins.swift. Should I signal this in any way?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is fine.

}

/// A list of components that make up the type name.
private let components: [Component]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we should just use Strings here which means we can simplify more of this type too.

Comment on lines 107 to 125
extension TypeName {
/// Returns the type name for the String type.
static var string: Self { .swift("String") }

/// Returns the type name for the Int type.
static var int: Self { .swift("Int") }

/// Returns a type name for a type with the specified name in the
/// Swift module.
/// - Parameter name: The name of the type.
/// - Returns: A TypeName representing the specified type within the Swift module.
static func swift(_ name: String) -> TypeName { TypeName(swiftKeyPath: ["Swift", name]) }

/// Returns a type name for a type with the specified name in the
/// Foundation module.
/// - Parameter name: The name of the type.
/// - Returns: A TypeName representing the specified type within the Foundation module.
static func foundation(_ name: String) -> TypeName {
TypeName(swiftKeyPath: ["Foundation", name])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is fine.

Comment on lines 66 to 67
func appending(component: String? = nil) -> Self {
precondition(component != nil, "The type name must be non-nil.")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a weird combination, we should just make the String non-optional

Copy link
Collaborator

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit, looks good otherwise

@glbrntt glbrntt added the version/v2 Relates to v2 label Nov 29, 2023
@glbrntt glbrntt merged commit bd0d0fd into grpc:main Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

version/v2 Relates to v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants