Skip to content

Commit f01e044

Browse files
committed
Ignore too long end of line comments when they're wrapped in printControl tokens that disable breaking.
The rationale here is that if the comment exists in a section of code where breaking is purposefully disabled, then there's probably a reason that the comment cannot be moved to another line. The `printControl` tokens that disable breaking are the signal to the pretty print algorithm for which sections are those special "no breaks allowed here", and it can just respect those when diagnosing long lines.
1 parent 01966bd commit f01e044

File tree

3 files changed

+57
-26
lines changed

3 files changed

+57
-26
lines changed

Sources/SwiftFormat/PrettyPrint/PrettyPrint.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,7 @@ public class PrettyPrinter {
515515

516516
write(comment.print(indent: currentIndentation))
517517
if wasEndOfLine {
518-
if comment.length > spaceRemaining {
518+
if comment.length > spaceRemaining && !isBreakingSuppressed {
519519
diagnose(.moveEndOfLineComment, category: .endOfLineComment)
520520
}
521521
} else {

Sources/SwiftFormat/PrettyPrint/TokenStreamCreator.swift

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2742,9 +2742,10 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
27422742
/// will stay inside the group.
27432743
///
27442744
/// * If the trailing comment is a line comment, we first append any enqueued after-tokens
2745-
/// that are *not* breaks or newlines, then we append the comment, and then the remaining
2746-
/// after-tokens. Due to visitation ordering, this ensures that a trailing line comment is
2747-
/// not incorrectly inserted into the token stream *after* a break or newline.
2745+
/// that are *not* related to breaks or newlines (e.g. includes print control tokens), then
2746+
/// we append the comment, and then the remaining after-tokens. Due to visitation ordering,
2747+
/// this ensures that a trailing line comment is not incorrectly inserted into the token stream
2748+
/// *after* a break or newline.
27482749
private func appendAfterTokensAndTrailingComments(_ token: TokenSyntax) {
27492750
let (wasLineComment, trailingCommentTokens) = afterTokensForTrailingComment(token)
27502751
let afterGroups = afterMap.removeValue(forKey: token) ?? []
@@ -2759,7 +2760,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
27592760
var shouldExtractTrailingComment = false
27602761
if wasLineComment && !hasAppendedTrailingComment {
27612762
switch afterToken {
2762-
case .break: shouldExtractTrailingComment = true
2763+
case .break, .printerControl: shouldExtractTrailingComment = true
27632764
default: break
27642765
}
27652766
}

Tests/SwiftFormatTests/PrettyPrint/CommentTests.swift

Lines changed: 51 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import _SwiftFormatTestSupport
2+
13
final class CommentTests: PrettyPrintTestCase {
24
func testDocumentationComments() {
35
let input =
@@ -408,7 +410,7 @@ final class CommentTests: PrettyPrintTestCase {
408410
case quux
409411
}
410412
"""
411-
413+
412414
let expected =
413415
"""
414416
struct Foo {
@@ -424,7 +426,7 @@ final class CommentTests: PrettyPrintTestCase {
424426
}
425427
426428
"""
427-
429+
428430
assertPrettyPrintEqual(input: input, expected: expected, linelength: 100)
429431
}
430432

@@ -594,25 +596,25 @@ final class CommentTests: PrettyPrintTestCase {
594596

595597
func testCommentsInIfStatements() {
596598
let input =
597-
"""
598-
if foo.bar && false && // comment about foo.bar
599-
baz && // comment about baz
600-
// comment about next
601-
next
602-
&& // other is important
603-
// second line about other
604-
other &&
605-
// comment about final on a new line
606-
final
607-
{
608-
}
609-
if foo.bar && foo.baz
610-
&& // comment about the next line
611-
// another comment line
612-
next.line
613-
{
614-
}
615-
"""
599+
"""
600+
if foo.bar && false && // comment about foo.bar
601+
baz && // comment about baz
602+
// comment about next
603+
next
604+
&& // other is important
605+
// second line about other
606+
other &&
607+
// comment about final on a new line
608+
final
609+
{
610+
}
611+
if foo.bar && foo.baz
612+
&& // comment about the next line
613+
// another comment line
614+
next.line
615+
{
616+
}
617+
"""
616618

617619
let expected =
618620
"""
@@ -682,4 +684,32 @@ final class CommentTests: PrettyPrintTestCase {
682684

683685
assertPrettyPrintEqual(input: input, expected: expected, linelength: 60)
684686
}
687+
688+
func testDiagnoseMoveEndOfLineComment() {
689+
assertPrettyPrintEqual(
690+
input: """
691+
import veryveryverylongmodulenameherebecauseitistypical // special sentinel comment
692+
693+
func fooBarBazRunningOutOfIdeas() { 1️⃣// comment that needs to move
694+
if foo { // comment is fine
695+
}
696+
}
697+
698+
""",
699+
expected: """
700+
import veryveryverylongmodulenameherebecauseitistypical // special sentinel comment
701+
702+
func fooBarBazRunningOutOfIdeas() { // comment that needs to move
703+
if foo { // comment is fine
704+
}
705+
}
706+
707+
""",
708+
linelength: 45,
709+
whitespaceOnly: true,
710+
findings: [
711+
FindingSpec("1️⃣", message: "move end-of-line comment that exceeds the line length"),
712+
]
713+
)
714+
}
685715
}

0 commit comments

Comments
 (0)