Merge pull request swiftlang#1496 from ahoppen/ahoppen/5.9/fixit

[5.9] Slightly clean up `FixIt.Change` and `FixIt.Changes`

Author: ahoppen

Sources/SwiftCompilerPluginMessageHandling/Diagnostics.swift

@@ -84,7 +84,7 @@ extension PluginMessage.Diagnostic {
     self.fixIts = syntaxDiag.fixIts.compactMap {
       PluginMessage.Diagnostic.FixIt(
         message: $0.message.message,
-        changes: $0.changes.changes.compactMap {
+        changes: $0.changes.compactMap {
           let range: SourceManager.SourceRange?
           let text: String
           switch $0 {

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,51 @@ 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)
+  }
+  public init(message: StaticParserFixIt, changes: [Change]) {
+    self.init(message: message as FixItMessage, changes: changes)
   }
 }
 
-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 +76,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 +116,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,
       let previousToken = node.previousToken(viewMode: .fixedUp),
       previousToken.presence == .present,
@@ -105,19 +132,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 +155,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 +176,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

@@ -192,7 +192,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()
@@ -206,7 +206,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))])
       ]
     case .equalMustHaveConsistentWhitespaceOnBothSides:
       let hasLeadingSpace = token.previousToken(viewMode: .all)?.trailingTrivia.contains(where: { $0.isSpaceOrTab }) ?? false
@@ -226,7 +226,7 @@ public extension SwiftSyntax.TokenDiagnostic {
       }
 
       return [
-        FixIt(message: .insertWhitespace, changes: FixIt.Changes(changes: changes))
+        FixIt(message: .insertWhitespace, changes: changes)
       ]
     default:
       return []

Sources/SwiftParserDiagnostics/MissingNodesError.swift

@@ -316,7 +316,7 @@ extension ParseDiagnosticsGenerator {
   func handleMissingSyntax<T: SyntaxProtocol>(
     _ node: T,
     overridePosition: AbsolutePosition? = nil,
-    additionalChanges: [FixIt.Changes] = [],
+    additionalChanges: [FixIt.MultiNodeChange] = [],
     additionalHandledNodes: [SyntaxIdentifier] = []
   ) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
@@ -356,7 +356,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: [])]),
+              FixIt.MultiNodeChange(.replaceTrailingTrivia(token: previous, newTrivia: [])),
             ]
           )
         ],

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,21 @@ 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(var payload):
+      payload.presence = newValue
+      return RawSyntax(arena: arena, payload: .parsedToken(payload))
+    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)