[fix][Observation]: further attempts to resolve macro expansion

interaction with comments

Adds logic to insert newlines in various places to try and resolve the
fact that the current expansion produces invalid code in some cases
depending on comment location. Adds some basic tests of the expansion
output.

Author: jamieQ

lib/Macros/Sources/ObservationMacros/ObservableMacro.swift

@@ -272,18 +272,33 @@ extension PatternBindingListSyntax {
 
 extension VariableDeclSyntax {
   func privatePrefixed(_ prefix: String, addingAttribute attribute: AttributeSyntax, removingAttribute toRemove: AttributeSyntax, in context: LocalMacroExpansionContext<some MacroExpansionContext>) -> VariableDeclSyntax {
+    var newAttribute = attribute
+    newAttribute.leadingTrivia = .newline
+
     let newAttributes = attributes.filter { attribute in
       switch attribute {
       case .attribute(let attr):
         attr.attributeName.identifier != toRemove.attributeName.identifier
       default: true
       }
-    } + [.attribute(attribute)]
+    } + [.attribute(newAttribute)]
+
+    var newModifiers = modifiers.privatePrefixed(prefix, in: context)
+    let hasModifiers = !newModifiers.isEmpty
+    if hasModifiers {
+      newModifiers.leadingTrivia += .newline
+    }
+
     return VariableDeclSyntax(
       leadingTrivia: leadingTrivia,
       attributes: newAttributes,
-      modifiers: modifiers.privatePrefixed(prefix, in: context),
-      bindingSpecifier: TokenSyntax(bindingSpecifier.tokenKind, leadingTrivia: .newline, trailingTrivia: .space, presence: .present),
+      modifiers: newModifiers,
+      bindingSpecifier: TokenSyntax(
+        bindingSpecifier.tokenKind,
+        leadingTrivia: hasModifiers ? .space : .newline,
+        trailingTrivia: .space,
+        presence: .present
+      ),
       bindings: bindings.privatePrefixed(prefix, in: context),
       trailingTrivia: trailingTrivia
     )

test/stdlib/Observation/Macros/observation_macro_expansion_observable.swift

@@ -0,0 +1,69 @@
+// REQUIRES: swift_swift_parser, asserts
+//
+// UNSUPPORTED: back_deploy_concurrency
+// REQUIRES: concurrency
+// REQUIRES: observation
+//
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t-scratch)
+
+// RUN: %target-swift-frontend -swift-version 5 -typecheck -plugin-path %swift-plugin-dir -I %t -dump-macro-expansions %s 2>&1 | %FileCheck %s --color
+
+import Observation
+
+// Test cases for comment handling with Observable macro
+_ = 0 // absorbs trivia so file check expectations don't leak into macro expansions
+
+@available(macOS 14.0, iOS 17.0, watchOS 10.0, tvOS 17.0, *)
+@Observable
+final class CommentAfterGlobalActorAnnotation {
+  @MainActor // Innocent comment
+  internal var it = 0
+}
+
+// CHECK-LABEL: @__swiftmacro{{.*}}CommentAfterGlobalActorAnnotation{{.*}}ObservationTracked{{.*}}.swift
+// CHECK:       @MainActor // Innocent comment
+// CHECK-NEXT:  @ObservationIgnored
+// CHECK-NEXT:  private var _it = 0
+_ = 0
+
+@available(macOS 14.0, iOS 17.0, watchOS 10.0, tvOS 17.0, *)
+@Observable
+final class CommentAfterAvailabilityAnnotation {
+  @available(*, deprecated) // Innocent comment
+  internal var it = 0
+}
+
+// CHECK-LABEL: @__swiftmacro{{.*}}CommentAfterAvailabilityAnnotation{{.*}}ObservationTracked{{.*}}.swift
+// CHECK-NEXT:  {{-+}}
+// CHECK-NEXT:  @available(*, deprecated) // Innocent comment
+// CHECK-NEXT:  @ObservationIgnored
+// CHECK-NEXT:  private  var _it  = 0
+_ = 0
+
+@available(macOS 14.0, iOS 17.0, watchOS 10.0, tvOS 17.0, *)
+@Observable
+final class CommentOnSameLineAsOtherAnnotation {
+  @MainActor /* Innocent comment */ public var it = 0
+}
+
+// CHECK-LABEL: @__swiftmacro{{.*}}CommentOnSameLineAsOtherAnnotation{{.*}}ObservationTracked{{.*}}.swift
+// CHECK:       @MainActor /* Innocent comment */
+// CHECK-NEXT:  @ObservationIgnored
+// CHECK-NEXT:  private var _it = 0
+_ = 0
+
+@available(macOS 14.0, iOS 17.0, watchOS 10.0, tvOS 17.0, *)
+@Observable
+final class CommentOnSameLineNoAnnotation {
+  /* Innocent comment */ public /*1*/ final /*2*/ var /*3*/ it /*4*/ = 0
+}
+
+// Note: seems there's some weirdness with the existing macro eating/duplicating trivia in some
+// cases but we'll just test for the current behavior since it doesn't seem to be covered elsewhere:
+
+// CHECK-LABEL: @__swiftmacro{{.*}}CommentOnSameLineNoAnnotation{{.*}}ObservationTracked{{.*}}.swift
+// CHECK:       /* Innocent comment */
+// CHECK-NEXT:  @ObservationIgnored
+// CHECK-NEXT:  private final /*2*/ var _it /*4*/ /*4*/ = 0
+_ = 0