Fix wrong formatting of multiline string literal

Sources/SwiftBasicFormat/BasicFormat.swift

@@ -143,35 +143,50 @@ open class BasicFormat: SyntaxRewriter {
     return node.requiresIndent
   }
 
-  /// Whether a leading newline on `token` should be added.
-  open func requiresLeadingNewline(_ token: TokenSyntax) -> Bool {
-    // We don't want to add newlines inside string interpolation
-    if isInsideStringInterpolation(token) {
+  open func requiresNewline(between first: TokenSyntax?, and second: TokenSyntax?) -> Bool {
+    // We don't want to add newlines inside string interpolation.
+    // When first or second `TokenSyntax` is a multiline quote we want special handling
+    // even if it's inside a string interpolation, because it still requires newline
+    // after open quote and before close quote.
+    if let first,
+      isInsideStringInterpolation(first),
+      first.tokenKind != .multilineStringQuote,
+      second?.tokenKind != .multilineStringQuote
+    {
       return false
-    }
-
-    if token.requiresLeadingNewline {
-      return true
-    }
-
-    var ancestor: Syntax = Syntax(token)
-    while let parent = ancestor.parent {
-      ancestor = parent
-      if ancestor.position != token.position || ancestor.firstToken(viewMode: viewMode) != token {
-        break
-      }
-      if let ancestorsParent = ancestor.parent, childrenSeparatedByNewline(ancestorsParent) {
+    } else if let second {
+      if second.requiresLeadingNewline {
         return true
       }
-      switch ancestor.keyPathInParent {
-      case \IfConfigClauseSyntax.elements:
-        return true
-      default:
-        break
+
+      var ancestor: Syntax = Syntax(second)
+      while let parent = ancestor.parent {
+        ancestor = parent
+        if ancestor.position != second.position || ancestor.firstToken(viewMode: viewMode) != second {
+          break
+        }
+        if let ancestorsParent = ancestor.parent, childrenSeparatedByNewline(ancestorsParent) {
+          return true
+        }
+        switch ancestor.keyPathInParent {
+        case \IfConfigClauseSyntax.elements:
+          return true
+        default:
+          break
+        }
       }
     }
 
-    return false
+    switch (first?.tokenKind, second?.tokenKind) {
+    case (.multilineStringQuote, .backslash),  // string interpolation segment inside a multi-line string literal
+      (.multilineStringQuote, .multilineStringQuote),  // empty multi-line string literal
+      (.multilineStringQuote, .stringSegment),  // segment starting a multi-line string literal
+      (.stringSegment, .multilineStringQuote),  // ending a multi-line string literal that has a string interpolation segment at its end
+      (.rightParen, .multilineStringQuote):  // ending a multi-line string literal that has a string interpolation segment at its end
+      return true
+    default:
+      return false
+    }
   }
 
   open func requiresWhitespace(between first: TokenSyntax?, and second: TokenSyntax?) -> Bool {
@@ -281,6 +296,12 @@ open class BasicFormat: SyntaxRewriter {
     let previousToken = self.previousToken ?? token.previousToken(viewMode: viewMode)
     let nextToken = token.nextToken(viewMode: viewMode)
 
+    /// In addition to existing trivia of `previousToken`, also considers
+    /// `previousToken` as ending with whitespace if it and `token` should be
+    /// separated by whitespace.
+    /// It does not consider whether a newline should be added between
+    /// `previousToken` and the `token` because that newline should be added to
+    /// the next token's trailing trivia.
     lazy var previousTokenWillEndWithWhitespace: Bool = {
       guard let previousToken = previousToken else {
         return false
@@ -289,6 +310,8 @@ open class BasicFormat: SyntaxRewriter {
         || (requiresWhitespace(between: previousToken, and: token) && isMutable(previousToken))
     }()
 
+    /// This method does not consider any posssible mutations to `previousToken`
+    /// because newlines should be added to the next token's leading trivia.
     lazy var previousTokenWillEndWithNewline: Bool = {
       guard let previousToken = previousToken else {
         // Assume that the start of the tree is equivalent to a newline so we
@@ -298,10 +321,7 @@ open class BasicFormat: SyntaxRewriter {
       if previousToken.trailingTrivia.endsWithNewline {
         return true
       }
-      if case .stringSegment(let segment) = previousToken.tokenKind, segment.last?.isNewline ?? false {
-        return true
-      }
-      return false
+      return previousToken.isStringSegmentWithLastCharacterBeingNewline
     }()
 
     lazy var previousTokenIsStringLiteralEndingInNewline: Bool = {
@@ -310,26 +330,29 @@ open class BasicFormat: SyntaxRewriter {
         // don't add a leading newline to the file.
         return true
       }
-      if case .stringSegment(let segment) = previousToken.tokenKind, segment.last?.isNewline ?? false {
-        return true
-      }
-      return false
+      return previousToken.isStringSegmentWithLastCharacterBeingNewline
     }()
 
+    /// Also considers `nextToken` as starting with a whitespace if a newline
+    /// should be added to it. It does not check whether `token` and `nextToken`
+    /// should be separated by whitespace because the whitespace should be added
+    /// to the `token`’s leading trivia.
     lazy var nextTokenWillStartWithWhitespace: Bool = {
       guard let nextToken = nextToken else {
         return false
       }
       return nextToken.leadingTrivia.startsWithWhitespace
-        || (requiresLeadingNewline(nextToken) && isMutable(nextToken))
+        || (requiresNewline(between: token, and: nextToken) && isMutable(nextToken))
     }()
 
+    /// Also considers `nextToken` as starting with a leading newline if `token`
+    /// and `nextToken` should be separated by a newline.
     lazy var nextTokenWillStartWithNewline: Bool = {
       guard let nextToken = nextToken else {
         return false
       }
       return nextToken.leadingTrivia.startsWithNewline
-        || (requiresLeadingNewline(nextToken) && isMutable(nextToken))
+        || (requiresNewline(between: token, and: nextToken) && isMutable(nextToken) && !token.trailingTrivia.endsWithNewline && !token.isStringSegmentWithLastCharacterBeingNewline)
     }()
 
     /// This token's trailing trivia + any spaces or tabs at the start of the
@@ -342,7 +365,7 @@ open class BasicFormat: SyntaxRewriter {
     var leadingTrivia = token.leadingTrivia
     var trailingTrivia = token.trailingTrivia
 
-    if requiresLeadingNewline(token) {
+    if requiresNewline(between: previousToken, and: token) {
       // Add a leading newline if the token requires it unless
       //  - it already starts with a newline or
       //  - the previous token ends with a newline
@@ -407,3 +430,14 @@ open class BasicFormat: SyntaxRewriter {
     return token.detach().with(\.leadingTrivia, leadingTrivia).with(\.trailingTrivia, trailingTrivia)
   }
 }
+
+fileprivate extension TokenSyntax {
+  var isStringSegmentWithLastCharacterBeingNewline: Bool {
+    switch self.tokenKind {
+    case .stringSegment(let segment):
+      return segment.last?.isNewline ?? false
+    default:
+      return false
+    }
+  }
+}

Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift

@@ -152,11 +152,12 @@ extension FixIt.MultiNodeChange {
       }
     }
 
-    if let previousToken = node.previousToken(viewMode: .all),
+    if let previousToken = node.previousToken(viewMode: .fixedUp),
       previousToken.presence == .present,
       let firstToken = node.firstToken(viewMode: .all),
       previousToken.trailingTrivia.allSatisfy({ $0.isWhitespace }),
-      !BasicFormat().requiresWhitespace(between: previousToken, and: firstToken)
+      !BasicFormat().requiresWhitespace(between: previousToken, and: firstToken),
+      !BasicFormat().requiresNewline(between: previousToken, and: firstToken)
     {
       // If the previous token and this node don't need to be separated, remove
       // the separation.

Sources/SwiftParserDiagnostics/MissingNodesError.swift

@@ -22,7 +22,7 @@ fileprivate func findCommonAncestor(_ nodes: [Syntax]) -> Syntax? {
 }
 
 class NoNewlinesFormat: BasicFormat {
-  override func requiresLeadingNewline(_ token: TokenSyntax) -> Bool {
+  override func requiresNewline(between first: TokenSyntax?, and second: TokenSyntax?) -> Bool {
     return false
   }
 }

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

@@ -1252,12 +1252,10 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     }
     if case .stringSegment(let segment) = node.segments.last {
       if let invalidContent = segment.unexpectedBeforeContent?.onlyToken(where: { $0.trailingTrivia.contains(where: { $0.isBackslash }) }) {
-        let leadingTrivia = segment.content.leadingTrivia
-        let trailingTrivia = segment.content.trailingTrivia
         let fixIt = FixIt(
           message: .removeBackslash,
           changes: [
-            .makePresent(segment.content, leadingTrivia: leadingTrivia, trailingTrivia: trailingTrivia),
+            .makePresent(segment.content),
             .makeMissing(invalidContent, transferTrivia: false),
           ]
         )

Tests/SwiftParserTest/ExpressionTests.swift

@@ -603,7 +603,8 @@ final class ExpressionTests: XCTestCase {
         )
       ],
       fixedSource: ##"""
-        """""""
+        """"
+        """
         """##
     )
 
@@ -619,7 +620,8 @@ final class ExpressionTests: XCTestCase {
         )
       ],
       fixedSource: ##"""
-        """"""""
+        """""
+        """
         """##
     )
 
@@ -656,7 +658,8 @@ final class ExpressionTests: XCTestCase {
         DiagnosticSpec(message: ##"expected '"""#' to end string literal"##, fixIts: [##"insert '"""#'"##])
       ],
       fixedSource: ##"""
-        #""""""#
+        #"""
+        """#
         """##
     )
 
@@ -668,7 +671,8 @@ final class ExpressionTests: XCTestCase {
         DiagnosticSpec(message: ##"expected '"""#' to end string literal"##, fixIts: [##"insert '"""#'"##])
       ],
       fixedSource: ##"""
-        #"""a"""#
+        #"""a
+        """#
         """##
     )
 

Tests/SwiftParserTest/translated/AvailabilityQueryTests.swift

@@ -100,7 +100,7 @@ final class AvailabilityQueryTests: XCTestCase {
         DiagnosticSpec(message: "expected ',' joining parts of a multi-clause condition", fixIts: ["replace '&&' with ','"])
       ],
       fixedSource: """
-        if #available(OSX 10.51, *) , #available(OSX 10.52, *) {
+        if #available(OSX 10.51, *), #available(OSX 10.52, *) {
         }
         """
     )
@@ -440,7 +440,7 @@ final class AvailabilityQueryTests: XCTestCase {
         DiagnosticSpec(message: "expected ',' joining platforms in availability condition", fixIts: ["replace '||' with ','"])
       ],
       fixedSource: """
-        if #available(OSX 10.51 , iOS 8.0) {
+        if #available(OSX 10.51, iOS 8.0) {
         }
         """
     )

Tests/SwiftParserTest/translated/AvailabilityQueryUnavailabilityTests.swift

@@ -103,7 +103,7 @@ final class AvailabilityQueryUnavailabilityTests: XCTestCase {
         )
       ],
       fixedSource: """
-        if #unavailable(OSX 10.51) , #unavailable(OSX 10.52) {
+        if #unavailable(OSX 10.51), #unavailable(OSX 10.52) {
         }
         """
     )
@@ -447,7 +447,7 @@ final class AvailabilityQueryUnavailabilityTests: XCTestCase {
         )
       ],
       fixedSource: """
-        if #unavailable(OSX 10.51 , iOS 8.0) {
+        if #unavailable(OSX 10.51, iOS 8.0) {
         }
         """
     )

Tests/SwiftParserTest/translated/MultilineErrorsTests.swift

@@ -705,7 +705,8 @@ final class MultilineErrorsTests: XCTestCase {
       ],
       fixedSource: ##"""
         _ = """
-          foo\"""
+          foo\
+        """
         """##
     )
   }
@@ -741,6 +742,7 @@ final class MultilineErrorsTests: XCTestCase {
       fixedSource: #"""
         _ = """
          \#u{20}
+          
           """
         """#
     )

Tests/SwiftParserTest/translated/MultilinePoundDiagnosticArgRdar41154797Tests.swift

@@ -35,7 +35,8 @@ final class MultilinePoundDiagnosticArgRdar41154797Tests: XCTestCase {
         ),
       ],
       fixedSource: ##"""
-        #error("""""")
+        #error("""
+        """)
         """##
     )
   }

Tests/SwiftParserTest/translated/MultilineStringTests.swift

@@ -411,6 +411,82 @@ final class MultilineStringTests: XCTestCase {
     )
   }
 
+  func testMultilineString47() {
+    assertParse(
+      #"""
+      _ = """1️⃣"""
+      """#,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "multi-line string literal closing delimiter must begin on a new line",
+          fixIts: ["insert newline"]
+        )
+      ],
+      fixedSource: #"""
+        _ = """
+        """
+        """#
+    )
+  }
+
+  func testMultilineString48() {
+    assertParse(
+      #"""
+      _ = """A1️⃣"""
+      """#,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "multi-line string literal closing delimiter must begin on a new line",
+          fixIts: ["insert newline"]
+        )
+      ],
+      fixedSource: #"""
+        _ = """A
+        """
+        """#
+    )
+  }
+
+  func testMultilineString49() {
+    assertParse(
+      #"""
+      _ = ℹ️"""1️⃣
+      """#,
+      diagnostics: [
+        DiagnosticSpec(
+          message: #"expected '"""' to end string literal"#,
+          notes: [NoteSpec(message: #"to match this opening '"""'"#)],
+          fixIts: [#"insert '"""'"#]
+        )
+      ],
+      fixedSource: #"""
+        _ = """
+        """
+        """#
+    )
+  }
+
+  func testMultilineString50() {
+    assertParse(
+      #"""
+      _ = ℹ️"""
+      A1️⃣
+      """#,
+      diagnostics: [
+        DiagnosticSpec(
+          message: #"expected '"""' to end string literal"#,
+          notes: [NoteSpec(message: #"to match this opening '"""'"#)],
+          fixIts: [#"insert '"""'"#]
+        )
+      ],
+      fixedSource: #"""
+        _ = """
+        A
+        """
+        """#
+    )
+  }
+
   func testEscapeNewlineInRawString() {
     assertParse(
       ##"""