Required spacing is not added in `SwiftSyntaxBuilder` for interpolated syntax node

[stevapple]

Description

For syntax nodes like FunctionEffectSpecifiersSyntax and FunctionParameterSyntax, some spacings are required internally. However, if a handcraft node is interpolated into another expression string, SwiftSyntaxBuilder fails to add these spacing, causing the parser to fail and the builder result being wrong.

Steps to Reproduce

let functionPrameterClause = FunctionParameterClauseSyntax {
  FunctionParameterSyntax(firstName: "_", secondName: "name", type: TypeSyntax("String"))
  FunctionParameterSyntax(firstName: "description", type: TypeSyntax("String"))
}
let functionSignature = FunctionSignatureSyntax(
  parameterClause: functionPrameterClause,
  effectSpecifiers: .init(asyncSpecifier: .keyword(.async), throwsSpecifier: .keyword(.throws))
)
assertBuildResult(functionSignature, "(_ name: String, description: String) async throws") // OK
let initializerDecl = InitializerDeclSyntax(signature: functionSignature) {
  "self.name = name"
  "self.description = description"
}
assertBuildResult(
  initializerDecl,
  """
  init(_ name: String, description: String) async throws {
      self.name = name
      self.description = description
  }
  """
) // OK
let initializerDeclFromString = try InitializerDeclSyntax("init\(functionSignature)") {
  "self.name = name"
  "self.description = description"
}
assertBuildResult(
  initializerDeclFromString,
  """
  init(_ name: String, description: String) async throws {
      self.name = name
      self.description = description
  }
  """
) // Error

Outputs:

xctest [ParseError] Parsing a `DeclSyntax` node from string interpolation produced the following parsing errors.
Set a breakpoint in `SyntaxParseable.logStringInterpolationParsingError()` to debug the failure.
1 │ init(_name:String,description:String)asyncthrows {}
  │                                      ╰─ error: unexpected code 'asyncthrows' in initializer
error: Test failed - Actual output (+) differed from expected output (-):
–init(_ name: String, description: String) async throws {
+init(_name: String, description: String) {
     self.name = name
     self.description = description
 }

[stevapple]

CC @ahoppen, this is a real bug I believe and I think we don't want it to get in the 509.0.0 release...

[ahoppen]

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

[ahoppen]

At the time that a syntax node is inserted into string interpolation, it will get re-parsed instead of being re-used as is, which is causing the behavior that you are seeing.

This behavior is currently inherent to the entire design of string interpolation and there’s no easy way to fix it. We would somehow need to inject the existing node (in your case FunctionSignatureSyntax) into the parser so that it’ll re-use instead of creating a new node from the input source. This essentially means that we would need to support incremental parsing for every single syntax node and hook up incremental parsing with string interpolation. But for performance reasons that’s not something we’re likely to be able to do since the incremental parse lookup comes at a cost.

So, for the foreseeable future, you will need to either format functionSignature before using it in string interpolation or insert the space after async and throws while creating FunctionSignatureSyntax.

[stevapple]

I guess there may be some simpler workarounds, eg:

1. Mark if a node is hand-crafted or not, and insert necessary whitespaces during string interpolation.
2. For all public initializers, try to ensure the freshly created syntax is valid (by default).

Wdyt?

[ahoppen]

I guess there may be some simpler workarounds, eg:

1. Mark if a node is hand-crafted or not, and insert necessary whitespaces during string interpolation.

We could theoretically format all syntax nodes before adding them to string interpolation but I believe that that will cause more issues than it solves. For example, the formatting might add newlines or indentation but you wanted to hand-craft the syntax tree and control all the trivia yourself. That’s why formatting is an explicit action and not something that’s being done implicitly.

2. For all public initializers, try to ensure the freshly created syntax is valid (by default).

There are situations where you want to create invalid syntax nodes. In your example alone, the following code is perfectly valid with the expectation that the missing spaces are inserted by .formatted

let functionPrameterClause = FunctionParameterClauseSyntax {
  FunctionParameterSyntax(firstName: "_", secondName: "name", type: TypeSyntax("String"))
  FunctionParameterSyntax(firstName: "description", type: TypeSyntax("String"))
}

[stevapple]

At the time that a syntax node is inserted into string interpolation, it will get re-parsed instead of being re-used as is, which is causing the behavior that you are seeing.

I think this is somewhat confusing given we require string interpolation in syntax to be marked as raw:. With no explicit label it’s natural to expect that syntax nodes will be used as-is (instead of being re-parsed to and from a string).

[ahoppen]

I absolutely agree that the better behavior would be to re-use the nodes instead of re-parsing them. But as I mentioned above, the only way of fixing that would be a huge undertaking in the parser with significant implications for parsing performance.