A lint for diverging expressions in else-case to encourage early retuns (and similar)

[porky11]

What it does

If you have a return/continue/break/panic or similar in an else case, it's often cleaner to use an early return/continue/break.

This also applies to turning if-let-else into let-else, if the else case always returns/continues/breaks.

I think it might even be one which can be enabled by default. I don't see any cases where it would be difficult to discover this, nor do I see any real drawbacks.

Advantage

• less indentation/nesting
• decreased complexity since else cases are closed
• often considered better style

Drawbacks

Not everybody might prefer early returns.
At least a completely functional style might be preferable.
(but probably almost never a "return" in an else case only)

Maybe the if_not_else lint might be confusing when not handled in a good way.

Example

fn sum_smaller_than(numbers: &[usize], max: usize) -> Option<usize> {
    let mut sum = 0;

    for &num in numbers {
        if sum < max {
            sum += num;
        } else {
            return None;
        }
    }

    Some(sum)
}

Here's a diverging expression (return) in an else case.

The suggestion would usually be to negate the expression and switch the branches like this:

fn sum_smaller_than(numbers: &[usize], max: usize) -> Option<usize> {
    let mut sum = 0;

    for &num in numbers {
        if !(sum < max) {
            return None;
        } else {
            sum += num;
        }
    }

    Some(sum)
}

Then when following the other lints nonminimal_bool and redundant_else lints, we get this:

fn sum_smaller_than(numbers: &[usize], max: usize) -> Option<usize> {
    let mut sum = 0;

    for &num in numbers {
        if sum >= max {
            return None;
        }

        sum += num;
    }

    Some(sum)
}

Following the if_not_else lint would just revert it back, so it should be disabled if a redundant_else lint is discovered for the same lint (probably a separate issue, which is useful by itself).

[Rudxain]

I was porting lpad to see how it would look like in "idiomatic" Rust, so I noticed this problem.

Here's the full `lib.rs`

#![warn(clippy::pedantic, clippy::nursery)]

const SPACE: &str = " ";

// to-do: generate with macro
const CACHE: [&str; 10] = [
    "",
    SPACE,
    "  ",
    "   ",
    "    ",
    "     ",
    "      ",
    "       ",
    "        ",
    "         ",
];

#[must_use]
pub fn left_pad_refactor(s: String, mut len: usize, mut ch: String) -> String {
    len = match len.checked_sub(s.len()) {
        None | Some(0) => return s,
        Some(l) => l,
    };
    ch = if ch.is_empty() {
        String::from(SPACE)
    } else {
        ch
    };
    if ch == SPACE && len < CACHE.len() {
        return format!("{}{s}", CACHE[len]);
    };
    let mut pad = String::new();
    loop {
        let len_is_odd = len & 1 != 0;
        if len_is_odd {
            pad.push_str(&ch);
        };
        len >>= 1;
        if len == 0 {
            break;
        }
        ch = format!("{ch}{ch}");
    }
    format!("{pad}{s}")
}

#[must_use]
pub fn left_pad_og(str: String, mut len: usize, mut ch: String) -> String {
    len = match len.checked_sub(str.len()) {
        None | Some(0) => return str,
        Some(l) => l,
    };
    ch = if ch.is_empty() {
        String::from(SPACE)
    } else {
        ch
    };
    if ch == SPACE && len < CACHE.len() {
        return format!("{}{str}", CACHE[len]);
    };
    let mut pad = String::new();
    loop {
        let len_is_odd = len & 1 != 0;
        if len_is_odd {
            pad.push_str(&ch);
        };
        len >>= 1;
        // `clippy::if_not_else` never triggers,
        // even if swapped blocks
        if len != 0 {
            ch = format!("{ch}{ch}");
        } else {
            break;
        };
    }
    format!("{pad}{str}")
}