Skip to content

Commit

Permalink
Update conditionalAssignment rule to support more complex lvalues
Browse files Browse the repository at this point in the history
  • Loading branch information
calda authored and nicklockwood committed Jun 9, 2024
1 parent 50f7f46 commit 1995910
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 15 deletions.
12 changes: 11 additions & 1 deletion Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,6 @@ Assign properties using if / switch expressions.
- bar = "bar"
+ "bar"
}
```

```diff
- let foo: String
Expand All @@ -534,6 +533,17 @@ Assign properties using if / switch expressions.
}
```

- switch condition {
+ foo.bar = switch condition {
case true:
- foo.bar = "baaz"
+ "baaz"
case false:
- foo.bar = "quux"
+ "quux"
}
```
</details>
<br/>
Expand Down
12 changes: 11 additions & 1 deletion Sources/Examples.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1652,7 +1652,6 @@ private struct Examples {
- bar = "bar"
+ "bar"
}
```
```diff
- let foo: String
Expand All @@ -1666,6 +1665,17 @@ private struct Examples {
+ "bar"
}
```
- switch condition {
+ foo.bar = switch condition {
case true:
- foo.bar = "baaz"
+ "baaz"
case false:
- foo.bar = "quux"
+ "quux"
}
```
"""

let sortTypealiases = """
Expand Down
29 changes: 16 additions & 13 deletions Sources/Rules.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7154,11 +7154,10 @@ public struct _FormatRules {
startOfFirstBranch = startOfNestedBranch
}

// Check if the first branch starts with the pattern `identifier =`.
guard let firstIdentifierIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: startOfFirstBranch),
let identifier = formatter.token(at: firstIdentifierIndex),
identifier.isIdentifier,
let equalsIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: firstIdentifierIndex),
// Check if the first branch starts with the pattern `lvalue =`.
guard let firstTokenIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: startOfFirstBranch),
let lvalueRange = formatter.parseExpressionRange(startingAt: firstTokenIndex),
let equalsIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: lvalueRange.upperBound),
formatter.tokens[equalsIndex] == .operator("=", .infix)
else { return }

Expand Down Expand Up @@ -7189,7 +7188,7 @@ public struct _FormatRules {

// Whether or not the given conditional branch body qualifies as a single statement
// that assigns a value to `identifier`. This is either:
// 1. a single assignment to `identifier =`
// 1. a single assignment to `lvalue =`
// 2. a single `if` or `switch` statement where each of the branches also qualify,
// and the statement is exhaustive.
func isExhaustiveSingleStatementAssignment(_ branch: Formatter.ConditionalBranch) -> Bool {
Expand All @@ -7216,9 +7215,10 @@ public struct _FormatRules {
&& isExhaustive
}

// Otherwise we expect this to be of the pattern `identifier = (statement)`
else if formatter.tokens[firstTokenIndex] == identifier,
let equalsIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: firstTokenIndex),
// Otherwise we expect this to be of the pattern `lvalue = (statement)`
else if let firstExpressionRange = formatter.parseExpressionRange(startingAt: firstTokenIndex),
formatter.tokens[firstExpressionRange] == formatter.tokens[lvalueRange],
let equalsIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: firstExpressionRange.upperBound),
formatter.tokens[equalsIndex] == .operator("=", .infix),
let valueStartIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: equalsIndex)
{
Expand Down Expand Up @@ -7270,7 +7270,8 @@ public struct _FormatRules {
func removeAssignmentFromAllBranches() {
formatter.forEachRecursiveConditionalBranch(in: conditionalBranches) { branch in
guard let firstTokenIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: branch.startOfBranch),
let equalsIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: firstTokenIndex),
let firstExpressionRange = formatter.parseExpressionRange(startingAt: firstTokenIndex),
let equalsIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: firstExpressionRange.upperBound),
formatter.tokens[equalsIndex] == .operator("=", .infix),
let valueStartIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: equalsIndex)
else { return }
Expand All @@ -7288,7 +7289,8 @@ public struct _FormatRules {
let propertyIdentifierIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: introducerIndex),
let propertyIdentifier = formatter.token(at: propertyIdentifierIndex),
propertyIdentifier.isIdentifier,
propertyIdentifier == identifier,
formatter.tokens[lvalueRange.lowerBound] == propertyIdentifier,
lvalueRange.count == 1,
let colonIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: propertyIdentifierIndex),
formatter.tokens[colonIndex] == .delimiter(":"),
let startOfTypeIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: colonIndex),
Expand Down Expand Up @@ -7352,12 +7354,13 @@ public struct _FormatRules {
(startOfFirstParentBranch ... endOfLastParentBranch).contains(startOfConditional)
{ return }

let lvalueTokens = formatter.tokens[lvalueRange]

// Now we can remove the `identifier =` from each branch,
// and instead add it before the if / switch expression.
removeAssignmentFromAllBranches()

let identifierEqualsTokens: [Token] = [
identifier,
let identifierEqualsTokens = lvalueTokens + [
.space(" "),
.operator("=", .infix),
.space(" "),
Expand Down
24 changes: 24 additions & 0 deletions Tests/RulesTests+Syntax.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4395,6 +4395,30 @@ class SyntaxTests: RulesTests {
testFormatting(for: input, [output], rules: [FormatRules.conditionalAssignment, FormatRules.wrapMultilineConditionalAssignment, FormatRules.indent], options: options)
}

func testConvertsSwitchStatementWithComplexLValueNotFollowingPropertyDefinition() {
let input = """
switch condition {
case true:
property?.foo!.bar["baaz"] = Foo("foo")
case false:
property?.foo!.bar["baaz"] = Foo("bar")
}
"""

let output = """
property?.foo!.bar["baaz"] =
switch condition {
case true:
Foo("foo")
case false:
Foo("bar")
}
"""

let options = FormatOptions(swiftVersion: "5.9")
testFormatting(for: input, [output], rules: [FormatRules.conditionalAssignment, FormatRules.wrapMultilineConditionalAssignment, FormatRules.indent], options: options)
}

func testDoesntMergePropertyWithUnrelatedCondition() {
let input = """
let differentProperty: Foo
Expand Down
23 changes: 23 additions & 0 deletions Tests/RulesTests+Wrapping.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5375,4 +5375,27 @@ class WrappingTests: RulesTests {

testFormatting(for: input, [output], rules: [FormatRules.wrapMultilineConditionalAssignment, FormatRules.indent])
}

func testWrapSwitchAssignmentWithComplexLValue() {
let input = """
property?.foo!.bar["baaz"] = switch condition {
case true:
Foo("foo")
case false:
Foo("bar")
}
"""

let output = """
property?.foo!.bar["baaz"] =
switch condition {
case true:
Foo("foo")
case false:
Foo("bar")
}
"""

testFormatting(for: input, [output], rules: [FormatRules.wrapMultilineConditionalAssignment, FormatRules.indent])
}
}

0 comments on commit 1995910

Please sign in to comment.