Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rule to add a line break after the assignment operator before a multi-line if or switch expressions #253

Merged
merged 4 commits into from Nov 22, 2023

Conversation

calda
Copy link
Member

@calda calda commented Nov 17, 2023

Summary

This PR proposes a new rule to add a line break after the assignment operator before a multi-line if or switch expression.

Autocorrection for this rule is implemented in the wrapMultilineConditionalAssignment SwiftFormat rule, added in nicklockwood/SwiftFormat#1574.

Reasoning

This makes it so that if and switch expressions always have the same "shape" as standard if and switch statements, where:

  1. The if / switch keyword is always the left-most token on a dedicated line of code
  2. The conditional branches are always to the right of and below the if / switch keyword.

This is most consistent with the formatting of if / switch statements in other contexts, and thus makes it easier to recognize that the code is using an if or switch expression at a glance.

This is independent of #250 and could be accepted even if we reject #250, but #250 depends on these changes given the discussion here.

Examples

// WRONG. Should have a line break after the `=`. 
let planetLocation = if let star = planet.star {
  "The \(star.name) system"
 } else {
  "Rogue planet"
}

// WRONG. The `=` should be on the line before the `if` / `switch` expression. 
let planetLocation 
  = if let star = planet.star {
    "The \(star.name) system"
  } else {
    "Rogue planet"
  }
  
// RIGHT 
let planetLocation = 
  if let star = planet.star {
    "The \(star.name) system"
  } else {
    "Rogue planet"
  }
  
// RIGHT 
let planetLocation = 
  if let star = planet.star {
    "The \(star.name) system"
  } else {
    "Rogue planet"
  }
  
// RIGHT
let planetType: PlanetType =
  switch planet {
  case .mercury, .venus, .earth, .mars:
    .terrestrial
  case .jupiter, .saturn, .uranus, .neptune:
    .gasGiant
  }
  
// ALSO RIGHT. A line break is not required because the declaration fits on a single line. 
let planet = if useFirst { mercury } else { neptune }

// ALSO RIGHT. A line break is always permitted if it helps with readability.
let planet =
  if useFirst { mercury } else { neptune }

Please react with 👍/👎 if you agree or disagree with this proposal.

@calda calda force-pushed the cal--wrap-conditional-assignment branch from 89bdb47 to a076f62 Compare November 17, 2023 19:13
@calda calda requested a review from bachand November 20, 2023 18:29
Copy link
Contributor

@bachand bachand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it! Thanks @calda .

@@ -900,6 +900,79 @@ _You can enable the following settings in Xcode by running [this script](resourc
}
```

</details>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
calda and others added 2 commits November 20, 2023 16:50
Co-authored-by: Michael Bachand <michael.bachand@airbnb.com>
@calda calda merged commit fb94276 into master Nov 22, 2023
5 checks passed
@calda calda deleted the cal--wrap-conditional-assignment branch November 22, 2023 18:26
let planet =
if useFirst { mercury } else { neptune }
let moonName =
if let moon = planet.moon { moon.name } else { "none" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants