Merge pull request #154 from natecook1000/argumentparser

Adopt ArgumentParser for the command-line tool
swiftlang · Mar 10, 2020 · 6449476 · 6449476
2 parents 70c38a2 + 8525dfe
commit 6449476
Show file tree

Hide file tree

Showing 7 changed files with 159 additions and 263 deletions.
diff --git a/Package.resolved b/Package.resolved
@@ -1,6 +1,15 @@
 {
   "object": {
     "pins": [
+      {
+        "package": "swift-argument-parser",
+        "repositoryURL": "https://github.com/apple/swift-argument-parser.git",
+        "state": {
+          "branch": null,
+          "revision": "35b76bf577d3cc74820f8991894ce3bcdf024ddc",
+          "version": "0.0.2"
+        }
+      },
       {
         "package": "SwiftSyntax",
         "repositoryURL": "https://github.com/apple/swift-syntax",

diff --git a/Package.swift b/Package.swift
@@ -26,6 +26,7 @@ let package = Package(
       .revision("swift-DEVELOPMENT-SNAPSHOT-2020-01-29-a")
     ),
     .package(url: "https://github.com/apple/swift-tools-support-core.git", from: "0.0.1"),
+    .package(url: "https://github.com/apple/swift-argument-parser.git", .upToNextMinor(from: "0.0.2")),
   ],
   targets: [
     .target(
@@ -69,6 +70,7 @@ let package = Package(
         "SwiftFormatCore",
         "SwiftSyntax",
         "SwiftToolsSupport-auto",
+        "ArgumentParser",
       ]
     ),
     .testTarget(

diff --git a/Sources/swift-format/CommandLineOptions.swift b/Sources/swift-format/CommandLineOptions.swift
@@ -10,191 +10,115 @@
 //
 //===----------------------------------------------------------------------===//
 
+import ArgumentParser
 import Foundation
 import SwiftFormat
 import TSCBasic
 import TSCUtility
 
 /// Collects the command line options that were passed to `swift-format`.
-struct CommandLineOptions {
-
+struct SwiftFormatCommand: ParsableCommand {
+  static var configuration = CommandConfiguration(
+    commandName: "swift-format",
+    abstract: "Format or lint Swift source code.",
+    discussion: "When no files are specified, it expects the source from standard input."
+  )
+
   /// The path to the JSON configuration file that should be loaded.
   ///
   /// If not specified, the default configuration will be used.
-  var configurationPath: String? = nil
+  @Option(
+    name: .customLong("configuration"),
+    help: "The path to a JSON file containing the configuration of the linter/formatter.")
+  var configurationPath: String?
 
   /// The filename for the source code when reading from standard input, to include in diagnostic
   /// messages.
   ///
   /// If not specified and standard input is used, a dummy filename is used for diagnostic messages
   /// about the source from standard input.
-  var assumeFilename: String? = nil
+  @Option(help: "When using standard input, the filename of the source to include in diagnostics.")
+  var assumeFilename: String?
 
+  enum ToolMode: String, CaseIterable, ExpressibleByArgument {
+    case format
+    case lint
+    case dumpConfiguration = "dump-configuration"
+  }
+
   /// The mode in which to run the tool.
   ///
   /// If not specified, the tool will be run in format mode.
-  var mode: ToolMode = .format
+  @Option(
+    default: .format,
+    help: "The mode to run swift-format in. Either 'format', 'lint', or 'dump-configuration'.")
+  var mode: ToolMode
 
   /// Whether or not to format the Swift file in-place
   ///
   /// If specified, the current file is overwritten when formatting
-  var inPlace: Bool = false
+  @Flag(
+    name: .shortAndLong,
+    help: "Overwrite the current file when formatting ('format' mode only).")
+  var inPlace: Bool
 
   /// Whether or not to run the formatter/linter recursively.
   ///
   /// If set, we recursively run on all ".swift" files in any provided directories.
-  var recursive: Bool = false
+  @Flag(
+    name: .shortAndLong,
+    help: "Recursively run on '.swift' files in any provided directories.")
+  var recursive: Bool
 
+  /// The list of paths to Swift source files that should be formatted or linted.
+  @Argument(help: "One or more input filenames")
+  var paths: [String]
+
+  @Flag(help: "Print the version and exit")
+  var version: Bool
+
+  @Flag(help: .hidden) var debugDisablePrettyPrint: Bool
+  @Flag(help: .hidden) var debugDumpTokenStream: Bool
+
   /// Advanced options that are useful for developing/debugging but otherwise not meant for general
   /// use.
-  var debugOptions: DebugOptions = []
-
-  /// The list of paths to Swift source files that should be formatted or linted.
-  var paths: [String] = []
-}
-
-/// Process the command line argument strings and returns an object containing their values.
-///
-/// - Parameters:
-///   - commandName: The name of the command that this tool was invoked as.
-///   - arguments: The remaining command line arguments after the command name.
-/// - Returns: A `CommandLineOptions` value that contains the parsed options.
-func processArguments(commandName: String, _ arguments: [String]) -> CommandLineOptions {
-  let parser = ArgumentParser(
-    commandName: commandName,
-    usage: "[options] [filename or path ...]",
-    overview:
-      """
-      Format or lint Swift source code.
-
-      When no files are specified, it expects the source from standard input.
-      """
-  )
-
-  let binder = ArgumentBinder<CommandLineOptions>()
-  binder.bind(
-    option: parser.add(
-      option: "--mode",
-      shortName: "-m",
-      kind: ToolMode.self,
-      usage: "The mode to run swift-format in. Either 'format', 'lint', or 'dump-configuration'."
-    )
-  ) {
-    $0.mode = $1
-  }
-  binder.bind(
-    option: parser.add(
-      option: "--version",
-      shortName: "-v",
-      kind: Bool.self,
-      usage: "Prints the version and exists"
-    )
-  ) { opts, _ in
-    opts.mode = .version
-  }
-  binder.bindArray(
-    positional: parser.add(
-      positional: "filenames or paths",
-      kind: [String].self,
-      optional: true,
-      strategy: .upToNextOption,
-      usage: "One or more input filenames",
-      completion: .filename
-    )
-  ) {
-    $0.paths = $1
-  }
-  binder.bind(
-    option: parser.add(
-      option: "--configuration",
-      kind: String.self,
-      usage: "The path to a JSON file containing the configuration of the linter/formatter."
-    )
-  ) {
-    $0.configurationPath = $1
-  }
-  binder.bind(
-    option: parser.add(
-      option: "--assume-filename",
-      kind: String.self,
-      usage: "When using standard input, the filename of the source to include in diagnostics."
-    )
-  ) {
-    $0.assumeFilename = $1
-  }
-  binder.bind(
-    option: parser.add(
-      option: "--in-place",
-      shortName: "-i",
-      kind: Bool.self,
-      usage: "Overwrite the current file when formatting ('format' mode only)."
-    )
-  ) {
-    $0.inPlace = $1
-  }
-  binder.bind(
-    option: parser.add(
-      option: "--recursive",
-      shortName: "-r",
-      kind: Bool.self,
-      usage: "Recursively run on '.swift' files in any provided directories."
-    )
-  ) {
-    $0.recursive = $1
+  var debugOptions: DebugOptions {
+    [
+      debugDisablePrettyPrint ? .disablePrettyPrint : [],
+      debugDumpTokenStream ? .dumpTokenStream : [],
+    ]
   }
 
-  // Add advanced debug/developer options. These intentionally have no usage strings, which omits
-  // them from the `--help` screen to avoid noise for the general user.
-  binder.bind(
-    option: parser.add(
-      option: "--debug-disable-pretty-print",
-      kind: Bool.self
-    )
-  ) {
-    $0.debugOptions.set(.disablePrettyPrint, enabled: $1)
-  }
-  binder.bind(
-    option: parser.add(
-      option: "--debug-dump-token-stream",
-      kind: Bool.self
-    )
-  ) {
-    $0.debugOptions.set(.dumpTokenStream, enabled: $1)
-  }
-
-  var opts = CommandLineOptions()
-  do {
-    let args = try parser.parse(arguments)
-    try binder.fill(parseResult: args, into: &opts)
-
-    if opts.inPlace && (ToolMode.format != opts.mode || opts.paths.isEmpty) {
-      throw ArgumentParserError.unexpectedArgument("--in-place, -i")
+  mutating func validate() throws {
+    if version {
+      throw CleanExit.message("0.0.1")
+    }
+
+    if inPlace && (mode != .format || paths.isEmpty) {
+      throw ValidationError("'--in-place' is only valid when formatting files")
     }
 
-    let modeSupportsRecursive = ToolMode.format == opts.mode || ToolMode.lint == opts.mode
-    if opts.recursive && (!modeSupportsRecursive || opts.paths.isEmpty) {
-      throw ArgumentParserError.unexpectedArgument("--recursive, -r")
+    let modeSupportsRecursive = mode == .format || mode == .lint
+    if recursive && (!modeSupportsRecursive || paths.isEmpty) {
+      throw ValidationError("'--recursive' is only valid when formatting or linting files")
     }
 
-    if opts.assumeFilename != nil && !opts.paths.isEmpty {
-      throw ArgumentParserError.unexpectedArgument("--assume-filename")
+    if assumeFilename != nil && !paths.isEmpty {
+      throw ValidationError("'--assume-filename' is only valid when reading from stdin")
     }
 
-    if !opts.paths.isEmpty && !opts.recursive {
-      for path in opts.paths {
+    if !paths.isEmpty && !recursive {
+      for path in paths {
         var isDir: ObjCBool = false
         if FileManager.default.fileExists(atPath: path, isDirectory: &isDir), isDir.boolValue {
-          throw ArgumentParserError.invalidValue(
-            argument: "'\(path)'",
-            error: ArgumentConversionError.custom("for directories, use --recursive option")
+          throw ValidationError(
+            """
+            '\(path)' is a path to a directory, not a Swift source file.
+            Use the '--recursive' option to handle directories.
+            """
           )
         }
       }
     }
-  } catch {
-    stderrStream.write("error: \(error)\n\n")
-    parser.printUsage(on: stderrStream)
-    exit(1)
   }
-  return opts
 }
diff --git a/Sources/swift-format/FormatError.swift b/Sources/swift-format/FormatError.swift
@@ -0,0 +1,13 @@
+import Foundation
+import SwiftSyntax
+
+struct FormatError: LocalizedError {
+  var message: String
+  var errorDescription: String? { message }
+
+  static var exitWithDiagnosticErrors: FormatError {
+    // The diagnostics engine has already printed errors to stderr.
+    FormatError(message: "")
+  }
+}
+
diff --git a/Sources/swift-format/Run.swift b/Sources/swift-format/Run.swift
@@ -30,15 +30,15 @@ import TSCBasic
 func lintMain(
   configuration: Configuration, sourceFile: FileHandle, assumingFilename: String?,
   debugOptions: DebugOptions, diagnosticEngine: DiagnosticEngine
-) -> Int {
+) {
   let linter = SwiftLinter(configuration: configuration, diagnosticEngine: diagnosticEngine)
   linter.debugOptions = debugOptions
   let assumingFileURL = URL(fileURLWithPath: assumingFilename ?? "<stdin>")
 
   guard let source = readSource(from: sourceFile) else {
     diagnosticEngine.diagnose(
       Diagnostic.Message(.error, "Unable to read source for linting from \(assumingFileURL.path)."))
-    return 1
+    return
   }
 
   do {
@@ -47,20 +47,19 @@ func lintMain(
     let path = assumingFileURL.path
     diagnosticEngine.diagnose(
       Diagnostic.Message(.error, "Unable to lint \(path): file is not readable or does not exist."))
-    return 1
+    return
   } catch SwiftFormatError.fileContainsInvalidSyntax(let position) {
     let path = assumingFileURL.path
     let location = SourceLocationConverter(file: path, source: source).location(for: position)
     diagnosticEngine.diagnose(
       Diagnostic.Message(.error, "file contains invalid or unrecognized Swift syntax."),
       location: location)
-    return 1
+    return
   } catch {
     let path = assumingFileURL.path
     diagnosticEngine.diagnose(Diagnostic.Message(.error, "Unable to lint \(path): \(error)"))
-    exit(1)
+    return
   }
-  return diagnosticEngine.diagnostics.isEmpty ? 0 : 1
 }
 
 /// Runs the formatting pipeline over the provided source file.
@@ -76,7 +75,7 @@ func lintMain(
 func formatMain(
   configuration: Configuration, sourceFile: FileHandle, assumingFilename: String?, inPlace: Bool,
   debugOptions: DebugOptions, diagnosticEngine: DiagnosticEngine
-) -> Int {
+) {
   // Even though `diagnosticEngine` is defined, it's use is reserved for fatal messages. Pass nil
   // to the formatter to suppress other messages since they will be fixed or can't be automatically
   // fixed anyway.
@@ -88,7 +87,7 @@ func formatMain(
     diagnosticEngine.diagnose(
       Diagnostic.Message(
         .error, "Unable to read source for formatting from \(assumingFileURL.path)."))
-    return 1
+    return
   }
 
   do {
@@ -110,20 +109,19 @@ func formatMain(
     diagnosticEngine.diagnose(
       Diagnostic.Message(
         .error, "Unable to format \(path): file is not readable or does not exist."))
-    return 1
+    return
   } catch SwiftFormatError.fileContainsInvalidSyntax(let position) {
     let path = assumingFileURL.path
     let location = SourceLocationConverter(file: path, source: source).location(for: position)
     diagnosticEngine.diagnose(
       Diagnostic.Message(.error, "file contains invalid or unrecognized Swift syntax."),
       location: location)
-    return 1
+    return
   } catch {
     let path = assumingFileURL.path
     diagnosticEngine.diagnose(Diagnostic.Message(.error, "Unable to format \(path): \(error)"))
-    exit(1)
+    return
   }
-  return 0
 }
 
 /// Reads from the given file handle until EOF is reached, then returns the contents as a UTF8