From 3ca204bb18c4a20bcb4402c22ddb8c6241c906f3 Mon Sep 17 00:00:00 2001 From: Ben Barham Date: Thu, 9 Jan 2025 20:32:55 -0800 Subject: [PATCH 1/4] Optimize pretty printing performance swift-format#883 fixed outputting incorrect line numbers, but introduced a performance regression. swift-format#901 improved this back to around the original, but had to be reverted as it introduced a an issue due to counting codepoints rather than characters. Introduce a similar optimization again, but only for the first portion of the string (prior to the last newline). Fixes swift-format#894 again. --- .../PrettyPrint/PrettyPrintBuffer.swift | 25 ++++++++------ .../PrettyPrint/LineNumbersTests.swift | 34 +++++++++++++++++++ 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift b/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift index 4b02d372e..9ea726206 100644 --- a/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift +++ b/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift @@ -119,18 +119,21 @@ struct PrettyPrintBuffer { consecutiveNewlineCount = 0 pendingSpaces = 0 - // In case of comments, we may get a multi-line string. - // To account for that case, we need to correct the lineNumber count. - // The new column is only the position within the last line. - let lines = text.split(separator: "\n") - lineNumber += lines.count - 1 - if lines.count > 1 { - // in case we have inserted new lines, we need to reset the column - column = lines.last?.count ?? 0 + // In case of comments, we may get a multi-line string. To account for that case, we need to correct the + // `lineNumber` count. The new `column` is the position within the last line. + + var lastNewlineIndex: String.Index? = nil + for i in text.utf8.indices { + if text.utf8[i] == UInt8(ascii: "\n") { + lastNewlineIndex = i + lineNumber += 1 + } + } + + if let lastNewlineIndex { + column = text.distance(from: text.utf8.index(after: lastNewlineIndex), to: text.endIndex) } else { - // in case it is an end of line comment or a single line comment, - // we just add to the current column - column += lines.last?.count ?? 0 + column += text.count } } diff --git a/Tests/SwiftFormatTests/PrettyPrint/LineNumbersTests.swift b/Tests/SwiftFormatTests/PrettyPrint/LineNumbersTests.swift index 1bb58f7ab..c377e7cad 100644 --- a/Tests/SwiftFormatTests/PrettyPrint/LineNumbersTests.swift +++ b/Tests/SwiftFormatTests/PrettyPrint/LineNumbersTests.swift @@ -81,4 +81,38 @@ final class LineNumbersTests: PrettyPrintTestCase { ] ) } + + func testCharacterVsCodepoint() { + let input = + """ + let fo = 1 // 🤥 + + """ + + assertPrettyPrintEqual( + input: input, + expected: input, + linelength: 16, + whitespaceOnly: true, + findings: [] + ) + } + + func testCharacterVsCodepointMultiline() { + let input = + #""" + /// This is a multiline + /// comment that is in 🤥 + /// fact perfectly sized + + """# + + assertPrettyPrintEqual( + input: input, + expected: input, + linelength: 25, + whitespaceOnly: true, + findings: [] + ) + } } From 83b5f010d250e3b4d3009562ed55414766201514 Mon Sep 17 00:00:00 2001 From: Rintaro Ishizaki Date: Mon, 13 Jan 2025 10:06:44 -0800 Subject: [PATCH 2/4] [WhitespaceLinter] Use hand crafted "is whitespace" function * `UnicodeScalar(_:)` on arbitrary UTF8 code point was wrong. It only works correctly if the code point is < 0x80 * `UnicodeScalar.Properties.isWhitespace` is slow. Profiling 'lint' shows it's taking 13.6 of the entire time * Whitespaces in Unicode "Basic Latin" block are well defined, there's no need to consult `UnicodeScalar.Properties` --- .../SwiftFormat/PrettyPrint/WhitespaceLinter.swift | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Sources/SwiftFormat/PrettyPrint/WhitespaceLinter.swift b/Sources/SwiftFormat/PrettyPrint/WhitespaceLinter.swift index c5c5e2ae8..30f733952 100644 --- a/Sources/SwiftFormat/PrettyPrint/WhitespaceLinter.swift +++ b/Sources/SwiftFormat/PrettyPrint/WhitespaceLinter.swift @@ -339,9 +339,16 @@ public class WhitespaceLinter { startingAt offset: Int, in data: [UTF8.CodeUnit] ) -> ArraySlice { + func isWhitespace(_ char: UTF8.CodeUnit) -> Bool { + switch char { + case UInt8(ascii: " "), UInt8(ascii: "\n"), UInt8(ascii: "\t"), UInt8(ascii: "\r"), /*VT*/ 0x0B, /*FF*/ 0x0C: + return true + default: + return false + } + } guard - let whitespaceEnd = - data[offset...].firstIndex(where: { !UnicodeScalar($0).properties.isWhitespace }) + let whitespaceEnd = data[offset...].firstIndex(where: { !isWhitespace($0) }) else { return data[offset.. Date: Sun, 12 Jan 2025 06:27:05 +0900 Subject: [PATCH 3/4] Gradually deprecate running swift-format without input paths; require '-' for stdin in the future --- Sources/swift-format/Frontend/Frontend.swift | 18 +++++++++++++++++- .../Subcommands/LintFormatOptions.swift | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/Sources/swift-format/Frontend/Frontend.swift b/Sources/swift-format/Frontend/Frontend.swift index a3ea18a4f..b0e262a94 100644 --- a/Sources/swift-format/Frontend/Frontend.swift +++ b/Sources/swift-format/Frontend/Frontend.swift @@ -97,7 +97,23 @@ class Frontend { /// Runs the linter or formatter over the inputs. final func run() { - if lintFormatOptions.paths.isEmpty { + if lintFormatOptions.paths == ["-"] { + processStandardInput() + } else if lintFormatOptions.paths.isEmpty { + diagnosticsEngine.emitWarning( + """ + Running swift-format without input paths is deprecated and will be removed in the future. + + Please update your invocation to do either of the following: + + - Pass `-` to read from stdin (e.g., `cat MyFile.swift | swift-format -`). + - Pass one or more paths to Swift source files or directories containing + Swift source files. When passing directories, make sure to include the + `--recursive` flag. + + For more information, use the `--help` option. + """ + ) processStandardInput() } else { processURLs( diff --git a/Sources/swift-format/Subcommands/LintFormatOptions.swift b/Sources/swift-format/Subcommands/LintFormatOptions.swift index 520eaf970..737de42fc 100644 --- a/Sources/swift-format/Subcommands/LintFormatOptions.swift +++ b/Sources/swift-format/Subcommands/LintFormatOptions.swift @@ -108,7 +108,7 @@ struct LintFormatOptions: ParsableArguments { var experimentalFeatures: [String] = [] /// The list of paths to Swift source files that should be formatted or linted. - @Argument(help: "Zero or more input filenames.") + @Argument(help: "Zero or more input filenames. Use `-` for stdin.") var paths: [String] = [] @Flag(help: .hidden) var debugDisablePrettyPrint: Bool = false From 4bba23b83cb6a1cbe191197907d220a3409c50fc Mon Sep 17 00:00:00 2001 From: Dmytro Mishchenko Date: Thu, 30 Jan 2025 12:54:58 +0100 Subject: [PATCH 4/4] Document missing configuration Documented missing config values in `Configuration.md`. Added the same description as inside inline docs. Documented values: * `fileScopedDeclarationPrivacy` * `indentSwitchCaseLabels` * `noAssignmentInExpressions` * `reflowMultilineStringLiterals` --- Documentation/Configuration.md | 88 ++++++++++++++++++++++++++++++++-- 1 file changed, 84 insertions(+), 4 deletions(-) diff --git a/Documentation/Configuration.md b/Documentation/Configuration.md index 42f044fb1..f84fb1c60 100644 --- a/Documentation/Configuration.md +++ b/Documentation/Configuration.md @@ -137,11 +137,40 @@ top-level keys and values: --- -## FIXME: fileScopedDeclarationPrivacy +## `fileScopedDeclarationPrivacy` +**type:** object + +**description:** Declarations at file scope with effective private access should be consistently declared as either `fileprivate` or `private`, determined by configuration. + +- `accessLevel` _(string)_: The formal access level to use when encountering a file-scoped declaration with effective private access. Allowed values are `private` and `fileprivate`. + +**default:** `{ "accessLevel" : "private" }` --- -### FIXME: indentSwitchCaseLabels +### `indentSwitchCaseLabels` +**type:** boolean + +**description:** Determines if `case` statements should be indented compared to the containing `switch` block. + +When `false`, the correct form is: +```swift +switch someValue { +case someCase: + someStatement +... +} +``` +When `true`, the correct form is: +```swift +switch someValue { + case someCase: + someStatement + ... +} +``` + +**default:** `false` --- @@ -154,7 +183,14 @@ top-level keys and values: --- -### FIXME: noAssignmentInExpressions +### `noAssignmentInExpressions` +**type:** object + +**description:** Assignment expressions must be their own statements. Assignment should not be used in an expression context that expects a `Void` value. For example, assigning a variable within a `return` statement existing a `Void` function is prohibited. + +- `allowedFunctions` _(strings array)_: A list of function names where assignments are allowed to be embedded in expressions that are passed as parameters to that function. + +**default:** `{ "allowedFunctions" : ["XCTAssertNoThrow"] }` --- @@ -167,7 +203,51 @@ top-level keys and values: --- -### FIXME: reflowMultilineStringLiterals +### `reflowMultilineStringLiterals` +**type:** `string` + +**description:** Determines how multiline string literals should reflow when formatted. + +- `never`: Never reflow multiline string literals. +- `onlyLinesOverLength`: Reflow lines in string literal that exceed the maximum line length. +For example with a line length of 10: +```swift +""" +an escape\ + line break +a hard line break +""" +``` +will be formatted as: +```swift +""" +an esacpe\ + line break +a hard \ +line break +""" +``` +- `always`: Always reflow multiline string literals, this will ignore existing escaped newlines in the literal and reflow each line. Hard linebreaks are still respected. +For example, with a line length of 10: +```swift +""" +one \ +word \ +a line. +this is too long. +""" +``` +will be formatted as: +```swift +""" +one word \ +a line. +this is \ +too long. +""" +``` + +**default:** `"never"` ---