break; else / return; else

[llogiq]

We may want to match needless else blocks in conditions whose then-block ends with break or return.

Example:

loop {
    if x {
        break;
    } else {
        y();
    }
}

could be

loop {
    if x { break; }
    y();
}

[killercup]

That reminds me: Is there a lint for if x == y { return 1; } else { return 2; } that suggests return if … (in some cases best without the return)?

Also, what I've seen in JS code a lot: if x == y {return 1;} return 2;, i.e. to make it an early return instead of using else.

[llogiq]

Yeah, but in Rust, the return is optional if it is the last statement, so you'd usually write if x == y { 1 } else { 2 } anyway.

Actually this could be two lints:

• Warn on superfluous return and
• Warn on duplicated code in both if-arms, e.g. if x == y { x * 42 + y } else { x * 42 + z } could be x * 42 + if x == y { y } else { z }.

The former would be quite simple and very fitting to clippy's stated mission, while the latter would probably be a bit complex (it could use much of the machinery of eq_op), but be very useful.

[killercup]

I'm sorry for hijacking this issue. (Feel free to copy my comments into new issues. I'm a work and just wanted to dump my thoughts.)

so you'd usually write if x == y { 1 } else { 2 } anyway.

And we should have a lint to tell beginners just that. :) You are right, @llogiq, these can be two (if not three, see below) separate lints.

I agree that detecting superfluous returns should be a lint.

Your second point is actually a superset of what I wanted to suggest; since returns in the conditional branches are quite common (as early returns), beginners may also write return in else branches. My suggestion is to detect if there are returns in all branches and suggest to return the if expression. (If this is the last expression, the return keyword will not be necessary.) We could actually do this (easy) version first and later remove it when a more powerful version (that you described) is available.

What do you think of extracting code of else blocks that return? I'm talking about code like:

fn stuff(x: i32, y: i32) -> bool {
    if x == y {
        true
    } else {
        // 20 lines of code
    }
}

which could be more readable as this:

fn stuff(x: i32, y: i32) -> bool {
    if x == y {
        return true;
    }
    // 20 lines of code
}

This could be another lint, but it's also a concern of code style.

[llogiq]

I just submitted #113 Warn on Superfluous returns.

As for the other proposal, I'm unsure if I want to warn on this (at least yet). Some people frown at early returns, readability be damned. 😄 So we would preempt a discussion on the true Rust style that has not yet taken place, if I'm up to date. I happen to own a good set of flame-retardant underwear, but even thusly armed I dare not go there.

Oh, and no need to be sorry.