Macro expansion sometimes adds leading spaces inside string literals

[tgoyne]

Description

struct Names: MemberMacro {
    public static func expansion(
        of node: AttributeSyntax,
        providingMembersOf declaration: some DeclGroupSyntax,
        in context: some MacroExpansionContext
    ) throws -> [DeclSyntax] {
        guard let declaration = declaration.as(ClassDeclSyntax.self) else { return [] }
        let className = declaration.identifier.trimmed
        let names = declaration.memberBlock.members.compactMap { (member) -> String? in
            guard let property = member.decl.as(VariableDeclSyntax.self),
                  let binding = property.bindings.first,
                  let identifier = binding.pattern.as(IdentifierPatternSyntax.self)
            else {
                return nil
            }
            return identifier.identifier.text
        }
        let body = names.map { name in
            ExprSyntax("\(literal: name): \\\(className).\(raw: name)")
        }
        return ["""
        public static var names: [String: AnyKeyPath] = [\(raw: body.map(\.trimmedDescription).joined(separator: ", "))]
        public static var names2: [String: AnyKeyPath] = [\(raw: body.map(\.trimmedDescription).joined(separator: ", "))]
        """]
    }
}
assertMacroExpansion(
    #"""
    @Names class Foo {
        var value1: Int
        var value2: Int
        var value3: Int
    }
    """#,
    expandedSource: #"""
    @Names class Foo {
        var value1: Int
        var value2: Int
        var value3: Int
        public static var names: [String: AnyKeyPath] = ["value1": \Foo.value1, "value2": \Foo.value2, "value3": \Foo.value3]
        public static var names2: [String: AnyKeyPath] = ["value1": \Foo.value1, "value2": \Foo.value2, "value3": \Foo.value3]
    }
    """#,
    macros: [
        "Names": Names.self,
    ])

The actual result of this macro is:

@Names class Foo {
    var value1: Int
    var value2: Int
    var value3: Int
    public static var names: [String: AnyKeyPath] = ["value1": \Foo.value1, "value2": \Foo.value2, "value3": \Foo.value3]
    public static var names2: [String: AnyKeyPath] = [" value1": \Foo.value1, " value2": \Foo.value2, " value3": \Foo.value3]
}

Somehow each of the string literals in names2 has add a space added to the beginning even though it is generated by the exact same code as names, which does not have the spaces.

Steps to Reproduce

No response

[ahoppen]

Tracked in Apple’s issue tracker as rdar://110797352

[ahoppen]

Looking at the code, there are two issues:

You are creating an ExprSyntax for "\(literal: name): \\\(className).\(raw: name)" but that is not actually a valid expression. I would suggest that you create the DictionaryExprSyntax as in the following

https://github.com/apple/swift-syntax/blob/338a626b87d5386a42c9f1b4edb0c9f97cb2bae7/Tests/SwiftSyntaxBuilderTest/DictionaryExprTests.swift#L19-L23

The second issue is that you are returning a single DeclSyntax node that contains both the names and names2 declaration. Those should be two separate nodes, i.e. you probably want the following where you create namesDict using the builder syntax above.

return [
  "public static var names: [String: AnyKeyPath] = \(namesDict)",
  "public static var names2: [String: AnyKeyPath] = \(namesDict)"
]

[tgoyne]

The first is just a not quite valid bit of reduction to a simple test case; the original thing did have an expression there and the same thing happens.

Incorrectly returning multiple declarations in one DeclSyntax node is indeed a problem in the original code too. This is an extremely weird way for it to break, though. I'm really not sure how I'm supposed to know what is valid to do with various nodes if weird broken things is the expected result of invalid input given the lack of documentation.

[ahoppen]

Incorrectly returning multiple declarations in one DeclSyntax node is indeed a problem in the original code too. This is an extremely weird way for it to break, though. I'm really not sure how I'm supposed to know what is valid to do with various nodes if weird broken things is the expected result of invalid input given the lack of documentation.

We just merged #1785, which should emit an assertion failure from assertMacroExpansion, which will tell you that the second declaration is unexpected. If you could give this a try by depending on the release/5.9 branch of SwiftSyntax and let us know if that would have helped you find the issue, that would be really appreciated.

[tgoyne]

Oh, that's great. That should totally fix the problem of not being sure if what I'm generating is correct or just appears to work, and it's also caught a few other places where I was doing something invalid.

I also poked a little more into figuring out where exactly the space was coming from and it's specifically the formatter trying to add a space after the end of string literals. Since the AST is invalid it ends up doing that at the start of string literals too, which makes sense as a failure case.

[ahoppen]

Also, just for your information, I’m thinking about adding error messages to the log when an invalid syntax node is being created. #1790. Do you think such an error message like I mentioned in #1792 (comment) would have directly pointed you to the issue where the incorrect node was being created?

[tgoyne]

Yeah, that sounds very useful. Using OSLog also indirectly helps with the problem of Xcode swallowing output from compiler plugins.