Rule Request: If as Guard

[Jeehut]

This is a rule request. I'm willing to implement this myself and send a PR, but before doing that I'd like to know if the rule is wanted or not. To explain the desired rule, I'd like to first post the custom rule we are already using in our projects:

if_as_guard:
    included: ".*.swift"
    regex: '\n *if [^\{]+\{\s*return[^\n]*\n *\}(?! *else)'
    name: "If as Guard"
    message: "Don't use an if statement to just return – use guard for such cases instead."
    severity: warning

I felt like this rule might be useful for other projects as well, which is why I'm posting it. What do you think about it? For us it helps a lot to prevent common issues and enforce a shared style.

Rationale

Early returns are considered bad practice amongst several languages due to the fact that they make following the code logic harder. Sometimes they're useful though to keep the code clean. In Swift we have the guard statement which already indicates to people, that there is some kind of early return at the beginning of a line (unlike the if statement where any code without a return could be in the body). Therefore when an if statement is used just to return something, then in all cases a guard would be better used instead. This is actually what guard was designed for, so it should be used.

Note that this rule does not cover more complicated cases with multi-line if statement bodies to ensure it is always better to have a guard statement instead (otherwise there might be edge cases).

A few examples:

// ❌ not acceptable
if x > 5 {
    return false
}

// ✅ acceptable
guard x <= 5 else {
    return false
}

[Uncommon]

It should look for continue and break as well.

[marcelofabri]

I think this would be welcome, but I'd make it opt-in:

1. Sometimes negating a condition makes it harder to read (i.e. !foo.notAvailable)
2. Any function that returns Never (e.g. fatalError) can be used instead of return/break/continue

[Jeehut]

Thanks for the pointers. I agree that continue and break should be added, too.

I also understand why this is not turned on by default, although we might work around the issue you mentioned by checking the condition for negations and not showing an error in those cases. That would make it hard for people to understand when the rule applies though, so I'm not sure if we should do that.

I'm not sure if I understood the second point right, but if I understood it correctly, what you're saying is that this rule may not cover some cases with functions in the if-statement that might return the entire program (like fatalError). If so, I'd say that you're right, but I don't feel like this makes this rule less useful. We could try to add those cases later with an option as well, but this rule is not even covering multi-line if statements and therefore it's not covering all early returns anyways. Just the simple ones, which I find still pretty useful (as they happen often).

[marcelofabri]

My second point basically means that probably this rule should be opt-in or we should check for common functions that return Never.

Related rule request: #596.

[Jeehut]

I think just making it opt-in should be fine.