Repository: apple/swift-syntax (swift)

Issue:
Accessor Macros applied to properties with trailing trivia comments lead to Swift that fails to compile.
### Description

The following Swift code fails to compile:

```swift
var x: Int // comment {
  get {
    return 1
  }
}
```

The trailing trivia comment captures the opening round brace… which leads to the compiler error.

We can attach an accessor macro to a variable declaration with similar trailing trivia:

```swift
@constantOne
var x: Int // comment
```

Which expands to:

```swift
var x: Int // comment {
  get {
    return 1
  }
}
```

Maybe we should do something about that comment trivia… could we place the comment trivia after (to the right of) the opening curly brace?

```swift
var x: Int { // comment
  get {
    return 1
  }
}
```

Could we enclose the trailing trivia with asterisks?

```swift
var x: Int /* comment */ {
  get {
    return 1
  }
}
```

### Steps to Reproduce

Here is a diff on `swift-syntax` to repro:

```
diff --git a/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift b/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift
index 471a34c7..97e2b8bc 100644
--- a/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift
+++ b/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift
@@ -43,6 +43,24 @@ fileprivate struct ConstantOneGetter: AccessorMacro {
 
 final class AccessorMacroTests: XCTestCase {
   private let indentationWidth: Trivia = .spaces(2)
+  
+  func testAccessorWithComment() {
+    assertMacroExpansion(
+      """
+      @constantOne
+      var x: Int // comment
+      """,
+      expandedSource: """
+        var x: Int {
+          get {
+            return 1
+          }
+        }
+        """,
+      macros: ["constantOne": ConstantOneGetter.self],
+      indentationWidth: indentationWidth
+    )
+  }
 
   func testAccessorOnVariableDeclWithExistingGetter() {
     assertMacroExpansion(

```

Pull request description:
Fixed #2614

Patch:
diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift
index 910e69553c6..bed3e1082f3 100644
--- a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift
+++ b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift
@@ -923,7 +923,9 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
       return DeclSyntax(node)
     }
 
-    guard node.bindings.count == 1, let binding = node.bindings.first else {
+    guard node.bindings.count == 1,
+      var binding = node.bindings.first
+    else {
       contextGenerator(Syntax(node)).addDiagnostics(
         from: MacroApplicationError.accessorMacroOnVariableWithMultipleBindings,
         node: node
@@ -931,17 +933,27 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
       return DeclSyntax(node)
     }
 
-    let expansion = expandAccessors(of: node, existingAccessors: binding.accessorBlock)
+    var expansion = expandAccessors(of: node, existingAccessors: binding.accessorBlock)
+
     if expansion.accessors != binding.accessorBlock {
+      if binding.accessorBlock == nil {
+        // remove the trailing trivia of the variable declaration and move it
+        // to the trailing trivia of the left brace of the newly created accessor block
+        expansion.accessors?.leftBrace.trailingTrivia = binding.trailingTrivia
+        binding.trailingTrivia = []
+      }
+
       if binding.initializer != nil, expansion.expandsGetSet {
         // The accessor block will have a leading space, but there will already be a
         // space between the variable and the to-be-removed initializer. Remove the
         // leading trivia on the accessor block so we don't double up.
-        node.bindings[node.bindings.startIndex].accessorBlock = expansion.accessors?.with(\.leadingTrivia, [])
-        node.bindings[node.bindings.startIndex].initializer = nil
+        binding.accessorBlock = expansion.accessors?.with(\.leadingTrivia, [])
+        binding.initializer = nil
       } else {
-        node.bindings[node.bindings.startIndex].accessorBlock = expansion.accessors
+        binding.accessorBlock = expansion.accessors
       }
+
+      node.bindings = [binding]
     }
 
     return DeclSyntax(node)


Test patch:
diff --git a/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift b/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift
index 471a34c7684..3b4c2265911 100644
--- a/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift
+++ b/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift
@@ -44,6 +44,127 @@ fileprivate struct ConstantOneGetter: AccessorMacro {
 final class AccessorMacroTests: XCTestCase {
   private let indentationWidth: Trivia = .spaces(2)
 
+  func testAccessorOnVariableDeclWithTrailingLineCommentAndNoAccessorBlock() {
+    assertMacroExpansion(
+      """
+      @constantOne
+      var x: Int /*1*/ // hello
+      """,
+      expandedSource: """
+        var x: Int { /*1*/ // hello
+          get {
+            return 1
+          }
+        }
+        """,
+      macros: ["constantOne": ConstantOneGetter.self],
+      indentationWidth: indentationWidth
+    )
+
+    assertMacroExpansion(
+      """
+      @constantOne
+      var x: Int /// hello
+      """,
+      expandedSource: """
+        var x: Int { /// hello
+          get {
+            return 1
+          }
+        }
+        """,
+      macros: ["constantOne": ConstantOneGetter.self],
+      indentationWidth: indentationWidth
+    )
+
+    assertMacroExpansion(
+      """
+      @constantOne
+      var x: Int = 1 /// hello
+      """,
+      expandedSource: """
+        var x: Int { /// hello
+          get {
+            return 1
+          }
+        }
+        """,
+      macros: ["constantOne": ConstantOneGetter.self],
+      indentationWidth: indentationWidth
+    )
+
+    assertMacroExpansion(
+      """
+      @constantOne
+      var x /// hello
+      """,
+      expandedSource: """
+        var x { /// hello
+          get {
+            return 1
+          }
+        }
+        """,
+      macros: ["constantOne": ConstantOneGetter.self],
+      indentationWidth: indentationWidth
+    )
+  }
+
+  func testAccessorOnVariableDeclWithTrailingLineCommentAndAccessorBlock() {
+    assertMacroExpansion(
+      """
+      @constantOne
+      var x: Int /*h*/ { // hello
+        1
+      }
+      """,
+      expandedSource: """
+        var x: Int /*h*/ { // hello
+        get {
+            1
+        }
+          get {
+            return 1
+          }
+        }
+        """,
+      macros: ["constantOne": ConstantOneGetter.self],
+      indentationWidth: indentationWidth
+    )
+  }
+
+  func testAccessorOnVariableDeclWithTrailingCommentsAndSingleLineGetterExpansion() {
+    struct ConstantOneSingleLineGetter: AccessorMacro {
+      static let formatMode: FormatMode = .disabled
+
+      static func expansion(
+        of node: AttributeSyntax,
+        providingAccessorsOf declaration: some DeclSyntaxProtocol,
+        in context: some MacroExpansionContext
+      ) throws -> [AccessorDeclSyntax] {
+        return [
+          """
+          get { 1 }
+          """
+        ]
+      }
+    }
+
+    assertMacroExpansion(
+      """
+      @constantOne
+      var x: Int // hello
+      """,
+      expandedSource: """
+        var x: Int { // hello
+          get { 1 }
+        }
+        """,
+      macros: ["constantOne": ConstantOneSingleLineGetter.self],
+      indentationWidth: indentationWidth
+    )
+  }
+
   func testAccessorOnVariableDeclWithExistingGetter() {
     assertMacroExpansion(
       """