Skip to content

Commit

Permalink
Fix redundantClosure when redundantReturn disabled
Browse files Browse the repository at this point in the history
  • Loading branch information
@nicklockwood
nicklockwood committed Nov 4, 2023
1 parent 1614652 commit 5455a18
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 21 deletions.
13 changes: 10 additions & 3 deletions Sources/FormattingHelpers.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}
Expand All @@ -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
Expand Down
28 changes: 23 additions & 5 deletions Sources/Rules.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down
108 changes: 95 additions & 13 deletions Tests/RulesTests+Redundancy.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand All @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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 = """
Expand Down

0 comments on commit 5455a18

Please sign in to comment.