Skip to content

Fix line breaking for attributes in inheritance clauses by grouping them with their type #1039

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 26, 2025
Merged

Fix line breaking for attributes in inheritance clauses by grouping them with their type #1039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Sources/SwiftFormat/PrettyPrint/TokenStreamCreator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2385,13 +2385,15 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
}

override func visit(_ node: AttributedTypeSyntax) -> SyntaxVisitorContinueKind {
before(node.firstToken(viewMode: .sourceAccurate), tokens: .open)
arrangeAttributeList(node.attributes)
for specifier in node.specifiers {
after(
specifier.firstToken(viewMode: .sourceAccurate),
tokens: .break(.continue, newlines: .elective(ignoresDiscretionary: true))
)
}
after(node.lastToken(viewMode: .sourceAccurate), tokens: .close)
return .visitChildren
}

Expand Down
21 changes: 21 additions & 0 deletions Tests/SwiftFormatTests/PrettyPrint/AttributeTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,4 +623,25 @@ final class AttributeTests: PrettyPrintTestCase {

assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
}

func testAttributeLineBreakInInheritanceClause() {
let input =
"""
public class MyClass: Foo, @unchecked Sendable, Bar {
// …
}

"""
let expected =
"""
public class MyClass: Foo,
@unchecked Sendable, Bar
{
// …
}

"""

assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
}
}
4 changes: 2 additions & 2 deletions Tests/SwiftFormatTests/PrettyPrint/FunctionDeclTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1129,8 +1129,8 @@ final class FunctionDeclTests: PrettyPrintTestCase {
.InnerMember,
) {}
func foo(
cmp: @escaping (R) ->
()
cmp:
@escaping (R) -> ()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I was hoping we'd find some others with that change.

Copy link
Contributor

@bnbarham bnbarham Jul 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this one is causing a bunch of differences in eg. sourcekit-lsp:

-    requestHandler: @Sendable @escaping (
-      RequestType, Workspace, LanguageService
-    ) async throws ->
+    requestHandler:
+      @Sendable @escaping (
+        RequestType, Workspace, LanguageService
+      ) async throws ->

(and others like it)

I wouldn't have expected any change there though 🤔? The @Sendable @escaping ( still fits on the same line as requestHandler:. EDIT: Although maybe it does make sense - the type doesn't fit on the same line, it's now fully grouped, and thus put on a new line + indented.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the group around the whole type now forces the line break since the whole type doesn't fit. I personally prefer that behavior (citing The Rectangle Rule), because I think a case like the one in the test diff above is one where the new version is clearly better, even though the one you posted from SourceKit doesn't seem so bad.

The difference between those two might be that in the test, we took something that didn't fit on a single line and made it fit on a single line, so that's (unambiguously, IMO) an improvement. In the SourceKit example, that's not the case, so it's less of a big win.

A future version of the formatter could try to make decisions like that, but it would require backtracking, so I've tried to resist that complexity.

) {}
func foo(
cmp:
Expand Down
Loading