Skip to content

Commit

Permalink
Add new rules related to blank lines in switch statements (#1621)
Browse files Browse the repository at this point in the history
  • Loading branch information
calda authored and nicklockwood committed Feb 3, 2024
1 parent 0fb8edc commit 7625b54
Show file tree
Hide file tree
Showing 9 changed files with 889 additions and 10 deletions.
91 changes: 91 additions & 0 deletions Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,10 @@
# Opt-in Rules (disabled by default)

* [acronyms](#acronyms)
* [blankLineAfterMultilineSwitchCase](#blankLineAfterMultilineSwitchCase)
* [blankLinesBetweenImports](#blankLinesBetweenImports)
* [blockComments](#blockComments)
* [consistentSwitchStatementSpacing](#consistentSwitchStatementSpacing)
* [docComments](#docComments)
* [isEmpty](#isEmpty)
* [markTypes](#markTypes)
Expand Down Expand Up @@ -238,6 +240,38 @@ Insert blank line after import statements.
</details>
<br/>

## blankLineAfterMultilineSwitchCase

Insert a blank line after multiline switch cases (excluding the last case,
which is followed by a closing brace).

<details>
<summary>Examples</summary>

```diff
func handle(_ action: SpaceshipAction) {
switch action {
case .engageWarpDrive:
navigationComputer.destination = targetedDestination
await warpDrive.spinUp()
warpDrive.activate()
+
case let .scanPlanet(planet):
scanner.target = planet
scanner.scanAtmosphere()
scanner.scanBiosphere()
scanner.scanForArticialLife()
+
case .handleIncomingEnergyBlast:
await energyShields.prepare()
energyShields.engage()
}
}
```

</details>
<br/>

## blankLinesAroundMark

Insert blank line before and after `MARK:` comments.
Expand Down Expand Up @@ -543,6 +577,63 @@ Replace consecutive spaces with a single space.
</details>
<br/>

## consistentSwitchStatementSpacing

Ensures consistent spacing among all of the cases in a switch statement.

<details>
<summary>Examples</summary>

```diff
func handle(_ action: SpaceshipAction) {
switch action {
case .engageWarpDrive:
navigationComputer.destination = targetedDestination
await warpDrive.spinUp()
warpDrive.activate()

case .enableArtificialGravity:
artificialGravityEngine.enable(strength: .oneG)
+
case let .scanPlanet(planet):
scanner.target = planet
scanner.scanAtmosphere()
scanner.scanBiosphere()
scanner.scanForArtificialLife()

case .handleIncomingEnergyBlast:
energyShields.engage()
}
}
```

```diff
var name: PlanetType {
switch self {
case .mercury:
"Mercury"
-
case .venus:
"Venus"
case .earth:
"Earth"
case .mars:
"Mars"
-
case .jupiter:
"Jupiter"
case .saturn:
"Saturn"
case .uranus:
"Uranus"
case .neptune:
"Neptune"
}
```

</details>
<br/>

## docComments

Use doc comments for API declarations, otherwise use regular comments.
Expand Down
72 changes: 72 additions & 0 deletions Sources/Examples.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1754,6 +1754,29 @@ private struct Examples {
```
"""

let blankLineAfterMultilineSwitchCase = #"""
```diff
func handle(_ action: SpaceshipAction) {
switch action {
case .engageWarpDrive:
navigationComputer.destination = targetedDestination
await warpDrive.spinUp()
warpDrive.activate()
+
case let .scanPlanet(planet):
scanner.target = planet
scanner.scanAtmosphere()
scanner.scanBiosphere()
scanner.scanForArticialLife()
+
case .handleIncomingEnergyBlast:
await energyShields.prepare()
energyShields.engage()
}
}
```
"""#

let wrapMultilineConditionalAssignment = #"""
```diff
- let planetLocation = if let star = planet.star {
Expand All @@ -1769,4 +1792,53 @@ private struct Examples {
+ }
```
"""#

let consistentSwitchStatementSpacing = #"""
```diff
func handle(_ action: SpaceshipAction) {
switch action {
case .engageWarpDrive:
navigationComputer.destination = targetedDestination
await warpDrive.spinUp()
warpDrive.activate()
case .enableArtificialGravity:
artificialGravityEngine.enable(strength: .oneG)
+
case let .scanPlanet(planet):
scanner.target = planet
scanner.scanAtmosphere()
scanner.scanBiosphere()
scanner.scanForArtificialLife()
case .handleIncomingEnergyBlast:
energyShields.engage()
}
}
```
```diff
var name: PlanetType {
switch self {
case .mercury:
"Mercury"
-
case .venus:
"Venus"
case .earth:
"Earth"
case .mars:
"Mars"
-
case .jupiter:
"Jupiter"
case .saturn:
"Saturn"
case .uranus:
"Uranus"
case .neptune:
"Neptune"
}
```
"""#
}
110 changes: 108 additions & 2 deletions Sources/FormattingHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1344,11 +1344,18 @@ extension Formatter {
while let conditionalBranchIndex = nextConditionalBranchIndex,
tokens[conditionalBranchIndex].isSwitchCaseOrDefault,
let startOfBody = index(of: .startOfScope(":"), after: conditionalBranchIndex),
let endOfBody = endOfScope(at: startOfBody)
var endOfBody = endOfScope(at: startOfBody)
{
// If the next case has the `@unknown` prefix, make sure that token isn't included in the body of this branch.
if let unknownKeyword = index(of: .nonSpaceOrCommentOrLinebreak, before: endOfBody),
tokens[unknownKeyword] == .keyword("@unknown")
{
endOfBody = unknownKeyword
}

branches.append((startOfBranch: startOfBody, endOfBranch: endOfBody))

if tokens[endOfBody].isSwitchCaseOrDefault {
if tokens[endOfBody].isSwitchCaseOrDefault || tokens[endOfBody] == .keyword("@unknown") {
nextConditionalBranchIndex = endOfBody
} else if tokens[startOfBody ..< endOfBody].contains(.startOfScope("#if")) {
return nil
Expand Down Expand Up @@ -1423,6 +1430,105 @@ extension Formatter {
return allSatisfy
}

/// Context describing the structure of a case in a switch statement
struct SwitchStatementBranchWithSpacingInfo {
let startOfBranchExcludingLeadingComments: Int
let endOfBranchExcludingTrailingComments: Int
let spansMultipleLines: Bool
let isLastCase: Bool
let isFollowedByBlankLine: Bool
let linebreakBeforeEndOfScope: Int?
let linebreakBeforeBlankLine: Int?

/// Inserts a blank line at the end of the switch case
func insertTrailingBlankLine(using formatter: Formatter) {
guard let linebreakBeforeEndOfScope = linebreakBeforeEndOfScope else {
return
}

formatter.insertLinebreak(at: linebreakBeforeEndOfScope)
}

/// Removes the trailing blank line from the switch case if present
func removeTrailingBlankLine(using formatter: Formatter) {
guard let linebreakBeforeEndOfScope = linebreakBeforeEndOfScope,
let linebreakBeforeBlankLine = linebreakBeforeBlankLine
else { return }

formatter.removeTokens(in: (linebreakBeforeBlankLine + 1) ... linebreakBeforeEndOfScope)
}
}

/// Finds all of the branch bodies in a switch statement, and derives additional information
/// about the structure of each branch / case.
func switchStatementBranchesWithSpacingInfo(at switchIndex: Int) -> [SwitchStatementBranchWithSpacingInfo]? {
guard let switchStatementBranches = switchStatementBranches(at: switchIndex) else { return nil }

return switchStatementBranches.enumerated().compactMap { caseIndex, switchCase -> SwitchStatementBranchWithSpacingInfo? in
// Exclude any comments when considering if this is a single line or multi-line branch
var startOfBranchExcludingLeadingComments = switchCase.startOfBranch
while let tokenAfterStartOfScope = index(of: .nonSpace, after: startOfBranchExcludingLeadingComments),
tokens[tokenAfterStartOfScope].isLinebreak,
let commentAfterStartOfScope = index(of: .nonSpace, after: tokenAfterStartOfScope),
tokens[commentAfterStartOfScope].isComment,
let endOfComment = endOfScope(at: commentAfterStartOfScope),
let tokenBeforeEndOfComment = index(of: .nonSpace, before: endOfComment)
{
if tokens[endOfComment].isLinebreak {
startOfBranchExcludingLeadingComments = tokenBeforeEndOfComment
} else {
startOfBranchExcludingLeadingComments = endOfComment
}
}

var endOfBranchExcludingTrailingComments = switchCase.endOfBranch
while let tokenBeforeEndOfScope = index(of: .nonSpace, before: endOfBranchExcludingTrailingComments),
tokens[tokenBeforeEndOfScope].isLinebreak,
let commentBeforeEndOfScope = index(of: .nonSpace, before: tokenBeforeEndOfScope),
tokens[commentBeforeEndOfScope].isComment,
let startOfComment = startOfScope(at: commentBeforeEndOfScope),
tokens[startOfComment].isComment
{
endOfBranchExcludingTrailingComments = startOfComment
}

guard let firstTokenInBody = index(of: .nonSpaceOrLinebreak, after: startOfBranchExcludingLeadingComments),
let lastTokenInBody = index(of: .nonSpaceOrLinebreak, before: endOfBranchExcludingTrailingComments)
else { return nil }

let isLastCase = caseIndex == switchStatementBranches.indices.last
let spansMultipleLines = !onSameLine(firstTokenInBody, lastTokenInBody)

var isFollowedByBlankLine = false
var linebreakBeforeEndOfScope: Int?
var linebreakBeforeBlankLine: Int?

if let tokenBeforeEndOfScope = index(of: .nonSpace, before: endOfBranchExcludingTrailingComments),
tokens[tokenBeforeEndOfScope].isLinebreak
{
linebreakBeforeEndOfScope = tokenBeforeEndOfScope
}

if let linebreakBeforeEndOfScope = linebreakBeforeEndOfScope,
let tokenBeforeBlankLine = index(of: .nonSpace, before: linebreakBeforeEndOfScope),
tokens[tokenBeforeBlankLine].isLinebreak
{
linebreakBeforeBlankLine = tokenBeforeBlankLine
isFollowedByBlankLine = true
}

return SwitchStatementBranchWithSpacingInfo(
startOfBranchExcludingLeadingComments: startOfBranchExcludingLeadingComments,
endOfBranchExcludingTrailingComments: endOfBranchExcludingTrailingComments,
spansMultipleLines: spansMultipleLines,
isLastCase: isLastCase,
isFollowedByBlankLine: isFollowedByBlankLine,
linebreakBeforeEndOfScope: linebreakBeforeEndOfScope,
linebreakBeforeBlankLine: linebreakBeforeBlankLine
)
}
}

/// Whether the given index is in a function call (not declaration)
func isFunctionCall(at index: Int) -> Bool {
if let openingParenIndex = self.index(of: .startOfScope("("), before: index + 1) {
Expand Down
69 changes: 69 additions & 0 deletions Sources/Rules.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7747,4 +7747,73 @@ public struct _FormatRules {
}
}
}

public let blankLineAfterMultilineSwitchCase = FormatRule(
help: """
Insert a blank line after multiline switch cases (excluding the last case,
which is followed by a closing brace).
""",
disabledByDefault: true,
orderAfter: ["redundantBreak"]
) { formatter in
formatter.forEach(.keyword("switch")) { switchIndex, _ in
guard let switchCases = formatter.switchStatementBranchesWithSpacingInfo(at: switchIndex) else { return }

for switchCase in switchCases.reversed() {
// Any switch statement that spans multiple lines should be followed by a blank line
// (excluding the last case, which is followed by a closing brace).
if switchCase.spansMultipleLines,
!switchCase.isLastCase,
!switchCase.isFollowedByBlankLine
{
switchCase.insertTrailingBlankLine(using: formatter)
}

// The last case should never be followed by a blank line, since it's
// already followed by a closing brace.
if switchCase.isLastCase,
switchCase.isFollowedByBlankLine
{
switchCase.removeTrailingBlankLine(using: formatter)
}
}
}
}

public let consistentSwitchStatementSpacing = FormatRule(
help: "Ensures consistent spacing among all of the cases in a switch statement.",
disabledByDefault: true,
orderAfter: ["blankLineAfterMultilineSwitchCase"]
) { formatter in
formatter.forEach(.keyword("switch")) { switchIndex, _ in
guard let switchCases = formatter.switchStatementBranchesWithSpacingInfo(at: switchIndex) else { return }

// When counting the switch cases, exclude the last case (which should never have a trailing blank line).
let countWithTrailingBlankLine = switchCases.filter { $0.isFollowedByBlankLine && !$0.isLastCase }.count
let countWithoutTrailingBlankLine = switchCases.filter { !$0.isFollowedByBlankLine && !$0.isLastCase }.count

// We want the spacing to be consistent for all switch cases,
// so use whichever formatting is used for the majority of cases.
var allCasesShouldHaveBlankLine = countWithTrailingBlankLine >= countWithoutTrailingBlankLine

// When the `blankLinesBetweenChainedFunctions` rule is enabled, and there is a switch case
// that is required to span multiple lines, then all cases must span multiple lines.
// (Since if this rule removed the blank line from that case, it would contradict the other rule)
if formatter.options.enabledRules.contains(FormatRules.blankLineAfterMultilineSwitchCase.name),
switchCases.contains(where: { $0.spansMultipleLines && !$0.isLastCase })
{
allCasesShouldHaveBlankLine = true
}

for switchCase in switchCases.reversed() {
if !switchCase.isFollowedByBlankLine, allCasesShouldHaveBlankLine, !switchCase.isLastCase {
switchCase.insertTrailingBlankLine(using: formatter)
}

if switchCase.isFollowedByBlankLine, !allCasesShouldHaveBlankLine || switchCase.isLastCase {
switchCase.removeTrailingBlankLine(using: formatter)
}
}
}
}
}
Loading

0 comments on commit 7625b54

Please sign in to comment.