Skip to content

Commit

Permalink
Fix issue where if/switch expressions with 'as?' cast would break bui…
Browse files Browse the repository at this point in the history
…ld in Swift 5.9
  • Loading branch information
calda authored and nicklockwood committed Oct 27, 2023
1 parent e1b3873 commit c2c9dc2
Show file tree
Hide file tree
Showing 5 changed files with 369 additions and 7 deletions.
48 changes: 45 additions & 3 deletions Sources/FormattingHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1222,14 +1222,28 @@ extension Formatter {
includingConditionalStatements,
let conditionalBranches = conditionalBranches(at: firstTokenInBody)
{
let isSingleStatement = conditionalBranches.allSatisfy { branch in
blockBodyHasSingleStatement(atStartOfScope: branch.startOfBranch, includingConditionalStatements: true)
let isSupportedSingleStatement = conditionalBranches.allSatisfy { branch in
// In Swift 5.9, there's a bug that prevents you from writing an
// if or switch expression using an `as?` on one of the branches:
// https://github.com/apple/swift/issues/68764
//
// if condition {
// foo as? String
// } else {
// "bar"
// }
//
if conditionalBranchHasUnsupportedCastOperator(startOfScopeIndex: branch.startOfBranch) {
return false
}

return blockBodyHasSingleStatement(atStartOfScope: branch.startOfBranch, includingConditionalStatements: true)
}

let endOfStatement = conditionalBranches.last?.endOfBranch ?? firstTokenInBody
let isOnlyStatement = index(of: .nonSpaceOrCommentOrLinebreak, after: endOfStatement) == endOfScopeIndex

return isSingleStatement && isOnlyStatement
return isSupportedSingleStatement && isOnlyStatement
}

guard let expressionRange = parseExpressionRange(startingAt: firstTokenInBody),
Expand Down Expand Up @@ -1338,6 +1352,34 @@ extension Formatter {
return branches
}

/// In Swift 5.9, there's a bug that prevents you from writing an
/// if or switch expression using an `as?` on one of the branches:
/// https://github.com/apple/swift/issues/68764
///
/// if condition {
/// foo as? String
/// } else {
/// "bar"
/// }
///
/// This helper returns whether or not the branch starting at the given `startOfScopeIndex`
/// includes an `as?` operator, so wouldn't be permitted in a if/switch exprssion in Swift 5.9
func conditionalBranchHasUnsupportedCastOperator(startOfScopeIndex: Int) -> Bool {
if options.swiftVersion == "5.9",
let asIndex = index(of: .keyword("as"), after: startOfScopeIndex),
let endOfScopeIndex = endOfScope(at: startOfScopeIndex),
asIndex < endOfScopeIndex,
next(.nonSpaceOrCommentOrLinebreak, after: asIndex) == .operator("?", .postfix),
// Make sure the as? is at the top level, not nested in some
// inner scope like a function call or closure
startOfScope(at: asIndex) == startOfScopeIndex
{
return true
}

return false
}

/// Performs a closure for each conditional branch in the given conditional statement,
/// including any recursive conditional inside an individual branch.
/// Iterates backwards to support removing tokens in `handle`.
Expand Down
52 changes: 49 additions & 3 deletions Sources/Rules.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3199,7 +3199,12 @@ public struct _FormatRules {
return
}

// Removes return statements in the given single-statement scope
// Find all of the return keywords to remove before we remove any of them,
// so we can apply additional validation first.
var returnKeywordRangesToRemove = [Range<Int>]()
var hasReturnThatCantBeRemoved = false

/// Finds the return keywords to remove and stores them in `returnKeywordRangesToRemove`
func removeReturn(atStartOfScope startOfScopeIndex: Int) {
// If this scope is a single-statement if or switch statement then we have to recursively
// remove the return from each branch of the if statement
Expand All @@ -3209,6 +3214,23 @@ public struct _FormatRules {
let conditionalBranches = formatter.conditionalBranches(at: firstTokenInBody)
{
for branch in conditionalBranches.reversed() {
// In Swift 5.9, there's a bug that prevents you from writing an
// if or switch expression using an `as?` on one of the branches:
// https://github.com/apple/swift/issues/68764
//
// if condition {
// foo as? String
// } else {
// "bar"
// }
//
if formatter.conditionalBranchHasUnsupportedCastOperator(
startOfScopeIndex: branch.startOfBranch)
{
hasReturnThatCantBeRemoved = true
return
}

removeReturn(atStartOfScope: branch.startOfBranch)
}
}
Expand All @@ -3229,11 +3251,17 @@ public struct _FormatRules {
returnIndices[i] -= range.count
}
}
formatter.removeTokens(in: range)
returnKeywordRangesToRemove.append(range)
}
}

removeReturn(atStartOfScope: startOfScopeIndex)

guard !hasReturnThatCantBeRemoved else { return }

for returnKeywordRangeToRemove in returnKeywordRangesToRemove.sorted(by: { $0.startIndex > $1.startIndex }) {
formatter.removeTokens(in: returnKeywordRangeToRemove)
}
}
}

