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

Attribute panic when applying clippy attributes to new #519

Closed
thedrow opened this issue Jul 2, 2019 · 8 comments
Closed

Attribute panic when applying clippy attributes to new #519

thedrow opened this issue Jul 2, 2019 · 8 comments
Labels

Comments

@thedrow
Copy link
Contributor

thedrow commented Jul 2, 2019

🐛 Bug Reports

When reporting a bug, please provide the following information. If this is not a bug report you can just discard this template.

🌍 Environment

  • Your operating system and version: Ubuntu 18.04
  • Your python version: 3.7
  • How did you install python (e.g. apt or pyenv)? Did you use a virtualenv?: pyenv, yes
  • Your rust version (rustc --version): rustc 1.37.0-nightly (17e62f77f 2019-07-01)
  • Are you using the latest pyo3 version? Have you tried using latest master (replace version = "0.x.y" with git = "https://github.com/PyO3/pyo3")? I'm using 0.7.0. Haven't tried with master

💥 Reproducing

   #[pymethods]
    impl UUID {
        #[new]
        #[allow(clippy::too_many_arguments)]
        #[allow(clippy::new_ret_no_self)]
        fn new(
            obj: &PyRawObject,
            hex: Option<&str>,
            bytes: Option<Py<PyBytes>>,
            bytes_le: Option<Py<PyBytes>>,
            fields: Option<Py<PyTuple>>,
            int: Option<u128>,
            version: Option<u8>,
            py: Python,
        ) -> PyResult<()> {}

This results in the following error:

error: custom attribute panicked
  --> src/lib.rs:23:5
   |
23 |     #[pymethods]
   |     ^^^^^^^^^^^^
   |
   = help: message: called `Option::unwrap()` on a `None` value

I'd like to disable some clippy warnings which are not relevant in PyO3 or for my code.
This is currently not possible.

@konstin konstin added the bug label Jul 9, 2019
@vitaly-burovoy
Copy link

It seems the correct variant is to move "allow" lines right before the "impl" (wrapped by the "pymethods") block:

#[allow(clippy::new_ret_no_self)]
#[allow(clippy::too_many_arguments)]
#[pymethods]
impl UUID {
    #[new]
    fn new(
        obj: &PyRawObject,
        hex: Option<&str>,
        bytes: Option<Py<PyBytes>>,
        bytes_le: Option<Py<PyBytes>>,
        fields: Option<Py<PyTuple>>,
        int: Option<u128>,
        version: Option<u8>,
        py: Python,
    ) -> PyResult<()> {}

@thedrow
Copy link
Contributor Author

thedrow commented Jul 23, 2019

Which is fine I guess but it applies to the entire implementation instead of one function.

@althonos
Copy link
Member

althonos commented Jul 23, 2019

What happens with

#[pymethods]
impl UUID {
        #[allow(clippy::too_many_arguments)]
        #[allow(clippy::new_ret_no_self)]
        #[new]
        fn new(
           ...
        ) -> PyResult<()> {}
}

?

@thedrow
Copy link
Contributor Author

thedrow commented Jul 23, 2019

Same panic.

@vitaly-burovoy
Copy link

Which is fine I guess but it applies to the entire implementation instead of one function.

No, it applies just to a block. You can limit this attribute to a single function just moving it to a different "impl" block:

#[allow(clippy::new_ret_no_self)]
#[allow(clippy::too_many_arguments)]
#[pymethods]
impl UUID {
    #[new]
    fn new(
        obj: &PyRawObject,
        hex: Option<&str>,
        bytes: Option<Py<PyBytes>>,
        bytes_le: Option<Py<PyBytes>>,
        fields: Option<Py<PyTuple>>,
        int: Option<u128>,
        version: Option<u8>,
        py: Python,
    ) -> PyResult<()> {...}
}

#[pymethods]
impl UUID {
// Here is no "clippy::too_many_arguments" allowed
    fn other_method(...) {...}
...
}

@thedrow
Copy link
Contributor Author

thedrow commented Jul 24, 2019

I guess that's a workaround but we should fix this bug anyway.

@Alexander-N
Copy link
Member

Can't reproduce. @thedrow Could you try with the current master?

@thedrow
Copy link
Contributor Author

thedrow commented Oct 6, 2019

This bug is fixed with 0.8.0.
I didn't even need to try master.

@thedrow thedrow closed this as completed Oct 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants