Refactor ConvertToTernaryExpr by a7maad-ayman · Pull Request #3252 · swiftlang/swift-syntax

a7maad-ayman · 2026-01-21T20:46:39Z

Refactor ConvertToTernaryExpr

Implements a new Swift refactoring that converts if-else statements with assignments into ternary expressions.

What it does

Converts this:

let result: Type
if condition {
  result = trueValue
} else {
  result = falseValue
}

Into this:

let result: Type = condition ? trueValue : falseValue

Supported patterns

Variable declaration + if-else assignment
Standalone if-else assignment
Tuple assignments: (a, b) = condition ? (1, 2) : (3, 4)
Simple assignments: x = condition ? value1 : value2

Contributes to #2424

ahoppen

Thanks for starting this, @a7maad-ayman. I left some initial comments from my first scan through the PR. Once those are resolved, I’ll have another more detailed look at it.

ahoppen · 2026-01-29T17:33:13Z

Sources/SwiftRefactor/ConvertToTernaryExpression.swift

+/// - Optionally, the variable is declared immediately before the if statement
+public struct ConvertToTernaryExpression: SyntaxRefactoringProvider {
+
+  public static func refactor(syntax: CodeBlockItemListSyntax, in context: Void) throws -> CodeBlockItemListSyntax {


Could you run swift-format on your changes. Instructions should be in CONTRIBUTING.md

I've already run swift format -i -r . to format the code according to the project's style guidelines.

Ah, sorry. I meant whether you could remove this superfluous newline. Same for all the other functions as well

ahoppen · 2026-01-29T17:39:16Z

Sources/SwiftRefactor/ConvertToTernaryExpression.swift

+    let items = Array(codeBlock)
+    guard !items.isEmpty else { return nil }
+
+    /// Variable declaration followed by if statement.


Would it be simpler to find the if expression to extract first and then just look up if the previous statement is a variable declaration of that name? I feel like that could unify this branch and the one below a little.

ahoppen · 2026-01-29T17:42:25Z

Sources/SwiftRefactor/ConvertToTernaryExpression.swift

+  private static func extractCondition(from ifExpr: IfExprSyntax) -> ExprSyntax? {
+    guard let firstCondition = ifExpr.conditions.first else {
+      return nil
+    }
+
+    guard case .expression(let condition) = firstCondition.condition else {
+      return nil
+    }
+
+    return condition
+  }
+


I think the entire code in analyzePattern would read easier if these checks would just be inline. This one, for example could just be a single

guard case .expression(let condition) = ifExpr.conditions.first.condition else { return nil }

Similar for most of the other checks

ahoppen · 2026-01-29T17:43:01Z

Sources/SwiftRefactor/ConvertToTernaryExpression.swift

+  }
+
+  private static func extractCondition(from ifExpr: IfExprSyntax) -> ExprSyntax? {
+    guard let firstCondition = ifExpr.conditions.first else {


We should only apply this transformation if there’s only a single condition. You can use .only for that.

Sources/SwiftRefactor/ConvertToTernaryExpression.swift

ahoppen · 2026-01-29T17:46:36Z

Sources/SwiftRefactor/ConvertToTernaryExpression.swift

+    if expr.as(TernaryExprSyntax.self) != nil { return true }
+    if expr.as(ClosureExprSyntax.self) != nil { return true }


Why are these too complex for a ternary?

I think nested ternaries hurt readability x = a ? b : (c ? d : e) is harder to follow than an if-else. Closures with captures are also clearer in if-else form since they benefit from proper line breaks and indentation rather than being collapsed into a single line. Both cases are better left as-is.

Happy to hear your thoughts — do you have a preference on which cases should be excluded?

I think if a user requests a conversion to a ternary expression, we should offer it to them. Users should be able to decide for themself whether applying the code action has a benefit for their code.

ahoppen · 2026-01-29T17:48:50Z

Sources/SwiftRefactor/ConvertToTernaryExpression.swift

+// MARK: - Alternative API for single if statement refactoring
+extension ConvertToTernaryExpression {
+
+  public static func refactor(


AFAICT this doesn’t satisfy any requirement in SyntaxRefactoringProvider. Since most clients will enter the refactoring action through that protocol, this method is practically unreachable. We should be able to perform this refactoring using the main refactor entry point.

ahoppen · 2026-01-29T17:48:50Z

Sources/SwiftRefactor/ConvertToTernaryExpression.swift

+  }
+
+  // MARK: - Builders
+  private static func withoutTrivia<T: SyntaxProtocol>(_ node: T) -> T {


We have .trimmed, which does exactly this.

ahoppen · 2026-01-29T17:48:50Z

Tests/SwiftRefactorTest/ConvertToTernaryExpressionTests.swift

+
+  func testNamedTupleAssignment() throws {
+    let baseline: CodeBlockItemListSyntax = """
+      let coordinates: (x: Int, y: Int)


Should we keep the type annotation here?

Type annotations are now handled differently based on assignment type:

Tuples: Type annotation is preserved (e.g., (x: Int, y: Int))

Simple types: Type annotation is removed and inferred (e.g., Int)

- Add `Collection.only` to `SyntaxUtils.swift` (mirrors the pattern in `SwiftParserDiagnostics` and `CodeGeneration`) - Preserve the type annotation in the output declaration when the declared type is a named tuple (e.g. `(x: Int, y: Int)`) - Delete `extractCondition`, `extractElseBlock` & `validateIfExpr` and inline their checks directly into `analyzePattern`. - Add inline comments to `isExpressionTooComplexForTernary` explaining why nested ternaries and closures are excluded

ahoppen

I have a few more comments inline.

ahoppen · 2026-02-28T17:30:41Z

Sources/SwiftRefactor/ConvertToTernaryExpression.swift

+  }
+
+  // MARK: - Models
+  /// ConvertibleIfElse


This comment doesn’t provide any value.

I think an example that shows an if expression and highlights which member represents which part of the if expression would be helpful.

ahoppen · 2026-02-28T17:32:08Z

Sources/SwiftRefactor/ConvertToTernaryExpression.swift

+    let isTupleAssignment: Bool
+  }
+
+  /// AssignmentInfo


Similar here, this comment doesn’t provide any value but documenting the members would be valuable.

ahoppen · 2026-02-28T17:34:06Z

Sources/SwiftRefactor/ConvertToTernaryExpression.swift

+/// - Optionally, the variable is declared immediately before the if statement
+public struct ConvertToTernaryExpression: SyntaxRefactoringProvider {
+
+  public static func refactor(syntax: CodeBlockItemListSyntax, in context: Void) throws -> CodeBlockItemListSyntax {


Should the node that this refactoring action is invoked on be an IfExprSyntax. If we invoke it on CodeBlockItemListSyntax, it’s not clear what you want to convert if you have multiple if expressions within the code block.

ahoppen · 2026-02-28T17:36:20Z

Sources/SwiftRefactor/SyntaxUtils.swift

  }
 }

+extension Collection {


Let’s move this to its own file `Collection+only.swift because it’s more of a general utility and not really related to syntax nodes.

ahoppen · 2026-02-28T17:38:10Z

Sources/SwiftRefactor/ConvertToTernaryExpression.swift

+      varDecl.bindings.count == 1,
+      let binding = varDecl.bindings.first,


Can be .only. Same in a couple more places.

ahoppen · 2026-02-28T18:04:02Z

Tests/SwiftRefactorTest/ConvertToTernaryExpressionTests.swift

+    try assertRefactorConvert(baseline, expected: expected)
+  }
+
+  func testBasicIfElseWithVarDeclaration() throws {


This parses exactly the same way as testBasicIfElseWithLetDeclaration, so I don’t think there’s much value in this test.

ahoppen · 2026-02-28T18:04:10Z

Tests/SwiftRefactorTest/ConvertToTernaryExpressionTests.swift

+    try assertRefactorConvert(baseline, expected: expected)
+  }
+
+  func testBasicIfElseWithVarDeclaration() throws {


This parses exactly the same way as testBasicIfElseWithLetDeclaration, so I don’t think there’s much value in this test.va

ahoppen · 2026-02-28T18:04:47Z

Sources/SwiftRefactor/ConvertToTernaryExpression.swift

+    }
+
+    let keyword = decl.bindingSpecifier.tokenKind
+    return keyword == .keyword(.let) || keyword == .keyword(.var)


What exactly are you trying to guard for here? Variable declarations will always have var or let and should we introduce a new keyword here, we’d likely want to also support it.

ahoppen · 2026-02-28T18:05:20Z

Tests/SwiftRefactorTest/ConvertToTernaryExpressionTests.swift

+  func testParenthesizedCondition() throws {
+    let baseline: CodeBlockItemListSyntax = """
+      let output: Int
+      if (x > 0) {


Does this also work if x < 0 is not wrapped in parentheses?

ahoppen · 2026-02-28T18:06:08Z

Tests/SwiftRefactorTest/ConvertToTernaryExpressionTests.swift

+
+  // MARK: - Complex Expression Tests
+
+  func testFunctionCallInBranches() throws {


Since we just copy the expressions verbatim, I don’t think this provides any additional coverage over testBasicIfElseWithLetDeclaration. Similar for testDictionaryLiteralInBranches.

Refactor ConvertToTernaryExpression

c2cf18a

a7maad-ayman requested review from ahoppen, hamishknight, hborla and rintaro as code owners January 21, 2026 20:46

ahoppen mentioned this pull request Jan 29, 2026

Rewrite syntactic refactoring actions in sourcekitd using swift-syntax swiftlang/sourcekit-lsp#2424

Open

10 tasks

ahoppen reviewed Jan 29, 2026

View reviewed changes

a7maad-ayman requested a review from ahoppen February 26, 2026 21:13

ahoppen reviewed Feb 28, 2026

View reviewed changes

		if expr.as(TernaryExprSyntax.self) != nil { return true }
		if expr.as(ClosureExprSyntax.self) != nil { return true }

		varDecl.bindings.count == 1,
		let binding = varDecl.bindings.first,


		// MARK: - Complex Expression Tests

		func testFunctionCallInBranches() throws {

Conversation

a7maad-ayman commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Refactor ConvertToTernaryExpr

What it does

Supported patterns

Uh oh!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahoppen Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

a7maad-ayman commented Jan 21, 2026 •

edited

Loading

ahoppen Feb 28, 2026 •

edited

Loading