Restrict let_and_return if the let binding is large

[mrmonday]

Currently, the let_and_return lint is always triggered. I have quite a lot of code which looks like:

let something = match something_else {
    Variant1 => { /* something */},
    ...
    Variant_some_very_large_number => { /* whatever */ }
};

something

Here, I believe it is a lot clearer to explicitly have the let binding, since you may have to scroll quite far to see that there's no semi-colon at the end of the match block for it to be obvious that it's the return expression. I don't know what a suitable size threshold would be, or even if it's possible to reasonably make restrictions based on that.

Of course, I'm open to other ways of writing code blocks like this which would also solve the issue!

[llogiq]

I'm not sure it really is clearer. But perhaps we can split the lint so that if the let-statement is longer than N lines, only the more strict lint (which can be set to Allow) is triggered, whereas shorter statements trigger the more lenient one.

But how many lines are too much?

[mrmonday]

It's obviously up for debate as to whether it's clearer, we could bikeshed it forever.

As for the number of lines, I suspect it's a similar argument to number of columns.

I can fit about 49 lines of my fairly small screen, with a fairly small font - I don't know what the average is. I, personally, would try and set the threshold to be "larger than can reasonably fit on one page/screen of code" - since that's the point where I believe the let/return approach becomes clearer.

[llogiq]

IMHO, if the second-to-last statement of your method is that long, you should be refactoring anyway.

I have a ~25 line limit for functions I try to hold in 95% of all cases. In any language. Yes, that includes Java.

[mrmonday]

I agree, most of the time - that's basically impossible if you've got an enum with a lot of variants to handle though.

[As an aside: my code is ugly, and does need refactoring - I probably wouldn't have this issue if I took the time to refactor it, it's not something I have time for just now unfortunately]

[Manishearth]

IMO even for large matches we shouldn't let-and-return; and I think this is a case that folks should be #[allow()]ing.

At some point it might be interesting to add a clippy.toml with preferences for things like this.