Expand Down Expand Up @@ -6989,7 +7017,25 @@ public struct _FormatRules {
tempScopeTokens.append(.endOfScope("}"))

let tempFormatter = Formatter(tempScopeTokens, options: formatter.options)
return tempFormatter.blockBodyHasSingleStatement(atStartOfScope: 0)
guard tempFormatter.blockBodyHasSingleStatement(atStartOfScope: 0) else {
return false
}

// In Swift 5.9, there's a bug that prevents you from writing an
// if or switch expression using an `as?` on one of the branches:
// https://github.com/apple/swift/issues/68764
//
// let result = if condition {
// foo as? String
// } else {
// "bar"
// }
//
if tempFormatter.conditionalBranchHasUnsupportedCastOperator(startOfScopeIndex: 0) {
return false
}

return true
}

return false
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftFormat.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public let swiftVersionFile = ".swift-version"
/// Supported Swift versions
public let swiftVersions = [
"3.x", "4.0", "4.1", "4.2",
"5.0", "5.1", "5.2", "5.3", "5.4", "5.5", "5.6", "5.7", "5.8", "5.9",
"5.0", "5.1", "5.2", "5.3", "5.4", "5.5", "5.6", "5.7", "5.8", "5.9", "5.10",
]

/// An enumeration of the types of error that may be thrown by SwiftFormat
Expand Down
157 changes: 157 additions & 0 deletions Tests/RulesTests+Redundancy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3017,6 +3017,91 @@ class RedundancyTests: RulesTests {
exclude: ["wrapSwitchCases", "sortSwitchCases"])
}

func testDoesntRemoveReturnFromIfExpressionConditionalCastInSwift5_9() {
// The following code doesn't compile in Swift 5.9 due to this issue:
// https://github.com/apple/swift/issues/68764
//
// var result: String {
// if condition {
// foo as? String
// } else {
// "bar"
// }
// }
//
let input = """
var result1: String {
if condition {
return foo as? String
} else {
return "bar"
}
}
var result2: String {
switch condition {
case true:
return foo as? String
case false:
return "bar"
}
}
"""

let options = FormatOptions(swiftVersion: "5.9")
testFormatting(for: input, rule: FormatRules.redundantReturn, options: options)
}

func testRemovseReturnFromIfExpressionNestedConditionalCastInSwift5_9() {
let input = """
var result1: String {
if condition {
return method(foo as? String)
} else {
return "bar"
}
}
"""

let output = """
var result1: String {
if condition {
method(foo as? String)
} else {
"bar"
}
}
"""

let options = FormatOptions(swiftVersion: "5.9")
testFormatting(for: input, output, rule: FormatRules.redundantReturn, options: options)
}

func testRemovesReturnFromIfExpressionConditionalCastInSwift5_10() {
let input = """
var result: String {
if condition {
return foo as? String
} else {
return "bar"
}
}
"""

let output = """
var result: String {
if condition {
foo as? String
} else {
"bar"
}
}
"""

let options = FormatOptions(swiftVersion: "5.10")
testFormatting(for: input, output, rule: FormatRules.redundantReturn, options: options)
}

// MARK: - redundantBackticks

func testRemoveRedundantBackticksInLet() {
Expand Down Expand Up @@ -8549,6 +8634,78 @@ class RedundancyTests: RulesTests {
testFormatting(for: input, rule: FormatRules.redundantClosure, options: options)
}

func testDoesntRemoveClosureWithIfExpressionConditionalCastInSwift5_9() {
// The following code doesn't compile in Swift 5.9 due to this issue:
// https://github.com/apple/swift/issues/68764
//
// let result = if condition {
// foo as? String
// } else {
// "bar"
// }
//
let input = """
let result1: String? = {
if condition {
return foo as? String
} else {
return "bar"
}
}()
let result1: String? = {
switch condition {
case true:
return foo as? String
case false:
return "bar"
}
}()
"""

let options = FormatOptions(swiftVersion: "5.9")
testFormatting(for: input, rule: FormatRules.redundantClosure, options: options)
}

func testDoesRemoveClosureWithIfExpressionConditionalCastInSwift5_10() {
let input = """
let result1: String? = {
if condition {
foo as? String
} else {
"bar"
}
}()
let result2: String? = {
switch condition {
case true:
foo as? String
case false:
"bar"
}
}()
"""

let output = """
let result1: String? = if condition {
foo as? String
} else {
"bar"
}
let result2: String? = switch condition {
case true:
foo as? String
case false:
"bar"
}
"""

let options = FormatOptions(swiftVersion: "5.10")
testFormatting(for: input, output, rule: FormatRules.redundantClosure, options: options, exclude: ["indent"])
}

// MARK: Redundant optional binding

func testRemovesRedundantOptionalBindingsInSwift5_7() {
Expand Down
Loading

0 comments on commit c2c9dc2

Please sign in to comment.