SwiftSyntax Syntax.withLeadingTrivia/withTrailingTrivia

swift-syntax
(David Hart) #1

Currently, in SwiftSyntax, the withLeadingTrivia and withTrailingTrivia are only available on SyntaxToken, which makes sense as trivia are attached to tokens. But is there a design reason that there are no helper methods on Syntax to change the leading/trailing trivia of a first/last token of a tree?

With an example, in a SyntaxRewriter, why can't I do:

var conditions: ConditionElementListSyntax = ...
conditions = conditions.withTrailingTrivia(.spaces(1))

I would want that method to recursively find the last token, modify its trailing trivia, and recursively modify the syntax tree back up.

CC @harlanhaskins @akyrtzi

(Harlan Haskins) #2

I think this is totally reasonable, and the answer really boils down to “we just haven’t done it”. I’d love a PR if you’re interested!

(David Hart) #3

I've started work on the implementation and was wandering what you think should be the behaviour of those withLeadingTrivia and withTrailingTrivia functions when the node does not contain any child nodes.

For example, I've tweaked the SyntaxNodes.swift.gyb to generate the following implementations:

struct IfStmtSyntax: _SyntaxBase {
  // ...

  func withLeadingTrivia(_ leadingTrivia: Trivia) -> IfStmtSyntax {
    if let labelName = labelName {
        return withLabelName(labelName.withLeadingTrivia(leadingTrivia))
    } else if let labelColon = labelColon {
        return withLabelColon(labelColon.withLeadingTrivia(leadingTrivia))
    } else {
        return withIfKeyword(ifKeyword.withLeadingTrivia(leadingTrivia))
    }
  }

  func withTrailingTrivia(_ trailingTrivia: Trivia) -> IfStmtSyntax {
    if let elseBody = elseBody {
        return withElseBody(elseBody.withTrailingTrivia(trailingTrivia))
    } else if let elseKeyword = elseKeyword {
        return withElseKeyword(elseKeyword.withTrailingTrivia(trailingTrivia))
    } else {
        return withBody(body.withTrailingTrivia(trailingTrivia))
    }
  }

  // ...
}

Question 1: What do you think of this approach?

Question 2: For a IfStmtSyntax node, everything should be fine, there's always going to be a non-optional node to call withLeadingTrivia and withTrailingTrivia on. But that's not the case of UnknownDeclSyntax, UnknownExprSyntax, UnknownStmtSyntax, UnknownTypeSyntax, or UnknownPatternSyntax. Those represent invalid ASTs, so perhaps the best solution is a NOOP, but here may be other nodes I'm not aware of that don't have valid child nodes to call withLeadingTrivia or withTrailingTrivia on.

CC @Xi_Ge that reviewed my recent swift-syntax PRs

(Argyrios Kyrtzidis) #4

This is also the case for SyntaxCollections that are empty. It doesn't necessarily mean it is 'invalid' to call this on them, but it probably should be a noop.
I'd suggest to create a general function on Syntax to do this, that will be noop if the node contains no tokens. Adding the functions for every node and checking every child seems a bit too heavyweight to me.

(David Hart) #5

What were you thinking the signature of the functions on Syntax would be? I was expecting those functions to return the concrete type it's called on. For example, I would expect IfStmtSyntax.withLeadingTrivia(_:) to return a IfStmtSyntax. That doesn't seem possible if the function is implemented on Syntax.

(Argyrios Kyrtzidis) #6

I see it would be useful to have type-specific implementations but they should just call into a generic function on SyntaxData to do the transformation and just wrap the result and return it.

(David Hart) #7

Gotcha! Thanks.

(David Hart) #8

I've finished the implementation about opened a Pull Request if anybody wants to check it out: https://github.com/apple/swift-syntax/pull/91

I've also added the equivalent setters for consistency.