Fix some papercuts with the command plugin

Motivation:

The command plugin has a few rough edges which make it harder to use
than it should.

Modifications:

- Check if help was requested before doing any other arg parsing.
- Check all input files end in '.proto', if they don't it's likely an
  option was passed in the wrong place, so print a helpful error
  message.
- Change the default output path to be ".", i.e. the directory of the
  caller. Previously it was the plugin working directory which is buried
  in '.build' so it would appear as if nothing was generated.
- Improve the formatting of the error message if the protoc invocation
  failed. The error message now includes a better formatted command that
  the user can invoke directly. Remove the error description because it
  was always "NSTaskTermination" without any useful information
  attached.

Result:

Command plugin is a little easier to use.

Author: glbrntt

Plugins/GRPCProtobufGeneratorCommand/CommandConfig.swift

@@ -42,10 +42,8 @@ struct CommandConfig {
 }
 
 extension CommandConfig {
-  static func parse(
-    argumentExtractor argExtractor: inout ArgumentExtractor,
-    pluginWorkDirectory: URL
-  ) throws -> CommandConfig {
+  static func parse(args: [String]) throws -> CommandConfig {
+    var argExtractor = ArgumentExtractor(args)
     var config = CommandConfig.defaults
 
     for flag in OptionsAndFlags.allCases {
@@ -131,9 +129,7 @@ extension CommandConfig {
         config.common.protocPath = argExtractor.extractSingleOption(named: flag.rawValue)
 
       case .outputPath:
-        config.common.outputPath =
-          argExtractor.extractSingleOption(named: flag.rawValue)
-          ?? pluginWorkDirectory.absoluteStringNoScheme
+        config.common.outputPath = argExtractor.extractSingleOption(named: flag.rawValue) ?? "."
 
       case .verbose:
         let verbose = argExtractor.extractFlag(named: flag.rawValue)

Plugins/GRPCProtobufGeneratorCommand/CommandPluginError.swift

@@ -21,13 +21,12 @@ enum CommandPluginError: Error {
   case unknownAccessLevel(String)
   case unknownFileNamingStrategy(String)
   case conflictingFlags(String, String)
+  case invalidInputFiles([String])
   case generationFailure(
-    errorDescription: String,
     executable: String,
     arguments: [String],
     stdErr: String?
   )
-  case tooManyParameterSeparators
 }
 
 extension CommandPluginError: CustomStringConvertible {
@@ -45,21 +44,52 @@ extension CommandPluginError: CustomStringConvertible {
       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).
-        \tExecutable: \(executable)
-        \tArguments: \(arguments.joined(separator: " "))
-        """
+
+    case .invalidInputFiles(let files):
+      var lines: [String] = []
+      lines.append("Invalid input file(s)")
+      lines.append("")
+      lines.append("Found \(files.count) input(s) not ending in '.proto':")
+      for file in files {
+        lines.append("- \(file)")
+      }
+      lines.append("")
+      lines.append("All options must be before '--', and all input files must be")
+      lines.append("after '--'. Input files must end in '.proto'.")
+      lines.append("")
+      lines.append("See --help for more information.")
+      return lines.joined(separator: "\n")
+
+    case .generationFailure(let executable, let arguments, let stdErr):
+      var lines: [String] = []
+      lines.append("protoc failed to generate code")
+      lines.append("")
+      lines.append(String(repeating: "-", count: 80))
+      lines.append("Command run:")
+      lines.append("")
+      lines.append("\(executable) \\")
+      var iterator = arguments.makeIterator()
+      var current = iterator.next()
+      while let currentArg = current {
+        var nextArg = iterator.next()
+        defer { current = nextArg }
+
+        if nextArg != nil {
+          lines.append("  \(currentArg) \\")
+        } else {
+          lines.append("  \(currentArg)")
+        }
+      }
+
       if let stdErr {
-        message += """
-          \n\tprotoc error output:
-          \t\(stdErr)
-          """
+        lines.append("")
+        lines.append(String(repeating: "-", count: 80))
+        lines.append("Error output (stderr):")
+        lines.append("")
+        lines.append(stdErr)
       }
-      return message
-    case .tooManyParameterSeparators:
-      return "Unexpected parameter structure, too many '--' separators."
+
+      return lines.joined(separator: "\n")
     }
   }
 }

Plugins/GRPCProtobufGeneratorCommand/Plugin.swift

@@ -50,45 +50,29 @@ struct GRPCProtobufGeneratorCommandPlugin {
     tool: (String) throws -> PluginContext.Tool,
     pluginWorkDirectoryURL: URL
   ) throws {
-    let flagsAndOptions: [String]
-    let inputFiles: [String]
-
-    let separatorCount = arguments.count { $0 == CommandConfig.parameterGroupSeparator }
-    switch separatorCount {
-    case 0:
-      var argExtractor = ArgumentExtractor(arguments)
-      // check if help requested
-      if argExtractor.extractFlag(named: OptionsAndFlags.help.rawValue) > 0 {
-        OptionsAndFlags.printHelp(requested: true)
-        return
-      }
-
-      inputFiles = arguments
-      flagsAndOptions = []
+    // Check for help.
+    if arguments.isEmpty || arguments.contains(where: { $0 == "--help" || $0 == "-h" }) {
+      OptionsAndFlags.printHelp(requested: true)
+      return
+    }
 
-    case 1:
-      let splitIndex = arguments.firstIndex(of: CommandConfig.parameterGroupSeparator)!
-      flagsAndOptions = Array(arguments[..<splitIndex])
-      inputFiles = Array(arguments[splitIndex.advanced(by: 1)...])
+    // Split the input into options and input files.
+    let (flagsAndOptions, inputFiles) = self.splitArgs(arguments)
 
-    default:
-      throw CommandPluginError.tooManyParameterSeparators
+    // Check the input files all look like protos
+    let nonProtoInputs = inputFiles.filter { !$0.hasSuffix(".proto") }
+    if !nonProtoInputs.isEmpty {
+      throw CommandPluginError.invalidInputFiles(nonProtoInputs)
     }
 
-    var argExtractor = ArgumentExtractor(flagsAndOptions)
-    // help requested
-    if argExtractor.extractFlag(named: OptionsAndFlags.help.rawValue) > 0 {
-      OptionsAndFlags.printHelp(requested: true)
-      return
+    if inputFiles.isEmpty {
+      throw CommandPluginError.missingInputFile
     }
 
-    // MARK: Configuration
+    // Parse the config.
     let commandConfig: CommandConfig
     do {
-      commandConfig = try CommandConfig.parse(
-        argumentExtractor: &argExtractor,
-        pluginWorkDirectory: pluginWorkDirectoryURL
-      )
+      commandConfig = try CommandConfig.parse(args: flagsAndOptions)
     } catch {
       throw error
     }
@@ -159,6 +143,22 @@ struct GRPCProtobufGeneratorCommandPlugin {
       }
     }
   }
+
+  private func splitArgs(_ args: [String]) -> (options: [String], inputs: [String]) {
+    let inputs: [String]
+    let options: [String]
+
+    if let index = args.firstIndex(of: "--") {
+      let nextIndex = args.index(after: index)
+      inputs = Array(args[nextIndex...])
+      options = Array(args[..<index])
+    } else {
+      options = []
+      inputs = args
+    }
+
+    return (options, inputs)
+  }
 }
 
 /// Execute a single invocation of `protoc`, printing output and if in verbose mode the invocation
@@ -202,7 +202,6 @@ func executeProtocInvocation(
       stdErr = nil
     }
     throw CommandPluginError.generationFailure(
-      errorDescription: "\(error)",
       executable: executableURL.absoluteStringNoScheme,
       arguments: arguments,
       stdErr: stdErr
@@ -224,7 +223,6 @@ func executeProtocInvocation(
   }
   let problem = "\(process.terminationReason):\(process.terminationStatus)"
   throw CommandPluginError.generationFailure(
-    errorDescription: problem,
     executable: executableURL.absoluteStringNoScheme,
     arguments: arguments,
     stdErr: stdErr