From afe9acdb33cd79f030c4b9b4b15702b0757157fc Mon Sep 17 00:00:00 2001 From: Dylan Sturgeon Date: Thu, 26 Mar 2020 11:01:07 -0700 Subject: [PATCH 1/6] Force breaking in repeat body when while condition is too long. Repeat-while-stmt is intended to keep the right brace next to the `while` token, like if-else statements. Previously, there wasn't a group so the pretty printer was allowing the repeat body to not have any breaks when the while condition overflowed the line. --- .../TokenStreamCreator.swift | 16 +++++++++--- .../RepeatStmtTests.swift | 25 +++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift b/Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift index 0ceeb2404..9dd23752c 100644 --- a/Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift +++ b/Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift @@ -533,11 +533,19 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor { override func visit(_ node: RepeatWhileStmtSyntax) -> SyntaxVisitorContinueKind { arrangeBracesAndContents(of: node.body, contentsKeyPath: \.statements) - let whilePrecedingBreak = config.lineBreakBeforeControlFlowKeywords - ? Token.break(.same) : Token.space - before(node.whileKeyword, tokens: whilePrecedingBreak) + if config.lineBreakBeforeControlFlowKeywords { + before(node.whileKeyword, tokens: .break(.same), .open) + after(node.condition.lastToken, tokens: .close) + } else { + // The length of the condition needs to force the breaks around the braces of the repeat + // stmt's body, so that there's always a break before the right brace when the while & + // condition is too long to be on one line. + before(node.whileKeyword, tokens: .space) + // The `open` token occurs after the ending tokens for the braced `body` node. + before(node.body.rightBrace, tokens: .open) + after(node.condition.lastToken, tokens: .close) + } after(node.whileKeyword, tokens: .space) - return .visitChildren } diff --git a/Tests/SwiftFormatPrettyPrintTests/RepeatStmtTests.swift b/Tests/SwiftFormatPrettyPrintTests/RepeatStmtTests.swift index 875fa4b3b..787d69e1b 100644 --- a/Tests/SwiftFormatPrettyPrintTests/RepeatStmtTests.swift +++ b/Tests/SwiftFormatPrettyPrintTests/RepeatStmtTests.swift @@ -10,6 +10,10 @@ final class RepeatStmtTests: PrettyPrintTestCase { while x repeat { foo() } while longcondition + repeat { f() } + while long.condition + repeat { f() } while long.condition + repeat { f() } while long.condition.that.ison.many.lines repeat { let a = 123 var b = "abc" @@ -29,6 +33,16 @@ final class RepeatStmtTests: PrettyPrintTestCase { repeat { foo() } while longcondition + repeat { + f() + } while long.condition + repeat { + f() + } while long.condition + repeat { + f() + } while long.condition + .that.ison.many.lines repeat { let a = 123 var b = "abc" @@ -50,6 +64,10 @@ final class RepeatStmtTests: PrettyPrintTestCase { repeat {} while x repeat { f() } while x repeat { foo() } while longcondition + repeat { f() } + while long.condition + repeat { f() } while long.condition + repeat { f() } while long.condition.that.ison.many.lines repeat { let a = 123 var b = "abc" @@ -68,6 +86,13 @@ final class RepeatStmtTests: PrettyPrintTestCase { repeat { f() } while x repeat { foo() } while longcondition + repeat { f() } + while long.condition + repeat { f() } + while long.condition + repeat { f() } + while long.condition.that + .ison.many.lines repeat { let a = 123 var b = "abc" From cd035912d6c74f85f5ed91a1fa8c7a6ecf23f154 Mon Sep 17 00:00:00 2001 From: Dylan Sturgeon Date: Thu, 26 Mar 2020 12:16:14 -0700 Subject: [PATCH 2/6] Test coverage for imports wrapped in conditional. These don't receive any special treatment, and instead are sorted as code blocks. That's done intentionally because the code block in a conditional can be complex, containing multiple imports or even non-import code (e.g. a typealias). Sorting them in with other imports would only be a sepcial case for the simplest conditional blocks. I considered treating these like ignored imports, which are kept as-is. That has the drawback of forcing all imports before and after the conditional to be kept in place, which prevents sorting. --- .../OrderedImportsTests.swift | 78 +++++++++++++++++++ .../XCTestManifests.swift | 2 + 2 files changed, 80 insertions(+) diff --git a/Tests/SwiftFormatRulesTests/OrderedImportsTests.swift b/Tests/SwiftFormatRulesTests/OrderedImportsTests.swift index e54dab12b..84c90aa77 100644 --- a/Tests/SwiftFormatRulesTests/OrderedImportsTests.swift +++ b/Tests/SwiftFormatRulesTests/OrderedImportsTests.swift @@ -541,4 +541,82 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase { XCTAssertFormatting(OrderedImports.self, input: input, expected: expected) } + + func testConditionalImports() { + let input = + """ + import Zebras + import Apples + #if canImport(Darwin) + import Darwin + #elseif canImport(Glibc) + import Glibc + #endif + import Aardvarks + + foo() + bar() + baz() + """ + + let expected = + """ + import Aardvarks + import Apples + import Zebras + + #if canImport(Darwin) + import Darwin + typealias SomeNativeTypeHandle = some_darwin_type_t + #elseif canImport(Glibc) + import Glibc + typealias SomeNativeTypeHandle = some_glibc_type_t + #endif + + foo() + bar() + baz() + """ + + XCTAssertFormatting(OrderedImports.self, input: input, expected: expected) + } + + func testIgnoredConditionalImports() { + let input = + """ + import Zebras + import Apples + #if canImport(Darwin) + import Darwin + #elseif canImport(Glibc) + import Glibc + #endif + // swift-format-ignore + import Aardvarks + + foo() + bar() + baz() + """ + + let expected = + """ + import Apples + import Zebras + + #if canImport(Darwin) + import Darwin + #elseif canImport(Glibc) + import Glibc + #endif + // swift-format-ignore + import Aardvarks + + foo() + bar() + baz() + """ + + XCTAssertFormatting(OrderedImports.self, input: input, expected: expected) + } } diff --git a/Tests/SwiftFormatRulesTests/XCTestManifests.swift b/Tests/SwiftFormatRulesTests/XCTestManifests.swift index d8d1d6d71..c50a441ef 100644 --- a/Tests/SwiftFormatRulesTests/XCTestManifests.swift +++ b/Tests/SwiftFormatRulesTests/XCTestManifests.swift @@ -264,12 +264,14 @@ extension OrderedImportsTests { // `swift test --generate-linuxmain` // to regenerate. static let __allTests__OrderedImportsTests = [ + ("testConditionalImports", testConditionalImports), ("testDisableOrderedImports", testDisableOrderedImports), ("testDisableOrderedImportsMovingComments", testDisableOrderedImportsMovingComments), ("testDuplicateAttributedImports", testDuplicateAttributedImports), ("testDuplicateCommentedImports", testDuplicateCommentedImports), ("testDuplicateIgnoredImports", testDuplicateIgnoredImports), ("testEmptyFile", testEmptyFile), + ("testIgnoredConditionalImports", testIgnoredConditionalImports), ("testImportsOrderWithDocComment", testImportsOrderWithDocComment), ("testImportsOrderWithoutModuleType", testImportsOrderWithoutModuleType), ("testInvalidImportsOrder", testInvalidImportsOrder), From 59949f3615b3af06949dfb5bdfe429c737dd27a2 Mon Sep 17 00:00:00 2001 From: Dylan Sturgeon Date: Thu, 26 Mar 2020 14:37:13 -0700 Subject: [PATCH 3/6] Fix for failing test. --- Tests/SwiftFormatRulesTests/OrderedImportsTests.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/Tests/SwiftFormatRulesTests/OrderedImportsTests.swift b/Tests/SwiftFormatRulesTests/OrderedImportsTests.swift index 84c90aa77..b86308c46 100644 --- a/Tests/SwiftFormatRulesTests/OrderedImportsTests.swift +++ b/Tests/SwiftFormatRulesTests/OrderedImportsTests.swift @@ -567,10 +567,8 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase { #if canImport(Darwin) import Darwin - typealias SomeNativeTypeHandle = some_darwin_type_t #elseif canImport(Glibc) import Glibc - typealias SomeNativeTypeHandle = some_glibc_type_t #endif foo() From 6fc8085eea062218caa14f6de2e68157c64ff996 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Wed, 1 Apr 2020 10:09:26 -0700 Subject: [PATCH 4/6] Remove swift-tools-support-core dependency. TSC was mostly used for its argument parser. Now that we've migrated to swift-argument-parser, the remaining file system and path APIs in TSC are only used by the Format command and can be fairly easily replaced by Foundation APIs instead. --- Package.resolved | 9 ------- Package.swift | 4 +-- .../Subcommands/DumpConfiguration.swift | 2 -- Sources/swift-format/Subcommands/Format.swift | 27 ++++++++----------- .../swift-format/Subcommands/LegacyMain.swift | 1 - Sources/swift-format/Subcommands/Lint.swift | 1 - .../FileHandle+TextOutputStream.swift | 19 +++++++++++++ Sources/swift-format/Utilities/Helpers.swift | 1 - 8 files changed, 31 insertions(+), 33 deletions(-) create mode 100644 Sources/swift-format/Utilities/FileHandle+TextOutputStream.swift diff --git a/Package.resolved b/Package.resolved index bdb5ff924..23a57b66f 100644 --- a/Package.resolved +++ b/Package.resolved @@ -18,15 +18,6 @@ "revision": "0688b9cfc4c3dd234e4f55f1f056b2affc849873", "version": "0.50200.0" } - }, - { - "package": "swift-tools-support-core", - "repositoryURL": "https://github.com/apple/swift-tools-support-core.git", - "state": { - "branch": null, - "revision": "693aba4c4c9dcc4767cc853a0dd38bf90ad8c258", - "version": "0.0.1" - } } ] }, diff --git a/Package.swift b/Package.swift index e7fbce2d4..e1b53d160 100644 --- a/Package.swift +++ b/Package.swift @@ -22,7 +22,6 @@ let package = Package( ], dependencies: [ .package(url: "https://github.com/apple/swift-syntax", from: "0.50200.0"), - .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.4")), ], targets: [ @@ -62,12 +61,11 @@ let package = Package( .target( name: "swift-format", dependencies: [ + "ArgumentParser", "SwiftFormat", "SwiftFormatConfiguration", "SwiftFormatCore", "SwiftSyntax", - "SwiftToolsSupport-auto", - "ArgumentParser", ] ), .testTarget( diff --git a/Sources/swift-format/Subcommands/DumpConfiguration.swift b/Sources/swift-format/Subcommands/DumpConfiguration.swift index a4fd79771..b32b9c8fe 100644 --- a/Sources/swift-format/Subcommands/DumpConfiguration.swift +++ b/Sources/swift-format/Subcommands/DumpConfiguration.swift @@ -13,8 +13,6 @@ import ArgumentParser import Foundation import SwiftFormatConfiguration -import TSCBasic -import TSCUtility extension SwiftFormatCommand { /// Dumps the tool's default configuration in JSON format to standard output. diff --git a/Sources/swift-format/Subcommands/Format.swift b/Sources/swift-format/Subcommands/Format.swift index 0f4ac69fa..7ad2f55b9 100644 --- a/Sources/swift-format/Subcommands/Format.swift +++ b/Sources/swift-format/Subcommands/Format.swift @@ -15,7 +15,6 @@ import Foundation import SwiftFormat import SwiftFormatConfiguration import SwiftSyntax -import TSCBasic extension SwiftFormatCommand { /// Formats one or more files containing Swift code. @@ -89,31 +88,29 @@ private func formatMain( // fixed anyway. let formatter = SwiftFormatter(configuration: configuration, diagnosticEngine: nil) formatter.debugOptions = debugOptions - let assumingFileURL = URL(fileURLWithPath: assumingFilename ?? "") + + let path = assumingFilename ?? "" + let assumingFileURL = URL(fileURLWithPath: path) guard let source = readSource(from: sourceFile) else { diagnosticEngine.diagnose( - Diagnostic.Message( - .error, "Unable to read source for formatting from \(assumingFileURL.path).")) + Diagnostic.Message(.error, "Unable to read source for formatting from \(path).")) return } + var stdoutStream = FileHandle.standardOutput do { if inPlace { - let cwd = FileManager.default.currentDirectoryPath - var buffer = BufferedOutputByteStream() + var buffer = "" try formatter.format(source: source, assumingFileURL: assumingFileURL, to: &buffer) - buffer.flush() - try localFileSystem.writeFileContents( - AbsolutePath(assumingFileURL.path, relativeTo: AbsolutePath(cwd)), - bytes: buffer.bytes - ) + + let bufferData = buffer.data(using: .utf8)! // Conversion to UTF-8 cannot fail + try bufferData.write(to: assumingFileURL, options: .atomic) } else { try formatter.format(source: source, assumingFileURL: assumingFileURL, to: &stdoutStream) - stdoutStream.flush() + stdoutStream.synchronizeFile() } } catch SwiftFormatError.fileNotReadable { - let path = assumingFileURL.path diagnosticEngine.diagnose( Diagnostic.Message( .error, "Unable to format \(path): file is not readable or does not exist.")) @@ -125,17 +122,15 @@ private func formatMain( return } stdoutStream.write(source) - stdoutStream.flush() + stdoutStream.synchronizeFile() return } - 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 } catch { - let path = assumingFileURL.path diagnosticEngine.diagnose(Diagnostic.Message(.error, "Unable to format \(path): \(error)")) return } diff --git a/Sources/swift-format/Subcommands/LegacyMain.swift b/Sources/swift-format/Subcommands/LegacyMain.swift index f4a1e45bf..2668a646f 100644 --- a/Sources/swift-format/Subcommands/LegacyMain.swift +++ b/Sources/swift-format/Subcommands/LegacyMain.swift @@ -16,7 +16,6 @@ import SwiftFormat import SwiftFormatConfiguration import SwiftFormatCore import SwiftSyntax -import TSCBasic extension SwiftFormatCommand { /// Keep the legacy `-m/--mode` flag working temporarily when no other subcommand is specified. diff --git a/Sources/swift-format/Subcommands/Lint.swift b/Sources/swift-format/Subcommands/Lint.swift index 31c55cb38..e5f31adaf 100644 --- a/Sources/swift-format/Subcommands/Lint.swift +++ b/Sources/swift-format/Subcommands/Lint.swift @@ -15,7 +15,6 @@ import Foundation import SwiftFormat import SwiftFormatConfiguration import SwiftSyntax -import TSCBasic extension SwiftFormatCommand { /// Emits style diagnostics for one or more files containing Swift code. diff --git a/Sources/swift-format/Utilities/FileHandle+TextOutputStream.swift b/Sources/swift-format/Utilities/FileHandle+TextOutputStream.swift new file mode 100644 index 000000000..b9e0c3928 --- /dev/null +++ b/Sources/swift-format/Utilities/FileHandle+TextOutputStream.swift @@ -0,0 +1,19 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2020 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 Foundation + +extension FileHandle: TextOutputStream { + public func write(_ string: String) { + self.write(string.data(using: .utf8)!) // Conversion to UTF-8 cannot fail + } +} diff --git a/Sources/swift-format/Utilities/Helpers.swift b/Sources/swift-format/Utilities/Helpers.swift index dd7d8bf6a..1ae0933e8 100644 --- a/Sources/swift-format/Utilities/Helpers.swift +++ b/Sources/swift-format/Utilities/Helpers.swift @@ -16,7 +16,6 @@ import SwiftFormat import SwiftFormatConfiguration import SwiftFormatCore import SwiftSyntax -import TSCBasic /// Throws an error that causes the current command to exit the process with a failure exit code if /// any of the preceding operations emitted diagnostics. From a727ab7f23005ee5b724ecac2fd26f110e8d8d8b Mon Sep 17 00:00:00 2001 From: Dylan Sturgeon Date: Wed, 1 Apr 2020 13:42:49 -0700 Subject: [PATCH 5/6] Discard newlines when sorting imports. When the author inserts a newline between the `import` token and the imported path, that newline shouldn't be considered when sorting the paths. --- Sources/SwiftFormatRules/OrderedImports.swift | 2 +- .../XCTestManifests.swift | 2 -- .../OrderedImportsTests.swift | 24 +++++++++++++++++++ .../XCTestManifests.swift | 1 + 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/Sources/SwiftFormatRules/OrderedImports.swift b/Sources/SwiftFormatRules/OrderedImports.swift index 87f53a733..080fa4639 100644 --- a/Sources/SwiftFormatRules/OrderedImports.swift +++ b/Sources/SwiftFormatRules/OrderedImports.swift @@ -503,7 +503,7 @@ fileprivate class Line { else { return "" } - return importDecl.path.description.trimmingCharacters(in: .whitespaces) + return importDecl.path.description.trimmingCharacters(in: .whitespacesAndNewlines) } /// Returns the first `TokenSyntax` in the code block(s) from this Line, or nil when this Line diff --git a/Tests/SwiftFormatPrettyPrintTests/XCTestManifests.swift b/Tests/SwiftFormatPrettyPrintTests/XCTestManifests.swift index 173048dc8..d12c3db33 100644 --- a/Tests/SwiftFormatPrettyPrintTests/XCTestManifests.swift +++ b/Tests/SwiftFormatPrettyPrintTests/XCTestManifests.swift @@ -207,9 +207,7 @@ extension DifferentiationAttributeTests { // `swift test --generate-linuxmain` // to regenerate. static let __allTests__DifferentiationAttributeTests = [ - ("testDerivative", testDerivative), ("testDifferentiable", testDifferentiable), - ("testTranspose", testTranspose), ] } diff --git a/Tests/SwiftFormatRulesTests/OrderedImportsTests.swift b/Tests/SwiftFormatRulesTests/OrderedImportsTests.swift index b86308c46..b3d2f2527 100644 --- a/Tests/SwiftFormatRulesTests/OrderedImportsTests.swift +++ b/Tests/SwiftFormatRulesTests/OrderedImportsTests.swift @@ -396,6 +396,30 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase { ) } + func testImportsContainingNewlines() { + let input = + """ + import + zeta + import Zeta + import + Alpha + import Beta + """ + + let expected = + """ + import + Alpha + import Beta + import Zeta + import + zeta + """ + + XCTAssertFormatting(OrderedImports.self, input: input, expected: expected) + } + func testRemovesDuplicateImports() { let input = """ diff --git a/Tests/SwiftFormatRulesTests/XCTestManifests.swift b/Tests/SwiftFormatRulesTests/XCTestManifests.swift index c50a441ef..41d3f412f 100644 --- a/Tests/SwiftFormatRulesTests/XCTestManifests.swift +++ b/Tests/SwiftFormatRulesTests/XCTestManifests.swift @@ -272,6 +272,7 @@ extension OrderedImportsTests { ("testDuplicateIgnoredImports", testDuplicateIgnoredImports), ("testEmptyFile", testEmptyFile), ("testIgnoredConditionalImports", testIgnoredConditionalImports), + ("testImportsContainingNewlines", testImportsContainingNewlines), ("testImportsOrderWithDocComment", testImportsOrderWithDocComment), ("testImportsOrderWithoutModuleType", testImportsOrderWithoutModuleType), ("testInvalidImportsOrder", testInvalidImportsOrder), From 1b46d56f4a8ee7e29639115b7e3d2c64b7c76f89 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Wed, 1 Apr 2020 14:21:51 -0700 Subject: [PATCH 6/6] Move conditional compilation directive inside tests. This ensures that the test function always exists, even if the symbol is not defined, so that `--generate-linuxmain` contains the symbol in both cases. When the symbol is undefined, the empty test will vacuously succeed. --- .../DifferentiationAttributeTests.swift | 14 ++++++++------ .../XCTestManifests.swift | 2 ++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/Tests/SwiftFormatPrettyPrintTests/DifferentiationAttributeTests.swift b/Tests/SwiftFormatPrettyPrintTests/DifferentiationAttributeTests.swift index 8f8eef4ec..3d5440733 100644 --- a/Tests/SwiftFormatPrettyPrintTests/DifferentiationAttributeTests.swift +++ b/Tests/SwiftFormatPrettyPrintTests/DifferentiationAttributeTests.swift @@ -43,8 +43,8 @@ final class DifferentiationAttributeTests: PrettyPrintTestCase { assertPrettyPrintEqual(input: input, expected: expected, linelength: 43) } - #if HAS_DERIVATIVE_REGISTRATION_ATTRIBUTE - func testDerivative() { + func testDerivative() { + #if HAS_DERIVATIVE_REGISTRATION_ATTRIBUTE let input = """ @derivative(of: foo, wrt: x) @@ -78,9 +78,11 @@ final class DifferentiationAttributeTests: PrettyPrintTestCase { """ assertPrettyPrintEqual(input: input, expected: expected, linelength: 28) - } + #endif + } - func testTranspose() { + func testTranspose() { + #if HAS_DERIVATIVE_REGISTRATION_ATTRIBUTE let input = """ @transpose(of: foo, wrt: 0) @@ -114,6 +116,6 @@ final class DifferentiationAttributeTests: PrettyPrintTestCase { """ assertPrettyPrintEqual(input: input, expected: expected, linelength: 27) - } - #endif + #endif + } } diff --git a/Tests/SwiftFormatPrettyPrintTests/XCTestManifests.swift b/Tests/SwiftFormatPrettyPrintTests/XCTestManifests.swift index d12c3db33..173048dc8 100644 --- a/Tests/SwiftFormatPrettyPrintTests/XCTestManifests.swift +++ b/Tests/SwiftFormatPrettyPrintTests/XCTestManifests.swift @@ -207,7 +207,9 @@ extension DifferentiationAttributeTests { // `swift test --generate-linuxmain` // to regenerate. static let __allTests__DifferentiationAttributeTests = [ + ("testDerivative", testDerivative), ("testDifferentiable", testDifferentiable), + ("testTranspose", testTranspose), ] }