Warn about blanket let _ =

[yshui]

What it does

What does this lint do?

Suggest replacing let _ = <expr> with let _: <type> = <expr>

Categories (optional)

• Kind: Tentatively clippy::pedantic

What is the advantage of the recommended code over the original code

It's somewhat common to use let _ = <expr> to ignore results that would otherwise generate a warning when not used. e.g. ignore an unimportant error.

However, if changes elsewhere cause the type of <expr> to change, bugs could be silently introduced, as let _ = would accept any type.

For example, <expr> might go from Result<_> to impl Future<Output=Result<_>>, because an upstream crate changed a function to async fn. And this could cause some operation to never be performed.

Asking the user to always specify the type in let _ = would avoid this kind of bugs.

Drawbacks

Slightly more verbose code.

Example

let _ = do_something_and_ignore_error();

Could be written as:

let _: Result<_, _> = do_something_and_ignore_error();

Alternatives

1. Warn when an impl Future is dropped in let _ =, it almost never makes sense to construct a future then immediately drop it.
2. Suggest using <expr>.ok() instead of let _ = <expr>, for the case where <expr> is Result<_, _>

Either option works for the example above, but I think my suggestion is more general.

[camsteffen]

Interesting idea. I like this as an answer to "What's the idiomatic way to drop a must_use value?"

I believe we should implement this by enhancing the let_underscore_* lints, but possibly as an opt-in configuration (allow_let_underscore_annotated = true). Though I think it would be good to just add it in without config. We can also add to the suggestion, help: add a type annotation if you really want to do this.

I like your suggestion more than the alternatives. But a let_underscore_future lint might make sense. I don't have enough experience with async to know, is it a correctness issue like let_underscore_lock, or could it be a false positive?

[yshui]

is it a correctness issue like let_underscore_lock, or could it be a false positive?

I would like to think that it is, I couldn't come up with a counterexample myself. Maybe someone more familiar with async could chime in.

[jplatte]

I like let _ = <expr>; to explicitly discard a result. The docs for must_use also say that this is the idiomatic way for it. .ok() is a conversion function, the fact it "gets rid of" the must_use is more of a coincidence rather than its actual purpose. In fact I'd really like that function to be #[must_use].

See also rust-lang/rust#66116

[jkelleyrtp]

Just chiming in that I just ran into this an our codebase with a future whose result I didn't care about.

We did a let binding

let _ = update_status_but_its_okay_to_fail();

But since the let binding swallows the must_use on the future, we never got the proper lint about futures needing to be polled.

It would be great to have at the bare minimum, a lint let_underscore_future to prevent swallowing unpolled futures since it's a very subtle bug.

[JirkaVebr]

@rustbot claim