Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unnecessary_wraps suggests to unwrap unit instead of omitting return value #6640

Closed
msakuta opened this issue Jan 25, 2021 · 1 comment · Fixed by #6665
Closed

unnecessary_wraps suggests to unwrap unit instead of omitting return value #6640

msakuta opened this issue Jan 25, 2021 · 1 comment · Fixed by #6665
Labels
C-bug Category: Clippy is not doing the correct thing C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@msakuta
Copy link

msakuta commented Jan 25, 2021

I tried this code:

fn always_ok() -> Result<(), String> {
    Ok(())
}

I expected that clippy suggests omitting return value at all:

fn always_ok() {
}

Instead, it suggests returning unit:

warning: this function's return value is unnecessarily wrapped by `Result`
  --> src/main.rs:22:1
   |
22 | / fn always_ok() -> Result<(), String> {
23 | |     Ok(())
24 | | }
   | |_^
   |
   = note: `#[warn(clippy::unnecessary_wraps)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_wraps
help: remove `Result` from the return type...
   |
22 | fn always_ok() -> () {
   |                   ^^
help: ...and change the returning expressions
   |
23 |     ()
   |

and it's stupid because if you follow the instructions it shows another lint warning:

warning: unneeded unit return type
  --> src/main.rs:22:15
   |
22 | fn always_ok() -> () {
   |               ^^^^^^ help: remove the `-> ()`
   |
   = note: `#[warn(clippy::unused_unit)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit

warning: unneeded unit expression
  --> src/main.rs:23:5
   |
23 |     ()
   |     ^^ help: remove the final `()`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit

warning: 2 warnings emitted

It should suggest omitting return value in the first place.

Tested on Rust playgound

Meta

  • cargo clippy -V: e.g. clippy 0.1.51 (2021-01-19 c5a96fb)
  • rustc 1.51.0-nightly
@msakuta msakuta added the C-bug Category: Clippy is not doing the correct thing label Jan 25, 2021
@giraffate giraffate added L-suggestion Lint: Improving, adding or fixing lint suggestions C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages labels Jan 25, 2021
@camsteffen camsteffen added the good-first-issue These issues are a good way to get started with Clippy label Feb 1, 2021
@pag4k
Copy link
Contributor

pag4k commented Feb 1, 2021

I'll try to do this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants