let_unit_value should be allowed when I explicitly write "let () = ..."

[safinaskar]

Summary

Sometimes I write code like this: let () = some_function_call(). I do this to make sure that this function returns unit value, i. e. to make sure that I didn't miss any return value. But let_unit_value warns on this code. So, please, make it not to warn on this code

Lint Name

let_unit_value

Reproducer

I tried this code:

fn main() {
    let mut v = vec![2, 1];
    let () = v.sort();
}

I saw this happen:

    Checking playground v0.0.1 (/playground)
warning: this let-binding has unit value
 --> src/main.rs:3:5
  |
3 |     let () = v.sort();
  |     ^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `v.sort();`
  |
  = note: `#[warn(clippy::let_unit_value)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value

warning: `playground` (bin "playground") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.42s

I expected to see this happen:
(no warn)

Version

Version from https://play.rust-lang.org/ as on today (2022-06-25)

Additional Labels

No response

[hellow554]

Related to #8998

Here are my two cents:

i. e. to make sure that I didn't miss any return value

If you can miss a return value, e.g. it's crucial that you use that return value, than that function should be attributed with must_use, which is a seperate clippy lint: muse_use_candidate.
I don't think that assigning to the unit value is a good thing in your case.

On the other hand, I like your style more than let _: () = ..., which is more verbose and less likely to be useful.

The only place, where I think a assignment to the unit value is good, is when the return value is a type parameter and must be specified.

Is this understandable?

[kpreid]

It looks like #9056 will fix this.

[Jarcho]

#9056 will allow it when used for inference, not in general.

[kpreid]

Oh. Then I think it ought to be changed to also allow let () =. The situation in which I met this lint is when the value was an associated type returned from a method — so the () pattern won't help inference, but it does serve the role of statically asserting that the type must be (). Minimized:

// Generic things
pub struct ThingError;
pub trait DoThing {
    type Output;
    fn do_thing(&self) -> Result<Self::Output, ThingError>;
}

// Specific concrete type
struct NoOutput;
impl DoThing for NoOutput {
    type Output = ();
    fn do_thing(&self) -> Result<Self::Output, ThingError> {
        Ok(())
    }
}

pub fn example() -> Result<&'static str, ThingError> {
    // This should stop compiling if the output type changes to something nontrivial
    let () = NoOutput.do_thing()?;
    
    Ok("some other return value")
}

Note that #[must_use] annotations cannot help here, since the associated type occurs inside a Result, which is already #[must_use], so a #[must_use] on fn do_thing() would have no effect — it would only check that the Result is used.

In the particular case this came up, replacing () with a custom empty struct is a reasonable option which I'm going to take, but in a situation where the Output was part of some public library API, adding such a type would create a fair bit of noise for a small benefit. I'd like to have the small benefit without the noise.

(A better overall solution would be some way to express nested #[must_use] on the trait's declaration of fn do_thing(), so that all calls catch ignored return values without callers going to any effort. That'd be a bigger piece of language design, though.)

[lilyball]

I agree that let () = … should be allowed. The static assertion that the type is unit is useful for the reader even if the compiler doesn't need it. Similarly () = … (no let keyword) should be allowed, which is also currently tripping let_unit_value (despite not actually being a let-binding).

[arxanas]

Just ran into this today. Agreed that let () = ... should be allowed. Furthermore, the justification at https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value is confusing:

Why is this bad?
A unit value cannot usefully be used anywhere. So binding one is kind of pointless.

Example

let x = {
    1;
};

because there are no actual variable bindings in the form let () = .... It took me a moment to realize why this check was triggering on my code.

[Thomasdezeeuw]

I have another use case were let () = ... can be useful: purposefully overshadowing a variable (name). We have a case where we have a SQL transaction and a SQL pool in the same code block, so it's very easy to use the SQL pool instead of the SQL transaction. To avoid using the pool instead of the transaction we overshadow the variable name so it can't be used any more.

fn do_something(pool: SqlPool) {
    let mut transaction = pool.start_transaction();
    #[allow(unused_variables)]
    let pool = (); // NOTE: shadowing the name to ensure we use `transaction`.
}

[safinaskar]

@Thomasdezeeuw , I would recommend drop(pool) instead

[safinaskar]

@Thomasdezeeuw , oops, this would have different semantics

[Thomasdezeeuw]

@Thomasdezeeuw , oops, this would have different semantics

And it's also not possible if the transaction depends on the pool and thus is connection via e.g. a lifetime.

[JeremyRubin]

+1 on let () -- yes, there can be other must_use lints, but you can't always attribute must_use because let () can happen in e.g. closures which i don't think have must_use support (imagine returing Result<(), E> and you do let () = a()?;.

[kevincox]

let () = ... can be required for some type inference. The fact that clippy can auto-apply this "fix" can break code which IIUC is a bug in clippy.

At the very least this should be downgraded from MachineApplicable to MaybeIncorrect.

[safinaskar]

Workaround! Use this:

let mut v = vec![2, 1];
[()] = [v.sort()];

[safinaskar]

rust-lang/rust#112380 is about to merge. It will add similar lint to rustc. Unfortunately, as well as I understand, the new lint will be allow-by-default, as opposed to clippy's (buggy) lint, which is warning-by-default, and which will be kept for now. So, clippy in its default configuration will still be buggy :(