Add the new lint same_item_push#5825
Conversation
|
r? @Manishearth (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
I couldn't run rustfmt at nightly so I haven't run rustfmt yet. |
|
I run rustfmt in a little old nightly. |
clippy_lints/src/loops.rs
Outdated
| } | ||
|
|
||
| // Delegate that traverses expression and detects mutable variables being used | ||
| struct UsesMutableDelegate { |
There was a problem hiding this comment.
Can this stuff be in utils?
There was a problem hiding this comment.
I used mutated_variables in utils. How about this?
clippy_lints/src/loops.rs
Outdated
| } | ||
|
|
||
| // Scans for the usage of the for loop pattern | ||
| struct ForPatternVisitor<'a, 'tcx> { |
There was a problem hiding this comment.
This seems unnecessary, we just need to walk and look for a path that overlaps, we don't need to do any highly bespoke visiting for that.
There was a problem hiding this comment.
I think it's actually better to just check if the for pattern contains any new bindings (i.e. it only contains _ and constants). Unused bindings will be warned about by the unused variable lint
There was a problem hiding this comment.
Thanks for your review! I fixed to just check if for pattern contains _ in 5670bc6. Is my understanding correct?
|
Test failed where it seems unrelated. |
|
☔ The latest upstream changes (presumably #5868) made this pull request unmergeable. Please resolve the merge conflicts. |
giraffate
force-pushed
the
same_item_push
branch
from
5670bc6 to
e48685e
Compare
|
@Manishearth Thanks for your review! I fixed it and tests passed now, so could you take another look into this? |
|
@bors r+ |
|
📌 Commit 610d4e3 has been approved by |
Add the new lint `same_item_push` changelog: Add the new lint `same_item_push` Fixed #4078. As I said in #4078 (comment), I referrerd to #4647.
|
💔 Test failed - checks-action_test |
|
@bors retry |
Add the new lint `same_item_push` changelog: Add the new lint `same_item_push` Fixed #4078. As I said in #4078 (comment), I referrerd to #4647.
flip1995
added
S-waiting-on-bors
|
💔 Test failed - checks-action_test |
|
@bors retry |
Add the new lint `same_item_push` changelog: Add the new lint `same_item_push` Fixed #4078. As I said in #4078 (comment), I referrerd to #4647.
|
@bors retry (yeet) |
Rollup of 5 pull requests Successful merges: - #5825 (Add the new lint `same_item_push`) - #5869 (New lint against `Self` as an arbitrary self type) - #5870 (enable #[allow(clippy::unsafe_derive_deserialize)]) - #5871 (Lint .min(x).max(y) with x < y) - #5874 (Make the docs clearer for new contributors) Failed merges: r? @ghost changelog: rollup
5 participants
changelog: Add the new lint
same_item_pushFixed #4078. As I said in #4078 (comment), I referrerd to #4647.