From bd82efdcddcd1d0ce9123d39aa72727bf3ecfcea Mon Sep 17 00:00:00 2001 From: Ethan Lam Date: Wed, 9 Oct 2019 22:52:39 -0500 Subject: [PATCH 1/5] Addresses Issue #4078 --- CHANGELOG.md | 1 + README.md | 2 +- clippy_lints/src/lib.rs | 2 + clippy_lints/src/loops.rs | 211 +++++++++++++++++++++++++++++++++ src/lintlist/mod.rs | 9 +- tests/ui/same_item_push.rs | 77 ++++++++++++ tests/ui/same_item_push.stderr | 35 ++++++ 7 files changed, 335 insertions(+), 2 deletions(-) create mode 100644 tests/ui/same_item_push.rs create mode 100644 tests/ui/same_item_push.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fe97097fda4..52051ae8e287 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1166,6 +1166,7 @@ Released 2018-09-13 [`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else [`result_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unwrap_used [`reverse_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#reverse_range_loop +[`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push [`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some [`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse [`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse diff --git a/README.md b/README.md index 9a75d2ced080..aff8a36eca7c 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 319 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 320 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 4300ed418333..a3c3f0b21afa 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -774,6 +774,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con loops::NEEDLESS_RANGE_LOOP, loops::NEVER_LOOP, loops::REVERSE_RANGE_LOOP, + loops::SAME_ITEM_PUSH, loops::WHILE_IMMUTABLE_CONDITION, loops::WHILE_LET_LOOP, loops::WHILE_LET_ON_ITERATOR, @@ -960,6 +961,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con loops::EMPTY_LOOP, loops::FOR_KV_MAP, loops::NEEDLESS_RANGE_LOOP, + loops::SAME_ITEM_PUSH, loops::WHILE_LET_ON_ITERATOR, main_recursion::MAIN_RECURSION, map_clone::MAP_CLONE, diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index ea496a0294ab..aac387142d26 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -453,6 +453,39 @@ declare_clippy_lint! { "variables used within while expression are not mutated in the body" } +declare_clippy_lint! { + /// **What it does:** Checks whether a for loop is being used to push a constant + /// value into a Vec. + /// + /// **Why is this bad?** This kind of operation can be expressed more succinctly with + /// `vec![item;SIZE]` or `vec.resize(NEW_SIZE, item)` and using these alternatives may also + /// have better performance. + /// **Known problems:** None + /// + /// **Example:** + /// ```rust + /// let item1 = 2; + /// let item2 = 3; + /// let mut vec: Vec = Vec::new(); + /// for _ in 0..20 { + /// vec.push(item1); + /// } + /// for _ in 0..30 { + /// vec.push(item2); + /// } + /// ``` + /// could be written as + /// ```rust + /// let item1 = 2; + /// let item2 = 3; + /// let mut vec: Vec = vec![item1; 20]; + /// vec.resize(20 + 30, item2); + /// ``` + pub SAME_ITEM_PUSH, + style, + "the same item is pushed inside of a for loop" +} + declare_lint_pass!(Loops => [ MANUAL_MEMCPY, NEEDLESS_RANGE_LOOP, @@ -471,6 +504,7 @@ declare_lint_pass!(Loops => [ NEVER_LOOP, MUT_RANGE_BOUND, WHILE_IMMUTABLE_CONDITION, + SAME_ITEM_PUSH ]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops { @@ -751,6 +785,7 @@ fn check_for_loop<'a, 'tcx>( check_for_loop_over_map_kv(cx, pat, arg, body, expr); check_for_mut_range_bound(cx, arg, body); detect_manual_memcpy(cx, pat, arg, body, expr); + detect_same_item_push(cx, pat, arg, body, expr); } fn same_var<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &Expr, var: HirId) -> bool { @@ -1035,6 +1070,182 @@ fn detect_manual_memcpy<'a, 'tcx>( } } +// Delegate that traverses expression and detects mutable variables being used +struct UsesMutableDelegate { + found_mutable: bool, +} + +impl<'a, 'tcx> Delegate<'tcx> for UsesMutableDelegate { + fn consume(&mut self, _: &cmt_<'tcx>, _: ConsumeMode) {} + + fn borrow(&mut self, _: &cmt_<'tcx>, bk: rustc::ty::BorrowKind) { + // Mutable variable is found + if let rustc::ty::BorrowKind::MutBorrow = bk { + self.found_mutable = true; + } + } + + fn mutate(&mut self, _: &cmt_<'tcx>) {} +} + +// Uses UsesMutableDelegate to find mutable variables in an expression expr +fn has_mutable_variables<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> bool { + let mut delegate = UsesMutableDelegate { found_mutable: false }; + let def_id = def_id::DefId::local(expr.hir_id.owner); + let region_scope_tree = &cx.tcx.region_scope_tree(def_id); + ExprUseVisitor::new( + &mut delegate, + cx.tcx, + def_id, + cx.param_env, + region_scope_tree, + cx.tables, + ) + .walk_expr(expr); + + delegate.found_mutable +} + +// Scans for the usage of the for loop pattern +struct ForPatternVisitor<'a, 'tcx> { + found_pattern: bool, + // Pattern that we are searching for + for_pattern_hir_id: HirId, + cx: &'a LateContext<'a, 'tcx>, +} + +impl<'a, 'tcx> Visitor<'tcx> for ForPatternVisitor<'a, 'tcx> { + fn visit_expr(&mut self, expr: &'tcx Expr) { + // Recursively explore an expression until a ExprKind::Path is found + match &expr.kind { + ExprKind::Box(expr) => self.visit_expr(expr), + ExprKind::Array(expr_list) => { + for expr in expr_list { + self.visit_expr(expr) + } + }, + ExprKind::MethodCall(_, _, expr_list) => { + for expr in expr_list { + self.visit_expr(expr) + } + }, + ExprKind::Tup(expr_list) => { + for expr in expr_list { + self.visit_expr(expr) + } + }, + ExprKind::Binary(_, lhs_expr, rhs_expr) => { + self.visit_expr(lhs_expr); + self.visit_expr(rhs_expr); + }, + ExprKind::Unary(_, expr) => self.visit_expr(expr), + ExprKind::Cast(expr, _) => self.visit_expr(expr), + ExprKind::Type(expr, _) => self.visit_expr(expr), + ExprKind::AddrOf(_, expr) => self.visit_expr(expr), + ExprKind::Struct(_, _, Some(expr)) => self.visit_expr(expr), + _ => { + // Exploration cannot continue ... calculate the hir_id of the current + // expr assuming it is a Path + if let Some(hir_id) = var_def_id(self.cx, &expr) { + // Pattern is found + if hir_id == self.for_pattern_hir_id { + self.found_pattern = true; + } + } + }, + } + } + + // This is triggered by walk_expr() for the case of vec.push(pat) + fn visit_qpath(&mut self, qpath: &'tcx QPath, _: HirId, _: Span) { + if_chain! { + if let rustc::hir::QPath::Resolved(_, path) = qpath; + if let rustc::hir::def::Res::Local(hir_id) = &path.res; + if *hir_id == self.for_pattern_hir_id; + then { + self.found_pattern = true; + } + } + } + + fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { + NestedVisitorMap::None + } +} + +// Given some statement, determine if that statement is a push on a Vec. If it is, return +// the Vec being pushed into and the item being pushed +fn get_vec_push<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt) -> Option<(&'tcx Expr, &'tcx Expr)> { + if_chain! { + // Extract method being called + if let rustc::hir::StmtKind::Semi(semi_stmt) = &stmt.kind; + if let rustc::hir::ExprKind::MethodCall(path, _, args) = &semi_stmt.kind; + // Figure out the parameters for the method call + if let Some(self_expr) = args.get(0); + if let Some(pushed_item) = args.get(1); + // Check that the method being called is push() on a Vec + if match_type(cx, cx.tables.expr_ty(self_expr), &paths::VEC); + if path.ident.name.as_str() == "push"; + then { + return Some((self_expr, pushed_item)) + } + } + None +} + +/// Detects for loop pushing the same item into a Vec +fn detect_same_item_push<'a, 'tcx>( + cx: &LateContext<'a, 'tcx>, + pat: &'tcx Pat, + _: &'tcx Expr, + body: &'tcx Expr, + _: &'tcx Expr, +) { + // Extract for loop body + if let rustc::hir::ExprKind::Block(block, _) = &body.kind { + // Analyze only for loops with 1 push statement + let pushes: Vec<(&Expr, &Expr)> = block + .stmts + .iter() + // Map each statement to a Vec push (if possible) + .map(|stmt| get_vec_push(cx, stmt)) + // Filter out statements that are not pushes + .filter(|stmt_option| stmt_option.is_some()) + // Unwrap + .map(|stmt_option| stmt_option.unwrap()) + .collect(); + if pushes.len() == 1 { + // Make sure that the push does not involve possibly mutating values + if !has_mutable_variables(cx, pushes[0].1) { + // Walk through the expression being pushed and make sure that it + // does not contain the for loop pattern + let mut for_pat_visitor = ForPatternVisitor { + found_pattern: false, + for_pattern_hir_id: pat.hir_id, + cx, + }; + intravisit::walk_expr(&mut for_pat_visitor, pushes[0].1); + + if !for_pat_visitor.found_pattern { + let vec_str = snippet(cx, pushes[0].0.span, ""); + let item_str = snippet(cx, pushes[0].1.span, ""); + + span_help_and_lint( + cx, + SAME_ITEM_PUSH, + pushes[0].0.span, + "it looks like the same item is being pushed into this Vec", + &format!( + "try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})", + item_str, vec_str, item_str + ), + ) + } + } + } + } +} + /// Checks for looping over a range and then indexing a sequence with it. /// The iteratee must be a range literal. #[allow(clippy::too_many_lines)] diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 2a38c2968d20..a548b9fa397a 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 319] = [ +pub const ALL_LINTS: [Lint; 320] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -1645,6 +1645,13 @@ pub const ALL_LINTS: [Lint; 319] = [ deprecation: None, module: "loops", }, + Lint { + name: "same_item_push", + group: "style", + desc: "same item is being pushed inside of a for loop", + deprecation: None, + module: "loops", + }, Lint { name: "search_is_some", group: "complexity", diff --git a/tests/ui/same_item_push.rs b/tests/ui/same_item_push.rs new file mode 100644 index 000000000000..0ef396bc2714 --- /dev/null +++ b/tests/ui/same_item_push.rs @@ -0,0 +1,77 @@ +#![warn(clippy::same_item_push)] + +fn mutate_increment(x: &mut u8) -> u8 { + *x += 1; + *x +} + +fn increment(x: u8) -> u8 { + x + 1 +} + +fn main() { + // Test for basic case + let mut spaces = Vec::with_capacity(10); + for _ in 0..10 { + spaces.push(vec![b' ']); + } + + let mut vec2: Vec = Vec::new(); + let item = 2; + for _ in 5..=20 { + vec2.push(item); + } + + let mut vec3: Vec = Vec::new(); + for _ in 0..15 { + let item = 2; + vec3.push(item); + } + + let mut vec4: Vec = Vec::new(); + for _ in 0..15 { + vec4.push(13); + } + + // Suggestion should not be given as pushed variable can mutate + let mut vec5: Vec = Vec::new(); + let mut item: u8 = 2; + for _ in 0..30 { + vec5.push(mutate_increment(&mut item)); + } + + let mut vec6: Vec = Vec::new(); + let mut item: u8 = 2; + let mut item2 = &mut mutate_increment(&mut item); + for _ in 0..30 { + vec6.push(mutate_increment(item2)); + } + + let mut vec7: Vec = Vec::new(); + for i in 0..30 { + vec7.push(i); + } + + let mut vec8: Vec = Vec::new(); + for i in 0..30 { + vec8.push(increment(i)); + } + + let mut vec9: Vec = Vec::new(); + for i in 0..30 { + vec9.push(i + i * i); + } + + // Suggestion should not be given as there are multiple pushes that are not the same + let mut vec10: Vec = Vec::new(); + let item: u8 = 2; + for _ in 0..30 { + vec10.push(item); + vec10.push(item * 2); + } + + // Suggestion should not be given as Vec is not involved + for _ in 0..5 { + println!("Same Item Push"); + } +} diff --git a/tests/ui/same_item_push.stderr b/tests/ui/same_item_push.stderr new file mode 100644 index 000000000000..aa7ce0d53ba7 --- /dev/null +++ b/tests/ui/same_item_push.stderr @@ -0,0 +1,35 @@ +error: it looks like the same item is being pushed into this Vec + --> $DIR/same_item_push.rs:16:9 + | +LL | spaces.push(vec![b' ']); + | ^^^^^^ + | + = note: `-D clippy::same-item-push` implied by `-D warnings` + = help: try using vec![< [_] > :: into_vec (box [$ ($ x), *]);SIZE] or spaces.resize(NEW_SIZE, < [_] > :: into_vec (box [$ ($ x), *])) + +error: it looks like the same item is being pushed into this Vec + --> $DIR/same_item_push.rs:22:9 + | +LL | vec2.push(item); + | ^^^^ + | + = help: try using vec![item;SIZE] or vec2.resize(NEW_SIZE, item) + +error: it looks like the same item is being pushed into this Vec + --> $DIR/same_item_push.rs:28:9 + | +LL | vec3.push(item); + | ^^^^ + | + = help: try using vec![item;SIZE] or vec3.resize(NEW_SIZE, item) + +error: it looks like the same item is being pushed into this Vec + --> $DIR/same_item_push.rs:33:9 + | +LL | vec4.push(13); + | ^^^^ + | + = help: try using vec![13;SIZE] or vec4.resize(NEW_SIZE, 13) + +error: aborting due to 4 previous errors + From 17be043a235d0c5bfd49cbd3e97c639b991ee4f3 Mon Sep 17 00:00:00 2001 From: Ethan Lam Date: Wed, 9 Oct 2019 23:56:06 -0500 Subject: [PATCH 2/5] Lint is not suggested for for loops with possible non-determinant behavior --- clippy_lints/src/loops.rs | 106 +++++++++++++++++++++++++------------- 1 file changed, 70 insertions(+), 36 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index aac387142d26..ca1a565e9caa 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1118,18 +1118,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ForPatternVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'tcx Expr) { // Recursively explore an expression until a ExprKind::Path is found match &expr.kind { - ExprKind::Box(expr) => self.visit_expr(expr), - ExprKind::Array(expr_list) => { - for expr in expr_list { - self.visit_expr(expr) - } - }, - ExprKind::MethodCall(_, _, expr_list) => { - for expr in expr_list { - self.visit_expr(expr) - } - }, - ExprKind::Tup(expr_list) => { + ExprKind::Array(expr_list) | ExprKind::MethodCall(_, _, expr_list) | ExprKind::Tup(expr_list) => { for expr in expr_list { self.visit_expr(expr) } @@ -1138,11 +1127,12 @@ impl<'a, 'tcx> Visitor<'tcx> for ForPatternVisitor<'a, 'tcx> { self.visit_expr(lhs_expr); self.visit_expr(rhs_expr); }, - ExprKind::Unary(_, expr) => self.visit_expr(expr), - ExprKind::Cast(expr, _) => self.visit_expr(expr), - ExprKind::Type(expr, _) => self.visit_expr(expr), - ExprKind::AddrOf(_, expr) => self.visit_expr(expr), - ExprKind::Struct(_, _, Some(expr)) => self.visit_expr(expr), + ExprKind::Box(expr) + | ExprKind::Unary(_, expr) + | ExprKind::Cast(expr, _) + | ExprKind::Type(expr, _) + | ExprKind::AddrOf(_, expr) + | ExprKind::Struct(_, _, Some(expr)) => self.visit_expr(expr), _ => { // Exploration cannot continue ... calculate the hir_id of the current // expr assuming it is a Path @@ -1173,6 +1163,55 @@ impl<'a, 'tcx> Visitor<'tcx> for ForPatternVisitor<'a, 'tcx> { } } +// Scans the body of the for loop and determines whether lint should be given +struct SameItemPushVisitor<'a, 'tcx> { + should_lint: bool, + // this field holds the last vec push operation visited, which should be the only push seen + vec_push: Option<(&'tcx Expr, &'tcx Expr)>, + cx: &'a LateContext<'a, 'tcx>, +} + +impl<'a, 'tcx> Visitor<'tcx> for SameItemPushVisitor<'a, 'tcx> { + fn visit_expr(&mut self, expr: &'tcx Expr) { + match &expr.kind { + // Non-determinism may occur ... don't give a lint + ExprKind::Loop(_, _, _) | ExprKind::Match(_, _, _) => self.should_lint = false, + ExprKind::Block(block, _) => self.visit_block(block), + _ => {}, + } + } + + fn visit_block(&mut self, b: &'tcx Block) { + for stmt in b.stmts.iter() { + self.visit_stmt(stmt); + } + } + + fn visit_stmt(&mut self, s: &'tcx Stmt) { + let vec_push_option = get_vec_push(self.cx, s); + if vec_push_option.is_none() { + // Current statement is not a push so visit inside + match &s.kind { + rustc::hir::StmtKind::Expr(expr) | rustc::hir::StmtKind::Semi(expr) => self.visit_expr(&expr), + _ => {}, + } + } else { + // Current statement is a push ...check whether another + // push had been previously done + if self.vec_push.is_none() { + self.vec_push = vec_push_option; + } else { + // There are multiple pushes ... don't lint + self.should_lint = false; + } + } + } + + fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { + NestedVisitorMap::None + } +} + // Given some statement, determine if that statement is a push on a Vec. If it is, return // the Vec being pushed into and the item being pushed fn get_vec_push<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt) -> Option<(&'tcx Expr, &'tcx Expr)> { @@ -1201,22 +1240,17 @@ fn detect_same_item_push<'a, 'tcx>( body: &'tcx Expr, _: &'tcx Expr, ) { - // Extract for loop body - if let rustc::hir::ExprKind::Block(block, _) = &body.kind { - // Analyze only for loops with 1 push statement - let pushes: Vec<(&Expr, &Expr)> = block - .stmts - .iter() - // Map each statement to a Vec push (if possible) - .map(|stmt| get_vec_push(cx, stmt)) - // Filter out statements that are not pushes - .filter(|stmt_option| stmt_option.is_some()) - // Unwrap - .map(|stmt_option| stmt_option.unwrap()) - .collect(); - if pushes.len() == 1 { + // Determine whether it is safe to lint the body + let mut same_item_push_visitor = SameItemPushVisitor { + should_lint: true, + vec_push: None, + cx, + }; + intravisit::walk_expr(&mut same_item_push_visitor, body); + if same_item_push_visitor.should_lint { + if let Some((vec, pushed_item)) = same_item_push_visitor.vec_push { // Make sure that the push does not involve possibly mutating values - if !has_mutable_variables(cx, pushes[0].1) { + if !has_mutable_variables(cx, pushed_item) { // Walk through the expression being pushed and make sure that it // does not contain the for loop pattern let mut for_pat_visitor = ForPatternVisitor { @@ -1224,16 +1258,16 @@ fn detect_same_item_push<'a, 'tcx>( for_pattern_hir_id: pat.hir_id, cx, }; - intravisit::walk_expr(&mut for_pat_visitor, pushes[0].1); + intravisit::walk_expr(&mut for_pat_visitor, pushed_item); if !for_pat_visitor.found_pattern { - let vec_str = snippet(cx, pushes[0].0.span, ""); - let item_str = snippet(cx, pushes[0].1.span, ""); + let vec_str = snippet(cx, vec.span, ""); + let item_str = snippet(cx, pushed_item.span, ""); span_help_and_lint( cx, SAME_ITEM_PUSH, - pushes[0].0.span, + vec.span, "it looks like the same item is being pushed into this Vec", &format!( "try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})", From 7211ea0d2f78a190fb49f347903ea00833b0edfe Mon Sep 17 00:00:00 2001 From: Ethan Lam Date: Thu, 10 Oct 2019 01:05:20 -0500 Subject: [PATCH 3/5] Fixed problem with tuple patterns --- clippy_lints/src/loops.rs | 26 +++++++++++++++++++++----- tests/ui/same_item_push.rs | 6 +++--- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index ca1a565e9caa..df005540d6f7 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1110,7 +1110,7 @@ fn has_mutable_variables<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) struct ForPatternVisitor<'a, 'tcx> { found_pattern: bool, // Pattern that we are searching for - for_pattern_hir_id: HirId, + for_pattern: &'a rustc::hir::Pat, cx: &'a LateContext<'a, 'tcx>, } @@ -1138,9 +1138,17 @@ impl<'a, 'tcx> Visitor<'tcx> for ForPatternVisitor<'a, 'tcx> { // expr assuming it is a Path if let Some(hir_id) = var_def_id(self.cx, &expr) { // Pattern is found - if hir_id == self.for_pattern_hir_id { + if hir_id == self.for_pattern.hir_id { self.found_pattern = true; } + // If the for loop pattern is a tuple, determine whether the current + // expr is inside that tuple pattern + if let PatKind::Tuple(pat_list, _) = &self.for_pattern.kind { + let hir_id_list: Vec = pat_list.iter().map(|p| p.hir_id).collect(); + if hir_id_list.contains(&hir_id) { + self.found_pattern = true; + } + } } }, } @@ -1151,9 +1159,17 @@ impl<'a, 'tcx> Visitor<'tcx> for ForPatternVisitor<'a, 'tcx> { if_chain! { if let rustc::hir::QPath::Resolved(_, path) = qpath; if let rustc::hir::def::Res::Local(hir_id) = &path.res; - if *hir_id == self.for_pattern_hir_id; then { - self.found_pattern = true; + if *hir_id == self.for_pattern.hir_id{ + self.found_pattern = true; + } + + if let PatKind::Tuple(pat_list, _) = &self.for_pattern.kind { + let hir_id_list: Vec = pat_list.iter().map(|p| p.hir_id).collect(); + if hir_id_list.contains(&hir_id) { + self.found_pattern = true; + } + } } } } @@ -1255,7 +1271,7 @@ fn detect_same_item_push<'a, 'tcx>( // does not contain the for loop pattern let mut for_pat_visitor = ForPatternVisitor { found_pattern: false, - for_pattern_hir_id: pat.hir_id, + for_pattern: pat, cx, }; intravisit::walk_expr(&mut for_pat_visitor, pushed_item); diff --git a/tests/ui/same_item_push.rs b/tests/ui/same_item_push.rs index 0ef396bc2714..e3a5a647f763 100644 --- a/tests/ui/same_item_push.rs +++ b/tests/ui/same_item_push.rs @@ -47,9 +47,9 @@ fn main() { vec6.push(mutate_increment(item2)); } - let mut vec7: Vec = Vec::new(); - for i in 0..30 { - vec7.push(i); + let mut vec7: Vec = Vec::new(); + for (a, b) in [0, 1, 4, 9, 16].iter().enumerate() { + vec7.push(a); } let mut vec8: Vec = Vec::new(); From e9f12f0abefd6754c28361cb000783b04e4200c4 Mon Sep 17 00:00:00 2001 From: Ethan Lam Date: Mon, 14 Oct 2019 15:22:51 -0500 Subject: [PATCH 4/5] Fixed Lint List --- README.md | 2 +- src/lintlist/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index a84ab0e8c252..986d920ac968 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 324 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 325 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index fdfaa6570346..0f49a38a4bee 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 324] = [ +pub const ALL_LINTS: [Lint; 325] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", From 39d015d25fbbc1f842283a38f44e2a2af5b898ca Mon Sep 17 00:00:00 2001 From: Ethan Lam Date: Fri, 18 Oct 2019 11:37:51 -0500 Subject: [PATCH 5/5] Removed Merge Markers --- README.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/README.md b/README.md index 00c249dbdbce..87ef441eadd5 100644 --- a/README.md +++ b/README.md @@ -6,11 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -<<<<<<< HEAD [There are 332 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) -======= -[There are 332 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) ->>>>>>> master We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: