swiftlang/swift-syntax

FunctionDeclSyntax with TokenSyntax.binaryOperator leads to different a trailing trivia space on 5.10 and 6.0.

vanvoorden opened this issue · 2 comments

Description

I'm seeing some different behavior between 5.10 and 6.0 when declaring a function with a TokenSyntax.binaryOperator.

Here is a macro:

DeclSyntax(
  FunctionDeclSyntax(
    modifiers: [DeclModifierSyntax(name: .keyword(.public)), DeclModifierSyntax(name: .keyword(.static))],
    name: TokenSyntax.binaryOperator("=="),
    signature: FunctionSignatureSyntax(
      parameterClause: FunctionParameterClauseSyntax(
        parameters: FunctionParameterListSyntax {
          FunctionParameterSyntax(firstName: TokenSyntax.identifier("lhs"), colon: .colonToken(), type: TypeSyntax("String"))
          FunctionParameterSyntax(firstName: TokenSyntax.identifier("rhs"), colon: .colonToken(), type: TypeSyntax("String"))
        }
      ),
      returnClause: ReturnClauseSyntax(
        type: IdentifierTypeSyntax(name: TokenSyntax.identifier("Bool"))
      )
    ),
    bodyBuilder: {
      ReturnStmtSyntax(
        expression: SequenceExprSyntax(
          elements: ExprListSyntax {
            DeclReferenceExprSyntax(baseName: .identifier("lhs"))
            BinaryOperatorExprSyntax(operator: .binaryOperator("<"))
            DeclReferenceExprSyntax(baseName: .identifier("rhs"))
          }
        )
      )
    }
  )
)

Here is the 5.10 expansion:

public static func == (lhs: String, rhs: String) -> Bool {
    return lhs < rhs
}

Here is the 6.0 (and main) expansion:

public static func ==(lhs: String, rhs: String) -> Bool {
    return lhs < rhs
}

Any ideas about that different syntax (the missing space) between the two versions? My belief here is that the Swift convention for a function like this (a static function on a binary operator) should come with a space before the round brace. This was the 5.10 behavior.

Should we consider the 6.0 behavior to be the "correct" behavior of this macro? Should we consider the 5.10 behavior to have been a bug? Should we consider the 5.10 behavior to have been correct and the 6.0 behavior is just a temporary regression that we should fix before release? Any more ideas about that? Thanks!

Steps to Reproduce

Here is a diff on swift-syntax to repro:

diff --git a/Tests/SwiftSyntaxBuilderTest/FunctionTests.swift b/Tests/SwiftSyntaxBuilderTest/FunctionTests.swift
index c18f22f6..3ad61eeb 100644
--- a/Tests/SwiftSyntaxBuilderTest/FunctionTests.swift
+++ b/Tests/SwiftSyntaxBuilderTest/FunctionTests.swift
@@ -208,7 +208,7 @@ final class FunctionTests: XCTestCase {
         DeclSyntax(
           FunctionDeclSyntax(
             modifiers: [DeclModifierSyntax(name: .keyword(.public)), DeclModifierSyntax(name: .keyword(.static))],
-            name: TokenSyntax.identifier("=="),
+            name: TokenSyntax.binaryOperator("=="),
             signature: FunctionSignatureSyntax(
               parameterClause: FunctionParameterClauseSyntax(
                 parameters: FunctionParameterListSyntax {
@@ -234,7 +234,7 @@ final class FunctionTests: XCTestCase {
           )
         ),
         """
-        public static func ==(lhs: String, rhs: String) -> Bool {
+        public static func == (lhs: String, rhs: String) -> Bool {
             return lhs < rhs
         }
         """

This change is intentional and not having a space between the operator name and the ( is the intended behavior. This was changed by #2595 in release/6.0 and #2592 on main.

@ahoppen Sounds good. Thanks!