`FunctionCallExprSyntax.addArgument` does not add separating comma between arguments

[bnickel]

Description

I have the following two functions that should produce identical results:

• asSyntaxSimple creates two different function expressions based on whether text has a value.
• asSyntaxAdvanced creates a function expression uses addArgument to add an argument if needed.

struct Node {
    var name: String?
    var text: String?
    
    func asSyntaxSimple() -> ExprSyntax {
        
        if let text = text {
            return ".init(\(literal: name), text: \(literal: text))"
        } else {
            return ".init(\(literal: name))"
        }
    }
    
    func asSyntaxAdvanced() -> ExprSyntax {
        
        var syntax = (".init(\(literal: name))" as ExprSyntax).cast(FunctionCallExprSyntax.self)
        
        if let text = text {
            syntax = syntax.addArgument(.init(label: "text", expression: StringLiteralExprSyntax(content: text)))
        }
        
        return syntax.cast(ExprSyntax.self)
    }
}

When testing with assertMacroExpansion, the function with addArgument fails to insert the comma between the existing argument and the new one. It similarly fails when calling addArgument multiple times.

Steps to Reproduce

Test the following code:

import XCTest
import SwiftSyntaxMacrosTestSupport
import SwiftSyntax
import SwiftSyntaxMacros

struct Node {
    var name: String?
    var text: String?
    
    func asSyntaxSimple() -> ExprSyntax {
        
        if let text = text {
            return ".init(\(literal: name), text: \(literal: text))"
        } else {
            return ".init(\(literal: name))"
        }
    }
    
    func asSyntaxAdvanced() -> ExprSyntax {
        
        var syntax = (".init(\(literal: name))" as ExprSyntax).cast(FunctionCallExprSyntax.self)
        
        if let text = text {
            syntax = syntax.addArgument(.init(label: "text", expression: StringLiteralExprSyntax(content: text)))
        }
        
        return syntax.cast(ExprSyntax.self)
    }
}

public struct NodeSimple: ExpressionMacro {
  public static func expansion(
    of node: some FreestandingMacroExpansionSyntax,
    in context: some MacroExpansionContext
  ) -> ExprSyntax {
      return Node(name: "NAME", text: "TEXT").asSyntaxSimple()
  }
}

public struct NodeAdvanced: ExpressionMacro {
  public static func expansion(
    of node: some FreestandingMacroExpansionSyntax,
    in context: some MacroExpansionContext
  ) -> ExprSyntax {
      return Node(name: "NAME", text: "TEXT").asSyntaxAdvanced()
  }
}

final class AddArgumentsTests: XCTestCase {
    
    func testSimple() throws {
        assertMacroExpansion(
            "#NodeSimple",
            expandedSource: #".init("NAME", text: "TEXT")"#,
            macros: [
                "NodeSimple": NodeSimple.self,
            ])
    }
    
    func testAdvanced() throws {
        assertMacroExpansion(
            "#NodeAdvanced",
            expandedSource: #".init("NAME", text: "TEXT")"#,
            macros: [
                "NodeAdvanced": NodeAdvanced.self,
            ])
    }
}

testAdvanced fails with the following error:

failed - Actual output (+) differed from expected output (-):
–.init("NAME", text: "TEXT")
+.init("NAME" text: "TEXT")

Actual expanded source:
.init("NAME" text: "TEXT")

[bnickel]

As an aside, the name should be addingArgument or similar since it isn't mutating.

[ahoppen]

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

[ahoppen]

I’ve wanted to deprecate the add* methods on the syntax nodes since they don’t fit into the current API design anymore in which you modify syntax nodes using the with function. That way you can also construct the new argument list using the result builder initializer, which takes care of inserting the comma to the previous argument.

So, the code I would suggest is the following

var functionCall = ExprSyntax("test(x: 1)").cast(FunctionCallExprSyntax.self)
functionCall = functionCall.with(\.arguments, TupleExprElementListSyntax {
  functionCall.arguments
  TupleExprElementSyntax(label: "y", expression: ExprSyntax("2"))
})
print(functionCall) // prints 'test(x: 1,y:2)'

The macro will take care of formatting the second argument and adding a space after , and :. If you wanted to do this manually, do

print(functionCall.formatted()) // prints 'test(x: 1, y: 2)'

[ahoppen]

I deprecated the addArgument in #1958. I agree that a function to add an element while ensuring that the previous element has a trailing comma would be useful. I filed #1989 with some ideas of how that should be done. Let’s use that issue to keep track of adding such an add method to SyntaxCollection because it has slightly more information of how an implementation could look like.