unit_cmp: special handling of assert_eq!(Option.unwrap(), ());

[matthiaskrgr]

pub fn main() {
    let x: Option<()> = Some(());
    assert_eq!(x.unwrap(), ());
}

Clippy warns here ``assert_eq of unit values detected. This will always succeed, however this code will actually check and panic if x is None.

Can we suggest to replace it with
assert!(x.is_some()) in this case?

Found in cargo codebase of cargo.

clippy 0.0.212 (5cb9833 2019-10-08)

[devonhollowood]

I've been working on this a bit, but I'm having trouble getting the left and right sides of the assert_eq!(). What's the best way to get the Expr for opt.unwrap() in assert_eq!(opt.unwrap(), ())? The left binding in UnitCmp::check_expr() (here) seems to be bound to something different, I'm assuming due to the macro expansion.

[flip1995]

There will be a utils function for this (hopefully) soon, see #4694

[hellow554]

Clippy warns here assert_eq of unit values detected. This will always succeed, however this code will actually check and panic if x is None.

Yes and no. The assert_eq! will always succeed, because the unwrap panics and not the assert_eq in case of None. So clippy is right here.

Can we suggest to replace it with
assert!(x.is_some()) in this case?

Would be a smart play :)

[martin-t]

Would it make sense to allow cases where either left or right is explicitly ()?

My usecase is testing that a macro i wrote always evaluates to () as part of testing that it's usable in expression position - e.g. assert_eq!(my_macro!(), ());. It'll fail at compile time if it's wrong so it always succeeds but i don't think there's an easier way to test this. If there is, then the lint should suggest it instead.

[flip1995]

Maybe let _: () = my_macro!(); is the better alternative here. assert! is not really meant for compile time checks IMO.

OTOH, assert maybe conveys the intention of this line better.

For .unwrap special casing it to is_some might make sense too.