Merge pull request #1643 from kimdv/kimdv/add-correct-diagnostic-for-set-modifier-in-variable

Fix wrong fix-it in modifier for variable decl

Author: kimdv

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

@@ -1525,6 +1525,47 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     if shouldSkip(node) {
       return .skipChildren
     }
+
+    if let modifiers = node.modifiers, modifiers.hasError {
+      for modifier in modifiers {
+        guard let detail = modifier.detail else {
+          continue
+        }
+
+        let unexpectedTokens: [TokenSyntax] = [detail.unexpectedBetweenLeftParenAndDetail, detail.unexpectedBetweenDetailAndRightParen]
+          .compactMap { $0?.tokens(viewMode: .all) }
+          .flatMap { $0 }
+
+        // If there is no unexpected tokens it means we miss a paren or set keyword.
+        // So we just skip the handling here
+        guard let firstUnexpected = unexpectedTokens.first else {
+          continue
+        }
+
+        let fixItMessage: ParserFixIt
+
+        if detail.detail.presence == .missing {
+          fixItMessage = ReplaceTokensFixIt(replaceTokens: unexpectedTokens, replacements: [detail.detail])
+        } else {
+          fixItMessage = RemoveNodesFixIt(unexpectedTokens)
+        }
+
+        addDiagnostic(
+          firstUnexpected,
+          MissingNodesError(missingNodes: [Syntax(detail.detail)]),
+          fixIts: [
+            FixIt(
+              message: fixItMessage,
+              changes: [
+                FixIt.MultiNodeChange.makePresent(detail.detail)
+              ] + unexpectedTokens.map { FixIt.MultiNodeChange.makeMissing($0) }
+            )
+          ],
+          handledNodes: [detail.id] + unexpectedTokens.map(\.id)
+        )
+      }
+    }
+
     let missingTries = node.bindings.compactMap({
       return $0.initializer?.value.as(TryExprSyntax.self)?.tryKeyword
     })

Tests/SwiftParserTest/DeclarationTests.swift

@@ -343,11 +343,10 @@ final class DeclarationTests: XCTestCase {
       private(1️⃣get) var a = 0
       """,
       diagnostics: [
-        DiagnosticSpec(message: "expected 'set' in modifier", fixIts: ["insert 'set'"]),
-        DiagnosticSpec(message: "unexpected code 'get' in modifier"),
+        DiagnosticSpec(message: "expected 'set' in modifier", fixIts: ["replace 'get' with 'set'"])
       ],
       fixedSource: """
-        private(setget) var a = 0
+        private(set) var a = 0
         """
     )
 
@@ -374,17 +373,29 @@ final class DeclarationTests: XCTestCase {
       private(1️⃣get, set) var a = 0
       """,
       diagnostics: [
-        DiagnosticSpec(message: "unexpected code 'get,' in modifier")
-      ]
+        DiagnosticSpec(
+          message: "expected 'set' in modifier",
+          fixIts: ["remove 'get,'"]
+        )
+      ],
+      fixedSource: """
+        private(set) var a = 0
+        """
     )
 
     assertParse(
       """
       private(1️⃣get: set) var a = 0
       """,
       diagnostics: [
-        DiagnosticSpec(message: "unexpected code 'get:' in modifier")
-      ]
+        DiagnosticSpec(
+          message: "expected 'set' in modifier",
+          fixIts: ["remove 'get:'"]
+        )
+      ],
+      fixedSource: """
+        private(set) var a = 0
+        """
     )
 
     assertParse(
@@ -410,12 +421,17 @@ final class DeclarationTests: XCTestCase {
 
     assertParse(
       """
-      private(1️⃣get, set2️⃣, didSet) var a = 0
+      private(1️⃣get, set, didSet) var a = 0
       """,
       diagnostics: [
-        DiagnosticSpec(locationMarker: "1️⃣", message: "unexpected code 'get,' in modifier"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code ', didSet' in modifier"),
-      ]
+        DiagnosticSpec(
+          message: "expected 'set' in modifier",
+          fixIts: ["remove 'get, , didSet'"]
+        )
+      ],
+      fixedSource: """
+        private(set) var a = 0
+        """
     )
 
     assertParse(