Feat(SwiftRefactor): Implement InvertIfCondition refactoring by rickhohler · Pull Request #3263 · swiftlang/swift-syntax

rickhohler · 2026-02-03T17:34:19Z

Description

This PR implements the InvertIfCondition refactoring action, which supports SourceKit-LSP #2408.

Transformation:

if !x {
  foo()
} else {
  bar()
}
// becomes:
if x {
  bar()
} else {
  foo()
}

Detailed Design

Adds InvertIfCondition provider to SwiftRefactor.
Criteria (Strict Scope):
- Input must be an IfExprSyntax.
- Must have an else block (CodeBlock).
- Must have exactly one condition.
- Condition must be negated (PrefixOperatorExpr with !).
Logic:
- Unwraps the negation (!condition -> condition).
- Swaps the body and elseBody.
- Preserves trivia (comments/whitespace) by swapping leading/trailing trivia between the blocks.

Fixes

Addresses logic for swiftlang/sourcekit-lsp#2408.

Pre-PR Checklist

Code builds and passes tests.
Ran swift-format.
Added unit tests in InvertIfConditionTest.swift.

…rceKit-LSP swiftlang#2451)

…it-LSP swiftlang#2408)

ahoppen

Very nice. Some small comments inline.

ahoppen · 2026-02-09T10:05:13Z

Sources/SwiftRefactor/InvertIfCondition.swift

