Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
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: "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: [
.target(name: "protoc-gen-grpc-swift"),
.product(name: "protoc-gen-swift", package: "swift-protobuf"),
]
),
]

let package = Package(
Expand Down
10 changes: 4 additions & 6 deletions Plugins/GRPCProtobufGenerator/BuildPluginConfig.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

import Foundation

let configFileName = "grpc-swift-proto-generator-config.json"

/// The config of the build plugin.
struct BuildPluginConfig: Codable {
/// Config defining which components should be considered when generating source.
Expand Down Expand Up @@ -193,14 +191,14 @@ extension BuildPluginConfig.Protoc: Codable {

extension GenerationConfig {
init(buildPluginConfig: BuildPluginConfig, configFilePath: URL, outputPath: URL) {
self.server = buildPluginConfig.generate.servers
self.client = buildPluginConfig.generate.clients
self.message = buildPluginConfig.generate.messages
self.servers = buildPluginConfig.generate.servers
self.clients = buildPluginConfig.generate.clients
self.messages = buildPluginConfig.generate.messages
// Use path to underscores as it ensures output files are unique (files generated from
// "foo/bar.proto" won't collide with those generated from "bar/bar.proto" as they'll be
// uniquely named "foo_bar.(grpc|pb).swift" and "bar_bar.(grpc|pb).swift".
self.fileNaming = .pathToUnderscores
self.visibility = buildPluginConfig.generatedSource.accessLevel
self.accessLevel = buildPluginConfig.generatedSource.accessLevel
self.accessLevelOnImports = buildPluginConfig.generatedSource.accessLevelOnImports
// Generate absolute paths for the imports relative to the config file in which they are specified
self.importPaths = buildPluginConfig.protoc.importPaths.map { relativePath in
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2024, gRPC Authors All rights reserved.
* 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.
Expand All @@ -14,13 +14,12 @@
* limitations under the License.
*/

enum PluginError: Error {
// Build plugin
enum BuildPluginError: Error {
case incompatibleTarget(String)
case noConfigFilesFound
}

extension PluginError: CustomStringConvertible {
extension BuildPluginError: CustomStringConvertible {
var description: String {
switch self {
case .incompatibleTarget(let target):
Expand Down
8 changes: 4 additions & 4 deletions Plugins/GRPCProtobufGenerator/Plugin.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ extension GRPCProtobufGenerator: BuildToolPlugin {
/// Create build commands, the entry-point when using a Package manifest.
func createBuildCommands(context: PluginContext, target: Target) async throws -> [Command] {
guard let swiftTarget = target as? SwiftSourceModuleTarget else {
throw PluginError.incompatibleTarget(target.name)
throw BuildPluginError.incompatibleTarget(target.name)
}
let configFiles = swiftTarget.sourceFiles(withSuffix: configFileName).map { $0.url }
let inputFiles = swiftTarget.sourceFiles(withSuffix: ".proto").map { $0.url }
Expand Down Expand Up @@ -78,7 +78,7 @@ struct GRPCProtobufGenerator {
var commands: [Command] = []
for inputFile in inputFiles {
guard let (configFilePath, config) = configs.findApplicableConfig(for: inputFile) else {
throw PluginError.noConfigFilesFound
throw BuildPluginError.noConfigFilesFound
}

let protocPath = try deriveProtocPath(using: config, tool: tool)
Expand All @@ -90,7 +90,7 @@ struct GRPCProtobufGenerator {
}

// unless *explicitly* opted-out
if config.client || config.server {
if config.clients || config.servers {
let grpcCommand = try protocGenGRPCSwiftCommand(
inputFile: inputFile,
config: config,
Expand All @@ -104,7 +104,7 @@ struct GRPCProtobufGenerator {
}

// unless *explicitly* opted-out
if config.message {
if config.messages {
let protoCommand = try protocGenSwiftCommand(
inputFile: inputFile,
config: config,
Expand Down
240 changes: 240 additions & 0 deletions Plugins/GRPCProtobufGeneratorCommand/CommandConfig.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,240 @@
/*
* 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 verbose: Bool
var dryRun: Bool

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

extension CommandConfig {
static func helpRequested(
argumentExtractor: inout ArgumentExtractor
) -> Bool {
let help = argumentExtractor.extractFlag(named: OptionsAndFlags.help.rawValue)
return help != 0
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function is tiny and only used one time so I think the indirection hurts it.

You'd be better off just inlining it:

if argumentExtractor.extractFlag(named: OptionsAndFlags.help.rawValue) > 0 {
  // ...
}


static func parse(
argumentExtractor argExtractor: inout ArgumentExtractor,
pluginWorkDirectory: URL
) throws -> (CommandConfig, [String]) {
var config = CommandConfig.defaults

for flag in OptionsAndFlags.allCases {
switch flag {
case .accessLevel:
let accessLevel = argExtractor.extractOption(named: flag.rawValue)
if let value = extractSingleValue(flag, values: accessLevel) {
if let accessLevel = GenerationConfig.AccessLevel(rawValue: value) {
config.common.accessLevel = accessLevel
} else {
Diagnostics.error("Unknown access level '--\(flag.rawValue)' \(value)")
}
}

case .servers, .noServers:
if flag == .noServers { continue } // only process this once
Copy link
Collaborator

Choose a reason for hiding this comment

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

This threw me for a second because I expected to be looping over the arguments and checking for known flags/options as opposed to looping over the known flags/options and extracting them (as that requires scanning the args multiple times) although this is how the arg extractor wants you to do it.

Very minor but I think it'd be more obvious if they were treated as separate cases:

case .noServers:
  // Handled by `.servers`
  continue

let servers = argExtractor.extractFlag(named: OptionsAndFlags.servers.rawValue)
let noServers = argExtractor.extractFlag(named: OptionsAndFlags.noServers.rawValue)
if noServers > servers {
config.common.servers = false
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think if the user specifies both flags then they've made a mistake and we should emit a diagnostic and fail. It should be okay to specify the same flag more than once though.

Same comment for clients and messages.


case .clients, .noClients:
if flag == .noClients { continue } // only process this once
let clients = argExtractor.extractFlag(named: OptionsAndFlags.clients.rawValue)
let noClients = argExtractor.extractFlag(named: OptionsAndFlags.noClients.rawValue)
if noClients > clients {
config.common.clients = false
}

case .messages, .noMessages:
if flag == .noMessages { continue } // only process this once
let messages = argExtractor.extractFlag(named: OptionsAndFlags.messages.rawValue)
let noMessages = argExtractor.extractFlag(named: OptionsAndFlags.noMessages.rawValue)
if noMessages > messages {
config.common.messages = false
}

case .fileNaming:
let fileNaming = argExtractor.extractOption(named: flag.rawValue)
if let value = extractSingleValue(flag, values: fileNaming) {
if let fileNaming = GenerationConfig.FileNaming(rawValue: value) {
config.common.fileNaming = fileNaming
} else {
Diagnostics.error("Unknown file naming strategy '--\(flag.rawValue)' \(value)")
}
}

case .accessLevelOnImports:
let accessLevelOnImports = argExtractor.extractOption(named: flag.rawValue)
if let value = extractSingleValue(flag, values: accessLevelOnImports) {
guard let accessLevelOnImports = Bool(value) else {
throw CommandPluginError.invalidArgumentValue(name: flag.rawValue, value: value)
Copy link
Collaborator

Choose a reason for hiding this comment

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

When do we use a diagnostic vs an error?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The idea was to try to be permissive and just limp on but that got a bit muddied so I've reversed that now and I just throw an error if there's an issue.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great, I think that's the better approach.

}
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 = extractSingleValue(flag, values: protocPath)

case .output:
let output = argExtractor.extractOption(named: flag.rawValue)
config.common.outputPath =
extractSingleValue(flag, values: output) ?? pluginWorkDirectory.absoluteStringNoScheme

case .verbose:
let verbose = argExtractor.extractFlag(named: flag.rawValue)
config.verbose = verbose != 0

case .dryRun:
let dryRun = argExtractor.extractFlag(named: flag.rawValue)
config.dryRun = dryRun != 0

case .help:
() // handled elsewhere
}
}

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

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

return (config, argExtractor.remainingArguments)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This setup is such that we allow inputs to be interspersed with options:

--clients foo.proto -verbose --servers bar.proto --dry-run

That's a bit unexpected for a CLI and you can end up with a mistyped flag/option as an input (e.g. -verbose). I think the easiest thing to do is have args after a "--" and then split the args on that, everything before goes to the extractor, everything after is an input (everything is input if there's no "--"):

--clients -verbose --servers --dry-run -- foo.proto bar.proto

}
}

func extractSingleValue(_ flag: OptionsAndFlags, values: [String]) -> String? {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems like an appropriate extension to have on arg extractor

if values.count > 1 {
Stderr.print(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a warning diagnostic?

Copy link
Collaborator Author

@rnro rnro Mar 3, 2025

Choose a reason for hiding this comment

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

There is, I've used it now :)

"Warning: '--\(flag.rawValue)' was unexpectedly repeated, the first value will be used."
)
}
return values.first
}

/// All valid input options/flags
enum OptionsAndFlags: String, CaseIterable {
case servers
case noServers = "no-servers"
case clients
case noClients = "no-clients"
case messages
case noMessages = "no-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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be output-path? (We have import path and protoc path 🤷‍♂️)

case verbose
case dryRun = "dry-run"

case help
}

extension OptionsAndFlags {
func usageDescription() -> String {
switch self {
case .servers:
return "Indicate that server code is to be generated. Generated by default."
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: brevity is important, if you can say it as clearly in fewer words then you should. I think "Generate server code" says the same as "Indicate that server code is to be generated"

case .noServers:
return "Indicate that server code is not to be generated. Generated by default."
case .clients:
return "Indicate that client code is to be generated. Generated by default."
case .noClients:
return "Indicate that client code is not to be generated. Generated by default."
case .messages:
return "Indicate that message code is to be generated. Generated by default."
case .noMessages:
return "Indicate that message code is not to be generated. Generated by default."
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."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Worth noting that it can be specified multiple times and IIUC the current working directory is used if not specified?

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 .verbose:
return "Emit verbose output."
case .help:
return "Print this help."
}
}

static func printHelp(requested: Bool) {
let printMessage: (String) -> Void
if requested {
printMessage = { message in print(message) }
} else {
printMessage = Stderr.print
}

printMessage("Usage: swift package generate-grpc-code-from-protos [flags] [input files]")
printMessage("")
printMessage("Flags:")
printMessage("")

let spacing = 3
let maxLength =
(OptionsAndFlags.allCases.map(\.rawValue).max(by: { $0.count < $1.count })?.count ?? 0)
+ spacing
for flag in OptionsAndFlags.allCases {
printMessage(
" --\(flag.rawValue.padding(toLength: maxLength, withPad: " ", startingAt: 0))\(flag.usageDescription())"
)
}
}
}
37 changes: 37 additions & 0 deletions Plugins/GRPCProtobufGeneratorCommand/CommandPluginError.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* 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 missingArgumentValue
case invalidArgumentValue(name: String, value: String)
case missingInputFile
case unknownOption(String)
}

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

Choose a reason for hiding this comment

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

Which option?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It turns out this was left over from an earlier era. I have removed it now.

case .invalidArgumentValue(let name, let value):
"Invalid value '\(value)', for '\(name)'."
case .missingInputFile:
"No input file(s) specified."
case .unknownOption(let value):
"Provided option is unknown: \(value)."
}
}
}
Loading