Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,27 @@ let targets: [Target] = [
.product(name: "protoc-gen-swift", package: "swift-protobuf"),
]
),

// Code generator SwiftPM command
.plugin(
name: "GRPCGeneratorCommand",
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should include "Protobuf" in the name. I find "Command" a weird suffix but that stems more from me finding the "command plugin" naming weird as well.

Suggested change
name: "GRPCGeneratorCommand",
name: "GRPCProtobufGeneratorCommand",

capability: .command(
intent: .custom(
verb: "generate-grpc-code-from-protos",
description: "Generate Swift code for gRPC services from protobuf definitions."
),
permissions: [
.writeToPackageDirectory(
reason:
"To write the generated Swift files back into the source directory of the package."
)
]
),
dependencies: [
"protoc-gen-grpc-swift",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"protoc-gen-grpc-swift",
.target(name: "protoc-gen-grpc-swift"),

.product(name: "protoc-gen-swift", package: "swift-protobuf"),
]
),
]

let package = Package(
Expand Down
264 changes: 264 additions & 0 deletions Plugins/GRPCGeneratorCommand/CommandConfig.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,264 @@
/*
* Copyright 2024, gRPC Authors All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import Foundation
import PackagePlugin

struct CommandConfig {
var common: GenerationConfig

var dryRun: Bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

Love this, great idea!


static let defaults = Self(
common: .init(
accessLevel: .internal,
servers: true,
clients: true,
messages: true,
fileNaming: .fullPath,
accessLevelOnImports: false,
importPaths: [],
outputPath: ""
),
dryRun: false
)
}

extension CommandConfig {
static func parse(
arguments: [String],
pluginWorkDirectory: URL
) throws -> (CommandConfig, [String]) {
var config = CommandConfig.defaults

var argExtractor = ArgumentExtractor(arguments)

for flag in Flag.allCases {
switch flag {
case .accessLevel:
let accessLevel = argExtractor.extractOption(named: flag.rawValue)
if let value = accessLevel.first {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If there's more than one value we should either warn or throw an error.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(This feedback applies to all of these options.)

switch value.lowercased() {
case "internal":
config.common.accessLevel = .`internal`
case "public":
config.common.accessLevel = .`public`
case "package":
config.common.accessLevel = .`package`
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have an init for this!

default:
Diagnostics.error("Unknown accessLevel \(value)")
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should parrot out the name of the argument as passed by the user ("--access-level") rather than the property name.

}
}
case .servers:
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: add a newline between cases for long enums, makes them much easier to read

let servers = argExtractor.extractOption(named: flag.rawValue)
if let value = servers.first {
guard let servers = Bool(value) else {
throw CommandPluginError.invalidArgumentValue(value)
}
config.common.servers = servers
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Booleans are more naturally represented as flags, e.g. "--servers"/"--no-servers". We should support both for client/server/messages where the default is true.

case .clients:
let clients = argExtractor.extractOption(named: flag.rawValue)
if let value = clients.first {
guard let clients = Bool(value) else {
throw CommandPluginError.invalidArgumentValue(value)
}
config.common.clients = clients
}
case .messages:
let messages = argExtractor.extractOption(named: flag.rawValue)
if let value = messages.first {
guard let messages = Bool(value) else {
throw CommandPluginError.invalidArgumentValue(value)
}
config.common.messages = messages
}
case .fileNaming:
let fileNaming = argExtractor.extractOption(named: flag.rawValue)
if let value = fileNaming.first {
switch value.lowercased() {
case "fullPath":
config.common.fileNaming = .fullPath
case "pathToUnderscores":
config.common.fileNaming = .pathToUnderscores
case "dropPath":
config.common.fileNaming = .dropPath
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we move this to a failable init on the FileNaming type as we did for the access level?

Also, this is always going to fail as currently implemented, all of your cases contain uppercase characters 🙃

default:
Diagnostics.error("Unknown file naming strategy \(value)")
}
}
case .accessLevelOnImports:
let accessLevelOnImports = argExtractor.extractOption(named: flag.rawValue)
if let value = accessLevelOnImports.first {
guard let accessLevelOnImports = Bool(value) else {
throw CommandPluginError.invalidArgumentValue(value)
}
config.common.accessLevelOnImports = accessLevelOnImports
}
case .importPath:
config.common.importPaths = argExtractor.extractOption(named: flag.rawValue)
case .protocPath:
let protocPath = argExtractor.extractOption(named: flag.rawValue)
config.common.protocPath = protocPath.first
case .output:
let output = argExtractor.extractOption(named: flag.rawValue)
config.common.outputPath = output.first ?? pluginWorkDirectory.absoluteStringNoScheme
case .dryRun:
let dryRun = argExtractor.extractFlag(named: flag.rawValue)
config.dryRun = dryRun != 0
case .help:
let help = argExtractor.extractFlag(named: flag.rawValue)
if help != 0 {
throw CommandPluginError.helpRequested
}
}
}

if argExtractor.remainingArguments.isEmpty {
throw CommandPluginError.missingInputFile
}

for argument in argExtractor.remainingArguments {
if argument.hasPrefix("--") {
throw CommandPluginError.unknownOption(argument)
}
}

return (config, argExtractor.remainingArguments)
}
}

/// All valid input options/flags
enum Flag: CaseIterable, RawRepresentable {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not actually flags but names of options and flags ;)

typealias RawValue = String

case servers
case clients
case messages
case fileNaming
case accessLevel
case accessLevelOnImports
case importPath
case protocPath
case output
case dryRun

case help

init?(rawValue: String) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason we should just set these on each case and let the compiler synthesise the init and rawValue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean we could store these strings as another property and not use the rawValue for storing the strings which map to then command-line spelling?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not quite, I mean why not just do:

enum Flag: String {
  case servers
  // ...
  case fileNaming = "file-naming"
  // (etc.)
}

because then you don't have to implement init?(rawValue:) or var rawValue: String { ... }

switch rawValue {
case "servers":
self = .servers
case "clients":
self = .clients
case "messages":
self = .messages
case "file-naming":
self = .fileNaming
case "access-level":
self = .accessLevel
case "use-access-level-on-imports":
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm guessing we don't want the "use-" prefix here.

self = .accessLevelOnImports
case "import-path":
self = .importPath
case "protoc-path":
self = .protocPath
case "output":
self = .output
case "dry-run":
self = .dryRun
case "help":
self = .help
default:
return nil
}
return nil
}

var rawValue: String {
switch self {
case .servers:
"servers"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wondering whether we should mirror the config a bit more, e.g. "generate-servers"? It might be a bit too unwieldy though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I think I prefer the brevity - no strong feelings though

case .clients:
"clients"
case .messages:
"messages"
case .fileNaming:
"file-naming"
case .accessLevel:
"access-level"
case .accessLevelOnImports:
"access-level-on-imports"
case .importPath:
"import-path"
case .protocPath:
"protoc-path"
case .output:
"output"
case .dryRun:
"dry-run"
case .help:
"help"
}
}
}

extension Flag {
func usageDescription() -> String {
switch self {
case .servers:
return "Whether server code is generated. Defaults to true."
case .clients:
return "Whether client code is generated. Defaults to true."
case .messages:
return "Whether message code is generated. Defaults to true."
case .fileNaming:
return
"The naming scheme for output files [fullPath/pathToUnderscores/dropPath]. Defaults to fullPath."
case .accessLevel:
return
"The access level of the generated source [internal/public/package]. Defaults to internal."
case .accessLevelOnImports:
return "Whether imports should have explicit access levels. Defaults to false."
case .importPath:
return "The directory in which to search for imports."
case .protocPath:
return "The path to the `protoc` binary."
case .dryRun:
return "Print but do not execute the protoc commands."
case .output:
return "The path into which the generated source files are created."
case .help:
return "Print this help."
}
}

static func printHelp() {
print("Usage: swift package generate-grpc-code-from-protos [flags] [input files]")
print("")
print("Flags:")
print("")

let spacing = 3
let maxLength =
(Flag.allCases.map(\.rawValue).max(by: { $0.count < $1.count })?.count ?? 0) + spacing
for flag in Flag.allCases {
print(
" --\(flag.rawValue.padding(toLength: maxLength, withPad: " ", startingAt: 0))\(flag.usageDescription())"
)
}
}
}
40 changes: 40 additions & 0 deletions Plugins/GRPCGeneratorCommand/CommandPluginError.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright 2025, gRPC Authors All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

enum CommandPluginError: Error {
case helpRequested
case missingArgumentValue
case invalidArgumentValue(String)
case missingInputFile
case unknownOption(String)
}

extension CommandPluginError: CustomStringConvertible {
var description: String {
switch self {
case .helpRequested:
"User requested help."
case .missingArgumentValue:
"Provided option does not have a value."
case .invalidArgumentValue:
"Invalid option value."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can these be more specific? They should at least include the name of the option/flag

case .missingInputFile:
"No input file(s) specified."
case .unknownOption(let value):
"Provided option is unknown: \(value)."
}
}
}
Loading