lint suggestion: unnecessary_move

[sgued]

What it does

Lints against uses of move before a closure or async block that captures nothing by reference. 

Advantage

This is likely the result of a mistake from the implementer, which does not have the proper intuition about how closure borrow their contents. Sometimes it can be hard to reason about it.

For example the developer might not realise that a capture that is passed by value is always captured by value.

Drawbacks

No response

Example

    let a = String::new();
    let closure = move || {
        drop(a);  
    };

Could be written as:

    let a = String::new();
    let closure = || {
        drop(a);  
    };

Because the String is always captured by value, since it needs to be passed by value to drop.

[kpreid]

This is likely the result of a mistake from the implementer, which does not have the proper intuition about how closure borrow their contents. Sometimes it can be hard to reason about it.

I think this is not a good principle to follow. A programmer should not be obligated to understand every nuance of the language's DWIM behaviors; they should be able to follow simpler principles which produce the right answer, where that does not harm readability, and move's behavior is simpler to describe (move/copy everything) than non-move (borrow or move according to usage sites).

Furthermore, this is at odds with reasonable consistency principles, like “always write spawn(move || { ... }), not spawn(|| { ... }), because either it's necessary or it's moot”. I'd rather read

spawn(move || job_1(...));
spawn(move || job_2(...));

than

spawn(move || job_1(...));
spawn(|| job_2(...)); // this one has only by-move arguments

I understand that this is being implemented as a pedantic lint and therefore not enabled by default. Still, I think this goes beyond pedantic territory into “arguably the opposite of this is better” — it's putting a greater obligation on the programmer to comprehend every detail of their program and put it in a form that is only optimal in the direction of “fewer tokens”, not “clearer”. Readability and good style is not just about using fewer tokens.

Please consider putting this in the restriction category (it even has an obvious possible opposite restriction lint, of "write all closures as move closures"), not adding it at all, or at least putting the lack of consistency as a documented caveat.