Unexpected trivia when removing attributes in `MacroSystem`

[gohanlon]

Description

On 509.0.0 and main (8966ec7), I'm running into the following unexpected trivia handling in a peer macro:

fileprivate struct NoOpPeerMacro: PeerMacro {
  static func expansion(
    of node: AttributeSyntax,
    providingPeersOf declaration: some DeclSyntaxProtocol,
    in context: some MacroExpansionContext
  ) throws -> [DeclSyntax] {
    return []
  }
}

final class PeerMacroTests: XCTestCase {
  // …
  func testEmptyBeforeAttributeOnSameLineAsMemberVariable() {
    assertMacroExpansion(
      """
      struct Foo {
        @Test @State var x: Int
      }
      """,
      expandedSource: """
        struct Foo {
          @State var x: Int
        }
        """,
      macros: ["Test": NoOpPeerMacro.self]
    )
  }
  // …
}

Output:

–struct Foo {
–  @State var x: Int
+struct Foo {@State 
+  var x: Int
 }

Quick reference: AttributeRemover.

To work around, I can tailor the whitespace in my test in a specific manner as follows:

struct Foo {
  @Test
  @State 
  var x: Int
}

What is the expected trivia treatment in this case? Should trivia in test cases be deliberately crafted, or should the MacroSystem strive to normalize all syntactically valid trivia in some predictable way?

Aside: Here are a couple other tests and outputs that I found surprising:

Click to expand!

  func testAttributeOnOwnLineThenEmptyBeforeVariable() {
    assertMacroExpansion(
      """
      @State
      @Test var x: Int
      """,
      expandedSource: """
        @State
        var x: Int
        """,
      macros: [
        "Test": NoOpPeerMacro.self
      ]
    )
  }

Output:

–@State
–var x: Int
+@Statevar x: Int

---

  func testEmptyAfterAttributeOnSameLineAsMemberVariable() {
    assertMacroExpansion(
      """
      struct Foo {
        @State @Test var x: Int
      }
      """,
      expandedSource: """
        struct Foo {
          @State var x: Int
        }
        """,
      macros: ["Test": NoOpPeerMacro.self]
    )
  }

Output:

 struct Foo {
–  @State var x: Int
+  @State 
+  var x: Int
 }

---

  func testEmptyOnOwnLineThenAttributedMemberVariable() {
    assertMacroExpansion(
      """
      struct Foo {
        @Test
        @State var x: Int
      }
      """,
      expandedSource: """
        struct Foo {
          @State var x: Int
        }
        """,
      macros: ["Test": NoOpPeerMacro.self]
    )
  }

Output:

 struct Foo {
–  @State var x: Int
+  @State 
+  var x: Int
 }

Steps to Reproduce

Add the tests included in the description to "Tests/SwiftMacroExpansionTest/PeerMacroTests.swift" and run them.

[ahoppen]

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

[ahoppen]

What is the expected trivia treatment in this case? Should trivia in test cases be deliberately crafted, or should the MacroSystem strive to normalize all syntactically valid trivia in some predictable way?

MacroSystem should take care of adding the trivia and you shouldn’t need to worry about them in the macro itself, so this is a bug in AttributeRemover.

Would you like to try fixing it since it seems that you already looked into the code?

[gohanlon]

Sure! I’ll start with a draft PR featuring some test cases to help guide the discussion on the expected behavior.