Sligthly clean up FixIt.Change and FixIt.Changes

The existing API with `FixIt.Change` and `FixIt.Changes` and the fact that we passed `[FixIt.Changes]` around what fairly weird.

Ideally, we would only have `FixIt.Change` and make it powerful enough to represent all the trivia transfer that `FixIt.Changes` is doing. But given that that will be a larger effort, let’s make some smaller changes now that reduce the badness of the public API impact:

- Move `FixIt.Changes` to `SwiftParserDiagnostics`, make it internal and rename it to `MultiNodeChange`. That way this type is no longer exposed in the public API, so we can iterate on it or remove it without breaking clients. The only thing that remains exposed is `FixIt.Change`, which we want to continue to support. We will probably add more cases to it in the future.
- Make `FixIt.Changes` no longer conform to `ExpressibleByArrayLiteral`. That just makes everything a lot more explicit and easier to follow.
- Remove some unecessary initializations of `FixIt.Changes` where `FixIt.Change` was sufficient.
- Make `presence` on `TokenSyntax` settable so you can do `token.with(\.presence, .missing)`
- Slightly unrelated: Rename SyntaxOtherNodes.swift to TokenSyntax.swift because that’s the only node it contains.

Author: ahoppen

Sources/SwiftDiagnostics/FixIt.swift

