diff --git a/Sources/FormattingHelpers.swift b/Sources/FormattingHelpers.swift index bf38b0625..ecf75eda1 100644 --- a/Sources/FormattingHelpers.swift +++ b/Sources/FormattingHelpers.swift @@ -1198,7 +1198,9 @@ extension Formatter { /// has a single statement. This makes it eligible to be used with implicit return. func blockBodyHasSingleStatement( atStartOfScope startOfScopeIndex: Int, - includingConditionalStatements: Bool = true + includingConditionalStatements: Bool, + includingReturnStatements: Bool, + includingReturnInConditionalStatements: Bool? = nil ) -> Bool { guard let endOfScopeIndex = endOfScope(at: startOfScopeIndex) else { return false } let startOfBody = self.startOfBody(atStartOfScope: startOfScopeIndex) @@ -1211,7 +1213,7 @@ extension Formatter { } // Skip over any optional `return` keyword - if tokens[firstTokenInBody] == .keyword("return") { + if includingReturnStatements, tokens[firstTokenInBody] == .keyword("return") { guard let tokenAfterReturnKeyword = index(of: .nonSpaceOrCommentOrLinebreak, after: firstTokenInBody) else { return false } firstTokenInBody = tokenAfterReturnKeyword } @@ -1237,7 +1239,12 @@ extension Formatter { return false } - return blockBodyHasSingleStatement(atStartOfScope: branch.startOfBranch, includingConditionalStatements: true) + return blockBodyHasSingleStatement( + atStartOfScope: branch.startOfBranch, + includingConditionalStatements: true, + includingReturnStatements: includingReturnInConditionalStatements ?? includingReturnStatements, + includingReturnInConditionalStatements: includingReturnInConditionalStatements + ) } let endOfStatement = conditionalBranches.last?.endOfBranch ?? firstTokenInBody diff --git a/Sources/Rules.swift b/Sources/Rules.swift index 8d4bbfb2b..cf6bd4add 100644 --- a/Sources/Rules.swift +++ b/Sources/Rules.swift @@ -3186,7 +3186,11 @@ public struct _FormatRules { } // Make sure the body only has a single statement - guard formatter.blockBodyHasSingleStatement(atStartOfScope: startOfScopeIndex) else { + guard formatter.blockBodyHasSingleStatement( + atStartOfScope: startOfScopeIndex, + includingConditionalStatements: true, + includingReturnStatements: true + ) else { return } @@ -6230,13 +6234,22 @@ public struct _FormatRules { { /// Whether or not this closure has a single, simple expression in its body. /// These closures can always be simplified / removed regardless of the context. - let hasSingleSimpleExpression = formatter.blockBodyHasSingleStatement(atStartOfScope: closureStartIndex, includingConditionalStatements: false) + let hasSingleSimpleExpression = formatter.blockBodyHasSingleStatement( + atStartOfScope: closureStartIndex, + includingConditionalStatements: false, + includingReturnStatements: true + ) /// Whether or not this closure has a single if/switch expression in its body. /// Since if/switch expressions are only valid in the `return` position or as an `=` assignment, /// these closures can only sometimes be simplified / removed. let hasSingleConditionalExpression = !hasSingleSimpleExpression && - formatter.blockBodyHasSingleStatement(atStartOfScope: closureStartIndex, includingConditionalStatements: true) + formatter.blockBodyHasSingleStatement( + atStartOfScope: closureStartIndex, + includingConditionalStatements: true, + includingReturnStatements: true, + includingReturnInConditionalStatements: false + ) guard hasSingleSimpleExpression || hasSingleConditionalExpression else { return @@ -6920,7 +6933,8 @@ public struct _FormatRules { } public let conditionalAssignment = FormatRule( - help: "Assign properties using if / switch expressions." + help: "Assign properties using if / switch expressions.", + orderAfter: ["redundantReturn"] ) { formatter in // If / switch expressions were added in Swift 5.9 (SE-0380) guard formatter.options.swiftVersion >= "5.9" else { @@ -7017,7 +7031,11 @@ public struct _FormatRules { tempScopeTokens.append(.endOfScope("}")) let tempFormatter = Formatter(tempScopeTokens, options: formatter.options) - guard tempFormatter.blockBodyHasSingleStatement(atStartOfScope: 0) else { + guard tempFormatter.blockBodyHasSingleStatement( + atStartOfScope: 0, + includingConditionalStatements: true, + includingReturnStatements: false + ) else { return false } diff --git a/Tests/RulesTests+Redundancy.swift b/Tests/RulesTests+Redundancy.swift index 74dc3eb06..8a7698ebe 100644 --- a/Tests/RulesTests+Redundancy.swift +++ b/Tests/RulesTests+Redundancy.swift @@ -2722,6 +2722,22 @@ class RedundancyTests: RulesTests { testFormatting(for: input, output, rule: FormatRules.redundantReturn, options: options) } + func testClosureAroundConditionalAssignmentNotRedundantForExplicitReturn() { + let input = """ + let myEnum = MyEnum.a + let test: Int = { + switch myEnum { + case .a: + return 0 + case .b: + return 1 + } + }() + """ + let options = FormatOptions(swiftVersion: "5.9") + testFormatting(for: input, rule: FormatRules.redundantClosure, options: options, exclude: ["redundantReturn"]) + } + func testNonRedundantSwitchStatementReturnInFunction() { let input = """ func foo(condition: Bool) -> String { @@ -7833,7 +7849,8 @@ class RedundancyTests: RulesTests { """ let options = FormatOptions(swiftVersion: "5.9") - testFormatting(for: input, output, rule: FormatRules.redundantClosure, options: options, exclude: ["indent"]) + testFormatting(for: input, [output], rules: [FormatRules.redundantReturn, FormatRules.redundantClosure], + options: options, exclude: ["indent"]) } func testRedundantClosureWithExplicitReturn2() { @@ -7869,7 +7886,7 @@ class RedundancyTests: RulesTests { } """ - testFormatting(for: input, output, rule: FormatRules.redundantClosure, exclude: ["redundantReturn"]) + testFormatting(for: input, [output], rules: [FormatRules.redundantReturn, FormatRules.redundantClosure]) } func testKeepsClosureThatIsNotCalled() { @@ -7936,7 +7953,8 @@ class RedundancyTests: RulesTests { } """ - testFormatting(for: input, [output], rules: [FormatRules.redundantClosure, FormatRules.semicolons]) + testFormatting(for: input, [output], rules: [FormatRules.redundantReturn, FormatRules.redundantClosure, + FormatRules.semicolons]) } func testRemoveRedundantClosureInWrappedPropertyDeclaration_beforeFirst() { @@ -8222,14 +8240,14 @@ class RedundancyTests: RulesTests { """ let output = """ let user2: User? = if let data2 = defaults.data(forKey: defaultsKey) { - return try PropertyListDecoder().decode(User.self, from: data2) + try PropertyListDecoder().decode(User.self, from: data2) } else { - return nil + nil } """ let options = FormatOptions(swiftVersion: "5.9") - testFormatting(for: input, output, rule: FormatRules.redundantClosure, options: options, - exclude: ["redundantReturn", "indent"]) + testFormatting(for: input, [output], rules: [FormatRules.redundantReturn, FormatRules.redundantClosure], + options: options, exclude: ["indent"]) } func testRedundantClosureDoesntLeaveStrayTryAwait() { @@ -8244,14 +8262,14 @@ class RedundancyTests: RulesTests { """ let output = """ let user2: User? = if let data2 = defaults.data(forKey: defaultsKey) { - return try await PropertyListDecoder().decode(User.self, from: data2) + try await PropertyListDecoder().decode(User.self, from: data2) } else { - return nil + nil } """ let options = FormatOptions(swiftVersion: "5.9") - testFormatting(for: input, output, rule: FormatRules.redundantClosure, options: options, - exclude: ["redundantReturn", "indent"]) + testFormatting(for: input, [output], rules: [FormatRules.redundantReturn, FormatRules.redundantClosure], + options: options, exclude: ["indent"]) } func testRedundantClosureDoesntLeaveInvalidSwitchExpressionInOperatorChain() { @@ -8525,7 +8543,8 @@ class RedundancyTests: RulesTests { """ let options = FormatOptions(swiftVersion: "5.9") - testFormatting(for: input, output, rule: FormatRules.redundantClosure, options: options, exclude: ["redundantReturn", "indent"]) + testFormatting(for: input, [output], rules: [FormatRules.redundantClosure], + options: options, exclude: ["redundantReturn", "indent"]) } func testRedundantClosureRemovesClosureAsImplicitReturnStatement() { @@ -8706,7 +8725,70 @@ class RedundancyTests: RulesTests { testFormatting(for: input, output, rule: FormatRules.redundantClosure, options: options, exclude: ["indent"]) } - // MARK: Redundant optional binding + func testRedundantClosureDoesntBreakBuildWithRedundantReturnRuleDisabled() { + let input = """ + enum MyEnum { + case a + case b + } + let myEnum = MyEnum.a + let test: Int = { + return 0 + }() + """ + + let output = """ + enum MyEnum { + case a + case b + } + let myEnum = MyEnum.a + let test: Int = 0 + """ + + let options = FormatOptions(swiftVersion: "5.9") + testFormatting(for: input, output, rule: FormatRules.redundantClosure, options: options, + exclude: ["redundantReturn", "blankLinesBetweenScopes"]) + } + + func testRedundantClosureWithSwitchExpressionDoesntBreakBuildWithRedundantReturnRuleDisabled() { + // From https://github.com/nicklockwood/SwiftFormat/issues/1565 + let input = """ + enum MyEnum { + case a + case b + } + let myEnum = MyEnum.a + let test: Int = { + switch myEnum { + case .a: + return 0 + case .b: + return 1 + } + }() + """ + + let output = """ + enum MyEnum { + case a + case b + } + let myEnum = MyEnum.a + let test: Int = switch myEnum { + case .a: + 0 + case .b: + 1 + } + """ + + let options = FormatOptions(swiftVersion: "5.9") + testFormatting(for: input, [output], rules: [FormatRules.redundantReturn, FormatRules.redundantClosure], + options: options, exclude: ["indent", "blankLinesBetweenScopes"]) + } + + // MARK: - redundantOptionalBinding func testRemovesRedundantOptionalBindingsInSwift5_7() { let input = """