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

Consider using #![allow(clippy)] at top of generated files #89

Closed
j-t-d opened this issue Mar 1, 2018 · 11 comments
Closed

Consider using #![allow(clippy)] at top of generated files #89

j-t-d opened this issue Mar 1, 2018 · 11 comments

Comments

@j-t-d
Copy link

j-t-d commented Mar 1, 2018

Clippy is giving a lot of warnings in the generated rust files. Using #![allow(clippy)] will disable clippy according to rust-lang/rust-clippy#702

@dwrensha
Copy link
Member

dwrensha commented Mar 2, 2018

How dire are the warnings? Would it be feasible to instead enhance capnpc-rust so that it generates clippy-appeasing code?

If not, then #![allow(clippy)] sounds good to me.

@j-t-d
Copy link
Author

j-t-d commented Mar 2, 2018

They're not too bad:

Lots of these

warning: long literal lacking separators
    |
689 |         pub const TYPE_ID: u64 = 0x8bc07f8159567336;
    |                                  ^^^^^^^^^^^^^^^^^^
    |
    = help: consider: 0x8bc0_7f81_5956_7336
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.186/index.html#unreadable_literal

and these:

warning: explicit lifetimes given in parameter types where they could be elided
   |
39 | /     pub fn borrow<'b>(&'b self) -> Reader<'b,> {
40 | |       Reader { .. *self }
41 | |     }
   | |_____^
   |
   = note: #[warn(needless_lifetimes)] on by default
   = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.186/index.html#needless_lifetimes

And these:

warning: unneeded return statement
    |
280 |           x => return ::std::result::Result::Err(::capnp::NotInSchema(x))
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return` as shown: `::std::result::Result::Err(::capnp::NotInSchema(x))`
    |
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.186/index.html#needless_return

@Deedasmi
Copy link

There are also a few:

  • redundant_field_names. There are a few struct initializations that do reader: reader which can be shortened to reader

  • wrong_self_convention because as_reader() should be called into_reader()

  • module_inception This one actually probably should be just be marked allowed.

@dwrensha
Copy link
Member

I suspect we would need to increase our minimum required rustc version to address the redundant_field_names thing. (Currently we're enforcing 1.15.)

@Deedasmi
Copy link

Yeah, looks like that was added in 1.17. still, should block the lint with a comment why it's being blocked.

@ysimonson
Copy link
Contributor

Most (all?) warnings should be addressed in #112 and #113

@dwrensha
Copy link
Member

Closing, as #112 and #113 have been merged.

@mraerino
Copy link

mraerino commented Jul 6, 2020

i'm still getting a bunch of warnings, mostly clippy::redundant-field-names

is the plan still to fix individual instances of this instead of ignoring lints in the generated files?

@dwrensha
Copy link
Member

dwrensha commented Jul 6, 2020

Thanks for the report. Does this fix it? 9405636
If not, can you please show the errors that you're seeing?

@mraerino
Copy link

mraerino commented Jul 6, 2020

yup, that seems to have done it. thank you!
are you going to release this soon?

@dwrensha
Copy link
Member

dwrensha commented Jul 7, 2020

Pushed 0.13.1 just now: https://crates.io/crates/capnpc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants