Address PR feedback

Author: natecook1000

Package.resolved

@@ -23,10 +23,10 @@ let package = Package(
   dependencies: [
     .package(
       url: "https://github.com/apple/swift-syntax",
-      .branch("master")
+      .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", from: "0.0.1"),
+    .package(url: "https://github.com/apple/swift-argument-parser.git", .upToNextMinor(from: "0.0.1")),
   ],
   targets: [
     .target(

Package.swift

@@ -94,7 +94,7 @@ struct SwiftFormatCommand: ParsableCommand {
       throw CleanExit.message("0.0.1")
     }
 
-    if inPlace && (mode == .format || paths.isEmpty) {
+    if inPlace && (mode != .format || paths.isEmpty) {
       throw ValidationError("'--in-place' is only valid when formatting files")
     }
 

Sources/swift-format/CommandLineOptions.swift

@@ -3,25 +3,8 @@ import SwiftSyntax
 
 struct FormatError: LocalizedError {
   var message: String
-  
   var errorDescription: String? { message }
 
-  static func readSource(path: String) -> FormatError {
-    FormatError(message: "Unable to read source for linting from \(path).")
-  }
-  
-  static func unableToLint(path: String, message: String) -> FormatError {
-    FormatError(message: "Unable to lint \(path): \(message).")
-  }
-  
-  static func unableToFormat(path: String, message: String) -> FormatError {
-    FormatError(message: "Unable to format \(path): \(message).")
-  }
-  
-  static func invalidSyntax(location: SourceLocation, message: String) -> FormatError {
-    FormatError(message: "Unable to format at \(location): \(message).")
-  }
-  
   static var exitWithDiagnosticErrors: FormatError {
     // The diagnostics engine has already printed errors to stderr.
     FormatError(message: "")

Sources/swift-format/FormatError.swift

@@ -29,33 +29,36 @@ import TSCBasic
 /// - Returns: Zero if there were no lint errors, otherwise a non-zero number.
 func lintMain(
   configuration: Configuration, sourceFile: FileHandle, assumingFilename: String?,
-  debugOptions: DebugOptions
-) throws {
-  let diagnosticEngine = makeDiagnosticEngine()
+  debugOptions: DebugOptions, diagnosticEngine: DiagnosticEngine
+) {
   let linter = SwiftLinter(configuration: configuration, diagnosticEngine: diagnosticEngine)
   linter.debugOptions = debugOptions
   let assumingFileURL = URL(fileURLWithPath: assumingFilename ?? "<stdin>")
 
   guard let source = readSource(from: sourceFile) else {
-    throw FormatError.readSource(path: assumingFileURL.path)
+    diagnosticEngine.diagnose(
+      Diagnostic.Message(.error, "Unable to read source for linting from \(assumingFileURL.path)."))
+    return
   }
 
   do {
     try linter.lint(source: source, assumingFileURL: assumingFileURL)
   } catch SwiftFormatError.fileNotReadable {
-    throw FormatError.unableToLint(
-      path: assumingFileURL.path,
-      message: "file is not readable or does not exist")
+    let path = assumingFileURL.path
+    diagnosticEngine.diagnose(
+      Diagnostic.Message(.error, "Unable to lint \(path): file is not readable or does not exist."))
+    return
   } catch SwiftFormatError.fileContainsInvalidSyntax(let position) {
     let path = assumingFileURL.path
     let location = SourceLocationConverter(file: path, source: source).location(for: position)
-    throw FormatError.invalidSyntax(location: location, message: "file contains invalid or unrecognized Swift syntax.")
+    diagnosticEngine.diagnose(
+      Diagnostic.Message(.error, "file contains invalid or unrecognized Swift syntax."),
+      location: location)
+    return
   } catch {
-    throw FormatError.unableToLint(path: assumingFileURL.path, message: "\(error)")
-  }
-  
-  if !diagnosticEngine.diagnostics.isEmpty {
-    throw FormatError.exitWithDiagnosticErrors
+    let path = assumingFileURL.path
+    diagnosticEngine.diagnose(Diagnostic.Message(.error, "Unable to lint \(path): \(error)"))
+    return
   }
 }
 
@@ -71,15 +74,17 @@ func lintMain(
 /// - Returns: Zero if there were no format errors, otherwise a non-zero number.
 func formatMain(
   configuration: Configuration, sourceFile: FileHandle, assumingFilename: String?, inPlace: Bool,
-  debugOptions: DebugOptions
-) throws {
-  let diagnosticEngine = makeDiagnosticEngine()
+  debugOptions: DebugOptions, diagnosticEngine: DiagnosticEngine
+) {
   let formatter = SwiftFormatter(configuration: configuration, diagnosticEngine: diagnosticEngine)
   formatter.debugOptions = debugOptions
   let assumingFileURL = URL(fileURLWithPath: assumingFilename ?? "<stdin>")
 
   guard let source = readSource(from: sourceFile) else {
-    throw FormatError.readSource(path: assumingFileURL.path)
+    diagnosticEngine.diagnose(
+      Diagnostic.Message(
+        .error, "Unable to read source for formatting from \(assumingFileURL.path)."))
+    return
   }
 
   do {
@@ -97,30 +102,25 @@ func formatMain(
       stdoutStream.flush()
     }
   } catch SwiftFormatError.fileNotReadable {
-    throw FormatError.unableToFormat(
-      path: assumingFileURL.path,
-      message: "file is not readable or does not exist")
+    let path = assumingFileURL.path
+    diagnosticEngine.diagnose(
+      Diagnostic.Message(
+        .error, "Unable to format \(path): file is not readable or does not exist."))
+    return
   } catch SwiftFormatError.fileContainsInvalidSyntax(let position) {
     let path = assumingFileURL.path
     let location = SourceLocationConverter(file: path, source: source).location(for: position)
-    throw FormatError.invalidSyntax(location: location, message: "file contains invalid or unrecognized Swift syntax.")
+    diagnosticEngine.diagnose(
+      Diagnostic.Message(.error, "file contains invalid or unrecognized Swift syntax."),
+      location: location)
+    return
   } catch {
-    throw FormatError.unableToFormat(path: assumingFileURL.path, message: "\(error)")
-  }
-  
-  if !diagnosticEngine.diagnostics.isEmpty {
-    throw FormatError.exitWithDiagnosticErrors
+    let path = assumingFileURL.path
+    diagnosticEngine.diagnose(Diagnostic.Message(.error, "Unable to format \(path): \(error)"))
+    return
   }
 }
 
-/// Makes and returns a new configured diagnostic engine.
-private func makeDiagnosticEngine() -> DiagnosticEngine {
-  let engine = DiagnosticEngine()
-  let consumer = PrintingDiagnosticConsumer()
-  engine.addConsumer(consumer)
-  return engine
-}
-
 /// Reads from the given file handle until EOF is reached, then returns the contents as a UTF8
 /// encoded string.
 fileprivate func readSource(from fileHandle: FileHandle) -> String? {

Sources/swift-format/Run.swift

@@ -19,43 +19,50 @@ import TSCBasic
 
 extension SwiftFormatCommand {
   func run() throws {
+    let diagnosticEngine = makeDiagnosticEngine()
     switch mode {
     case .format:
       if paths.isEmpty {
         let configuration = try loadConfiguration(
           forSwiftFile: nil, configFilePath: configurationPath)
-        try formatMain(
+        formatMain(
           configuration: configuration, sourceFile: FileHandle.standardInput,
           assumingFilename: assumeFilename, inPlace: false,
-          debugOptions: debugOptions)
+          debugOptions: debugOptions, diagnosticEngine: diagnosticEngine)
       } else {
-        try processSources(from: paths, configurationPath: configurationPath) {
+        try processSources(from: paths, configurationPath: configurationPath, diagnosticEngine: diagnosticEngine) {
           (sourceFile, path, configuration) in
-          try formatMain(
+          formatMain(
             configuration: configuration, sourceFile: sourceFile, assumingFilename: path,
-            inPlace: inPlace, debugOptions: debugOptions)
+            inPlace: inPlace, debugOptions: debugOptions, diagnosticEngine: diagnosticEngine)
         }
       }
 
     case .lint:
       if paths.isEmpty {
         let configuration = try loadConfiguration(
           forSwiftFile: nil, configFilePath: configurationPath)
-        try lintMain(
+        lintMain(
             configuration: configuration, sourceFile: FileHandle.standardInput,
-            assumingFilename: assumeFilename, debugOptions: debugOptions)
+            assumingFilename: assumeFilename, debugOptions: debugOptions, diagnosticEngine: diagnosticEngine)
       } else {
-        try processSources(from: paths, configurationPath: configurationPath) {
+        try processSources(from: paths, configurationPath: configurationPath, diagnosticEngine: diagnosticEngine) {
           (sourceFile, path, configuration) in
-          try lintMain(
+          lintMain(
             configuration: configuration, sourceFile: sourceFile, assumingFilename: path,
-            debugOptions: debugOptions)
+            debugOptions: debugOptions, diagnosticEngine: diagnosticEngine)
         }
       }
 
     case .dumpConfiguration:
       try dumpDefaultConfiguration()
     }
+    
+    // If any of the operations have generated diagnostics, throw an error
+    // to exit with the error status code.
+    if !diagnosticEngine.diagnostics.isEmpty {
+      throw FormatError.exitWithDiagnosticErrors
+    }
   }
 }
 
@@ -68,14 +75,17 @@ extension SwiftFormatCommand {
 ///   - transform: A closure that performs a transformation on a specific source file.
 private func processSources(
   from paths: [String], configurationPath: String?,
-  transform: (FileHandle, String, Configuration) throws -> Void
+  diagnosticEngine: DiagnosticEngine,
+  transform: (FileHandle, String, Configuration) -> Void
 ) throws {
   for path in FileIterator(paths: paths) {
     guard let sourceFile = FileHandle(forReadingAtPath: path) else {
-      throw FormatError(message: "Unable to create a file handle for source from \(path).")
+      diagnosticEngine.diagnose(
+        Diagnostic.Message(.error, "Unable to create a file handle for source from \(path)."))
+      return
     }
     let configuration = try loadConfiguration(forSwiftFile: path, configFilePath: configurationPath)
-    try transform(sourceFile, path, configuration)
+    transform(sourceFile, path, configuration)
   }
 }