Rust-clippy: Unable to allow same_item_push on beta and released Rust version

Created on 2 Sep 2020  路  7Comments  路  Source: rust-lang/rust-clippy

As part of preparing devicemapper-rs in Stratis for the 1.46 release, I was addressing a lint error message from clippy that appears to be a false one, since it's on a non-constant value (which would correspond to issue #5902).

(Related pull request from devicemapper-rs: stratis-storage/devicemapper-rs#576)

"rustup default beta" (rustc 1.47.0-beta.1)
...
error: it looks like the same item is being pushed into this Vec
   --> src/core/dm.rs:582:17
    |
582 |                 targets.push((targ.sector_start, targ.length, target_type, params));
    |                 ^^^^^^^
    |
    = note: `-D clippy::same-item-push` implied by `-D warnings`
    = help: try using vec![(targ.sector_start, targ.length, target_type, params);SIZE] or targets.resize(NEW_SIZE, (targ.sector_start, targ.length, target_type, params))
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push

As part of continuous integration testing, clippy is executed on both the beta, and the released version. However, in the case of 1.46.0, and same_item_push, the lint cannot be found in the released version:

"rustup default 1.46.0" (1.46.0-x86_64-unknown-linux-gnu)
...
error: unknown clippy lint: clippy::same_item_push
   --> src/core/dm.rs:558:13
    |
558 |     #[allow(clippy::same_item_push)]
    |             ^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::unknown-clippy-lints` implied by `-D warnings`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unknown_clippy_lints
    Checking tempfile v3.1.0
error: aborting due to previous error

For now, we changed the allow to #[allow(clippy::all)], but it would be preferable to be able to allow the specific lint on both the beta and released version.

L-bug

All 7 comments

We, like to run clippy on beta on our CI for various reasons. The principle one is so that when we move up to the new stable version of Rust we will have already fixed all the lints. But, as the above illustrates if the lint is a false positive, we will not be able to fix it, and if it is new we will not be able to allow it on stable. We have chosen to use clippy::all, which allows this lint that can't be named on stable. But, it's on a pretty largish expression, so that may allow other lints to creep in as well.

Ideally, the lint name would be introduced in one version, and then the lint would be implemented in the next version. This way, on stable, we could just allow the lint by name.

However, we may choose to drop the beta lint task from our CI, as this, while it has some benefits, and has caught at least one lint regression, it also has drawbacks.

One option would be to allow the lint from the command line only for the beta pipeline:

cargo clippy -- -A clippy::same_item_push

See here for the relevant documentation.

One option would be to allow the lint from the command line only for the beta pipeline:

cargo clippy -- -A clippy::same_item_push

See here for the relevant documentation.

That is true (and would not require radical surgery on our CI). Thanks for the suggestion!

Or allow clippy::unknown-clippy-lints

Or allow clippy::unknown-clippy-lints

Thanks!

@bgurney-rh @mulkieran does the last suggestion solve your problem? Just asking in case we can close this issue :)

Please feel free to close. thx.

Was this page helpful?
0 / 5 - 0 ratings