Repository: apple/swift-syntax (swift)

Issue:
BasicFormat outputs incomplete indentation when formatting multiline string literals containing blank lines
### Description

The following code is formatted using BasicFormat (spaces are replaced with `␣` for clarity),

```swift
assertionFailure("""
␣␣First␣line

␣␣Second␣line

␣␣""")
```

It produces the following code

```swift
assertionFailure("""
␣␣First␣line
␣␣␣␣
␣␣Second␣line
␣␣␣␣
␣␣␣␣␣␣""")
```

This is an incorrectly indented literal closing quote.
This causes a compilation error `Insufficient indentation of line in multi-line string literal`.

This problem can easily occur if both the "Automatically trim trailing whitespace" and "Including whitespace-only lines" settings in Xcode are enabled.

Also, macros now automatically format the code they generate by default, which can easily cause problems if the above settings are enabled.

For example, passing the following string to the `#stringify` macro created by the Swift Macro Project template will result in a compilation error due to the same issue.

```swift
#stringify(
␣␣"""
␣␣First line

␣␣Second line

␣␣"""
)
```

^ The above macro will generate the following code, which will result in a compiation error

```swift
(
␣␣"""
␣␣First␣line
␣␣
␣␣Second␣line
␣␣
␣␣␣␣""",␣#"\#n␣␣"""\#n␣␣First␣line\#n\#n␣␣Second␣line\#n\#n␣␣""""#)
```


### Steps to Reproduce

To reproduce this problem, run the following code

```swift
let source = #"""
  assertionFailure("""
    First line

    Second line

    """)
  """#
print(Parser.parse(source: source).formatted().description)
```

The result of the execution will be as follows This code results in a compilation error "Insufficient indentation of line in multi-line string literal".

```swift
assertionFailure("""
  First line
    
  Second line
    
      """)
```

Pull request:
The underlying issues of the malformatted multi-line string literal are that

1. We are setting an `anchorPoint` for empty lines. Anchor points are meant to be starting points at which the user didn’t provide any manual indentation relative to which we should format the rest of the tree. But for empty lines it’s not like the user didn’t provide any indentation. There simply wasn’t anything to indent.
2. `leadingTrivia.trimmingTrailingWhitespaceBeforeNewline` was not considering whether the token text itself contained a newline in `isBeforeNewline`.

Now, to detect if the token is followed by a newline, we should check if its text is empty, which is the case for empty string literals. Unfortunately, this is also the case for missing identifiers, which also have an empty text but whose empty text will be replace by a placeholder by `PresentMaker`. To distinguish between the two, I merged `PresentMaker` and `MissingNodesBasicFormatter` so that the `BasicFormat` also performs the text replacement to placeholders.

Fixes #1959

Patch:
diff --git a/Sources/SwiftBasicFormat/BasicFormat.swift b/Sources/SwiftBasicFormat/BasicFormat.swift
index 3566eb84e28..796f38a25d0 100644
--- a/Sources/SwiftBasicFormat/BasicFormat.swift
+++ b/Sources/SwiftBasicFormat/BasicFormat.swift
@@ -10,7 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-import SwiftSyntax
+@_spi(RawSyntax) import SwiftSyntax
 
 /// A rewriter that performs a "basic" format of the passed tree.
 ///
