Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
vanvoorden opened this issue Apr 22, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@vanvoorden
Copy link

vanvoorden commented Apr 22, 2024

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(

@vanvoorden vanvoorden added the bug Something isn't working label Apr 22, 2024
@ahoppen
Copy link
Collaborator

ahoppen commented Apr 22, 2024

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

@ahoppen
Copy link
Collaborator

ahoppen commented Apr 24, 2024

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

@vanvoorden
Copy link
Author

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
           }

@ahoppen
Copy link
Collaborator

ahoppen commented Apr 25, 2024

OK, thanks for confirming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants