Add an initializer that accepts [ExprSyntax] for ArrayExprSyntax#1693
Conversation
|
Hi @gibachan 👋 The files you have changed are inside a generated folder. Also I think the code would fit better inside the SwiftSyntaxBuilder, as this is about building the syntax code. You can then add test cases to |
|
Hi @kimdv, thank you so much! I moved the initializers to |
|
@swift-ci please test |
|
Would you mind to squash the two commits @gibachan ? 😁 |
gibachan
force-pushed
the
array-expr-syntax-initializer
branch
from
53fb44a to
8275367
Compare
|
I did it 👍 |
|
@swift-ci please test |
| // MARK: - ArrayElementList | ||
|
|
||
| extension ArrayElementListSyntax { | ||
| public init(expressions: [ExprSyntaxProtocol]) { |
There was a problem hiding this comment.
You should explicitly write any ExprSyntaxProtocol here.
There was a problem hiding this comment.
We prefer to use ExprSyntax here. I’m adding some documentation about it in #1696
|
|
||
| extension ArrayElementListSyntax { | ||
| public init(expressions: [ExprSyntaxProtocol]) { | ||
| let elements = expressions.enumerated().map { index, expression in |
There was a problem hiding this comment.
It's valid for the last element to have a trailing comma in an array literal, so there's no need for enumerated() or index-checking here.
There was a problem hiding this comment.
SwiftSyntaxBuilder doesn’t add a trailing comma for the last element, so I’d prefer to be consistent between the two initializers her and also not add a trailing comma here either. So: I’d prefer to keep it as you wrote it @gibachan.
ahoppen
left a comment
ahoppen
left a comment
There was a problem hiding this comment.
Thank you. I’ve got a couple of comments, but overall looks good
| // MARK: - ArrayElementList | ||
|
|
||
| extension ArrayElementListSyntax { | ||
| public init(expressions: [ExprSyntaxProtocol]) { |
There was a problem hiding this comment.
We prefer to use ExprSyntax here. I’m adding some documentation about it in #1696
|
|
||
| extension ArrayElementListSyntax { | ||
| public init(expressions: [ExprSyntaxProtocol]) { | ||
| let elements = expressions.enumerated().map { index, expression in |
There was a problem hiding this comment.
SwiftSyntaxBuilder doesn’t add a trailing comma for the last element, so I’d prefer to be consistent between the two initializers her and also not add a trailing comma here either. So: I’d prefer to keep it as you wrote it @gibachan.
| public init(expressions: [ExprSyntaxProtocol]) { | ||
| let elements = expressions.enumerated().map { index, expression in | ||
| let element = ArrayElementSyntax(expression: expression) | ||
| return index != (expressions.count - 1) ? element.with(\.trailingComma, .commaToken()) : element |
There was a problem hiding this comment.
This will override any potentially already existing trailing comma (and trivia associated with it). You should be able to use element.ensuringTrailingComma() instead.
Also, I’d prefer an if to the ternary expression. I just think it reads a lot more easily.
gibachan
force-pushed
the
array-expr-syntax-initializer
branch
from
8275367 to
f95fa76
Compare
|
Thank you for all your comments 😄 I have made the fixes! |
| } | ||
|
|
||
| func testInitializerWithExpressions() { | ||
| let expressions: [ExprSyntax] = [.init(literal: 0), .init(literal: 1), .init(literal: 2)] |
There was a problem hiding this comment.
You could also make this even simpler as follows.
| let expressions: [ExprSyntax] = [.init(literal: 0), .init(literal: 1), .init(literal: 2)] | |
| let expressions: [ExprSyntax] = ["0", "1", "2"] |
gibachan
force-pushed
the
array-expr-syntax-initializer
branch
from
f95fa76 to
50bc963
Compare
|
@swift-ci please test |
|
@swift-ci Please test |
4 participants
Resolve #1674
I have added initializers for
ArrayExprSyntaxandArrayElementListSyntaxthat allow codes like below( I'm not sure if I need to write a test for the
ArrayExprSyntaxinitializer or where to place it. )