Merge pull request #84833 from jamieQ/obs-macro-ii

[Observation]: fix @observable macro producing invalid code with certain comments

Author: bnbarham

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: .space, 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