swiftlang/swift-syntax

Accessor Macros applied to properties with trailing trivia comments lead to Swift that fails to compile.

vanvoorden opened this issue · 4 comments

Description

The following Swift code fails to compile:

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:

@constantOne
var x: Int // comment

Which expands to:

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?

var x: Int { // comment
  get {
    return 1
  }
}

Could we enclose the trailing trivia with asterisks?

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(

Tracked in Apple’s issue tracker as rdar://126880983

Just to confirm: This is only an issue for assertMacroExpansion and not during compilation, right?

Just to confirm: This is only an issue for assertMacroExpansion and not during compilation, right?

@ahoppen Correct. Here is a diff that compiles a playground and passes tests:

diff --git a/Examples/Sources/MacroExamples/Playground/AccessorMacrosPlayground.swift b/Examples/Sources/MacroExamples/Playground/AccessorMacrosPlayground.swift
index e100945a..f5b2cac5 100644
--- a/Examples/Sources/MacroExamples/Playground/AccessorMacrosPlayground.swift
+++ b/Examples/Sources/MacroExamples/Playground/AccessorMacrosPlayground.swift
@@ -24,7 +24,7 @@ private struct MyEnvironmentKey: EnvironmentKey {
 
 extension EnvironmentValues {
   @EnvironmentValue(for: MyEnvironmentKey.self)
-  var myCustomValue: String
+  var myCustomValue: String // comment
 }
 
 func runEnvironmentValueAccessorMacroPlayground() {
diff --git a/Examples/Sources/MacroExamples/Playground/ComplexMacrosPlayground.swift b/Examples/Sources/MacroExamples/Playground/ComplexMacrosPlayground.swift
index 8e02d839..c78671bd 100644
--- a/Examples/Sources/MacroExamples/Playground/ComplexMacrosPlayground.swift
+++ b/Examples/Sources/MacroExamples/Playground/ComplexMacrosPlayground.swift
@@ -36,8 +36,8 @@ struct Treat {}
 
 @Observable
 final class Dog {
-  var name: String?
-  var treat: Treat?
+  var name: String? // comment
+  var treat: Treat? // comment
 
   var isHappy: Bool = true
 
diff --git a/Examples/Tests/MacroExamples/Implementation/Accessor/EnvironmentValueMacroTests.swift b/Examples/Tests/MacroExamples/Implementation/Accessor/EnvironmentValueMacroTests.swift
index 73749476..eee8d6c5 100644
--- a/Examples/Tests/MacroExamples/Implementation/Accessor/EnvironmentValueMacroTests.swift
+++ b/Examples/Tests/MacroExamples/Implementation/Accessor/EnvironmentValueMacroTests.swift
@@ -23,12 +23,12 @@ final class EnvironmentValueMacroMacroTests: XCTestCase {
       """
       extension EnvironmentValues {
           @EnvironmentValue(for: MyEnvironmentKey.self)
-          var myCustomValue: String
+          var myCustomValue: String // comment
       }
       """,
       expandedSource: """
         extension EnvironmentValues {
-            var myCustomValue: String {
+            var myCustomValue: String // comment {
                 get {
                     self[MyEnvironmentKey.self]
                 }
diff --git a/Examples/Tests/MacroExamples/Implementation/ComplexMacros/ObservableMacroTests.swift b/Examples/Tests/MacroExamples/Implementation/ComplexMacros/ObservableMacroTests.swift
index 183cbb53..4b293a36 100644
--- a/Examples/Tests/MacroExamples/Implementation/ComplexMacros/ObservableMacroTests.swift
+++ b/Examples/Tests/MacroExamples/Implementation/ComplexMacros/ObservableMacroTests.swift
@@ -26,8 +26,8 @@ final class ObservableMacroTests: XCTestCase {
       """
       @Observable
       final class Dog {
-        var name: String?
-        var treat: Treat?
+        var name: String? // comment
+        var treat: Treat? // comment
 
         var isHappy: Bool = true
 
@@ -40,7 +40,7 @@ final class ObservableMacroTests: XCTestCase {
       """,
       expandedSource: #"""
         final class Dog {
-          var name: String? {
+          var name: String? // comment {
             get {
               _registrar.beginAccess(\.name)
               defer {
@@ -58,7 +58,7 @@ final class ObservableMacroTests: XCTestCase {
               _storage.name = newValue
             }
           }
-          var treat: Treat? {
+          var treat: Treat? // comment {
             get {
               _registrar.beginAccess(\.treat)
               defer {
@@ -122,8 +122,8 @@ final class ObservableMacroTests: XCTestCase {
 
           private struct Storage {
 
-            var name: String?
-            var treat: Treat?
+            var name: String? // comment
+            var treat: Treat? // comment
 
             var isHappy: Bool = true
           }

OK, thanks for confirming.