+/// ```
+public struct InvertIfCondition: SyntaxRefactoringProvider {
+  public static func refactor(syntax ifExpr: IfExprSyntax, in context: Void) -> IfExprSyntax {
+    // 1. Must have an `else` block (and it must be a CodeBlock, not another `if`).


Similar to my comment in your other PR, I wouldn’t have these numbered comments that explain exactly what the code blow does.

ahoppen · 2026-02-09T10:05:29Z

Sources/SwiftRefactor/InvertIfCondition.swift

+    guard ifExpr.conditions.count == 1,
+      let condition = ifExpr.conditions.first
+    else {


Fits on one line, same above.

Suggested change

guard ifExpr.conditions.count == 1,

let condition = ifExpr.conditions.first

else {

guard ifExpr.conditions.count == 1, let condition = ifExpr.conditions.first else {

ahoppen · 2026-02-09T10:06:30Z

Sources/SwiftRefactor/InvertIfCondition.swift

+    // Preserve trivia: The `!` might have leading trivia (e.g. comments/spaces).
+    // Usually standard formatting is `if !cond`.
+    // We should probably apply the `PrefixOperatorExpr`'s leading trivia to the inner expression
+    // to preserve any comments attached to the negation.


If there are things left to address, please address them. Otherwise please don’t talk about things that we should probably do. Either do them or don’t and explain why.

ahoppen · 2026-02-09T10:07:37Z

Sources/SwiftRefactor/InvertIfCondition.swift

+    // Preserve trivia: The `!` might have leading trivia (e.g. comments/spaces).
+    // Usually standard formatting is `if !cond`.
+    // We should probably apply the `PrefixOperatorExpr`'s leading trivia to the inner expression
+    // to preserve any comments attached to the negation.


You should be able to use merging(triviaOf:) to make sure we don’t loose trivia from !.

ahoppen · 2026-02-09T10:08:22Z

Sources/SwiftRefactor/RemoveRedundantParens.swift

This is an unrelated change. Please make sure your PR only addresses one issue.

ahoppen · 2026-02-09T10:08:45Z

Tests/SwiftRefactorTest/InvertIfConditionTest.swift

+
+final class InvertIfConditionTest: XCTestCase {
+  func testInvertIfCondition() throws {
+    let tests = [


Same comment about writing an assert function as in your other PR here.

This comment still applies

ahoppen · 2026-02-21T16:02:50Z

Looks like you pushed a new commit without addressing all of my comments. Please let me know when this is ready for another review.

…ents, reformat guards, improve trivia preservation, and add test helper

rickhohler · 2026-02-28T02:57:51Z

Thank you for the patience. I have now addressed all of your previous comments in the latest push:

Styling: Removed the numbered comments and reformatted the guard statements to fit on single lines.

Trivia: Implemented merging(triviaOf:) for the unwrapped condition to ensure that any trivia (comments/spacing) attached to the ! operator is correctly preserved.

Hygiene: Confirmed that the branch is now clean and free of the unrelated RemoveRedundantParens changes.

Tests: Added a private assertInvertIfCondition helper to the test suite to reduce boilerplate and match repository conventions.

This should be ready for another review now!

ahoppen

Looks good, just a few small things left.

ahoppen · 2026-02-28T19:16:17Z

Sources/SwiftRefactor/InvertIfCondition.swift

+public struct InvertIfCondition: SyntaxRefactoringProvider {
+  public static func refactor(syntax ifExpr: IfExprSyntax, in context: Void) -> IfExprSyntax {
+    guard let elseBody = ifExpr.elseBody, case .codeBlock(let elseBlock) = elseBody else {
+      return ifExpr


If there is nothing to refactor we should throw a RefactoringNotApplicableError. This will prevent the refactoring from showing up in SourceKit-LSP when it doesn’t apply.

ahoppen · 2026-02-28T19:19:41Z

Sources/SwiftRefactor/InvertIfCondition.swift

+      return ifExpr
+    }
+
+    guard let prefixOpExpr = expr.as(PrefixOperatorExprSyntax.self), prefixOpExpr.operator.text == "!" else {


Should we also support adding the ! if you want to invert an if expression that has a normal expression? The tricky part would likely be to decide whether parentheses need to be added around the expression for precedence rules. We can also do that in a follow-up PR.

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

Sources/SwiftRefactor/InvertIfCondition.swift

+    let newCondition = condition.with(\.condition, .expression(innerExpr))
+    let newConditions = ifExpr.conditions.with(\.[ifExpr.conditions.startIndex], newCondition)


Suggested change

let newCondition = condition.with(\.condition, .expression(innerExpr))

let newConditions = ifExpr.conditions.with(\.[ifExpr.conditions.startIndex], newCondition)

let newConditions = ifExpr.conditions.with(\.[ifExpr.conditions.startIndex].condition, .expression(innerExpr))

ahoppen · 2026-02-28T19:21:02Z

Tests/SwiftRefactorTest/InvertIfConditionTest.swift

+
+final class InvertIfConditionTest: XCTestCase {
+  func testInvertIfCondition() throws {
+    let tests = [


This comment still applies

ahoppen · 2026-02-28T19:21:56Z

Tests/SwiftRefactorTest/InvertIfConditionTest.swift

+        } else {
+          a
+        }
+        """


Could you also add a test that has comments in the bodies?

ahoppen · 2026-02-28T19:22:12Z

Tests/SwiftRefactorTest/InvertIfConditionTest.swift

+        }
+        """,
+        """
+        if (x == y) {


Should we strip the parentheses here since they are no longer necessary?

rickhohler added 2 commits February 3, 2026 11:23

Feat(SwiftRefactor): Implement RemoveRedundantParens refactoring (Sou…

624d0bf

…rceKit-LSP swiftlang#2451)

Feat(SwiftRefactor): Implement InvertIfCondition refactoring (SourceK…

ccc7b4d

…it-LSP swiftlang#2408)

rickhohler requested review from ahoppen, hamishknight, hborla and rintaro as code owners February 3, 2026 17:34

ahoppen reviewed Feb 9, 2026

View reviewed changes

feat(refactor): Implement InvertIfCondition (clean)

08c1c8b

Address reviewer feedback for InvertIfCondition: remove numbered comm…

58c1796

…ents, reformat guards, improve trivia preservation, and add test helper

ahoppen reviewed Feb 28, 2026

View reviewed changes

		let newCondition = condition.with(\.condition, .expression(innerExpr))
		let newConditions = ifExpr.conditions.with(\.[ifExpr.conditions.startIndex], newCondition)

Conversation

rickhohler commented Feb 3, 2026

Description

Detailed Design

Fixes

Pre-PR Checklist

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!

ahoppen commented Feb 21, 2026

Uh oh!

rickhohler commented Feb 28, 2026

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants