From 87467ee3a573c2b13e5b0078b93cb29af7bcc545 Mon Sep 17 00:00:00 2001 From: Josh Wisenbaker Date: Fri, 13 Dec 2024 16:32:35 -0500 Subject: [PATCH 1/4] PrettyPrinterPerformance PrettyPrinterPerformance PrettyPrinterPerformance PrettyPrinterPerformance Optimized the PrettyPrinter for #894 Worked to get the perfomance to be closer to where we were before the changes in #883. This code should be only about 1.5% slower rather than 7% slower. Using a lazy filter as `count(where:_)` isn't avaliable < Swift 6.0. One forward loop and using the UTF8 view makes this faster than the original code pre-#883. --- .../PrettyPrint/PrettyPrintBuffer.swift | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift b/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift index 4b02d372e..0752cd916 100644 --- a/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift +++ b/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift @@ -122,16 +122,17 @@ struct PrettyPrintBuffer { // 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 - } 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 + var lastLength = 0 + // We are only interested in "\n" we can use the UTF8 view and skip the grapheme clustering. + for element in text.utf8 { + if element == 10 { + lineNumber += 1 + lastLength = 0 + } else { + lastLength += 1 + } } + column += lastLength } /// Request that the given number of spaces be printed out before the next text token. From bb5b4c9b5aeaee2effc728aa496cfa2bc4da54ab Mon Sep 17 00:00:00 2001 From: Josh Wisenbaker Date: Thu, 19 Dec 2024 09:00:45 -0500 Subject: [PATCH 2/4] Update Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift Co-authored-by: Alex Hoppen --- Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift b/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift index 0752cd916..724f346b5 100644 --- a/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift +++ b/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift @@ -125,7 +125,7 @@ struct PrettyPrintBuffer { var lastLength = 0 // We are only interested in "\n" we can use the UTF8 view and skip the grapheme clustering. for element in text.utf8 { - if element == 10 { + if element == UInt8(ascii: "\n") { lineNumber += 1 lastLength = 0 } else { From 8073ddcb6de0b4397f094f10d7113eeb831a4732 Mon Sep 17 00:00:00 2001 From: Ben Barham Date: Fri, 20 Dec 2024 12:30:55 -0800 Subject: [PATCH 3/4] Revert "PrettyPrinterPerformance Optimized the PrettyPrinter for #894" --- .../PrettyPrint/PrettyPrintBuffer.swift | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift b/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift index 724f346b5..4b02d372e 100644 --- a/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift +++ b/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift @@ -122,17 +122,16 @@ struct PrettyPrintBuffer { // 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. - var lastLength = 0 - // We are only interested in "\n" we can use the UTF8 view and skip the grapheme clustering. - for element in text.utf8 { - if element == UInt8(ascii: "\n") { - lineNumber += 1 - lastLength = 0 - } else { - lastLength += 1 - } + 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 + } 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 += lastLength } /// Request that the given number of spaces be printed out before the next text token. From 3d1e03b9d48426f5aa50f442e84a580a34c11716 Mon Sep 17 00:00:00 2001 From: TTOzzi Date: Sat, 4 Jan 2025 22:34:44 +0900 Subject: [PATCH 4/4] Fix NoEmptyLinesOpeningClosingBraces to respect consecutive newlines when a function starts or ends with a comment --- .../NoEmptyLineOpeningClosingBraces.swift | 43 +++++++++++--- ...oEmptyLinesOpeningClosingBracesTests.swift | 56 +++++++++++++++++++ 2 files changed, 91 insertions(+), 8 deletions(-) diff --git a/Sources/SwiftFormat/Rules/NoEmptyLineOpeningClosingBraces.swift b/Sources/SwiftFormat/Rules/NoEmptyLineOpeningClosingBraces.swift index b01d173bf..5158dbb95 100644 --- a/Sources/SwiftFormat/Rules/NoEmptyLineOpeningClosingBraces.swift +++ b/Sources/SwiftFormat/Rules/NoEmptyLineOpeningClosingBraces.swift @@ -69,7 +69,9 @@ public final class NoEmptyLinesOpeningClosingBraces: SyntaxFormatRule { } func rewritten(_ token: TokenSyntax) -> TokenSyntax { - let (trimmedLeadingTrivia, count) = token.leadingTrivia.trimmingSuperfluousNewlines() + let (trimmedLeadingTrivia, count) = token.leadingTrivia.trimmingSuperfluousNewlines( + fromClosingBrace: token.tokenKind == .rightBrace + ) if trimmedLeadingTrivia.sourceLength != token.leadingTriviaLength { diagnose(.removeEmptyLinesBefore(count), on: token, anchor: .start) return token.with(\.leadingTrivia, trimmedLeadingTrivia) @@ -83,7 +85,7 @@ public final class NoEmptyLinesOpeningClosingBraces: SyntaxFormatRule { if let first = collection.first, first.leadingTrivia.containsNewlines, let index = collection.index(of: first) { - let (trimmedLeadingTrivia, count) = first.leadingTrivia.trimmingSuperfluousNewlines() + let (trimmedLeadingTrivia, count) = first.leadingTrivia.trimmingSuperfluousNewlines(fromClosingBrace: false) if trimmedLeadingTrivia.sourceLength != first.leadingTriviaLength { diagnose(.removeEmptyLinesAfter(count), on: first, anchor: .leadingTrivia(0)) var first = first @@ -96,24 +98,49 @@ public final class NoEmptyLinesOpeningClosingBraces: SyntaxFormatRule { } extension Trivia { - func trimmingSuperfluousNewlines() -> (Trivia, Int) { + func trimmingSuperfluousNewlines(fromClosingBrace: Bool) -> (Trivia, Int) { var trimmmed = 0 + var pendingNewlineCount = 0 let pieces = self.indices.reduce([TriviaPiece]()) { (partialResult, index) in let piece = self[index] // Collapse consecutive newlines into a single one if case .newlines(let count) = piece { - if let last = partialResult.last, last.isNewline { - trimmmed += count - return partialResult + if fromClosingBrace { + if index == self.count - 1 { + // For the last index(newline right before the closing brace), collapse into a single newline + trimmmed += count - 1 + return partialResult + [.newlines(1)] + } else { + pendingNewlineCount += count + return partialResult + } } else { - trimmmed += count - 1 - return partialResult + [.newlines(1)] + if let last = partialResult.last, last.isNewline { + trimmmed += count + return partialResult + } else if index == 0 { + // For leading trivia not associated with a closing brace, collapse the first newline into a single one + trimmmed += count - 1 + return partialResult + [.newlines(1)] + } else { + return partialResult + [piece] + } } } // Remove spaces/tabs surrounded by newlines if piece.isSpaceOrTab, index > 0, index < self.count - 1, self[index - 1].isNewline, self[index + 1].isNewline { return partialResult } + // Handle pending newlines if there are any + if pendingNewlineCount > 0 { + if index < self.count - 1 { + let newlines = TriviaPiece.newlines(pendingNewlineCount) + pendingNewlineCount = 0 + return partialResult + [newlines] + [piece] + } else { + return partialResult + [.newlines(1)] + [piece] + } + } // Retain other trivia pieces return partialResult + [piece] } diff --git a/Tests/SwiftFormatTests/Rules/NoEmptyLinesOpeningClosingBracesTests.swift b/Tests/SwiftFormatTests/Rules/NoEmptyLinesOpeningClosingBracesTests.swift index 30f9471fc..bb555739a 100644 --- a/Tests/SwiftFormatTests/Rules/NoEmptyLinesOpeningClosingBracesTests.swift +++ b/Tests/SwiftFormatTests/Rules/NoEmptyLinesOpeningClosingBracesTests.swift @@ -136,4 +136,60 @@ final class NoEmptyLinesOpeningClosingBracesTests: LintOrFormatRuleTestCase { ] ) } + + func testNoEmptyLinesOpeningClosingBracesInFunctionBeginningAndEndingWithComment() { + assertFormatting( + NoEmptyLinesOpeningClosingBraces.self, + input: """ + func myFunc() { + // Some comment here + + // Do a thing + var x = doAThing() + + // Do a thing + + var y = doAThing() + + // Some other comment here + } + """, + expected: """ + func myFunc() { + // Some comment here + + // Do a thing + var x = doAThing() + + // Do a thing + + var y = doAThing() + + // Some other comment here + } + """ + ) + } + + func testNoEmptyLinesOpeningClosingBracesInFunctionWithEmptyLinesOnly() { + assertFormatting( + NoEmptyLinesOpeningClosingBraces.self, + input: """ + func myFunc() { + + + + + + 1️⃣} + """, + expected: """ + func myFunc() { + } + """, + findings: [ + FindingSpec("1️⃣", message: "remove empty lines before '}'") + ] + ) + } }