unit_arg suggestions don't make sense when run against Option::map_or(unit_expr, ...)

[Arnavion]

#[deny(clippy::unit_arg)]
pub fn foo(o: Option<i32>, f: impl FnOnce(i32), g: impl FnOnce()) {
    o.map_or(g(), |i| f(i))
}

(For context, this is the code after taking the code in #5821 and applying clippy::option_if_let_else's suggestion.)

The lint's suggestion is:

help: move the expression in front of the call...
  |
3 |     g();
  |
help: ...and use a unit literal instead
  |
3 |     o.map_or((), |i| f(i))
  |              ^^

I'm not sure what the first suggestion is trying to say.

The second suggestion is removing the call to g entirely, which is definitely wrong.

Note that the expectation of this code was that it would actually trigger clippy::or_fun_call to suggest o.map_or_else(|| g(), |i| f(i)) (as mentioned in #5821), so the problem is not only that it ended up triggering clippy::unit_arg instead but also that the suggestions don't make sense.

Meta

• cargo clippy -V: clippy 0.0.212 (39d5a61 2020-07-17)
• rustc -Vv:

  rustc 1.47.0-nightly (39d5a61f2 2020-07-17)
  binary: rustc
  commit-hash: 39d5a61f2e4e237123837f5162cc275c2fd7e625
  commit-date: 2020-07-17
  host: x86_64-unknown-linux-gnu
  release: 1.47.0-nightly
  LLVM version: 10.0
  

[flip1995]

The suggestion suggests to move the g() call in front of the map_or call and replacing it with (). This suggestion could still be improved, since this doesn't seem to be clear.

Anyway, the or_fun_call lint should definitely trigger here.

[Arnavion]

Ah, I get it now. Perhaps it could emit the whole change as a single line so that the context is there, like:

help: move the expression in front of the call and replace the original value with a unit literal
  |
3 |     g(); o.map_or((), |i| f(i))
  |