From 1ca68450175a1106a0bd0d8f827b9bd2dd7e4367 Mon Sep 17 00:00:00 2001 From: stackotter Date: Tue, 4 Apr 2023 09:22:51 +1000 Subject: [PATCH] Fix formatting of import statements with attributes, and ensure that they **never** wrap (fixes #445) --- .../SwiftFormatPrettyPrint/PrettyPrint.swift | 21 ++++++++++++++----- Sources/SwiftFormatPrettyPrint/Token.swift | 6 ++++-- .../TokenStreamCreator.swift | 9 ++++++-- .../ImportTests.swift | 8 +++++++ 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/Sources/SwiftFormatPrettyPrint/PrettyPrint.swift b/Sources/SwiftFormatPrettyPrint/PrettyPrint.swift index a85173487..f35acc61b 100644 --- a/Sources/SwiftFormatPrettyPrint/PrettyPrint.swift +++ b/Sources/SwiftFormatPrettyPrint/PrettyPrint.swift @@ -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] { @@ -469,7 +474,7 @@ 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. @@ -477,7 +482,7 @@ public class PrettyPrinter { mustBreak = true } - let suppressBreaking = isBreakingSupressed && !overrideBreakingSuppressed + let suppressBreaking = isBreakingSuppressed && !overrideBreakingSuppressed if !suppressBreaking && ((!isAtStartOfLine && length > spaceRemaining) || mustBreak) { currentLineIsContinuation = isContinuationIfBreakFires writeNewlines(newline) @@ -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 } diff --git a/Sources/SwiftFormatPrettyPrint/Token.swift b/Sources/SwiftFormatPrettyPrint/Token.swift index 431edb288..fdfcb297f 100644 --- a/Sources/SwiftFormatPrettyPrint/Token.swift +++ b/Sources/SwiftFormatPrettyPrint/Token.swift @@ -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. diff --git a/Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift b/Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift index 5fc50c76f..10085ba9d 100644 --- a/Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift +++ b/Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift @@ -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)) @@ -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 } diff --git a/Tests/SwiftFormatPrettyPrintTests/ImportTests.swift b/Tests/SwiftFormatPrettyPrintTests/ImportTests.swift index befa073b5..326f36718 100644 --- a/Tests/SwiftFormatPrettyPrintTests/ImportTests.swift +++ b/Tests/SwiftFormatPrettyPrintTests/ImportTests.swift @@ -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 = @@ -17,6 +23,8 @@ final class ImportTests: PrettyPrintTestCase { import struct MyModule.MyStruct @testable import testModule + @_spi(STP) @testable import testModule + """ // Imports should not wrap