From 0067ab6766c9fa0b2c42da3da50957067bba8b68 Mon Sep 17 00:00:00 2001 From: Nate Cook Date: Thu, 9 Jan 2020 14:21:47 -0600 Subject: [PATCH 1/4] Adopt ArgumentParser for command-line tool --- Package.resolved | 13 +- Package.swift | 4 +- Sources/swift-format/CommandLineOptions.swift | 208 ++++++------------ Sources/swift-format/FormatError.swift | 30 +++ Sources/swift-format/Run.swift | 70 +++--- Sources/swift-format/ToolMode.swift | 39 ---- Sources/swift-format/main.swift | 128 +++++------ 7 files changed, 197 insertions(+), 295 deletions(-) create mode 100644 Sources/swift-format/FormatError.swift delete mode 100644 Sources/swift-format/ToolMode.swift diff --git a/Package.resolved b/Package.resolved index 08d3c74dd..b5ca7b177 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,12 +1,21 @@ { "object": { "pins": [ + { + "package": "swift-argument-parser", + "repositoryURL": "https://github.com/apple/swift-argument-parser.git", + "state": { + "branch": "master", + "revision": "2b80dc4a9fa21af31a3a46a91022c51e9245cc6b", + "version": null + } + }, { "package": "SwiftSyntax", "repositoryURL": "https://github.com/apple/swift-syntax", "state": { - "branch": "swift-DEVELOPMENT-SNAPSHOT-2020-01-29-a", - "revision": "16f6af54d9ad3cfcb35cd557288783dded1107fd", + "branch": "master", + "revision": "7251727261c6d2fb225245dc20ffded8f8d0f000", "version": null } }, diff --git a/Package.swift b/Package.swift index 9718325e3..99a8fcb2a 100644 --- a/Package.swift +++ b/Package.swift @@ -23,9 +23,10 @@ let package = Package( dependencies: [ .package( url: "https://github.com/apple/swift-syntax", - .revision("swift-DEVELOPMENT-SNAPSHOT-2020-01-29-a") + .branch("master") ), .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"), ], 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 index 92dbb0759..13b87430a 100644 --- 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() - 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 new file mode 100644 index 000000000..2d5b243e4 --- /dev/null +++ b/Sources/swift-format/FormatError.swift @@ -0,0 +1,30 @@ +import Foundation +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: "") + } +} + diff --git a/Sources/swift-format/Run.swift b/Sources/swift-format/Run.swift index 714286b34..c5d4c8b87 100644 --- a/Sources/swift-format/Run.swift +++ b/Sources/swift-format/Run.swift @@ -29,38 +29,34 @@ 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, diagnosticEngine: DiagnosticEngine -) -> Int { + debugOptions: DebugOptions +) throws { + let diagnosticEngine = makeDiagnosticEngine() let linter = SwiftLinter(configuration: configuration, diagnosticEngine: diagnosticEngine) linter.debugOptions = debugOptions let assumingFileURL = URL(fileURLWithPath: assumingFilename ?? "") guard let source = readSource(from: sourceFile) else { - diagnosticEngine.diagnose( - Diagnostic.Message(.error, "Unable to read source for linting from \(assumingFileURL.path).")) - return 1 + throw FormatError.readSource(path: assumingFileURL.path) } do { try linter.lint(source: source, assumingFileURL: assumingFileURL) } catch SwiftFormatError.fileNotReadable { - let path = assumingFileURL.path - diagnosticEngine.diagnose( - Diagnostic.Message(.error, "Unable to lint \(path): file is not readable or does not exist.")) - return 1 + throw FormatError.unableToLint( + path: assumingFileURL.path, + message: "file is not readable or does not exist") } 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 + throw FormatError.invalidSyntax(location: location, message: "file contains invalid or unrecognized Swift syntax.") } catch { - let path = assumingFileURL.path - diagnosticEngine.diagnose(Diagnostic.Message(.error, "Unable to lint \(path): \(error)")) - exit(1) + throw FormatError.unableToLint(path: assumingFileURL.path, message: "\(error)") + } + + if !diagnosticEngine.diagnostics.isEmpty { + throw FormatError.exitWithDiagnosticErrors } - return diagnosticEngine.diagnostics.isEmpty ? 0 : 1 } /// Runs the formatting pipeline over the provided source file. @@ -75,17 +71,15 @@ 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, diagnosticEngine: DiagnosticEngine -) -> Int { + debugOptions: DebugOptions +) throws { + let diagnosticEngine = makeDiagnosticEngine() let formatter = SwiftFormatter(configuration: configuration, diagnosticEngine: diagnosticEngine) formatter.debugOptions = debugOptions let assumingFileURL = URL(fileURLWithPath: assumingFilename ?? "") guard let source = readSource(from: sourceFile) else { - diagnosticEngine.diagnose( - Diagnostic.Message( - .error, "Unable to read source for formatting from \(assumingFileURL.path).")) - return 1 + throw FormatError.readSource(path: assumingFileURL.path) } do { @@ -103,24 +97,28 @@ func formatMain( stdoutStream.flush() } } catch SwiftFormatError.fileNotReadable { - let path = assumingFileURL.path - diagnosticEngine.diagnose( - Diagnostic.Message( - .error, "Unable to format \(path): file is not readable or does not exist.")) - return 1 + throw FormatError.unableToFormat( + path: assumingFileURL.path, + message: "file is not readable or does not exist") } 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 + throw FormatError.invalidSyntax(location: location, message: "file contains invalid or unrecognized Swift syntax.") } catch { - let path = assumingFileURL.path - diagnosticEngine.diagnose(Diagnostic.Message(.error, "Unable to format \(path): \(error)")) - exit(1) + throw FormatError.unableToFormat(path: assumingFileURL.path, message: "\(error)") + } + + if !diagnosticEngine.diagnostics.isEmpty { + throw FormatError.exitWithDiagnosticErrors } - return 0 +} + +/// 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 diff --git a/Sources/swift-format/ToolMode.swift b/Sources/swift-format/ToolMode.swift deleted file mode 100644 index 16d94e8c4..000000000 --- a/Sources/swift-format/ToolMode.swift +++ /dev/null @@ -1,39 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the Swift.org open source project -// -// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors -// Licensed under Apache License v2.0 with Runtime Library Exception -// -// See https://swift.org/LICENSE.txt for license information -// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors -// -//===----------------------------------------------------------------------===// - -import TSCUtility - -/// The mode in which the `swift-format` tool should run. -enum ToolMode: String, Codable, ArgumentKind { - case format - case lint - case dumpConfiguration = "dump-configuration" - case version - - static var completion: ShellCompletion { - return .values( - [ - ("format", "Format the provided files."), - ("lint", "Lint the provided files."), - ("dump-configuration", "Dump the default configuration as JSON to standard output."), - ]) - } - - /// Creates a `ToolMode` value from the given command line argument string, throwing an error if - /// the string is not valid. - init(argument: String) throws { - guard let mode = ToolMode(rawValue: argument) else { - throw ArgumentParserError.invalidValue(argument: argument, error: .unknown(value: argument)) - } - self = mode - } -} diff --git a/Sources/swift-format/main.swift b/Sources/swift-format/main.swift index cd091c0d1..97b495365 100644 --- a/Sources/swift-format/main.swift +++ b/Sources/swift-format/main.swift @@ -16,58 +16,46 @@ import SwiftFormatConfiguration import SwiftFormatCore import SwiftSyntax import TSCBasic -import TSCUtility -fileprivate func main(_ arguments: [String]) -> Int32 { - let url = URL(fileURLWithPath: arguments.first!) - let options = processArguments(commandName: url.lastPathComponent, Array(arguments.dropFirst())) - let diagnosticEngine = makeDiagnosticEngine() - switch options.mode { - case .format: - if options.paths.isEmpty { - let configuration = loadConfiguration( - forSwiftFile: nil, configFilePath: options.configurationPath) - return Int32( - formatMain( +extension SwiftFormatCommand { + func run() throws { + switch mode { + case .format: + if paths.isEmpty { + let configuration = try loadConfiguration( + forSwiftFile: nil, configFilePath: configurationPath) + try formatMain( configuration: configuration, sourceFile: FileHandle.standardInput, - assumingFilename: options.assumeFilename, inPlace: false, - debugOptions: options.debugOptions, diagnosticEngine: diagnosticEngine)) + assumingFilename: assumeFilename, inPlace: false, + debugOptions: debugOptions) + } else { + try processSources(from: paths, configurationPath: configurationPath) { + (sourceFile, path, configuration) in + try formatMain( + configuration: configuration, sourceFile: sourceFile, assumingFilename: path, + inPlace: inPlace, debugOptions: debugOptions) + } + } + + case .lint: + if paths.isEmpty { + let configuration = try loadConfiguration( + forSwiftFile: nil, configFilePath: configurationPath) + try lintMain( + configuration: configuration, sourceFile: FileHandle.standardInput, + assumingFilename: assumeFilename, debugOptions: debugOptions) + } else { + try processSources(from: paths, configurationPath: configurationPath) { + (sourceFile, path, configuration) in + try lintMain( + configuration: configuration, sourceFile: sourceFile, assumingFilename: path, + debugOptions: debugOptions) + } + } + + case .dumpConfiguration: + try dumpDefaultConfiguration() } - return processSources( - from: options.paths, configurationPath: options.configurationPath, - diagnosticEngine: diagnosticEngine - ) { - (sourceFile, path, configuration) in - formatMain( - configuration: configuration, sourceFile: sourceFile, assumingFilename: path, - inPlace: options.inPlace, debugOptions: options.debugOptions, - diagnosticEngine: diagnosticEngine) - } - case .lint: - if options.paths.isEmpty { - let configuration = loadConfiguration( - forSwiftFile: nil, configFilePath: options.configurationPath) - return Int32( - lintMain( - configuration: configuration, sourceFile: FileHandle.standardInput, - assumingFilename: options.assumeFilename, debugOptions: options.debugOptions, - diagnosticEngine: diagnosticEngine)) - } - return processSources( - from: options.paths, configurationPath: options.configurationPath, - diagnosticEngine: diagnosticEngine - ) { - (sourceFile, path, configuration) in - lintMain( - configuration: configuration, sourceFile: sourceFile, assumingFilename: path, - debugOptions: options.debugOptions, diagnosticEngine: diagnosticEngine) - } - case .dumpConfiguration: - dumpDefaultConfiguration() - return 0 - case .version: - print("0.0.1") // TODO: Automate updates to this somehow. - return 0 } } @@ -78,21 +66,17 @@ fileprivate func main(_ arguments: [String]) -> Int32 { /// - configurationPath: The file path to a swift-format configuration file. /// - diagnosticEngine: A diagnostic collector that handles diagnostic messages. /// - transform: A closure that performs a transformation on a specific source file. -fileprivate func processSources( - from paths: [String], configurationPath: String?, diagnosticEngine: DiagnosticEngine, - transform: (FileHandle, String, Configuration) -> Int -) -> Int32 { - var result = 0 +private func processSources( + from paths: [String], configurationPath: String?, + transform: (FileHandle, String, Configuration) throws -> Void +) throws { for path in FileIterator(paths: paths) { guard let sourceFile = FileHandle(forReadingAtPath: path) else { - diagnosticEngine.diagnose( - Diagnostic.Message(.error, "Unable to create a file handle for source from \(path).")) - return 1 + throw FormatError(message: "Unable to create a file handle for source from \(path).") } - let configuration = loadConfiguration(forSwiftFile: path, configFilePath: configurationPath) - result |= transform(sourceFile, path, configuration) + let configuration = try loadConfiguration(forSwiftFile: path, configFilePath: configurationPath) + try transform(sourceFile, path, configuration) } - return Int32(result) } /// Makes and returns a new configured diagnostic engine. @@ -106,15 +90,15 @@ fileprivate func makeDiagnosticEngine() -> DiagnosticEngine { /// Load the configuration. fileprivate func loadConfiguration( forSwiftFile swiftFilePath: String?, configFilePath: String? -) -> Configuration { +) throws -> Configuration { if let configFilePath = configFilePath { - return decodedConfiguration(fromFile: URL(fileURLWithPath: configFilePath)) + return try decodedConfiguration(fromFile: URL(fileURLWithPath: configFilePath)) } if let swiftFileURL = swiftFilePath.map(URL.init(fileURLWithPath:)), let configFileURL = Configuration.url(forConfigurationFileApplyingTo: swiftFileURL) { - return decodedConfiguration(fromFile: configFileURL) + return try decodedConfiguration(fromFile: configFileURL) } return Configuration() @@ -123,18 +107,16 @@ fileprivate func loadConfiguration( /// Loads and returns a `Configuration` from the given JSON file if it is found and is valid. If the /// file does not exist or there was an error decoding it, the program exits with a non-zero exit /// code. -fileprivate func decodedConfiguration(fromFile url: Foundation.URL) -> Configuration { +fileprivate func decodedConfiguration(fromFile url: Foundation.URL) throws -> Configuration { do { return try Configuration(contentsOf: url) } catch { - // TODO: Improve error message, write to stderr. - print("Could not load configuration at \(url): \(error)") - exit(1) + throw FormatError(message: "Could not load configuration at \(url): \(error)") } } /// Dumps the default configuration as JSON to standard output. -fileprivate func dumpDefaultConfiguration() { +private func dumpDefaultConfiguration() throws { let configuration = Configuration() do { let encoder = JSONEncoder() @@ -147,16 +129,12 @@ fileprivate func dumpDefaultConfiguration() { guard let jsonString = String(data: data, encoding: .utf8) else { // This should never happen, but let's make sure we fail more gracefully than crashing, just // in case. - // TODO: Improve error message, write to stderr. - print("Could not dump the default configuration: the JSON was not valid UTF-8") - exit(1) + throw FormatError(message: "Could not dump the default configuration: the JSON was not valid UTF-8") } print(jsonString) } catch { - // TODO: Improve error message, write to stderr. - print("Could not dump the default configuration: \(error)") - exit(1) + throw FormatError(message: "Could not dump the default configuration: \(error)") } } -exit(main(CommandLine.arguments)) +SwiftFormatCommand.main() From 73d072661a1e6307531c221bdbdcb98c3ebcf644 Mon Sep 17 00:00:00 2001 From: Nate Cook Date: Fri, 28 Feb 2020 14:41:22 -0600 Subject: [PATCH 2/4] Address PR feedback --- Package.resolved | 10 +-- Package.swift | 4 +- Sources/swift-format/CommandLineOptions.swift | 2 +- Sources/swift-format/FormatError.swift | 17 ----- Sources/swift-format/Run.swift | 68 +++++++++---------- Sources/swift-format/main.swift | 36 ++++++---- 6 files changed, 65 insertions(+), 72 deletions(-) diff --git a/Package.resolved b/Package.resolved index b5ca7b177..80d19dd99 100644 --- a/Package.resolved +++ b/Package.resolved @@ -5,17 +5,17 @@ "package": "swift-argument-parser", "repositoryURL": "https://github.com/apple/swift-argument-parser.git", "state": { - "branch": "master", - "revision": "2b80dc4a9fa21af31a3a46a91022c51e9245cc6b", - "version": null + "branch": null, + "revision": "f6ac7b8118ff5d1bc0faee7f37bf6f8fd8f95602", + "version": "0.0.1" } }, { "package": "SwiftSyntax", "repositoryURL": "https://github.com/apple/swift-syntax", "state": { - "branch": "master", - "revision": "7251727261c6d2fb225245dc20ffded8f8d0f000", + "branch": "swift-DEVELOPMENT-SNAPSHOT-2020-01-29-a", + "revision": "16f6af54d9ad3cfcb35cd557288783dded1107fd", "version": null } }, diff --git a/Package.swift b/Package.swift index 99a8fcb2a..56eacd911 100644 --- a/Package.swift +++ b/Package.swift @@ -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( diff --git a/Sources/swift-format/CommandLineOptions.swift b/Sources/swift-format/CommandLineOptions.swift index 13b87430a..f0490588d 100644 --- a/Sources/swift-format/CommandLineOptions.swift +++ b/Sources/swift-format/CommandLineOptions.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") } diff --git a/Sources/swift-format/FormatError.swift b/Sources/swift-format/FormatError.swift index 2d5b243e4..28e8fdea3 100644 --- a/Sources/swift-format/FormatError.swift +++ b/Sources/swift-format/FormatError.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: "") diff --git a/Sources/swift-format/Run.swift b/Sources/swift-format/Run.swift index c5d4c8b87..0b03d3b69 100644 --- a/Sources/swift-format/Run.swift +++ b/Sources/swift-format/Run.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 ?? "") 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 ?? "") 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? { diff --git a/Sources/swift-format/main.swift b/Sources/swift-format/main.swift index 97b495365..f02d6c047 100644 --- a/Sources/swift-format/main.swift +++ b/Sources/swift-format/main.swift @@ -19,21 +19,22 @@ 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) } } @@ -41,21 +42,27 @@ extension SwiftFormatCommand { 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) } } From 278849657645f50a7b96ae2ce33537c59ee05ad8 Mon Sep 17 00:00:00 2001 From: Nate Cook Date: Fri, 28 Feb 2020 18:41:51 -0600 Subject: [PATCH 3/4] Pass a nil DiagnosticEngine when formatting to match #155 --- Sources/swift-format/Run.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/swift-format/Run.swift b/Sources/swift-format/Run.swift index 0b03d3b69..11a4aa9a1 100644 --- a/Sources/swift-format/Run.swift +++ b/Sources/swift-format/Run.swift @@ -76,7 +76,7 @@ func formatMain( configuration: Configuration, sourceFile: FileHandle, assumingFilename: String?, inPlace: Bool, debugOptions: DebugOptions, diagnosticEngine: DiagnosticEngine ) { - let formatter = SwiftFormatter(configuration: configuration, diagnosticEngine: diagnosticEngine) + let formatter = SwiftFormatter(configuration: configuration, diagnosticEngine: nil) formatter.debugOptions = debugOptions let assumingFileURL = URL(fileURLWithPath: assumingFilename ?? "") From 8525dfecefabc17c0cd8627b29d0fe8d119da030 Mon Sep 17 00:00:00 2001 From: Nate Cook Date: Mon, 9 Mar 2020 17:14:57 -0500 Subject: [PATCH 4/4] Don't print an additional error message when exiting --- Package.resolved | 4 ++-- Package.swift | 2 +- Sources/swift-format/main.swift | 7 ++++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Package.resolved b/Package.resolved index 80d19dd99..458f8cc1d 100644 --- a/Package.resolved +++ b/Package.resolved @@ -6,8 +6,8 @@ "repositoryURL": "https://github.com/apple/swift-argument-parser.git", "state": { "branch": null, - "revision": "f6ac7b8118ff5d1bc0faee7f37bf6f8fd8f95602", - "version": "0.0.1" + "revision": "35b76bf577d3cc74820f8991894ce3bcdf024ddc", + "version": "0.0.2" } }, { diff --git a/Package.swift b/Package.swift index 56eacd911..5a0ada27c 100644 --- a/Package.swift +++ b/Package.swift @@ -26,7 +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.1")), + .package(url: "https://github.com/apple/swift-argument-parser.git", .upToNextMinor(from: "0.0.2")), ], targets: [ .target( diff --git a/Sources/swift-format/main.swift b/Sources/swift-format/main.swift index f02d6c047..387518f86 100644 --- a/Sources/swift-format/main.swift +++ b/Sources/swift-format/main.swift @@ -16,6 +16,7 @@ import SwiftFormatConfiguration import SwiftFormatCore import SwiftSyntax import TSCBasic +import ArgumentParser extension SwiftFormatCommand { func run() throws { @@ -58,10 +59,10 @@ extension SwiftFormatCommand { try dumpDefaultConfiguration() } - // If any of the operations have generated diagnostics, throw an error - // to exit with the error status code. + // If any of the operations have generated diagnostics, exit with the + // error status code. if !diagnosticEngine.diagnostics.isEmpty { - throw FormatError.exitWithDiagnosticErrors + throw ExitCode.failure } } }