Merge pull request swiftlang#501 from stackotter/fix-import-formatting

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

Author: allevato

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,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)
@@ -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
       }

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.

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

@@ -1394,7 +1394,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))
@@ -1723,9 +1723,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
   }
 

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