From 0d1e482cfe2d943f8ea86cf3735c6756b7d4aac7 Mon Sep 17 00:00:00 2001 From: Zev Eisenberg Date: Thu, 11 Sep 2025 10:34:28 -0400 Subject: [PATCH 1/2] [Observation] fix bug where a comment in an annotation would result in invalid macro expansion. The comment was 'eating' the generated variable declaration, preventing it from being visible to the compiler. --- lib/Macros/Sources/ObservationMacros/ObservableMacro.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Macros/Sources/ObservationMacros/ObservableMacro.swift b/lib/Macros/Sources/ObservationMacros/ObservableMacro.swift index 585a71c23395d..29afa2fa65a63 100644 --- a/lib/Macros/Sources/ObservationMacros/ObservableMacro.swift +++ b/lib/Macros/Sources/ObservationMacros/ObservableMacro.swift @@ -283,7 +283,7 @@ extension VariableDeclSyntax { leadingTrivia: leadingTrivia, attributes: newAttributes, modifiers: modifiers.privatePrefixed(prefix, in: context), - bindingSpecifier: TokenSyntax(bindingSpecifier.tokenKind, leadingTrivia: .space, trailingTrivia: .space, presence: .present), + bindingSpecifier: TokenSyntax(bindingSpecifier.tokenKind, leadingTrivia: .newline, trailingTrivia: .space, presence: .present), bindings: bindings.privatePrefixed(prefix, in: context), trailingTrivia: trailingTrivia ) From 21b57f785679308ec9e9ca16f6a2a3521e336cee Mon Sep 17 00:00:00 2001 From: Jamie <2119834+jamieQ@users.noreply.github.com> Date: Fri, 10 Oct 2025 09:19:18 -0500 Subject: [PATCH 2/2] [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. --- .../ObservationMacros/ObservableMacro.swift | 21 +++++- ...servation_macro_expansion_observable.swift | 69 +++++++++++++++++++ 2 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 test/stdlib/Observation/Macros/observation_macro_expansion_observable.swift diff --git a/lib/Macros/Sources/ObservationMacros/ObservableMacro.swift b/lib/Macros/Sources/ObservationMacros/ObservableMacro.swift index 29afa2fa65a63..8760edffce2ca 100644 --- a/lib/Macros/Sources/ObservationMacros/ObservableMacro.swift +++ b/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) -> 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 ) diff --git a/test/stdlib/Observation/Macros/observation_macro_expansion_observable.swift b/test/stdlib/Observation/Macros/observation_macro_expansion_observable.swift new file mode 100644 index 0000000000000..c99c0aae04fee --- /dev/null +++ b/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