Skip to content

Commit 3477df6

Browse files
committed
Address review comments
1 parent a079352 commit 3477df6

File tree

8 files changed

+76
-38
lines changed

8 files changed

+76
-38
lines changed

CodeGeneration/Sources/SyntaxSupport/DeclNodes.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public let DECL_NODES: [Node] = [
4646
kind: .accessorBlock,
4747
base: .syntax,
4848
nameForDiagnostics: nil,
49-
parserFunction: "parseGetSet",
49+
parserFunction: "parseAccessorBlock",
5050
traits: [
5151
"Braced"
5252
],

Sources/SwiftParser/CollectionNodes+Parsable.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,8 @@ extension MemberBlockItemListSyntax: SyntaxParseable {
119119

120120
//==========================================================================//
121121
// IMPORTANT: When adding a new conformance, also add a corrsponding //
122-
// conformance to SyntaxExpressibleByStringInterpolation. //
122+
// conformance to SyntaxExpressibleByStringInterpolation in //
123+
// SyntaxExpressibleByStringInterpolationConformances.swift //
123124
//==========================================================================//
124125

125126
//==========================================================================//

Sources/SwiftParser/Declarations.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1151,7 +1151,7 @@ extension Parser {
11511151
// Parse getter and setter.
11521152
let accessor: RawAccessorBlockSyntax?
11531153
if self.at(.leftBrace) || self.at(anyIn: AccessorDeclSyntax.AccessorSpecifierOptions.self) != nil {
1154-
accessor = self.parseGetSet()
1154+
accessor = self.parseAccessorBlock()
11551155
} else {
11561156
accessor = nil
11571157
}
@@ -1265,7 +1265,7 @@ extension Parser {
12651265

12661266
let accessors: RawAccessorBlockSyntax?
12671267
if self.at(.leftBrace) || (inMemberDeclList && self.at(anyIn: AccessorDeclSyntax.AccessorSpecifierOptions.self) != nil && !self.at(.keyword(.`init`))) {
1268-
accessors = self.parseGetSet()
1268+
accessors = self.parseAccessorBlock()
12691269
} else {
12701270
accessors = nil
12711271
}
@@ -1408,7 +1408,7 @@ extension Parser {
14081408

14091409
/// Parse the body of a variable declaration. This can include explicit
14101410
/// getters, setters, and observers, or the body of a computed property.
1411-
mutating func parseGetSet() -> RawAccessorBlockSyntax {
1411+
mutating func parseAccessorBlock() -> RawAccessorBlockSyntax {
14121412
// Parse getter and setter.
14131413
let unexpectedBeforeLBrace: RawUnexpectedNodesSyntax?
14141414
let lbrace: RawTokenSyntax

Sources/SwiftParser/generated/LayoutNodes+Parsable.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ extension AccessorBlockSyntax: SyntaxParseable {
3030
withExtendedLifetime(parser) {
3131
}
3232
}
33-
let node = parser.parseGetSet()
33+
let node = parser.parseAccessorBlock()
3434
let raw = RawSyntax(parser.parseRemainder(into: node))
3535
return Syntax(raw: raw, rawNodeArena: raw.arena).cast(Self.self)
3636
}

Sources/SwiftSyntaxMacroExpansion/IndentationUtils.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ extension SyntaxProtocol {
2727
// MARK: String.indented
2828

2929
extension String {
30-
/// Indents every new line in this string literal by `indentation`.
30+
/// Indents every new line in this string by `indentation`.
3131
///
3232
/// - Note: The first line in the string gets indented as well.
3333
func indented(by indentation: Trivia) -> String {

Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ public func collapse<Node: SyntaxProtocol>(
403403
}
404404

405405
var expansions = expansions
406-
var separator = "\n"
406+
var separator = "\n\n"
407407

408408
switch role {
409409
case .accessor:
@@ -427,6 +427,7 @@ public func collapse<Node: SyntaxProtocol>(
427427
expansions = expansions.map({ $0.indented(by: indentationWidth ?? .spaces(4)) })
428428
expansions[0] = "{\n" + expansions[0]
429429
expansions[expansions.count - 1] += "\n}"
430+
separator = "\n"
430431
}
431432
case .memberAttribute:
432433
separator = " "

Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -263,9 +263,11 @@ private func expandAccessorMacroWithoutExistingAccessors(
263263
return nil
264264
}
265265

266-
// `expandAttachedMacro` adds the `{` and `}` to wrap the accessor block.
267-
// Since the entire block is indented, we start it on a new line.
268-
let indentedSource = "\n" + expanded.indented(by: attachedTo.indentationOfFirstLine)
266+
// `expandAttachedMacro` adds the `{` and `}` to wrap the accessor block and
267+
// then indents it.
268+
// Remove any indentaiton from the first line using `drop(while:)` and then
269+
// prepend a space to separate it from the variable declaration
270+
let indentedSource = " " + expanded.indented(by: attachedTo.indentationOfFirstLine).drop(while: { $0.isWhitespace })
269271
return "\(raw: indentedSource)"
270272
}
271273

@@ -434,14 +436,14 @@ private enum MacroApplicationError: DiagnosticMessage, Error {
434436
switch self {
435437
case .accessorMacroOnVariableWithMultipleBindings:
436438
return """
437-
swift-syntax applies macros syntactically but there is no way to represent a variable \
439+
swift-syntax applies macros syntactically and there is no way to represent a variable \
438440
declaration with multiple bindings that have accessors syntactically. \
439441
While the compiler allows this expansion, swift-syntax cannot represent it and thus \
440442
disallows it.
441443
"""
442444
case .malformedAccessor:
443445
return """
444-
Macro returned a mal-formed accessor. Accessors should start with an introducer like 'get' or 'set'.
446+
Macro returned a malformed accessor. Accessors should start with an introducer like 'get' or 'set'.
445447
"""
446448
}
447449
}
@@ -452,8 +454,8 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
452454
let macroSystem: MacroSystem
453455
var context: Context
454456
var indentationWidth: Trivia
455-
/// Nodes that we have already handled in `visitAny` and that should be visited
456-
/// in the node-specific handling function.
457+
/// Nodes that we are currently handling in `visitAny` and that should be
458+
/// visited using the node-specific handling function.
457459
var skipVisitAnyHandling: Set<Syntax> = []
458460

459461
/// Store expanded extension while visiting member decls. This should be
@@ -481,8 +483,8 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
481483

482484
// Expand 'MacroExpansionExpr'.
483485
// Note that 'MacroExpansionExpr'/'MacroExpansionExprDecl' at code item
484-
// position are handled by 'visit(_:CodeBlockItemListSyntax)'. Here's only
485-
// expression inside other syntax node.
486+
// position are handled by 'visit(_:CodeBlockItemListSyntax)'.
487+
// Only expression expansions inside other syntax nodes is handled here.
486488
if let expanded = expandExpr(node: node) {
487489
return Syntax(visit(expanded))
488490
}
@@ -632,7 +634,7 @@ extension MacroApplication {
632634

633635
return attributes.compactMap {
634636
guard case let .attribute(attribute) = $0,
635-
let attributeName = attribute.attributeName.as(IdentifierTypeSyntax.self)?.name.text,
637+
let attributeName = attribute.attributeName.as(IdentifierTypeSyntax.self)?.name.text,
636638
let macro = macroSystem.lookup(attributeName)
637639
else {
638640
return nil
@@ -673,13 +675,13 @@ extension MacroApplication {
673675
) -> [ExpandedNode] {
674676
var result: [ExpandedNode] = []
675677

676-
for peerMacro in macroAttributes(attachedTo: decl, ofType: ofType) {
678+
for macroAttribute in macroAttributes(attachedTo: decl, ofType: ofType) {
677679
do {
678-
if let expanded = try expandMacro(peerMacro.attributeNode, peerMacro.definition) {
680+
if let expanded = try expandMacro(macroAttribute.attributeNode, macroAttribute.definition) {
679681
result += expanded
680682
}
681683
} catch {
682-
context.addDiagnostics(from: error, node: peerMacro.attributeNode)
684+
context.addDiagnostics(from: error, node: macroAttribute.attributeNode)
683685
}
684686
}
685687
return result

Tests/SwiftSyntaxMacroExpansionTest/MacroSystemTests.swift

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -608,13 +608,10 @@ public struct UnwrapMacro: CodeItemMacro {
608608
}
609609
}
610610

611-
public struct DeclsFromStringsMacro: DeclarationMacro {
612-
public static func expansion(
613-
of node: some FreestandingMacroExpansionSyntax,
614-
in context: some MacroExpansionContext
615-
) throws -> [DeclSyntax] {
611+
public struct DeclsFromStringsMacro: DeclarationMacro, MemberMacro {
612+
private static func decls(from arguments: LabeledExprListSyntax) -> [DeclSyntax] {
616613
var strings: [String] = []
617-
for arg in node.argumentList {
614+
for arg in arguments {
618615
guard
619616
let value = arg.expression.as(StringLiteralExprSyntax.self)?.representedLiteralValue
620617
else {
@@ -627,6 +624,24 @@ public struct DeclsFromStringsMacro: DeclarationMacro {
627624
"\(raw: $0)"
628625
}
629626
}
627+
628+
public static func expansion(
629+
of node: some FreestandingMacroExpansionSyntax,
630+
in context: some MacroExpansionContext
631+
) throws -> [DeclSyntax] {
632+
return decls(from: node.argumentList)
633+
}
634+
635+
public static func expansion(
636+
of node: AttributeSyntax,
637+
providingMembersOf declaration: some DeclGroupSyntax,
638+
in context: some MacroExpansionContext
639+
) throws -> [DeclSyntax] {
640+
guard case .argumentList(let arguments) = node.arguments else {
641+
return []
642+
}
643+
return decls(from: arguments)
644+
}
630645
}
631646

632647
public struct SendableConformanceMacro: ConformanceMacro {
@@ -857,8 +872,7 @@ final class MacroSystemTests: XCTestCase {
857872
var x: Int
858873
""",
859874
expandedSource: """
860-
var x: Int
861-
{
875+
var x: Int {
862876
get {
863877
_x.wrappedValue
864878
}
@@ -958,8 +972,7 @@ final class MacroSystemTests: XCTestCase {
958972
""",
959973
expandedSource: """
960974
struct Foo {
961-
subscript() -> Int
962-
{
975+
subscript() -> Int {
963976
get {
964977
return 1
965978
}
@@ -1064,7 +1077,7 @@ final class MacroSystemTests: XCTestCase {
10641077
diagnostics: [
10651078
DiagnosticSpec(
10661079
message:
1067-
"swift-syntax applies macros syntactically but there is no way to represent a variable declaration with multiple bindings that have accessors syntactically. While the compiler allows this expansion, swift-syntax cannot represent it and thus disallows it.",
1080+
"swift-syntax applies macros syntactically and there is no way to represent a variable declaration with multiple bindings that have accessors syntactically. While the compiler allows this expansion, swift-syntax cannot represent it and thus disallows it.",
10681081
line: 1,
10691082
column: 1,
10701083
severity: .error
@@ -1083,8 +1096,7 @@ final class MacroSystemTests: XCTestCase {
10831096
var x: Int
10841097
""",
10851098
expandedSource: """
1086-
var x: Int
1087-
{
1099+
var x: Int {
10881100
get {
10891101
return 1
10901102
}
@@ -1249,17 +1261,15 @@ final class MacroSystemTests: XCTestCase {
12491261
expandedSource: """
12501262
12511263
struct Point {
1252-
var x: Int
1253-
{
1264+
var x: Int {
12541265
get {
12551266
_storage[wrappedKeyPath: \\.x]
12561267
}
12571268
set {
12581269
_storage[wrappedKeyPath: \\.x] = newValue
12591270
}
12601271
}
1261-
var y: Int
1262-
{
1272+
var y: Int {
12631273
get {
12641274
_storage[wrappedKeyPath: \\.y]
12651275
}
@@ -1417,6 +1427,30 @@ final class MacroSystemTests: XCTestCase {
14171427
)
14181428
}
14191429

1430+
func testMemberDeclsFromStringLiterals() {
1431+
assertMacroExpansion(
1432+
"""
1433+
@decls("func foo() {}", "func bar() {}"
1434+
struct Foo {
1435+
var member: Int
1436+
}
1437+
""",
1438+
expandedSource: """
1439+
struct Foo {
1440+
var member: Int
1441+
1442+
func foo() {
1443+
}
1444+
1445+
func bar() {
1446+
}
1447+
}
1448+
""",
1449+
macros: ["decls": DeclsFromStringsMacro.self],
1450+
indentationWidth: indentationWidth
1451+
)
1452+
}
1453+
14201454
func testIndentationOfMultipleModifiers() {
14211455
assertMacroExpansion(
14221456
"""

0 commit comments

Comments
 (0)