Code generation command plugin

Plugins/GRPCProtobufGenerator/Plugin.swift

@@ -19,7 +19,6 @@ import PackagePlugin
 
 // Entry-point when using Package manifest
 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 BuildPluginError.incompatibleTarget(target.name)
@@ -41,7 +40,6 @@ import XcodeProjectPlugin
 
 // Entry-point when using Xcode projects
 extension GRPCProtobufGenerator: XcodeBuildToolPlugin {
-  /// Create build commands, the entry-point when using an Xcode project.
   func createBuildCommands(context: XcodePluginContext, target: XcodeTarget) throws -> [Command] {
     let configFiles = target.inputFiles.filter {
       $0.url.lastPathComponent == configFileName
@@ -62,7 +60,7 @@ extension GRPCProtobufGenerator: XcodeBuildToolPlugin {
 
 @main
 struct GRPCProtobufGenerator {
-  /// Build plugin code common to both invocation types: package manifest Xcode project
+  /// Build plugin common code
   func createBuildCommands(
     pluginWorkDirectory: URL,
     tool: (String) throws -> PluginContext.Tool,

Plugins/GRPCProtobufGeneratorCommand/CommandConfig.swift

@@ -37,88 +37,103 @@ struct CommandConfig {
     verbose: false,
     dryRun: false
   )
+
+  static let parameterGroupSeparator = "--"
 }
 
 extension CommandConfig {
-  static func helpRequested(
-    argumentExtractor: inout ArgumentExtractor
-  ) -> Bool {
-    let help = argumentExtractor.extractFlag(named: OptionsAndFlags.help.rawValue)
-    return help != 0
-  }
-
   static func parse(
     argumentExtractor argExtractor: inout ArgumentExtractor,
     pluginWorkDirectory: URL
-  ) throws -> (CommandConfig, [String]) {
+  ) throws -> CommandConfig {
     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 value = argExtractor.extractSingleOption(named: flag.rawValue) {
           if let accessLevel = GenerationConfig.AccessLevel(rawValue: value) {
             config.common.accessLevel = accessLevel
           } else {
-            Diagnostics.error("Unknown access level '--\(flag.rawValue)' \(value)")
+            throw CommandPluginError.unknownAccessLevel(value)
           }
         }
 
-      case .servers, .noServers:
-        if flag == .noServers { continue }  // only process this once
+      case .noServers:
+        // Handled by `.servers`
+        continue
+      case .servers:
         let servers = argExtractor.extractFlag(named: OptionsAndFlags.servers.rawValue)
         let noServers = argExtractor.extractFlag(named: OptionsAndFlags.noServers.rawValue)
-        if noServers > servers {
+        if servers > 0 && noServers > 0 {
+          throw CommandPluginError.conflictingFlags(
+            OptionsAndFlags.servers.rawValue,
+            OptionsAndFlags.noServers.rawValue
+          )
+        } else if servers > 0 {
+          config.common.servers = true
+        } else if noServers > 0 {
           config.common.servers = false
         }
 
-      case .clients, .noClients:
-        if flag == .noClients { continue }  // only process this once
+      case .noClients:
+        // Handled by `.clients`
+        continue
+      case .clients:
         let clients = argExtractor.extractFlag(named: OptionsAndFlags.clients.rawValue)
         let noClients = argExtractor.extractFlag(named: OptionsAndFlags.noClients.rawValue)
-        if noClients > clients {
+        if clients > 0 && noClients > 0 {
+          throw CommandPluginError.conflictingFlags(
+            OptionsAndFlags.clients.rawValue,
+            OptionsAndFlags.noClients.rawValue
+          )
+        } else if clients > 0 {
+          config.common.clients = true
+        } else if noClients > 0 {
           config.common.clients = false
         }
 
-      case .messages, .noMessages:
-        if flag == .noMessages { continue }  // only process this once
+      case .noMessages:
+        // Handled by `.messages`
+        continue
+      case .messages:
         let messages = argExtractor.extractFlag(named: OptionsAndFlags.messages.rawValue)
         let noMessages = argExtractor.extractFlag(named: OptionsAndFlags.noMessages.rawValue)
-        if noMessages > messages {
+        if messages > 0 && noMessages > 0 {
+          throw CommandPluginError.conflictingFlags(
+            OptionsAndFlags.messages.rawValue,
+            OptionsAndFlags.noMessages.rawValue
+          )
+        } else if messages > 0 {
+          config.common.messages = true
+        } else if noMessages > 0 {
           config.common.messages = false
         }
 
       case .fileNaming:
-        let fileNaming = argExtractor.extractOption(named: flag.rawValue)
-        if let value = extractSingleValue(flag, values: fileNaming) {
+        if let value = argExtractor.extractSingleOption(named: flag.rawValue) {
           if let fileNaming = GenerationConfig.FileNaming(rawValue: value) {
             config.common.fileNaming = fileNaming
           } else {
-            Diagnostics.error("Unknown file naming strategy '--\(flag.rawValue)' \(value)")
+            throw CommandPluginError.unknownFileNamingStrategy(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)
-          }
-          config.common.accessLevelOnImports = accessLevelOnImports
+        if argExtractor.extractFlag(named: flag.rawValue) > 0 {
+          config.common.accessLevelOnImports = true
         }
 
       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)
+        config.common.protocPath = argExtractor.extractSingleOption(named: flag.rawValue)
 
-      case .output:
-        let output = argExtractor.extractOption(named: flag.rawValue)
+      case .outputPath:
         config.common.outputPath =
-          extractSingleValue(flag, values: output) ?? pluginWorkDirectory.absoluteStringNoScheme
+          argExtractor.extractSingleOption(named: flag.rawValue)
+          ?? pluginWorkDirectory.absoluteStringNoScheme
 
       case .verbose:
         let verbose = argExtractor.extractFlag(named: flag.rawValue)
@@ -133,27 +148,24 @@ extension CommandConfig {
       }
     }
 
-    if argExtractor.remainingArguments.isEmpty {
-      throw CommandPluginError.missingInputFile
-    }
-
-    for argument in argExtractor.remainingArguments {
-      if argument.hasPrefix("--") {
-        throw CommandPluginError.unknownOption(argument)
-      }
+    if let argument = argExtractor.remainingArguments.first {
+      throw CommandPluginError.unknownOption(argument)
     }
 
-    return (config, argExtractor.remainingArguments)
+    return config
   }
 }
 
-func extractSingleValue(_ flag: OptionsAndFlags, values: [String]) -> String? {
-  if values.count > 1 {
-    Stderr.print(
-      "Warning: '--\(flag.rawValue)' was unexpectedly repeated, the first value will be used."
-    )
+extension ArgumentExtractor {
+  mutating func extractSingleOption(named optionName: String) -> String? {
+    let values = self.extractOption(named: optionName)
+    if values.count > 1 {
+      Diagnostics.warning(
+        "'--\(optionName)' was unexpectedly repeated, the first value will be used."
+      )
+    }
+    return values.first
   }
-  return values.first
 }
 
 /// All valid input options/flags
@@ -169,7 +181,7 @@ enum OptionsAndFlags: String, CaseIterable {
   case accessLevelOnImports = "access-level-on-imports"
   case importPath = "import-path"
   case protocPath = "protoc-path"
-  case output
+  case outputPath = "output-path"
   case verbose
   case dryRun = "dry-run"
 
@@ -180,17 +192,17 @@ extension OptionsAndFlags {
   func usageDescription() -> String {
     switch self {
     case .servers:
-      return "Indicate that server code is to be generated. Generated by default."
+      return "Generate server code. Generated by default."
     case .noServers:
-      return "Indicate that server code is not to be generated. Generated by default."
+      return "Do not generate server code. Generated by default."
     case .clients:
-      return "Indicate that client code is to be generated. Generated by default."
+      return "Generate client code. Generated by default."
     case .noClients:
-      return "Indicate that client code is not to be generated. Generated by default."
+      return "Do not generate client code. Generated by default."
     case .messages:
-      return "Indicate that message code is to be generated. Generated by default."
+      return "Generate message code. Generated by default."
     case .noMessages:
-      return "Indicate that message code is not to be generated. Generated by default."
+      return "Do not generate message code. Generated by default."
     case .fileNaming:
       return
         "The naming scheme for output files [fullPath/pathToUnderscores/dropPath]. Defaults to fullPath."
@@ -200,13 +212,14 @@ extension OptionsAndFlags {
     case .accessLevelOnImports:
       return "Whether imports should have explicit access levels. Defaults to false."
     case .importPath:
-      return "The directory in which to search for imports."
+      return
+        "The directory in which to search for imports. May be specified multiple times. If none are specified the current working directory is used."
     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 .outputPath:
+      return "The directory into which the generated source files are created."
     case .verbose:
       return "Emit verbose output."
     case .help:
@@ -222,7 +235,9 @@ extension OptionsAndFlags {
       printMessage = Stderr.print
     }
 
-    printMessage("Usage: swift package generate-grpc-code-from-protos [flags] [input files]")
+    printMessage(
+      "Usage: swift package generate-grpc-code-from-protos [flags] [\(CommandConfig.parameterGroupSeparator)] [input files]"
+    )
     printMessage("")
     printMessage("Flags:")
     printMessage("")

Plugins/GRPCProtobufGeneratorCommand/CommandPluginError.swift

@@ -15,23 +15,51 @@
  */
 
 enum CommandPluginError: Error {
-  case missingArgumentValue
   case invalidArgumentValue(name: String, value: String)
   case missingInputFile
   case unknownOption(String)
+  case unknownAccessLevel(String)
+  case unknownFileNamingStrategy(String)
+  case conflictingFlags(String, String)
+  case generationFailure(
+    errorDescription: String,
+    executable: String?,
+    arguments: [String]?,
+    stdErr: String?
+  )
+  case tooManyParameterSeparators
 }
 
 extension CommandPluginError: CustomStringConvertible {
   var description: String {
     switch self {
-    case .missingArgumentValue:
-      "Provided option does not have a value."
     case .invalidArgumentValue(let name, let value):
-      "Invalid value '\(value)', for '\(name)'."
+      return "Invalid value '\(value)', for '\(name)'."
     case .missingInputFile:
-      "No input file(s) specified."
-    case .unknownOption(let value):
-      "Provided option is unknown: \(value)."
+      return "No input file(s) specified."
+    case .unknownOption(let name):
+      return "Provided option is unknown: \(name)."
+    case .unknownAccessLevel(let value):
+      return "Provided access level is unknown: \(value)."
+    case .unknownFileNamingStrategy(let value):
+      return "Provided file naming strategy is unknown: \(value)."
+    case .conflictingFlags(let flag1, let flag2):
+      return "Provided flags conflict: '\(flag1)' and '\(flag2)'."
+    case .generationFailure(let errorDescription, let executable, let arguments, let stdErr):
+      var message = "Code generation failed with: \(errorDescription)."
+      if let executable {
+        message += "\n\tExecutable: \(executable)"
+      }
+      if let arguments {
+        message += "\n\tArguments: \(arguments.joined(separator: " "))"
+      }
+      if let stdErr {
+        message += "\n\tprotoc error output:"
+        message += "\n\t\(stdErr)"
+      }
+      return message
+    case .tooManyParameterSeparators:
+      return "Unexpected parameter structure, too many '--' separators."
     }
   }
 }