From ceb85743ace0c9131648c1285686726a33b3e1f4 Mon Sep 17 00:00:00 2001
From: Alex Hoppen <ahoppen@apple.com>
Date: Wed, 10 May 2023 07:25:32 -0700
Subject: [PATCH] In `nodesDescription` only merge tokens if they occur
 consecutively in the source code

This was motiviated by the following discussion, where the issue becomes more apparent: https://github.com/apple/swift-syntax/pull/1643#discussion_r1187871863.
---
 .../MissingNodesError.swift                     | 17 ++++++++++++++++-
 Tests/SwiftParserTest/DeclarationTests.swift    |  2 +-
 .../translated/RecoveryTests.swift              | 10 +++++-----
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/Sources/SwiftParserDiagnostics/MissingNodesError.swift b/Sources/SwiftParserDiagnostics/MissingNodesError.swift
index dd7ba898ab4..ecd5252a2ae 100644
--- a/Sources/SwiftParserDiagnostics/MissingNodesError.swift
+++ b/Sources/SwiftParserDiagnostics/MissingNodesError.swift
@@ -67,7 +67,12 @@ fileprivate enum NodesDescriptionPart {
   }
 
   static func descriptionParts(for nodes: some Sequence<Syntax>) -> [NodesDescriptionPart] {
+    // For each token that we modified (e.g. made it present), map to the original token in `nodes`
+    var originalTokens: [TokenSyntax: TokenSyntax] = [:]
+
+    // Accumulates the result
     var parts: [NodesDescriptionPart] = []
+
     for missingNode in nodes {
       if let token = missingNode.as(TokenSyntax.self) {
         let newPart: NodesDescriptionPart
@@ -77,10 +82,12 @@ fileprivate enum NodesDescriptionPart {
           let (rawKind, text) = token.tokenKind.decomposeToRaw()
           if let text = text, !text.isEmpty {
             let presentToken = token.with(\.presence, .present)
+            originalTokens[presentToken] = token
             newPart = .tokensWithDefaultText([presentToken])
           } else if let defaultText = rawKind.defaultText {
             let newKind = TokenKind.fromRaw(kind: rawKind, text: String(syntaxText: defaultText))
             let presentToken = token.with(\.tokenKind, newKind).with(\.presence, .present)
+            originalTokens[presentToken] = token
             newPart = .tokensWithDefaultText([presentToken])
           } else {
             newPart = .tokenWithoutDefaultText(token)
@@ -89,7 +96,15 @@ fileprivate enum NodesDescriptionPart {
 
         switch (parts.last, newPart) {
         case (.tokensWithDefaultText(let previousTokens), .tokensWithDefaultText(let newTokens)):
-          parts[parts.count - 1] = .tokensWithDefaultText(previousTokens + newTokens)
+          // Merge `tokensWithDefaultText` if they occur consecutively in the tree
+          if let lastPrevious = previousTokens.last,
+            let firstNew = newTokens.first,
+            originalTokens[lastPrevious, default: lastPrevious].nextToken(viewMode: .all) == originalTokens[firstNew, default: firstNew]
+          {
+            parts[parts.count - 1] = .tokensWithDefaultText(previousTokens + newTokens)
+          } else {
+            parts.append(newPart)
+          }
         default:
           parts.append(newPart)
         }
diff --git a/Tests/SwiftParserTest/DeclarationTests.swift b/Tests/SwiftParserTest/DeclarationTests.swift
index d6036fb36fd..4ad1f5f63e6 100644
--- a/Tests/SwiftParserTest/DeclarationTests.swift
+++ b/Tests/SwiftParserTest/DeclarationTests.swift
@@ -426,7 +426,7 @@ final class DeclarationTests: XCTestCase {
       diagnostics: [
         DiagnosticSpec(
           message: "expected 'set' in modifier",
-          fixIts: ["remove 'get, , didSet'"]
+          fixIts: ["remove 'get,' and ', didSet'"]
         )
       ],
       fixedSource: """
diff --git a/Tests/SwiftParserTest/translated/RecoveryTests.swift b/Tests/SwiftParserTest/translated/RecoveryTests.swift
index 0dda9d49fd7..5cfed060d3d 100644
--- a/Tests/SwiftParserTest/translated/RecoveryTests.swift
+++ b/Tests/SwiftParserTest/translated/RecoveryTests.swift
@@ -2017,7 +2017,7 @@ final class RecoveryTests: XCTestCase {
       diagnostics: [
         DiagnosticSpec(
           message: "expected ':' to begin inheritance clause",
-          fixIts: ["replace '()' with ':'"]
+          fixIts: ["replace '(' and ')' with ':'"]
         )
       ],
       fixedSource: """
@@ -2034,7 +2034,7 @@ final class RecoveryTests: XCTestCase {
       diagnostics: [
         DiagnosticSpec(
           message: "expected ':' to begin inheritance clause",
-          fixIts: ["replace '()' with ':'"]
+          fixIts: ["replace '(' and ')' with ':'"]
         )
       ],
       fixedSource: """
@@ -2051,7 +2051,7 @@ final class RecoveryTests: XCTestCase {
       diagnostics: [
         DiagnosticSpec(
           message: "expected ':' to begin inheritance clause",
-          fixIts: ["replace '()' with ':'"]
+          fixIts: ["replace '(' and ')' with ':'"]
         )
       ],
       fixedSource: """
@@ -2068,7 +2068,7 @@ final class RecoveryTests: XCTestCase {
       diagnostics: [
         DiagnosticSpec(
           message: "expected ':' to begin inheritance clause",
-          fixIts: ["replace '()' with ':'"]
+          fixIts: ["replace '(' and ')' with ':'"]
         )
       ],
       fixedSource: """
@@ -2085,7 +2085,7 @@ final class RecoveryTests: XCTestCase {
       diagnostics: [
         DiagnosticSpec(
           message: "expected ':' to begin inheritance clause",
-          fixIts: ["replace '()' with ':'"]
+          fixIts: ["replace '(' and ')' with ':'"]
         )
       ],
       fixedSource: """