false positives and crazy suggestsiong for unit_arg lint

[pm100]

I have this code:

    pub(crate) fn set_word(&mut self, addr: Word, value: Word) -> ExcResult<()> {
        Ok(self.ram.set_word(addr, value))
    }

ExcResult is Result<(), Exception>

clippy says

475 |         Ok(self.ram.set_word(addr, value))
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(clippy::unit_arg)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg
help: move the expression in front of the call...
    |
475 |         self.ram.set_word(addr, value);
    |
help: ...and use a unit literal instead
    |
475 |         Ok(())
    |            ^^

which is clearly not right.

I have half a dozen similar errors when calling functions that return ()

Meta

clippy 0.0.212 (04488afe3 2020-08-24)
cargo 1.46.0 (149022b1d 2020-07-17)
rustc 1.46.0 (04488afe3 2020-08-24)

[ghost]

It looks like this lint is working as intended to me.

Are you saying that Ok(self.ram.set_word(addr, value)) isn't equivalent to self.ram.set_word(addr, value); Ok(()) ? Why not?

[pm100]

ok i misundertood what it says, I read it as saying it could be reduced to Ok(()). THe text is not clear.

Having said that, I disagree with the lint. As the code stands the intent is very clear -> 'Okize' the result of calling this function. You dont have to look at set_word to see if it returns something that I am choosing to ignore, also the intent is 100% clear. And its very concise.

In clippy's approved version one is left wondering about that function call.

I have a long series of small functions that are proxying non Result returning functions and upscaling them to be Result returners. The fact that some of them return () as opposed to i32 or string is not relevant.

Having said that I must say that clippy is awesome. It found some real bugs. I ported c++ code that uses octal literals a lot (a PDP 11 emulator) and clippy found some literals I had not added 'o' to. Ty v much.

[ebroto]

FYI there's a PR that will be merged soon that improves the suggestion (#5931).

About your use case, to be fair the lint help mentions functions, so the fact that enum constructors are included may be an oversight in the implementation and could be special cased.

I'm adding the "needs-discussion" tag to hopefully have more eyes on this, as I think it's debatable.

[pm100]

also note that the message says line 475 should say Ok(()) thats why its confusing