Skip to content

Fix formatting of import with multiple attributes (fixes #445) and ensure that imports never wrap #501

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.

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
Apr 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 16 additions & 5 deletions Sources/SwiftFormatPrettyPrint/PrettyPrint.swift
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,16 @@ public class PrettyPrinter {
private var activeBreakSuppressionCount = 0

/// Whether breaks are supressed from firing. When true, no breaks should fire and the only way to
/// move to a new line is an explicit new line token.
private var isBreakingSupressed: Bool {
/// move to a new line is an explicit new line token. Discretionary breaks aren't suppressed
/// if ``allowSuppressedDiscretionaryBreaks`` is true.
private var isBreakingSuppressed: Bool {
return activeBreakSuppressionCount > 0
}

/// Indicates whether discretionary breaks should still be included even if break suppression is
/// enabled (see ``isBreakingSuppressed``).
private var allowSuppressedDiscretionaryBreaks = false

/// The computed indentation level, as a number of spaces, based on the state of any unclosed
/// delimiters and whether or not the current line is a continuation line.
private var currentIndentation: [Indent] {
Expand Down Expand Up @@ -469,15 +474,15 @@ public class PrettyPrinter {
case .soft(_, let discretionary):
// A discretionary newline (i.e. from the source) should create a line break even if the
// rules for breaking are disabled.
overrideBreakingSuppressed = discretionary
overrideBreakingSuppressed = discretionary && allowSuppressedDiscretionaryBreaks
mustBreak = true
case .hard:
// A hard newline must always create a line break, regardless of the context.
overrideBreakingSuppressed = true
mustBreak = true
}

let suppressBreaking = isBreakingSupressed && !overrideBreakingSuppressed
let suppressBreaking = isBreakingSuppressed && !overrideBreakingSuppressed
if !suppressBreaking && ((!isAtStartOfLine && length > spaceRemaining) || mustBreak) {
currentLineIsContinuation = isContinuationIfBreakFires
writeNewlines(newline)
Expand Down Expand Up @@ -527,8 +532,14 @@ public class PrettyPrinter {

case .printerControl(let kind):
switch kind {
case .disableBreaking:
case .disableBreaking(let allowDiscretionary):
activeBreakSuppressionCount += 1
// Override the supression of discretionary breaks if we're at the top level or
// discretionary breaks are currently allowed (false should override true, but not the other
// way around).
if activeBreakSuppressionCount == 1 || allowSuppressedDiscretionaryBreaks {
allowSuppressedDiscretionaryBreaks = allowDiscretionary
}
case .enableBreaking:
activeBreakSuppressionCount -= 1
}
Expand Down
6 changes: 4 additions & 2 deletions Sources/SwiftFormatPrettyPrint/Token.swift
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,10 @@ enum PrinterControlKind {
/// control token is encountered.
///
/// It's valid to nest `disableBreaking` and `enableBreaking` tokens. Breaks will be suppressed
/// long as there is at least 1 unmatched disable token.
case disableBreaking
/// long as there is at least 1 unmatched disable token. If `allowDiscretionary` is `true`, then
/// discretionary breaks aren't effected. An `allowDiscretionary` value of true never overrides a
/// value of false. Hard breaks are always inserted no matter what.
case disableBreaking(allowDiscretionary: Bool)

/// A signal that break tokens should be allowed to fire following this token, as long as there
/// are no other unmatched disable tokens.
Expand Down
9 changes: 7 additions & 2 deletions Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1339,7 +1339,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
]
)
} else if let condition = node.condition {
before(condition.firstToken, tokens: .printerControl(kind: .disableBreaking))
before(condition.firstToken, tokens: .printerControl(kind: .disableBreaking(allowDiscretionary: true)))
after(
condition.lastToken,
tokens: .printerControl(kind: .enableBreaking), .break(.reset, size: 0))
Expand Down Expand Up @@ -1668,9 +1668,14 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
}

override func visit(_ node: ImportDeclSyntax) -> SyntaxVisitorContinueKind {
after(node.attributes?.lastToken, tokens: .space)
// Import declarations should never be wrapped.
before(node.firstToken, tokens: .printerControl(kind: .disableBreaking(allowDiscretionary: false)))

arrangeAttributeList(node.attributes)
after(node.importTok, tokens: .space)
after(node.importKind, tokens: .space)

after(node.lastToken, tokens: .printerControl(kind: .enableBreaking))
return .visitChildren
}

Expand Down
8 changes: 8 additions & 0 deletions Tests/SwiftFormatPrettyPrintTests/ImportTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ final class ImportTests: PrettyPrintTestCase {
import class MyModule.MyClass
import struct MyModule.MyStruct
@testable import testModule

@_spi(
STP
)
@testable
import testModule
"""

let expected =
Expand All @@ -17,6 +23,8 @@ final class ImportTests: PrettyPrintTestCase {
import struct MyModule.MyStruct
@testable import testModule

@_spi(STP) @testable import testModule

"""

// Imports should not wrap
Expand Down