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 complains about function wrapped by proc-macro #6721

Open
alex opened this issue Feb 11, 2021 · 5 comments
Open

unnecessary_wraps complains about function wrapped by proc-macro #6721

alex opened this issue Feb 11, 2021 · 5 comments
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have S-needs-discussion Status: Needs further discussion before merging or work can be started

Comments

@alex
Copy link
Member

alex commented Feb 11, 2021

Lint name: unnecessary_wraps

I tried this code:

#[pyo3::prelude::pymodule]
fn _rust(_py: pyo3::Python<'_>, _m: &pyo3::types::PyModule) -> pyo3::PyResult<()> {
    Ok(())
}

I expected to see this happen: No warning, even though it always returns Ok(()), the proc-macro needs it to return a Result

Instead, this happened:

error: this function's return value is unnecessarily wrapped by `Result`
 --> src/lib.rs:6:1
  |
6 | / fn _rust(_py: pyo3::Python<'_>, _m: &pyo3::types::PyModule) -> pyo3::PyResult<()> {
7 | |     Ok(())
8 | | }
  | |_^
  |
  = note: `-D clippy::unnecessary-wraps` implied by `-D warnings`
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_wraps
help: remove `Result` from the return type...
  |
6 | fn _rust(_py: pyo3::Python<'_>, _m: &pyo3::types::PyModule) -> () {
  |                                                                ^^
help: ...and change the returning expressions
  |
7 |     ()
  |

Meta

Rust: 1.50.

mrtnzlml added a commit to adeira/universe that referenced this issue Feb 12, 2021
See: rust-lang/rust-clippy#6721 (similar but not identical)

Warp requires to return a `Result` and I don't know how to write it differently to comply with this lint.
kodiakhq bot pushed a commit to adeira/universe that referenced this issue Feb 12, 2021
See: rust-lang/rust-clippy#6721 (similar but not identical)

Warp requires to return a `Result` and I don't know how to write it differently to comply with this lint.
@camsteffen
Copy link
Contributor

This seems like an issue with the pyo3 library to me. I'm not sure it makes sense to disable the lint on any function with a proc-macro.

@camsteffen camsteffen added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have S-needs-discussion Status: Needs further discussion before merging or work can be started labels Feb 12, 2021
@alex
Copy link
Member Author

alex commented Feb 12, 2021

Why do you say it's an issue with pyo3?

@camsteffen
Copy link
Contributor

Because it requires you to return a Result.

@alex
Copy link
Member Author

alex commented Feb 12, 2021 via email

@rschoon
Copy link
Contributor

rschoon commented Feb 12, 2021

I suspect the proc-macro could probably be changed to handle detect and handle a function returning either pyo3::PyResult<T> or T, but my first inclination on encountering this would be to assume it doesn't support it and the lint needed to be ignored instead (which is almost certainly currently true).

bors added a commit that referenced this issue Feb 21, 2021
Change unnecessary_wraps to pedantic

changelog: Change unnecessary_wraps to pedantic

There seems to be enough evidence that this lint is not wanted as warn-by-default. Attempted before at #6380. False positives at #6721 and #6427. Actually requested to change the category at #6726.

Closes #6726
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 I-false-positive Issue: The lint was triggered on code it shouldn't have S-needs-discussion Status: Needs further discussion before merging or work can be started
Projects
None yet
Development

No branches or pull requests

3 participants