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

Restrict same_item_push to suppress false positives #6016

Merged
merged 6 commits into from
Sep 8, 2020

Conversation

giraffate
Copy link
Contributor

It only emits a lint when the pushed item is a literal, a constant and an immutable binding that are initialized with those, as discussed in #5997 (review).

Fix #5985

changelog: Restrict same_item_push to literals, constants and immutable bindings that are initialized with those.

r? @ebroto

It emits a lint when the pushed item is a literal, a constant and an immutable binding that are initialized with those.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 7, 2020
Copy link
Member

@ebroto ebroto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing this so quickly!

I've left some comments, mostly style remarks and some new tests.

clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
clippy_lints/src/loops.rs Show resolved Hide resolved
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
tests/ui/same_item_push.rs Show resolved Hide resolved
tests/ui/same_item_push.rs Outdated Show resolved Hide resolved
tests/ui/same_item_push.rs Show resolved Hide resolved
@ebroto ebroto added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Sep 8, 2020
Add tests in which the variable is initialized with a match expression and function call
@giraffate
Copy link
Contributor Author

giraffate commented Sep 8, 2020

Thanks for your review! I addressed these (Tests failed where it seems unrelated).

@ebroto
Copy link
Member

ebroto commented Sep 8, 2020

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Sep 8, 2020

📌 Commit 1d8ae3a has been approved by ebroto

@bors
Copy link
Collaborator

bors commented Sep 8, 2020

⌛ Testing commit 1d8ae3a with merge 8b54f1e...

@bors
Copy link
Collaborator

bors commented Sep 8, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: ebroto
Pushing 8b54f1e to master...

@bors bors merged commit 8b54f1e into rust-lang:master Sep 8, 2020
@giraffate giraffate deleted the restrict_same_item_push branch September 10, 2020 14:50
TianyiShi2001 added a commit to rust-bio/rust-bio that referenced this pull request Sep 11, 2020
this is a bug in clippy that has been fixed and will not cause this problem in future releases; see rust-lang/rust-clippy#6016
tedil pushed a commit to rust-bio/rust-bio that referenced this pull request Sep 11, 2020
* fix compiler warnings

unused return value of `std::iter::Iterator::collect` that must be used

note: `#[warn(unused_must_use)]` on by default
note: if you really need to exhaust the iterator, consider `.for_each(drop)` instead

* fix clippy warning

trait `Motif` has a `len` method but no (possibly inherited) `is_empty` method

* fix clippy warning

`if` chain can be rewritten with `match`

* fix clippy

it looks like the same item is being pushed into this Vec
try using vec![n;SIZE] or self.pos.resize(NEW_SIZE, n)

* fix clippy

you are implementing `Ord` explicitly but have derived `PartialOrd`

See also https://doc.rust-lang.org/std/cmp/trait.PartialOrd.html

* fix clippy

wrong_self_convention

* fix clippy

allow single-character variable names

* fix clippy (same_item_push)

* fix clippy (sort_unstable)

* fix clippy (needless_lifetimes)

* run cargo fmt

* un-fix clippy warning

this is a bug in clippy that has been fixed and will not cause this problem in future releases; see rust-lang/rust-clippy#6016

* Revert "fix clippy"

This reverts commit 3d414dd.
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 1, 2020
[beta][clippy] backport multiple FP fixes for a warn-by-default lint

This backports the PR rust-lang/rust-clippy#6016 fixing multiple FPs:

rust-lang/rust-clippy#5902
rust-lang/rust-clippy#5979
rust-lang/rust-clippy#5985

We didn't have any complaints about this lint, since me merged this PR.

cc `@ebroto` (sorry I forgot about this, since we talked about the backport 3 weeks ago 😐)

r? `@pietroalbini`
bors added a commit that referenced this pull request Oct 9, 2020
Add changelog for 1.48 beta

[Rendered](https://github.com/ebroto/rust-clippy/blob/changelog_1_48/CHANGELOG.md)

I've not added the PRs fixing `same_item_push` because those were backported, namely:
* [#5908](#5908)
* [#5997](#5997)
* [#6016](#6016)

The following PR was reverted, so I've ignored it too:
* [#5984](#5984)

~~Also, I took the liberty of adding a "Thanks" section, naming all the contributors to this release. I think they deserve visibility in the changelog. Please tell me if we want to add this or maybe it's redundant given we link to the PRs?~~

changelog: none

r? `@flip1995`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

same_item_push is incorrect in several cases
4 participants