Added FORCED_RETURN lint.

[daxpedda]

This adds a new lint that warns if return is not used, which is the opposite of clippy::needless_return.

Questions:

• Should I add a check for ; at the end? I thought that would be more a thing for rustfmt.
• Should I add more tests?
• I made the functions that I needed into associated functions instead of methods, is that fine? Could also make them into closures?
• I needed to add #[allow(clippy::needless_bool)] and #[allow(clippy::match_bool)] to the test to make it work properly. Other tests had it and others didn't, how should this be handled?

Thanks.
Fixes #2685.

[phansch]

Should I add a check for ; at the end? I thought that would be more a thing for rustfmt.

Yup, that's more a thing for rustfmt

Should I add more tests?

Looks like you re-used the existing needless_return tests, which seems good enough to me 👍

I made the functions that I needed into associated functions instead of methods, is that fine? Could also make them into closures?

It looks fine as it is. I don't really mind either way, apart from Closures.

I needed to add #[allow(clippy::needless_bool)] and #[allow(clippy::match_bool)] to the test to make it work properly. Other tests had it and others didn't, how should this be handled?

Generally we want to make the tests as simple as possible so that they are shorter and more readable. If allowing a lint helps with that, then it's fine. (as in this case)

[phansch]

just a nit regarding the documentation text

[oli-obk]

So. I read up on the RFC. Specifically relevant is this section

"restriction" lints follow all the rules for semantic changes,

This lint is no semantic change, OK

but do not bother with the rules for the lint being useful to most rust programmers.

Not useful to most rust programmers, OK

A restriction lint must still be such that you have a good reason to enable it — "I dislike such code" is insufficient — but will likely be a lint most programmers wish to keep off by default for most of their code.

The motivation in the docs is lacking imo.

The goal of restriction lints is to provide tools with which you can supplement the language checks in very specific cases where you need it, e.g. forbidding panics from a certain area of code.

This is not making the language stricter, as any expression is still a value, only the return position is affected. It might fit under the hypothetical defensive lint category: https://internals.rust-lang.org/t/defensive-programming-clippy-lint-category/8210 But that first needs an RFC

[daxpedda]

I could write something up if it helps?:
Why is this bad? Actually omitting the return keyword is idiomatic Rust code. Programmers coming from other languages might prefer the expressiveness of return. It's possible to miss the last returning statement because the only difference is a missing ;. Especially in bigger code with multiple return paths having a return keyword makes it easier to find the corresponding statements.

I'm not sure if I should write it all up under Why is this bad?, not sure where else though.
Otherwise we could also wait until the defensive lint category goes through?

[Manishearth]

Lints should be named after the thing they lint for, so this should be implicit-return or something, I think.

[daxpedda]

I added a check and test for replacing break with return as seen here:

fn test_loop() -> bool {
    loop {
        break true;
    }
}

resolves to:

fn test_loop() -> bool {
    loop {
        return true;
    }
}

Is that desirable or should I remove it?

[oli-obk]

I'm not sure if I should write it all up under Why is this bad?, not sure where else though.

The additional text you suggested seems good.