Implement ExpandTernaryExpr mirroring sourcekitd implementation#3247
Implement ExpandTernaryExpr mirroring sourcekitd implementation#3247redne-w wants to merge 1 commit intoswiftlang:mainfrom
Conversation
redne-w
requested review from
ahoppen,
hamishknight,
hborla and
rintaro
as code owners
redne-w
changed the title
| // | ||
| // This source file is part of the Swift.org open source project | ||
| // | ||
| // Copyright (c) 2014 - 2025 Apple Inc. and the Swift project authors |
There was a problem hiding this comment.
| // Copyright (c) 2014 - 2025 Apple Inc. and the Swift project authors | |
| // Copyright (c) 2014 - 2026 Apple Inc. and the Swift project authors |
😉
| #endif | ||
|
|
||
| /// Converts a ternary conditional expression to an if-else expression using | ||
| /// Swift 5.9's if-expression syntax. |
There was a problem hiding this comment.
I don’t think it’s worth mentioning the version in which the if-expressions were introduced 5.9 is sufficiently old now.
| /// Swift 5.9's if-expression syntax. | |
| /// Swift's if-expression syntax. |
| // Extract components with cleaned trivia | ||
| let condition = ternary.condition | ||
| .with(\.leadingTrivia, []) | ||
| .with(\.trailingTrivia, []) | ||
| let thenExpr = ternary.thenExpression | ||
| .with(\.leadingTrivia, []) | ||
| .with(\.trailingTrivia, []) | ||
| let elseExpr = ternary.elseExpression | ||
| .with(\.leadingTrivia, []) | ||
| .with(\.trailingTrivia, []) |
There was a problem hiding this comment.
This will remove any comments around these expressions. Handling comments is always a little tricky but could you try to make sure we do something reasonable for /*a*/ foo /*b*/ ? /*c*/ 1 /*d*/ : /*e*/ 2 /*f*/?
I believe that in most cases we only want to remove surrounding whitespace but not comments.
One other interesting case to handle would be
// test
foo ? 1 : 2Because // test will be attached to foo. I think you handle this well but it would be good to have a test case for that.
| CodeBlockSyntax( | ||
| statements: CodeBlockItemListSyntax { | ||
| CodeBlockItemSyntax( | ||
| leadingTrivia: .newline + indentStep, |
There was a problem hiding this comment.
I think we also need to add the base indentation of the ternary expression here, eg.
func foo() {
let x = true ? 1 : 2
}| let baseline = TernaryExprSyntax( | ||
| condition: ExprSyntax("a"), | ||
| thenExpression: ExprSyntax("b"), | ||
| elseExpression: ExprSyntax("c") | ||
| ) |
There was a problem hiding this comment.
I think this would read easier if you wrote this as ExprSyntax("a ? b : c").cast(TernaryExprSyntax.self). It would also be closer to what actual users would do based on real source code.
| try assertRefactor(baseline, context: (), provider: ExpandTernaryExpr.self, expected: expected) | ||
| } | ||
|
|
||
| func testTernaryWithStringLiterals() throws { |
There was a problem hiding this comment.
All of the following tests are effectively the same as the first one because we just copy the expression verbatim.
Instead, I’d prefer to have some more tests that involve comments because that’s usually the tricky thing to handle.
| ) | ||
|
|
||
| let expected: ExprSyntax = """ | ||
| if (x > 0 && y < 10) { |
There was a problem hiding this comment.
Should we remove the parentheses here? Would also be a good follow-up PR if you don’t want to do it in this one.
2 participants
ExpandTernaryExpr
Part of Sourcekit-LSP Issue#2424
Description
Implement ExpandTernaryExpr in Swift-Syntax to be used by lsp in place of current sourcekitd implementation.
Updated sourcekitd implementation to use Swift 5.9+ if-expression syntax.
Support: