From 624d0bf7182d7819a6e9d4522d806cb13506db0b Mon Sep 17 00:00:00 2001 From: Rick Hohler Date: Tue, 3 Feb 2026 11:23:37 -0600 Subject: [PATCH 1/4] Feat(SwiftRefactor): Implement RemoveRedundantParens refactoring (SourceKit-LSP #2451) --- .../SwiftRefactor/RemoveRedundantParens.swift | 85 ++++++++++++++++ .../RemoveRedundantParensTest.swift | 98 +++++++++++++++++++ 2 files changed, 183 insertions(+) create mode 100644 Sources/SwiftRefactor/RemoveRedundantParens.swift create mode 100644 Tests/SwiftRefactorTest/RemoveRedundantParensTest.swift diff --git a/Sources/SwiftRefactor/RemoveRedundantParens.swift b/Sources/SwiftRefactor/RemoveRedundantParens.swift new file mode 100644 index 00000000000..fb5a4ca69ea --- /dev/null +++ b/Sources/SwiftRefactor/RemoveRedundantParens.swift @@ -0,0 +1,85 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2026 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +#if compiler(>=6) +public import SwiftSyntax +#else +import SwiftSyntax +#endif + +/// Removes redundant parentheses from a single-element tuple. +/// +/// ## Before +/// +/// ```swift +/// let x = (10) +/// ``` +/// +/// ## After +/// +/// ```swift +/// let x = 10 +/// ``` +public struct RemoveRedundantParens: SyntaxRefactoringProvider { + public static func refactor(syntax tupleExpr: TupleExprSyntax, in context: Void) -> ExprSyntax { + // 1. Must be exactly one element. + guard tupleExpr.elements.count == 1, + let element = tupleExpr.elements.first + else { + return ExprSyntax(tupleExpr) + } + + // 2. Must not have a label. + guard element.label == nil else { + return ExprSyntax(tupleExpr) + } + + // 3. Extract the inner expression. + let innerExpr = element.expression + + // 4. Safety Check: Precedence Ambiguity + // If we are unwrapping a SequenceExpr (binary op) AND we are inside another SequenceExpr, + // we assume it is UNSAFE because we don't know operator precedence. + // Example: x * (y + z) -> unwrapping makes it x * y + z (different logic). + + // Check if inner is SequenceExpr + // Check if inner is SequenceExpr + if innerExpr.is(SequenceExprSyntax.self) { + // Case: Parent is ExprList (standard SequenceExpr elements) -> Grandparent is SequenceExpr + if let parent = tupleExpr.parent, + parent.is(ExprListSyntax.self), + let grandParent = parent.parent, + grandParent.is(SequenceExprSyntax.self) + { + return ExprSyntax(tupleExpr) + } + + // Case: Direct parent is SequenceExpr (fallback) + if let parent = tupleExpr.parent, parent.is(SequenceExprSyntax.self) { + return ExprSyntax(tupleExpr) + } + } + + // 5. Preserve Trivia + // We want to keep comments attached to the parens, but maybe not the newlines inside? + // A safe default is to take the tuple's leading/trailing trivia and apply it to the inner expression. + // This replaces the inner expression's existing leading/trailing trivia if we use .with, + // so we might want to append? + // Actually, usually `( /* comment */ x )` -> `/* comment */ x`. + // Let's replace. + + return + innerExpr + .with(\.leadingTrivia, tupleExpr.leadingTrivia) + .with(\.trailingTrivia, tupleExpr.trailingTrivia) + } +} diff --git a/Tests/SwiftRefactorTest/RemoveRedundantParensTest.swift b/Tests/SwiftRefactorTest/RemoveRedundantParensTest.swift new file mode 100644 index 00000000000..eb4883b83b8 --- /dev/null +++ b/Tests/SwiftRefactorTest/RemoveRedundantParensTest.swift @@ -0,0 +1,98 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2026 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import SwiftParser +import SwiftRefactor +import SwiftSyntax +import SwiftSyntaxBuilder +import XCTest +import _SwiftSyntaxTestSupport + +final class RemoveRedundantParensTest: XCTestCase { + func testRemoveRedundantParens() throws { + let tests = [ + ( + "(10)", + "10" + ), + ( + "((10))", + "(10)" + ), + ( + "(y + z)", + "y + z" + ), + ( + "/* c */ (x)", + "/* c */ x" + ), + ] + + for (input, expected) in tests { + let inputSyntax = try XCTUnwrap( + ExprSyntax.parse(from: input).as(TupleExprSyntax.self), + "Failed validity check: \(input)" + ) + // For expected, it might not be a TupleExpr (it's the unwrapped expr) + let expectedSyntax = ExprSyntax.parse(from: expected) + + try assertRefactor(inputSyntax, context: (), provider: RemoveRedundantParens.self, expected: expectedSyntax) + } + } + + func testRemoveRedundantParensFails() throws { + let tests = [ + // Multiple elements + "(1, 2)", + // Labeled element + "(x: 1)", + // Empty + "()", + // Safety: Binary op inside Binary op + "x * (y + z)", + "(a + b) / c", + ] + + for input in tests { + // Input might be a SequenceExpr containing a TupleExpr, not just a TupleExpr. + let fullSyntax = ExprSyntax.parse(from: input) + + // Find the first TupleExpr in the tree + let visitor = FindTupleVisitor(viewMode: .sourceAccurate) + visitor.walk(fullSyntax) + + guard let tupleExpr = visitor.tupleExpr else { + // If top level is tuple + if let tuple = fullSyntax.as(TupleExprSyntax.self) { + try assertRefactor(tuple, context: (), provider: RemoveRedundantParens.self, expected: ExprSyntax(tuple)) + continue + } + XCTFail("Could not find tuple in \(input)") + continue + } + + // We expect the refactoring to return the tuple unchanged (as ExprSyntax) + try assertRefactor(tupleExpr, context: (), provider: RemoveRedundantParens.self, expected: ExprSyntax(tupleExpr)) + } + } +} + +class FindTupleVisitor: SyntaxVisitor { + var tupleExpr: TupleExprSyntax? + override func visit(_ node: TupleExprSyntax) -> SyntaxVisitorContinueKind { + if tupleExpr == nil { + tupleExpr = node + } + return .visitChildren + } +} From ccc7b4d6f2a99a4e96cef4f522d4e8e2d348c2c6 Mon Sep 17 00:00:00 2001 From: Rick Hohler Date: Tue, 3 Feb 2026 11:32:41 -0600 Subject: [PATCH 2/4] Feat(SwiftRefactor): Implement InvertIfCondition refactoring (SourceKit-LSP #2408) --- Sources/SwiftRefactor/InvertIfCondition.swift | 104 +++++++++++++ .../InvertIfConditionTest.swift | 143 ++++++++++++++++++ .../RemoveRedundantParensTest.swift | 8 + 3 files changed, 255 insertions(+) create mode 100644 Sources/SwiftRefactor/InvertIfCondition.swift create mode 100644 Tests/SwiftRefactorTest/InvertIfConditionTest.swift diff --git a/Sources/SwiftRefactor/InvertIfCondition.swift b/Sources/SwiftRefactor/InvertIfCondition.swift new file mode 100644 index 00000000000..7061046cf99 --- /dev/null +++ b/Sources/SwiftRefactor/InvertIfCondition.swift @@ -0,0 +1,104 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2026 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +#if compiler(>=6) +public import SwiftSyntax +#else +import SwiftSyntax +#endif + +/// Inverts a negated `if` condition and swaps the branches. +/// +/// ## Before +/// +/// ```swift +/// if !x { +/// foo() +/// } else { +/// bar() +/// } +/// ``` +/// +/// ## After +/// +/// ```swift +/// if x { +/// bar() +/// } else { +/// foo() +/// } +/// ``` +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`). + guard let elseBody = ifExpr.elseBody, + case .codeBlock(let elseBlock) = elseBody + else { + return ifExpr + } + + // 2. Must have exactly one condition. + guard ifExpr.conditions.count == 1, + let condition = ifExpr.conditions.first + else { + return ifExpr + } + + // 3. Condition must be an expression (not a binding like `let x`). + guard case .expression(let expr) = condition.condition else { + return ifExpr + } + + // 4. Expression must be a PrefixOperatorExpr with operator "!". + guard let prefixOpExpr = expr.as(PrefixOperatorExprSyntax.self), + prefixOpExpr.operator.text == "!" + else { + return ifExpr + } + + // 5. Extract inner expression (remove the `!`). + // 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. + let innerExpr = prefixOpExpr.expression + .with(\.leadingTrivia, prefixOpExpr.leadingTrivia) + + // 6. Create new condition list. + let newCondition = condition.with(\.condition, .expression(innerExpr)) + let newConditions = ifExpr.conditions.with(\.[ifExpr.conditions.startIndex], newCondition) + + // 7. Swap bodies with Trivia preservation. + // We strictly swap the trivia: + // New body (was else) takes old body's trivia (space before `{`, space after `}`). + // New else (was body) takes old else's trivia (space before `{`?, newline after `}`). + + let oldBody = ifExpr.body + let oldElseBlock = elseBlock + + let newBody = + oldElseBlock + .with(\.leadingTrivia, oldBody.leadingTrivia) + .with(\.trailingTrivia, oldBody.trailingTrivia) + + let newElseBody = + oldBody + .with(\.leadingTrivia, oldElseBlock.leadingTrivia) + .with(\.trailingTrivia, oldElseBlock.trailingTrivia) + + return + ifExpr + .with(\.conditions, newConditions) + .with(\.body, newBody) + .with(\.elseBody, .codeBlock(newElseBody)) + } +} diff --git a/Tests/SwiftRefactorTest/InvertIfConditionTest.swift b/Tests/SwiftRefactorTest/InvertIfConditionTest.swift new file mode 100644 index 00000000000..cdd8c736d3c --- /dev/null +++ b/Tests/SwiftRefactorTest/InvertIfConditionTest.swift @@ -0,0 +1,143 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2026 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import SwiftParser +import SwiftRefactor +import SwiftSyntax +import SwiftSyntaxBuilder +import XCTest +import _SwiftSyntaxTestSupport + +final class InvertIfConditionTest: XCTestCase { + func testInvertIfCondition() throws { + let tests = [ + ( + """ + if !x { + foo() + } else { + bar() + } + """, + """ + if x { + bar() + } else { + foo() + } + """ + ), + ( + """ + if !(x == y) { + return + } else { + continue + } + """, + """ + if (x == y) { + continue + } else { + return + } + """ + ), + // Trivia preservation + ( + """ + if /* comment */ !x { + a + } else { + b + } + """, + """ + if /* comment */ x { + b + } else { + a + } + """ + ), + ] + + for (input, expected) in tests { + let inputSyntax = try XCTUnwrap( + ExprSyntax.parse(from: input).as(IfExprSyntax.self), + "Failed validity check: \(input)" + ) + let expectedSyntax = try XCTUnwrap(ExprSyntax.parse(from: expected), "Failed validity check: \(expected)") + + try assertRefactor(inputSyntax, context: (), provider: InvertIfCondition.self, expected: expectedSyntax) + } + } + + func testInvertIfConditionFails() throws { + let tests = [ + // Not negated + """ + if x { + a + } else { + b + } + """, + // No else + """ + if !x { + a + } + """, + // Else if (not a CodeBlock) + """ + if !x { + a + } else if y { + b + } + """, + // Multiple conditions + """ + if !x, !y { + a + } else { + b + } + """, + // Binding + """ + if let x = y { + a + } else { + b + } + """, + ] + + for input in tests { + let inputSyntax = try XCTUnwrap( + ExprSyntax.parse(from: input).as(IfExprSyntax.self), + "Failed validity check: \(input)" + ) + try assertRefactor(inputSyntax, context: (), provider: InvertIfCondition.self, expected: inputSyntax) + } + } +} + +// Private helper to avoid redeclaration conflicts +private extension ExprSyntax { + static func parse(from source: String) -> ExprSyntax { + var parser = Parser(source) + return ExprSyntax.parse(from: &parser) + } +} diff --git a/Tests/SwiftRefactorTest/RemoveRedundantParensTest.swift b/Tests/SwiftRefactorTest/RemoveRedundantParensTest.swift index eb4883b83b8..835111a0fc5 100644 --- a/Tests/SwiftRefactorTest/RemoveRedundantParensTest.swift +++ b/Tests/SwiftRefactorTest/RemoveRedundantParensTest.swift @@ -96,3 +96,11 @@ class FindTupleVisitor: SyntaxVisitor { return .visitChildren } } + +// Private helper to avoid redeclaration conflicts +private extension ExprSyntax { + static func parse(from source: String) -> ExprSyntax { + var parser = Parser(source) + return ExprSyntax.parse(from: &parser) + } +} From 08c1c8bab9202129c8f1793a06bee830e0bee224 Mon Sep 17 00:00:00 2001 From: Rick Hohler Date: Fri, 20 Feb 2026 15:46:53 -0600 Subject: [PATCH 3/4] feat(refactor): Implement InvertIfCondition (clean) --- Sources/SwiftRefactor/CMakeLists.txt | 1 + .../SwiftRefactor/RemoveRedundantParens.swift | 85 -------------- .../RemoveRedundantParensTest.swift | 106 ------------------ 3 files changed, 1 insertion(+), 191 deletions(-) delete mode 100644 Sources/SwiftRefactor/RemoveRedundantParens.swift delete mode 100644 Tests/SwiftRefactorTest/RemoveRedundantParensTest.swift diff --git a/Sources/SwiftRefactor/CMakeLists.txt b/Sources/SwiftRefactor/CMakeLists.txt index 8a7325f37ed..46df20e1e24 100644 --- a/Sources/SwiftRefactor/CMakeLists.txt +++ b/Sources/SwiftRefactor/CMakeLists.txt @@ -18,6 +18,7 @@ add_swift_syntax_library(SwiftRefactor ExpandEditorPlaceholder.swift FormatRawStringLiteral.swift IntegerLiteralUtilities.swift + InvertIfCondition.swift MigrateToNewIfLetSyntax.swift OpaqueParameterToGeneric.swift RefactoringProvider.swift diff --git a/Sources/SwiftRefactor/RemoveRedundantParens.swift b/Sources/SwiftRefactor/RemoveRedundantParens.swift deleted file mode 100644 index fb5a4ca69ea..00000000000 --- a/Sources/SwiftRefactor/RemoveRedundantParens.swift +++ /dev/null @@ -1,85 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the Swift.org open source project -// -// Copyright (c) 2014 - 2026 Apple Inc. and the Swift project authors -// Licensed under Apache License v2.0 with Runtime Library Exception -// -// See https://swift.org/LICENSE.txt for license information -// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors -// -//===----------------------------------------------------------------------===// - -#if compiler(>=6) -public import SwiftSyntax -#else -import SwiftSyntax -#endif - -/// Removes redundant parentheses from a single-element tuple. -/// -/// ## Before -/// -/// ```swift -/// let x = (10) -/// ``` -/// -/// ## After -/// -/// ```swift -/// let x = 10 -/// ``` -public struct RemoveRedundantParens: SyntaxRefactoringProvider { - public static func refactor(syntax tupleExpr: TupleExprSyntax, in context: Void) -> ExprSyntax { - // 1. Must be exactly one element. - guard tupleExpr.elements.count == 1, - let element = tupleExpr.elements.first - else { - return ExprSyntax(tupleExpr) - } - - // 2. Must not have a label. - guard element.label == nil else { - return ExprSyntax(tupleExpr) - } - - // 3. Extract the inner expression. - let innerExpr = element.expression - - // 4. Safety Check: Precedence Ambiguity - // If we are unwrapping a SequenceExpr (binary op) AND we are inside another SequenceExpr, - // we assume it is UNSAFE because we don't know operator precedence. - // Example: x * (y + z) -> unwrapping makes it x * y + z (different logic). - - // Check if inner is SequenceExpr - // Check if inner is SequenceExpr - if innerExpr.is(SequenceExprSyntax.self) { - // Case: Parent is ExprList (standard SequenceExpr elements) -> Grandparent is SequenceExpr - if let parent = tupleExpr.parent, - parent.is(ExprListSyntax.self), - let grandParent = parent.parent, - grandParent.is(SequenceExprSyntax.self) - { - return ExprSyntax(tupleExpr) - } - - // Case: Direct parent is SequenceExpr (fallback) - if let parent = tupleExpr.parent, parent.is(SequenceExprSyntax.self) { - return ExprSyntax(tupleExpr) - } - } - - // 5. Preserve Trivia - // We want to keep comments attached to the parens, but maybe not the newlines inside? - // A safe default is to take the tuple's leading/trailing trivia and apply it to the inner expression. - // This replaces the inner expression's existing leading/trailing trivia if we use .with, - // so we might want to append? - // Actually, usually `( /* comment */ x )` -> `/* comment */ x`. - // Let's replace. - - return - innerExpr - .with(\.leadingTrivia, tupleExpr.leadingTrivia) - .with(\.trailingTrivia, tupleExpr.trailingTrivia) - } -} diff --git a/Tests/SwiftRefactorTest/RemoveRedundantParensTest.swift b/Tests/SwiftRefactorTest/RemoveRedundantParensTest.swift deleted file mode 100644 index 835111a0fc5..00000000000 --- a/Tests/SwiftRefactorTest/RemoveRedundantParensTest.swift +++ /dev/null @@ -1,106 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the Swift.org open source project -// -// Copyright (c) 2014 - 2026 Apple Inc. and the Swift project authors -// Licensed under Apache License v2.0 with Runtime Library Exception -// -// See https://swift.org/LICENSE.txt for license information -// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors -// -//===----------------------------------------------------------------------===// - -import SwiftParser -import SwiftRefactor -import SwiftSyntax -import SwiftSyntaxBuilder -import XCTest -import _SwiftSyntaxTestSupport - -final class RemoveRedundantParensTest: XCTestCase { - func testRemoveRedundantParens() throws { - let tests = [ - ( - "(10)", - "10" - ), - ( - "((10))", - "(10)" - ), - ( - "(y + z)", - "y + z" - ), - ( - "/* c */ (x)", - "/* c */ x" - ), - ] - - for (input, expected) in tests { - let inputSyntax = try XCTUnwrap( - ExprSyntax.parse(from: input).as(TupleExprSyntax.self), - "Failed validity check: \(input)" - ) - // For expected, it might not be a TupleExpr (it's the unwrapped expr) - let expectedSyntax = ExprSyntax.parse(from: expected) - - try assertRefactor(inputSyntax, context: (), provider: RemoveRedundantParens.self, expected: expectedSyntax) - } - } - - func testRemoveRedundantParensFails() throws { - let tests = [ - // Multiple elements - "(1, 2)", - // Labeled element - "(x: 1)", - // Empty - "()", - // Safety: Binary op inside Binary op - "x * (y + z)", - "(a + b) / c", - ] - - for input in tests { - // Input might be a SequenceExpr containing a TupleExpr, not just a TupleExpr. - let fullSyntax = ExprSyntax.parse(from: input) - - // Find the first TupleExpr in the tree - let visitor = FindTupleVisitor(viewMode: .sourceAccurate) - visitor.walk(fullSyntax) - - guard let tupleExpr = visitor.tupleExpr else { - // If top level is tuple - if let tuple = fullSyntax.as(TupleExprSyntax.self) { - try assertRefactor(tuple, context: (), provider: RemoveRedundantParens.self, expected: ExprSyntax(tuple)) - continue - } - XCTFail("Could not find tuple in \(input)") - continue - } - - // We expect the refactoring to return the tuple unchanged (as ExprSyntax) - try assertRefactor(tupleExpr, context: (), provider: RemoveRedundantParens.self, expected: ExprSyntax(tupleExpr)) - } - } -} - -class FindTupleVisitor: SyntaxVisitor { - var tupleExpr: TupleExprSyntax? - override func visit(_ node: TupleExprSyntax) -> SyntaxVisitorContinueKind { - if tupleExpr == nil { - tupleExpr = node - } - return .visitChildren - } -} - -// Private helper to avoid redeclaration conflicts -private extension ExprSyntax { - static func parse(from source: String) -> ExprSyntax { - var parser = Parser(source) - return ExprSyntax.parse(from: &parser) - } -} From 58c1796a6f63299899cb032bd14005b1c9a2f293 Mon Sep 17 00:00:00 2001 From: Rick Hohler Date: Fri, 27 Feb 2026 20:54:55 -0600 Subject: [PATCH 4/4] Address reviewer feedback for InvertIfCondition: remove numbered comments, reformat guards, improve trivia preservation, and add test helper --- Sources/SwiftRefactor/InvertIfCondition.swift | 30 ++----------- .../InvertIfConditionTest.swift | 43 +++++++++++++------ 2 files changed, 35 insertions(+), 38 deletions(-) diff --git a/Sources/SwiftRefactor/InvertIfCondition.swift b/Sources/SwiftRefactor/InvertIfCondition.swift index 7061046cf99..8b246a0b531 100644 --- a/Sources/SwiftRefactor/InvertIfCondition.swift +++ b/Sources/SwiftRefactor/InvertIfCondition.swift @@ -39,49 +39,27 @@ import SwiftSyntax /// ``` 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`). - guard let elseBody = ifExpr.elseBody, - case .codeBlock(let elseBlock) = elseBody - else { + guard let elseBody = ifExpr.elseBody, case .codeBlock(let elseBlock) = elseBody else { return ifExpr } - // 2. Must have exactly one condition. - guard ifExpr.conditions.count == 1, - let condition = ifExpr.conditions.first - else { + guard ifExpr.conditions.count == 1, let condition = ifExpr.conditions.first else { return ifExpr } - // 3. Condition must be an expression (not a binding like `let x`). guard case .expression(let expr) = condition.condition else { return ifExpr } - // 4. Expression must be a PrefixOperatorExpr with operator "!". - guard let prefixOpExpr = expr.as(PrefixOperatorExprSyntax.self), - prefixOpExpr.operator.text == "!" - else { + guard let prefixOpExpr = expr.as(PrefixOperatorExprSyntax.self), prefixOpExpr.operator.text == "!" else { return ifExpr } - // 5. Extract inner expression (remove the `!`). - // 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. - let innerExpr = prefixOpExpr.expression - .with(\.leadingTrivia, prefixOpExpr.leadingTrivia) + let innerExpr = prefixOpExpr.expression.with(\.leadingTrivia, prefixOpExpr.leadingTrivia.merging(triviaOf: prefixOpExpr.operator)) - // 6. Create new condition list. let newCondition = condition.with(\.condition, .expression(innerExpr)) let newConditions = ifExpr.conditions.with(\.[ifExpr.conditions.startIndex], newCondition) - // 7. Swap bodies with Trivia preservation. - // We strictly swap the trivia: - // New body (was else) takes old body's trivia (space before `{`, space after `}`). - // New else (was body) takes old else's trivia (space before `{`?, newline after `}`). - let oldBody = ifExpr.body let oldElseBlock = elseBlock diff --git a/Tests/SwiftRefactorTest/InvertIfConditionTest.swift b/Tests/SwiftRefactorTest/InvertIfConditionTest.swift index cdd8c736d3c..5549fcdb8af 100644 --- a/Tests/SwiftRefactorTest/InvertIfConditionTest.swift +++ b/Tests/SwiftRefactorTest/InvertIfConditionTest.swift @@ -72,13 +72,7 @@ final class InvertIfConditionTest: XCTestCase { ] for (input, expected) in tests { - let inputSyntax = try XCTUnwrap( - ExprSyntax.parse(from: input).as(IfExprSyntax.self), - "Failed validity check: \(input)" - ) - let expectedSyntax = try XCTUnwrap(ExprSyntax.parse(from: expected), "Failed validity check: \(expected)") - - try assertRefactor(inputSyntax, context: (), provider: InvertIfCondition.self, expected: expectedSyntax) + try assertInvertIfCondition(input, expected: expected) } } @@ -125,13 +119,38 @@ final class InvertIfConditionTest: XCTestCase { ] for input in tests { - let inputSyntax = try XCTUnwrap( - ExprSyntax.parse(from: input).as(IfExprSyntax.self), - "Failed validity check: \(input)" - ) - try assertRefactor(inputSyntax, context: (), provider: InvertIfCondition.self, expected: inputSyntax) + try assertInvertIfCondition(input, expected: input) } } + + private func assertInvertIfCondition( + _ input: String, + expected: String, + file: StaticString = #filePath, + line: UInt = #line + ) throws { + let inputSyntax = try XCTUnwrap( + ExprSyntax.parse(from: input).as(IfExprSyntax.self), + "Failed validity check: \(input)", + file: file, + line: line + ) + let expectedSyntax = try XCTUnwrap( + ExprSyntax.parse(from: expected), + "Failed validity check: \(expected)", + file: file, + line: line + ) + + try assertRefactor( + inputSyntax, + context: (), + provider: InvertIfCondition.self, + expected: expectedSyntax, + file: file, + line: line + ) + } } // Private helper to avoid redeclaration conflicts