Skip to content

Commit

Permalink
MacroSystem should remove the initializer for an accessor macro
Browse files Browse the repository at this point in the history
SE-0389 specifies that a macro returning either a getter or setter
should remove the initializer, if one exists.

Resolves rdar://117442713 (swiftlang#2310)
  • Loading branch information
bnbarham committed Oct 27, 2023
1 parent ebd7026 commit 9197eb4
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 17 deletions.
60 changes: 43 additions & 17 deletions Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ private func expandAccessorMacroWithExistingAccessors(
return nil
}

// Separate the accessor from any existing accessors by two spaces
// Separate the accessor from any existing accessors by an empty line
let indentedSource = "\n" + expanded.indented(by: attachedTo.indentationOfFirstLine + indentationWidth)
return "\(raw: indentedSource)"
}
Expand Down Expand Up @@ -702,13 +702,24 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
context.addDiagnostics(from: MacroApplicationError.accessorMacroOnVariableWithMultipleBindings, node: node)
return DeclSyntax(node)
}
node.bindings[node.bindings.startIndex].accessorBlock = expandAccessors(of: node, existingAccessors: binding.accessorBlock)


let expansion = expandAccessors(of: node, existingAccessors: binding.accessorBlock)
if expansion.accessors != binding.accessorBlock {
if binding.initializer != nil, expansion.expandsGetSet {
node.bindings[node.bindings.startIndex].accessorBlock = expansion.accessors?.with(\.leadingTrivia, [])
node.bindings[node.bindings.startIndex].initializer = nil
} else {
node.bindings[node.bindings.startIndex].accessorBlock = expansion.accessors
}
}

return DeclSyntax(node)
}

override func visit(_ node: SubscriptDeclSyntax) -> DeclSyntax {
var node = super.visit(node).cast(SubscriptDeclSyntax.self)
node.accessorBlock = expandAccessors(of: node, existingAccessors: node.accessorBlock)
node.accessorBlock = expandAccessors(of: node, existingAccessors: node.accessorBlock).accessors
return DeclSyntax(node)
}
}
Expand Down Expand Up @@ -873,10 +884,15 @@ extension MacroApplication {
/// node.
///
/// - Returns: The storage node with all macro-synthesized accessors applied.
private func expandAccessors(of storage: some DeclSyntaxProtocol, existingAccessors: AccessorBlockSyntax?) -> AccessorBlockSyntax? {
private func expandAccessors(of storage: some DeclSyntaxProtocol, existingAccessors: AccessorBlockSyntax?) -> (accessors: AccessorBlockSyntax?, expandsGetSet: Bool) {
let accessorMacros = macroAttributes(attachedTo: DeclSyntax(storage), ofType: AccessorMacro.Type.self)

var newAccessorsBlock = existingAccessors
var expandsGetSet: Bool = false
func checkExpansions(_ accessors: AccessorDeclListSyntax?) {
guard let accessors else { return }
expandsGetSet = expandsGetSet || accessors.contains(where: \.isGetOrSet)
}

for macro in accessorMacros {
do {
Expand All @@ -894,6 +910,8 @@ extension MacroApplication {
in: context,
indentationWidth: indentationWidth
) {
checkExpansions(newAccessors)

// If existingAccessors is not `nil`, then we also set
// `newAccessorBlock` above to a a non-nil value, so
// `newAccessorsBlock` also isn’t `nil`.
Expand All @@ -902,20 +920,22 @@ extension MacroApplication {
indentationWidth: self.indentationWidth
)
}
} else {
let newAccessors = try expandAccessorMacroWithoutExistingAccessors(
definition: macro.definition,
attributeNode: macro.attributeNode,
attachedTo: DeclSyntax(storage),
in: context,
indentationWidth: indentationWidth
)
} else if let newAccessors = try expandAccessorMacroWithoutExistingAccessors(
definition: macro.definition,
attributeNode: macro.attributeNode,
attachedTo: DeclSyntax(storage),
in: context,
indentationWidth: indentationWidth
) {
guard case .accessors(let accessorList) = newAccessors.accessors else {
throw MacroApplicationError.malformedAccessor
}

checkExpansions(accessorList)

if newAccessorsBlock == nil {
newAccessorsBlock = newAccessors
} else if let newAccessors = newAccessors {
guard case .accessors(let accessorList) = newAccessors.accessors else {
throw MacroApplicationError.malformedAccessor
}
} else {
newAccessorsBlock = newAccessorsBlock!.addingAccessors(
from: accessorList,
indentationWidth: self.indentationWidth
Expand All @@ -926,7 +946,7 @@ extension MacroApplication {
context.addDiagnostics(from: error, node: macro.attributeNode)
}
}
return newAccessorsBlock
return (newAccessorsBlock, expandsGetSet)
}
}

Expand Down Expand Up @@ -1130,3 +1150,9 @@ private extension AttributeSyntax {
return (detach(in: context, foldingWith: operatorTable) as Syntax).cast(Self.self)
}
}

private extension AccessorDeclSyntax {
var isGetOrSet: Bool {
return accessorSpecifier.tokenKind == .keyword(.get) || accessorSpecifier.tokenKind == .keyword(.set)
}
}
91 changes: 91 additions & 0 deletions Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -320,4 +320,95 @@ final class AccessorMacroTests: XCTestCase {
macros: ["Test": TestMacro.self]
)
}

func testInitializerRemovedForGetSet() {
assertMacroExpansion(
"""
@constantOne
var x: Int = 1
""",
expandedSource: """
var x: Int {
get {
return 1
}
}
""",
macros: ["constantOne": ConstantOneGetter.self],
indentationWidth: indentationWidth
)

// Bit of an odd case, compiler has the type but we don't know it in `MacroSystem`
assertMacroExpansion(
"""
@constantOne
var x = 1
""",
expandedSource: """
var x {
get {
return 1
}
}
""",
macros: ["constantOne": ConstantOneGetter.self],
indentationWidth: indentationWidth
)
}

func testInitializerRemainsForObserver() {
struct DidSetAdder: AccessorMacro {
static func expansion(
of node: AttributeSyntax,
providingAccessorsOf declaration: some DeclSyntaxProtocol,
in context: some MacroExpansionContext
) throws -> [AccessorDeclSyntax] {
return [
"""
didSet {
}
"""
]
}
}

assertMacroExpansion(
"""
@addDidSet
var x = 1
""",
expandedSource: """
var x = 1 {
didSet {
}
}
""",
macros: ["addDidSet": DidSetAdder.self],
indentationWidth: indentationWidth
)

// Invalid semantically, but we shouldn't remove the initializer as the
// macro did not produce a getter/setter
assertMacroExpansion(
"""
@addDidSet
var x = 1 {
get {
return 1
}
}
""",
expandedSource: """
var x = 1 {
get {
return 1
}
didSet {
}
}
""",
macros: ["addDidSet": DidSetAdder.self],
indentationWidth: indentationWidth
)
}
}

0 comments on commit 9197eb4

Please sign in to comment.