@@ -48,6 +48,14 @@ open class BasicFormat: SyntaxRewriter {
   /// been visited yet.
   private var previousToken: TokenSyntax? = nil
 
+  /// The number of ancestors that are `StringLiteralExprSyntax`.
+  private var stringLiteralNestingLevel = 0
+
+  /// Whether we are currently visiting the subtree of a `StringLiteralExprSyntax`.
+  private var isInsideStringLiteral: Bool {
+    return stringLiteralNestingLevel > 0
+  }
+
   public init(
     indentationWidth: Trivia = .spaces(4),
     initialIndentation: Trivia = [],
@@ -83,6 +91,9 @@ open class BasicFormat: SyntaxRewriter {
   }
 
   open override func visitPre(_ node: Syntax) {
+    if node.is(StringLiteralExprSyntax.self) {
+      stringLiteralNestingLevel += 1
+    }
     if requiresIndent(node) {
       if let firstToken = node.firstToken(viewMode: viewMode),
         let tokenIndentation = firstToken.leadingTrivia.indentation(isOnNewline: false),
@@ -98,6 +109,9 @@ open class BasicFormat: SyntaxRewriter {
   }
 
   open override func visitPost(_ node: Syntax) {
+    if node.is(StringLiteralExprSyntax.self) {
+      stringLiteralNestingLevel -= 1
+    }
     if requiresIndent(node) {
       decreaseIndentationLevel()
     }
@@ -271,7 +285,7 @@ open class BasicFormat: SyntaxRewriter {
       (.keyword(.`init`), .leftParen),  // init()
       (.keyword(.self), .period),  // self.someProperty
       (.keyword(.Self), .period),  // self.someProperty
-      (.keyword(.set), .leftParen),  // var mYar: Int { set(value) {} }
+      (.keyword(.set), .leftParen),  // var mVar: Int { set(value) {} }
       (.keyword(.subscript), .leftParen),  // subscript(x: Int)
       (.keyword(.super), .period),  // super.someProperty
       (.leftBrace, .rightBrace),  // {}
@@ -348,6 +362,26 @@ open class BasicFormat: SyntaxRewriter {
     return true
   }
 
+  /// Change the text of a token during formatting.
+  ///
+  /// This allows formats to e.g. replace missing tokens by placeholder tokens.
+  ///
+  /// - Parameter token: The token whose text should be changed
+  /// - Returns: The new text or `nil` if the text should not be changed
+  open func transformTokenText(_ token: TokenSyntax) -> String? {
+    return nil
+  }
+
+  /// Change the presence of a token during formatting.
+  ///
+  /// This allows formats to e.g. replace missing tokens by placeholder tokens.
+  ///
+  /// - Parameter token: The token whose presence should be changed
+  /// - Returns: The new presence or `nil` if the presence should not be changed
+  open func transformTokenPresence(_ token: TokenSyntax) -> SourcePresence? {
+    return nil
+  }
+
   // MARK: - Formatting a token
 
   open override func visit(_ token: TokenSyntax) -> TokenSyntax {
@@ -357,6 +391,8 @@ open class BasicFormat: SyntaxRewriter {
     let isInitialToken = self.previousToken == nil
     let previousToken = self.previousToken ?? token.previousToken(viewMode: viewMode)
     let nextToken = token.nextToken(viewMode: viewMode)
+    let transformedTokenText = self.transformTokenText(token)
+    let transformedTokenPresence = self.transformTokenPresence(token)
 
     /// In addition to existing trivia of `previousToken`, also considers
     /// `previousToken` as ending with whitespace if it and `token` should be
@@ -372,7 +408,7 @@ open class BasicFormat: SyntaxRewriter {
         || (requiresWhitespace(between: previousToken, and: token) && isMutable(previousToken))
     }()
 
-    /// This method does not consider any posssible mutations to `previousToken`
+    /// This method does not consider any possible mutations to `previousToken`
     /// because newlines should be added to the next token's leading trivia.
     let previousTokenWillEndWithNewline: Bool = {
       guard let previousToken = previousToken else {
@@ -416,6 +452,14 @@ open class BasicFormat: SyntaxRewriter {
       if nextToken.leadingTrivia.startsWithNewline {
         return true
       }
+      if nextToken.leadingTrivia.isEmpty {
+        if nextToken.text.first?.isNewline ?? false {
+          return true
+        }
+        if nextToken.text.isEmpty && nextToken.trailingTrivia.startsWithNewline {
+          return true
+        }
+      }
       if requiresNewline(between: token, and: nextToken),
         isMutable(nextToken),
         !token.trailingTrivia.endsWithNewline,
@@ -436,6 +480,19 @@ open class BasicFormat: SyntaxRewriter {
       return trailingTrivia + Trivia(pieces: nextTokenLeadingWhitespace)
     }()
 
+    /// Whether the leading trivia of the token is followed by a newline.
+    let leadingTriviaIsFollowedByNewline: Bool = {
+      if (transformedTokenText ?? token.text).isEmpty && token.trailingTrivia.startsWithNewline {
+        return true
+      } else if token.text.first?.isNewline ?? false {
+        return true
+      } else if (transformedTokenText ?? token.text).isEmpty && token.trailingTrivia.isEmpty && nextTokenWillStartWithNewline {
+        return true
+      } else {
+        return false
+      }
+    }()
+
     if requiresNewline(between: previousToken, and: token) {
       // Add a leading newline if the token requires it unless
       //  - it already starts with a newline or
@@ -455,9 +512,12 @@ open class BasicFormat: SyntaxRewriter {
       }
     }
 
-    if leadingTrivia.indentation(isOnNewline: isInitialToken || previousTokenWillEndWithNewline) == [] {
+    if leadingTrivia.indentation(isOnNewline: isInitialToken || previousTokenWillEndWithNewline) == [] && !token.isStringSegment {
       // If the token starts on a new line and does not have indentation, this
-      // is the last non-indented token. Store its indentation level
+      // is the last non-indented token. Store its indentation level.
+      // But never consider string segments as anchor points since you can’t
+      // indent individual lines of a multi-line string literals without breaking
+      // their integrity.
       anchorPoints[token] = currentIndentationLevel
     }
 
@@ -485,14 +545,17 @@ open class BasicFormat: SyntaxRewriter {
     var leadingTriviaIndentation = self.currentIndentationLevel
     var trailingTriviaIndentation = self.currentIndentationLevel
 
-    // If the trivia contain user-defined indentation, find their anchor point
+    // If the trivia contains user-defined indentation, find their anchor point
     // and indent the token relative to that anchor point.
-    if leadingTrivia.containsIndentation(isOnNewline: previousTokenWillEndWithNewline),
+    // Always indent string literals relative to their anchor point because
+    // their indentation has structural meaning and we just want to maintain
+    // what the user wrote.
+    if leadingTrivia.containsIndentation(isOnNewline: previousTokenWillEndWithNewline) || isInsideStringLiteral,
       let anchorPointIndentation = self.anchorPointIndentation(for: token)
     {
       leadingTriviaIndentation = anchorPointIndentation
     }
-    if combinedTrailingTrivia.containsIndentation(isOnNewline: previousTokenWillEndWithNewline),
+    if combinedTrailingTrivia.containsIndentation(isOnNewline: previousTokenWillEndWithNewline) || isInsideStringLiteral,
       let anchorPointIndentation = self.anchorPointIndentation(for: token)
     {
       trailingTriviaIndentation = anchorPointIndentation
@@ -501,18 +564,36 @@ open class BasicFormat: SyntaxRewriter {
     leadingTrivia = leadingTrivia.indented(indentation: leadingTriviaIndentation, isOnNewline: previousTokenIsStringLiteralEndingInNewline)
     trailingTrivia = trailingTrivia.indented(indentation: trailingTriviaIndentation, isOnNewline: false)
 
-    leadingTrivia = leadingTrivia.trimmingTrailingWhitespaceBeforeNewline(isBeforeNewline: false)
+    leadingTrivia = leadingTrivia.trimmingTrailingWhitespaceBeforeNewline(isBeforeNewline: leadingTriviaIsFollowedByNewline)
     trailingTrivia = trailingTrivia.trimmingTrailingWhitespaceBeforeNewline(isBeforeNewline: nextTokenWillStartWithNewline)
 
-    if leadingTrivia == token.leadingTrivia && trailingTrivia == token.trailingTrivia {
-      return token
+    var result = token.detached
+    if leadingTrivia != result.leadingTrivia {
+      result = result.with(\.leadingTrivia, leadingTrivia)
     }
-
-    return token.detached.with(\.leadingTrivia, leadingTrivia).with(\.trailingTrivia, trailingTrivia)
+    if trailingTrivia != result.trailingTrivia {
+      result = result.with(\.trailingTrivia, trailingTrivia)
+    }
+    if let transformedTokenText {
+      let newKind = TokenKind.fromRaw(kind: token.tokenKind.decomposeToRaw().rawKind, text: transformedTokenText)
+      result = result.with(\.tokenKind, newKind).with(\.presence, .present)
+    }
+    if let transformedTokenPresence {
+      result = result.with(\.presence, transformedTokenPresence)
+    }
+    return result
   }
 }
 
 fileprivate extension TokenSyntax {
+  var isStringSegment: Bool {
+    if case .stringSegment = self.tokenKind {
+      return true
+    } else {
+      return false
+    }
+  }
+
   var isStringSegmentWithLastCharacterBeingNewline: Bool {
     switch self.tokenKind {
     case .stringSegment(let segment):
diff --git a/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift b/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift
index 6c0ec3072a8..912bd529f75 100644
--- a/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift
+++ b/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift
@@ -12,7 +12,7 @@
 
 import SwiftDiagnostics
 import SwiftBasicFormat
-import SwiftSyntax
+@_spi(RawSyntax) import SwiftSyntax
 
 extension FixIt {
   /// A more complex set of changes that affects multiple syntax nodes and thus
@@ -108,11 +108,46 @@ extension FixIt.MultiNodeChange {
 
 // MARK: - Make present
 
-class MissingNodesBasicFormatter: BasicFormat {
+class PresentMakingFormatter: BasicFormat {
+  init() {
+    super.init(viewMode: .fixedUp)
+  }
+
   override func isMutable(_ token: TokenSyntax) -> Bool {
     // Assume that all missing nodes will be made present by the Fix-It.
     return token.isMissing
   }
+
+  /// Change the text of all missing tokens to a placeholder with their
+  /// name for diagnostics.
+  override func transformTokenText(_ token: TokenSyntax) -> String? {
+    guard token.isMissing else {
+      return nil
+    }
+
+    let (rawKind, text) = token.tokenKind.decomposeToRaw()
+
+    guard let text = text else {
+      // The token has a default text that we cannot change.
+      return nil
+    }
+
+    if text.isEmpty && rawKind != .stringSegment {
+      // String segments are allowed to have empty text. Replace all other empty
+      // tokens (e.g. missing identifiers) by a placeholder.
+      return "<#\(token.tokenKind.nameForDiagnostics)#>"
+    }
+
+    return nil
+  }
+
+  /// Make all tokens present.
+  override func transformTokenPresence(_ token: TokenSyntax) -> SourcePresence? {
+    guard token.isMissing else {
+      return nil
+    }
+    return .present
+  }
 }
 
 extension FixIt.MultiNodeChange {
@@ -123,8 +158,7 @@ extension FixIt.MultiNodeChange {
     leadingTrivia: Trivia? = nil,
     trailingTrivia: Trivia? = nil
   ) -> Self {
-    var presentNode = MissingNodesBasicFormatter(viewMode: .fixedUp).rewrite(node, detach: true)
-    presentNode = PresentMaker().rewrite(presentNode)
+    var presentNode = PresentMakingFormatter().rewrite(node, detach: true)
 
     if let leadingTrivia {
       presentNode = presentNode.with(\.leadingTrivia, leadingTrivia)
diff --git a/Sources/SwiftParserDiagnostics/PresenceUtils.swift b/Sources/SwiftParserDiagnostics/PresenceUtils.swift
index fc9a3a1ca3c..b8acee29b73 100644
--- a/Sources/SwiftParserDiagnostics/PresenceUtils.swift
+++ b/Sources/SwiftParserDiagnostics/PresenceUtils.swift
@@ -43,29 +43,6 @@ extension SyntaxProtocol {
   }
 }
 
-/// Transforms a syntax tree by making all missing tokens present.
-class PresentMaker: SyntaxRewriter {
-  init() {
-    super.init(viewMode: .fixedUp)
-  }
-
-  override func visit(_ token: TokenSyntax) -> TokenSyntax {
-    if token.isMissing {
-      let presentToken: TokenSyntax
-      let (rawKind, text) = token.tokenKind.decomposeToRaw()
-      if let text = text, (!text.isEmpty || rawKind == .stringSegment) {  // string segments can have empty text
-        presentToken = token.with(\.presence, .present)
-      } else {
-        let newKind = TokenKind.fromRaw(kind: rawKind, text: rawKind.defaultText.map(String.init) ?? "<#\(token.tokenKind.nameForDiagnostics)#>")
-        presentToken = token.with(\.tokenKind, newKind).with(\.presence, .present)
-      }
-      return presentToken
-    } else {
-      return token
-    }
-  }
-}
-
 /// Transforms a syntax tree by making all present tokens missing.
 class MissingMaker: SyntaxRewriter {
   init() {


Test patch:
diff --git a/Tests/SwiftBasicFormatTest/BasicFormatTests.swift b/Tests/SwiftBasicFormatTest/BasicFormatTests.swift
index 8dd03faa837..680d94e7dfe 100644
--- a/Tests/SwiftBasicFormatTest/BasicFormatTests.swift
+++ b/Tests/SwiftBasicFormatTest/BasicFormatTests.swift
@@ -250,6 +250,117 @@ final class BasicFormatTest: XCTestCase {
     )
   }
 
+  func testMultilineStringLiteralWithBlankLines() {
+    let source = #"""
+      assertionFailure("""
+
+        First line
+
+        Second line
+
+        """)
+      """#
+    assertFormatted(source: source, expected: source)
+  }
+
+  func testMultilineStringLiteralWithFirstLineBlank() {
+    let source = #"""
+      assertionFailure("""
+
+        """)
+      """#
+    assertFormatted(source: source, expected: source)
+  }
+
+  func testNestedMultilineStringLiterals() {
+    let source = #"""
+      assertionFailure("""
+
+        \("""
+          First Line
+        """)
+      """)
+      """#
+
+    assertFormatted(source: source, expected: source)
+  }
+
+  func testIndentNestedMultilineStringLiterals() throws {
+    let stringLiteral: ExprSyntax = #"""
+      """
+
+        \("""
+          First Line
+        """)
+      """
+      """#
+
+    assertFormatted(tree: stringLiteral, expected: stringLiteral.description)
+
+    let tree = try FunctionDeclSyntax("func test()") {
+      stringLiteral
+    }
+
+    assertFormatted(
+      tree: tree,
+      expected: #"""
+        func test() {
+            """
+
+              \("""
+                First Line
+              """)
+            """
+        }
+        """#
+    )
+  }
+
+  func testIndentNestedIndentedMultilineStringLiterals() throws {
+    let stringLiteral: ExprSyntax = #"""
+      _ = """
+
+            \("""
+              First Line
+            """)
+          """
+      """#
+
+    assertFormatted(tree: stringLiteral, expected: stringLiteral.description)
+
+    let tree = try FunctionDeclSyntax("func test()") {
+      stringLiteral
+    }
+
+    assertFormatted(
+      tree: tree,
+      expected: #"""
+        func test() {
+            _ = """
+
+                  \("""
+                    First Line
+                  """)
+                """
+        }
+        """#
+    )
+  }
+
+  func testClosureInStringInterpolation() {
+    let source = #"""
+      """
+      \(gen { (x) in
+          return """
+          case
+      """
+      })
+      """
+      """#
+
+    assertFormatted(source: source, expected: source)
+  }
+
   func testNestedUserDefinedIndentation() {
     assertFormatted(
       source: """