diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 5fb1a988d148..1f6501f74e70 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -676,7 +676,8 @@ fn never_loop_expr(expr: &Expr, main_loop_id: HirId) -> NeverLoopResult { | ExprKind::Field(ref e, _) | ExprKind::AddrOf(_, ref e) | ExprKind::Struct(_, _, Some(ref e)) - | ExprKind::Repeat(ref e, _) => never_loop_expr(e, main_loop_id), + | ExprKind::Repeat(ref e, _) + | ExprKind::Use(ref e) => never_loop_expr(e, main_loop_id), ExprKind::Array(ref es) | ExprKind::MethodCall(_, _, ref es) | ExprKind::Tup(ref es) => { never_loop_expr_all(&mut es.iter(), main_loop_id) }, @@ -1458,54 +1459,46 @@ fn check_for_loop_explicit_counter<'a, 'tcx>( // For each candidate, check the parent block to see if // it's initialized to zero at the start of the loop. - let map = &cx.tcx.hir(); - let expr_node_id = expr.hir_id; - let parent_scope = map - .get_enclosing_scope(expr_node_id) - .and_then(|id| map.get_enclosing_scope(id)); - if let Some(parent_id) = parent_scope { - if let Node::Block(block) = map.get_by_hir_id(parent_id) { - for (id, _) in visitor.states.iter().filter(|&(_, v)| *v == VarState::IncrOnce) { - let mut visitor2 = InitializeVisitor { - cx, - end_expr: expr, - var_id: *id, - state: VarState::IncrOnce, - name: None, - depth: 0, - past_loop: false, - }; - walk_block(&mut visitor2, block); - - if visitor2.state == VarState::Warn { - if let Some(name) = visitor2.name { - let mut applicability = Applicability::MachineApplicable; - span_lint_and_sugg( - cx, - EXPLICIT_COUNTER_LOOP, - expr.span, - &format!("the variable `{}` is used as a loop counter.", name), - "consider using", - format!( - "for ({}, {}) in {}.enumerate()", - name, - snippet_with_applicability(cx, pat.span, "item", &mut applicability), - if higher::range(cx, arg).is_some() { - format!( - "({})", - snippet_with_applicability(cx, arg.span, "_", &mut applicability) - ) - } else { - format!( - "{}", - sugg::Sugg::hir_with_applicability(cx, arg, "_", &mut applicability) - .maybe_par() - ) - } - ), - applicability, - ); - } + if let Some(block) = get_enclosing_block(&cx, expr.hir_id) { + for (id, _) in visitor.states.iter().filter(|&(_, v)| *v == VarState::IncrOnce) { + let mut visitor2 = InitializeVisitor { + cx, + end_expr: expr, + var_id: *id, + state: VarState::IncrOnce, + name: None, + depth: 0, + past_loop: false, + }; + walk_block(&mut visitor2, block); + + if visitor2.state == VarState::Warn { + if let Some(name) = visitor2.name { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + EXPLICIT_COUNTER_LOOP, + expr.span, + &format!("the variable `{}` is used as a loop counter.", name), + "consider using", + format!( + "for ({}, {}) in {}.enumerate()", + name, + snippet_with_applicability(cx, pat.span, "item", &mut applicability), + if higher::range(cx, arg).is_some() { + format!( + "({})", + snippet_with_applicability(cx, arg.span, "_", &mut applicability) + ) + } else { + format!( + "{}", + sugg::Sugg::hir_with_applicability(cx, arg, "_", &mut applicability).maybe_par() + ) + } + ), + applicability, + ); } } } @@ -2042,7 +2035,7 @@ fn is_simple_break_expr(expr: &Expr) -> bool { // To trigger the EXPLICIT_COUNTER_LOOP lint, a variable must be // incremented exactly once in the loop body, and initialized to zero // at the start of the loop. -#[derive(PartialEq)] +#[derive(Debug, PartialEq)] enum VarState { Initial, // Not examined yet IncrOnce, // Incremented exactly once, may be a loop counter diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index 4ddae1d01c13..3936473f725d 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -495,6 +495,12 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { ExprKind::Err => { println!("Err = {}", current); }, + ExprKind::Use(ref expr) => { + let expr_pat = self.next("expr"); + println!("Use(ref {}) = {};", expr_pat, current); + self.current = expr_pat; + self.visit_expr(expr); + }, } } diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index e084ed8224c6..71f84d6910fd 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -156,6 +156,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { && self.eq_block(lb, rb) && both(ll, rl, |l, r| l.ident.as_str() == r.ident.as_str()) }, + (&ExprKind::Use(ref le), &ExprKind::Use(ref re)) => self.eq_expr(le, re), _ => false, } } @@ -606,6 +607,11 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> { } }, ExprKind::Err => {}, + ExprKind::Use(ref e) => { + let c: fn(_) -> _ = ExprKind::Use; + c.hash(&mut self.s); + self.hash_expr(e); + }, } } diff --git a/clippy_lints/src/utils/inspector.rs b/clippy_lints/src/utils/inspector.rs index 72c0b5ce365c..02725e510601 100644 --- a/clippy_lints/src/utils/inspector.rs +++ b/clippy_lints/src/utils/inspector.rs @@ -330,6 +330,10 @@ fn print_expr(cx: &LateContext<'_, '_>, expr: &hir::Expr, indent: usize) { hir::ExprKind::Err => { println!("{}Err", ind); }, + hir::ExprKind::Use(ref e) => { + println!("{}Use", ind); + print_expr(cx, e, indent + 1); + }, } } diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index 72740cee5006..acb996a3c9b8 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -115,6 +115,7 @@ impl<'a> Sugg<'a> { | hir::ExprKind::Struct(..) | hir::ExprKind::Tup(..) | hir::ExprKind::While(..) + | hir::ExprKind::Use(_) | hir::ExprKind::Err => Sugg::NonParen(snippet), hir::ExprKind::Assign(..) => Sugg::BinOp(AssocOp::Assign, snippet), hir::ExprKind::AssignOp(op, ..) => Sugg::BinOp(hirbinop2assignop(op), snippet), diff --git a/tests/ui/author/for_loop.stdout b/tests/ui/author/for_loop.stdout index e223ba1a8302..9402705355cc 100644 --- a/tests/ui/author/for_loop.stdout +++ b/tests/ui/author/for_loop.stdout @@ -1,9 +1,7 @@ if_chain! { - if let ExprKind::Block(ref block) = expr.node; - if let StmtKind::Local(ref local) = block.node; - if let Some(ref init) = local.init; - if let ExprKind::Match(ref expr, ref arms, MatchSource::ForLoopDesugar) = init.node; - if let ExprKind::Call(ref func, ref args) = expr.node; + if let ExprKind::Use(ref expr) = expr.node; + if let ExprKind::Match(ref expr1, ref arms, MatchSource::ForLoopDesugar) = expr.node; + if let ExprKind::Call(ref func, ref args) = expr1.node; if let ExprKind::Path(ref path) = func.node; if match_qpath(path, &["{{root}}", "std", "iter", "IntoIterator", "into_iter"]); if args.len() == 1; @@ -13,12 +11,12 @@ if_chain! { // unimplemented: field checks if arms.len() == 1; if let ExprKind::Loop(ref body, ref label, LoopSource::ForLoop) = arms[0].body.node; - if let StmtKind::Local(ref local1) = body.node; - if let PatKind::Binding(BindingAnnotation::Mutable, _, name, None) = local1.pat.node; + if let StmtKind::Local(ref local) = body.node; + if let PatKind::Binding(BindingAnnotation::Mutable, _, name, None) = local.pat.node; if name.node.as_str() == "__next"; - if let StmtKind::Expr(ref e, _) = local1.pat.node - if let ExprKind::Match(ref expr1, ref arms1, MatchSource::ForLoopDesugar) = e.node; - if let ExprKind::Call(ref func1, ref args1) = expr1.node; + if let StmtKind::Expr(ref e, _) = local.pat.node + if let ExprKind::Match(ref expr2, ref arms1, MatchSource::ForLoopDesugar) = e.node; + if let ExprKind::Call(ref func1, ref args1) = expr2.node; if let ExprKind::Path(ref path2) = func1.node; if match_qpath(path2, &["{{root}}", "std", "iter", "Iterator", "next"]); if args1.len() == 1; @@ -40,27 +38,23 @@ if_chain! { if arms1[1].pats.len() == 1; if let PatKind::Path(ref path7) = arms1[1].pats[0].node; if match_qpath(path7, &["{{root}}", "std", "option", "Option", "None"]); - if let StmtKind::Local(ref local2) = path7.node; - if let Some(ref init1) = local2.init; - if let ExprKind::Path(ref path8) = init1.node; + if let StmtKind::Local(ref local1) = path7.node; + if let Some(ref init) = local1.init; + if let ExprKind::Path(ref path8) = init.node; if match_qpath(path8, &["__next"]); - if let PatKind::Binding(BindingAnnotation::Unannotated, _, name1, None) = local2.pat.node; + if let PatKind::Binding(BindingAnnotation::Unannotated, _, name1, None) = local1.pat.node; if name1.node.as_str() == "y"; - if let StmtKind::Expr(ref e1, _) = local2.pat.node - if let ExprKind::Block(ref block1) = e1.node; - if let StmtKind::Local(ref local3) = block1.node; - if let Some(ref init2) = local3.init; - if let ExprKind::Path(ref path9) = init2.node; + if let StmtKind::Expr(ref e1, _) = local1.pat.node + if let ExprKind::Block(ref block) = e1.node; + if let StmtKind::Local(ref local2) = block.node; + if let Some(ref init1) = local2.init; + if let ExprKind::Path(ref path9) = init1.node; if match_qpath(path9, &["y"]); - if let PatKind::Binding(BindingAnnotation::Unannotated, _, name2, None) = local3.pat.node; + if let PatKind::Binding(BindingAnnotation::Unannotated, _, name2, None) = local2.pat.node; if name2.node.as_str() == "z"; if arms[0].pats.len() == 1; if let PatKind::Binding(BindingAnnotation::Mutable, _, name3, None) = arms[0].pats[0].node; if name3.node.as_str() == "iter"; - if let PatKind::Binding(BindingAnnotation::Unannotated, _, name4, None) = local.pat.node; - if name4.node.as_str() == "_result"; - if let ExprKind::Path(ref path10) = local.pat.node; - if match_qpath(path10, &["_result"]); then { // report your lint here }