@@ -25,22 +25,6 @@ public protocol FixItMessage {
 
 /// A Fix-It that can be applied to resolve a diagnostic.
 public struct FixIt {
-  public struct Changes: ExpressibleByArrayLiteral {
-    public var changes: [Change]
-
-    public init(changes: [Change]) {
-      self.changes = changes
-    }
-
-    public init(arrayLiteral elements: FixIt.Change...) {
-      self.init(changes: elements)
-    }
-
-    public init(combining: [Changes]) {
-      self.init(changes: combining.flatMap(\.changes))
-    }
-  }
-
   public enum Change {
     /// Replace `oldNode` by `newNode`.
     case replace(oldNode: Syntax, newNode: Syntax)
@@ -54,10 +38,10 @@ public struct FixIt {
   public let message: FixItMessage
 
   /// The changes that need to be performed when the Fix-It is applied.
-  public let changes: Changes
+  public let changes: [Change]
 
-  public init(message: FixItMessage, changes: Changes) {
-    precondition(!changes.changes.isEmpty, "A Fix-It must have at least one change associated with it")
+  public init(message: FixItMessage, changes: [Change]) {
+    precondition(!changes.isEmpty, "A Fix-It must have at least one change associated with it")
     self.message = message
     self.changes = changes
   }

Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift

@@ -15,24 +15,48 @@ import SwiftBasicFormat
 import SwiftSyntax
 
 extension FixIt {
-  public init(message: FixItMessage, changes: [Changes]) {
-    self.init(message: message, changes: FixIt.Changes(combining: changes))
+  /// A more complex set of changes that affects multiple syntax nodes and thus
+  /// produces multiple `FixIt.Change`s. This allows us to e.g. mark a node as
+  /// missing but keep the trivia by transferring it to the previous or next
+  /// token.
+  struct MultiNodeChange {
+    var primitiveChanges: [Change]
+
+    init(primitiveChanges: [Change]) {
+      self.primitiveChanges = primitiveChanges
+    }
+
+    init(_ primitiveChanges: Change...) {
+      self.init(primitiveChanges: primitiveChanges)
+    }
+
+    init(combining: [MultiNodeChange]) {
+      self.init(primitiveChanges: combining.flatMap(\.primitiveChanges))
+    }
+  }
+
+  init(message: FixItMessage, changes: [MultiNodeChange]) {
+    self.init(message: message, changes: MultiNodeChange(combining: changes))
+  }
+
+  init(message: FixItMessage, changes: MultiNodeChange) {
+    self.init(message: message, changes: changes.primitiveChanges)
   }
 
   // These overloads shouldn't be needed, but are currently required for the
   // Swift 5.5 compiler to handle non-trivial FixIt initializations using
   // leading-dot syntax.
   // TODO: These can be dropped once we require a minimum of Swift 5.6 to
   // compile the library.
-  init(message: StaticParserFixIt, changes: Changes) {
-    self.init(message: message as FixItMessage, changes: changes)
+  init(message: StaticParserFixIt, changes: MultiNodeChange) {
+    self.init(message: message as FixItMessage, changes: changes.primitiveChanges)
   }
-  init(message: StaticParserFixIt, changes: [Changes]) {
-    self.init(message: message as FixItMessage, changes: FixIt.Changes(combining: changes))
+  init(message: StaticParserFixIt, changes: [MultiNodeChange]) {
+    self.init(message: message as FixItMessage, changes: MultiNodeChange(combining: changes).primitiveChanges)
   }
 }
 
-extension FixIt.Changes {
+extension FixIt.MultiNodeChange {
   /// Replaced a present token with a missing node.
   /// If `transferTrivia` is `true`, the leading and trailing trivia of the
   /// removed node will be transferred to the trailing trivia of the previous token.
@@ -49,26 +73,26 @@ extension FixIt.Changes {
     var changes = tokens.map {
       FixIt.Change.replace(
         oldNode: Syntax($0),
-        newNode: Syntax(TokenSyntax($0.tokenKind, leadingTrivia: [], trailingTrivia: [], presence: .missing))
+        newNode: Syntax($0.with(\.presence, .missing))
       )
     }
     if transferTrivia {
-      changes += FixIt.Changes.transferTriviaAtSides(from: tokens).changes
+      changes += FixIt.MultiNodeChange.transferTriviaAtSides(from: tokens).primitiveChanges
     }
-    return FixIt.Changes(changes: changes)
+    return FixIt.MultiNodeChange(primitiveChanges: changes)
   }
 
   /// If `transferTrivia` is `true`, the leading and trailing trivia of the
   /// removed node will be transferred to the trailing trivia of the previous token.
   static func makeMissing<SyntaxType: SyntaxProtocol>(_ node: SyntaxType?, transferTrivia: Bool = true) -> Self {
     guard let node = node else {
-      return FixIt.Changes(changes: [])
+      return FixIt.MultiNodeChange(primitiveChanges: [])
     }
     var changes = [FixIt.Change.replace(oldNode: Syntax(node), newNode: MissingMaker().visit(Syntax(node)))]
     if transferTrivia {
-      changes += FixIt.Changes.transferTriviaAtSides(from: [node]).changes
+      changes += FixIt.MultiNodeChange.transferTriviaAtSides(from: [node]).primitiveChanges
     }
-    return FixIt.Changes(changes: changes)
+    return FixIt.MultiNodeChange(primitiveChanges: changes)
   }
 
   /// Make a node present. If `leadingTrivia` or `trailingTrivia` is specified,
@@ -89,13 +113,13 @@ extension FixIt.Changes {
       let nextToken = node.nextToken(viewMode: .sourceAccurate),
       leadingTrivia == nil
     {
-      return [
+      return FixIt.MultiNodeChange(
         .replace(
           oldNode: Syntax(node),
           newNode: Syntax(presentNode).with(\.leadingTrivia, nextToken.leadingTrivia)
         ),
-        .replaceLeadingTrivia(token: nextToken, newTrivia: []),
-      ]
+        .replaceLeadingTrivia(token: nextToken, newTrivia: [])
+      )
     } else if node.leadingTrivia?.isEmpty ?? true,
       let previousToken = node.previousToken(viewMode: .fixedUp),
       previousToken.presence == .present,
@@ -105,19 +129,19 @@ extension FixIt.Changes {
     {
       /// If neither this nor the previous token are punctionation make sure they
       /// are separated by a space.
-      return [
+      return FixIt.MultiNodeChange(
         .replace(
           oldNode: Syntax(node),
           newNode: Syntax(presentNode).with(\.leadingTrivia, .space)
         )
-      ]
+      )
     } else {
-      return [
+      return FixIt.MultiNodeChange(
         .replace(
           oldNode: Syntax(node),
           newNode: Syntax(presentNode)
         )
-      ]
+      )
     }
   }
 
@@ -128,10 +152,10 @@ extension FixIt.Changes {
       if !previousToken.trailingTrivia.isEmpty {
         presentToken = presentToken.with(\.trailingTrivia, previousToken.trailingTrivia)
       }
-      return [
+      return FixIt.MultiNodeChange(
         .replaceTrailingTrivia(token: previousToken, newTrivia: []),
-        .replace(oldNode: Syntax(token), newNode: Syntax(presentToken)),
-      ]
+        .replace(oldNode: Syntax(token), newNode: Syntax(presentToken))
+      )
     } else {
       return .makePresent(token)
     }
@@ -149,11 +173,11 @@ extension FixIt.Changes {
         // Punctuation is generally not followed by spaces in Swift.
         // If this action would only add spaces to the punctuation, drop it.
         // This generally yields better results.
-        return []
+        return FixIt.MultiNodeChange()
       }
-      return [.replaceTrailingTrivia(token: previousToken, newTrivia: mergedTrivia)]
+      return FixIt.MultiNodeChange(.replaceTrailingTrivia(token: previousToken, newTrivia: mergedTrivia))
     } else {
-      return []
+      return FixIt.MultiNodeChange()
     }
   }
 }

Sources/SwiftParserDiagnostics/LexerDiagnosticMessages.swift

@@ -184,7 +184,7 @@ public extension SwiftSyntax.TokenDiagnostic {
         .with(\.leadingTrivia, Trivia(pieces: token.leadingTrivia.map(replaceNonBreakingSpace)))
         .with(\.trailingTrivia, Trivia(pieces: token.trailingTrivia.map(replaceNonBreakingSpace)))
       return [
-        FixIt(message: .replaceNonBreakingSpaceBySpace, changes: [[.replace(oldNode: Syntax(token), newNode: Syntax(fixedToken))]])
+        FixIt(message: .replaceNonBreakingSpaceBySpace, changes: [.replace(oldNode: Syntax(token), newNode: Syntax(fixedToken))])
       ]
     case .unicodeCurlyQuote:
       let (rawKind, text) = token.tokenKind.decomposeToRaw()
@@ -198,7 +198,7 @@ public extension SwiftSyntax.TokenDiagnostic {
 
       let fixedToken = token.withKind(TokenKind.fromRaw(kind: rawKind, text: replacedText))
       return [
-        FixIt(message: .replaceCurlyQuoteByNormalQuote, changes: [[.replace(oldNode: Syntax(token), newNode: Syntax(fixedToken))]])
+        FixIt(message: .replaceCurlyQuoteByNormalQuote, changes: [.replace(oldNode: Syntax(token), newNode: Syntax(fixedToken))])
       ]
     default:
       return []

Sources/SwiftParserDiagnostics/MissingNodesError.swift

@@ -314,7 +314,7 @@ extension ParseDiagnosticsGenerator {
   func handleMissingSyntax<T: SyntaxProtocol>(
     _ node: T,
     overridePosition: AbsolutePosition? = nil,
-    additionalChanges: [FixIt.Changes] = [],
+    additionalChanges: [FixIt.MultiNodeChange] = [],
     additionalHandledNodes: [SyntaxIdentifier] = []
   ) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
@@ -354,7 +354,7 @@ extension ParseDiagnosticsGenerator {
       }
     }
 
-    let changes = missingNodes.enumerated().map { (index, missingNode) -> FixIt.Changes in
+    let changes = missingNodes.enumerated().map { (index, missingNode) -> FixIt.MultiNodeChange in
       if index == 0,
         let token = missingNode.as(TokenSyntax.self),
         let previousTokenKind = missingNode.previousToken(viewMode: .sourceAccurate)?.tokenKind

Sources/SwiftParserDiagnostics/MissingTokenError.swift

@@ -97,7 +97,7 @@ extension ParseDiagnosticsGenerator {
 
   private func handleInvalidPeriod(invalidToken: TokenSyntax, missingToken: TokenSyntax, invalidTokenContainer: UnexpectedNodesSyntax) -> Bool {
     // Trailing trivia is the cause of this diagnostic, don't transfer it.
-    let changes: [FixIt.Changes] = [
+    let changes: [FixIt.MultiNodeChange] = [
       .makeMissing(invalidToken, transferTrivia: false),
       .makePresent(missingToken),
     ]

Sources/SwiftParserDiagnostics/MultiLineStringLiteralDiagnosticsGenerator.swift

@@ -115,7 +115,7 @@ final class MultiLineStringLiteralIndentatinDiagnosticsGenerator: SyntaxVisitor
       message: InvalidIndentationInMultiLineStringLiteralError(kind: currentDiagnostic.kind, lines: currentDiagnostic.lines),
       highlights: [],
       notes: [Note(node: Syntax(closeQuote), message: .shouldMatchIndentationOfClosingQuote)],
-      fixIts: [FixIt(message: .changeIndentationToMatchClosingDelimiter, changes: FixIt.Changes(changes: currentDiagnostic.changes))]
+      fixIts: [FixIt(message: .changeIndentationToMatchClosingDelimiter, changes: currentDiagnostic.changes)]
     )
 
     finishedDiagnostics.append((diagnostic, currentDiagnostic.handledNodes))

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

@@ -143,23 +143,23 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
 
     // Ignore `correctTokens` that are already present.
     let correctAndMissingTokens = correctTokens.filter({ $0.presence == .missing })
-    var changes: [FixIt.Changes] = []
+    var changes: [FixIt.MultiNodeChange] = []
     if let misplacedToken = misplacedTokens.only, let correctToken = correctTokens.only,
       misplacedToken.nextToken(viewMode: .all) == correctToken || misplacedToken.previousToken(viewMode: .all) == correctToken,
       correctToken.presence == .missing
     {
       // We are exchanging two adjacent tokens, transfer the trivia from the incorrect token to the corrected token.
-      changes += misplacedTokens.map { FixIt.Changes.makeMissing($0, transferTrivia: false) }
+      changes += misplacedTokens.map { FixIt.MultiNodeChange.makeMissing($0, transferTrivia: false) }
       changes.append(
-        FixIt.Changes.makePresent(
+        FixIt.MultiNodeChange.makePresent(
           correctToken,
           leadingTrivia: misplacedToken.leadingTrivia,
           trailingTrivia: misplacedToken.trailingTrivia
         )
       )
     } else {
-      changes += misplacedTokens.map { FixIt.Changes.makeMissing($0) }
-      changes += correctAndMissingTokens.map { FixIt.Changes.makePresent($0) }
+      changes += misplacedTokens.map { FixIt.MultiNodeChange.makeMissing($0) }
+      changes += correctAndMissingTokens.map { FixIt.MultiNodeChange.makePresent($0) }
     }
     var fixIts: [FixIt] = []
     if changes.count > 1 {
@@ -309,7 +309,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
         node,
         .unexpectedSemicolon,
         fixIts: [
-          FixIt(message: RemoveNodesFixIt(semicolons), changes: semicolons.map { FixIt.Changes.makeMissing($0) })
+          FixIt(message: RemoveNodesFixIt(semicolons), changes: semicolons.map { FixIt.MultiNodeChange.makeMissing($0) })
         ]
       )
     } else if node.first?.as(TokenSyntax.self)?.tokenKind.isIdentifier == true,
@@ -327,7 +327,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
         FixIt(
           message: .joinIdentifiers,
           changes: [
-            [.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))],
+            FixIt.MultiNodeChange(.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))),
             .makeMissing(tokens),
           ]
         )
@@ -338,7 +338,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
           FixIt(
             message: .joinIdentifiersWithCamelCase,
             changes: [
-              [.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))],
+              FixIt.MultiNodeChange(.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))),
               .makeMissing(tokens),
             ]
           )
@@ -821,7 +821,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
             message: RemoveNodesFixIt(unexpectedName),
             changes: [
               .makeMissing(unexpectedName),
-              FixIt.Changes(changes: [.replaceTrailingTrivia(token: previous, newTrivia: .zero)]),
+              FixIt.MultiNodeChange(.replaceTrailingTrivia(token: previous, newTrivia: .zero)),
             ]
           )
         ],

Sources/SwiftSyntax/CMakeLists.txt

@@ -19,10 +19,10 @@ add_swift_host_library(SwiftSyntax
   SyntaxArena.swift
   SyntaxChildren.swift
   SyntaxData.swift
-  SyntaxOtherNodes.swift
   SyntaxText.swift
   SyntaxTreeViewMode.swift
   TokenDiagnostic.swift
+  TokenSyntax.swift
   Utils.swift
 
   Raw/RawSyntax.swift

Sources/SwiftSyntax/Raw/RawSyntaxTokenView.swift

@@ -197,6 +197,28 @@ public struct RawSyntaxTokenView {
     }
   }
 
+  /// Returns a `RawSyntax` node with the presence changed to `newValue`.
+  @_spi(RawSyntax)
+  public func withPresence(_ newValue: SourcePresence, arena: SyntaxArena) -> RawSyntax {
+    switch raw.rawData.payload {
+    case .parsedToken(_):
+      // The wholeText can't be continuous anymore. Make a materialized token.
+      return .makeMaterializedToken(
+        kind: formKind(),
+        leadingTrivia: formLeadingTrivia(),
+        trailingTrivia: formTrailingTrivia(),
+        presence: newValue,
+        tokenDiagnostic: tokenDiagnostic,
+        arena: arena
+      )
+    case .materializedToken(var payload):
+      payload.presence = newValue
+      return RawSyntax(arena: arena, payload: .materializedToken(payload))
+    default:
+      preconditionFailure("'withKind()' is called on non-token raw syntax")
+    }
+  }
+
   /// The length of the token without leading or trailing trivia, assuming this
   /// is a token node.
   @_spi(RawSyntax)

Sources/SwiftSyntax/SyntaxData.swift

@@ -336,6 +336,14 @@ struct SyntaxData {
       return self
     }
   }
+
+  func withPresence(_ presence: SourcePresence, arena: SyntaxArena) -> SyntaxData {
+    if let raw = raw.tokenView?.withPresence(presence, arena: arena) {
+      return replacingSelf(raw, arena: arena)
+    } else {
+      return self
+    }
+  }
 }
 
 #if DEBUG

Sources/SwiftSyntax/SyntaxOtherNodes.swift renamed to Sources/SwiftSyntax/TokenSyntax.swift

@@ -55,7 +55,12 @@ public struct TokenSyntax: SyntaxProtocol, SyntaxHashable {
   }
 
   public var presence: SourcePresence {
-    return tokenView.presence
+    get {
+      return tokenView.presence
+    }
+    set {
+      self = TokenSyntax(data.withPresence(newValue, arena: SyntaxArena()))
+    }
   }
 
   /// The text of the token as written in the source code, without any trivia.

Tests/SwiftParserTest/Assertions.swift

@@ -285,7 +285,7 @@ class FixItApplier: SyntaxRewriter {
           return true
         }
       }
-      .flatMap { $0.changes.changes }
+      .flatMap { $0.changes }
   }
 
   public override func visitAny(_ node: Syntax) -> Syntax? {