This commit is contained in:
Michael Wright 2019-04-28 09:11:20 +02:00
parent 910d538ef1
commit 5ad79c2b3d
6 changed files with 77 additions and 73 deletions

View file

@ -676,7 +676,8 @@ fn never_loop_expr(expr: &Expr, main_loop_id: HirId) -> NeverLoopResult {
| ExprKind::Field(ref e, _) | ExprKind::Field(ref e, _)
| ExprKind::AddrOf(_, ref e) | ExprKind::AddrOf(_, ref e)
| ExprKind::Struct(_, _, Some(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) => { ExprKind::Array(ref es) | ExprKind::MethodCall(_, _, ref es) | ExprKind::Tup(ref es) => {
never_loop_expr_all(&mut es.iter(), main_loop_id) 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 // For each candidate, check the parent block to see if
// it's initialized to zero at the start of the loop. // it's initialized to zero at the start of the loop.
let map = &cx.tcx.hir(); if let Some(block) = get_enclosing_block(&cx, expr.hir_id) {
let expr_node_id = expr.hir_id; for (id, _) in visitor.states.iter().filter(|&(_, v)| *v == VarState::IncrOnce) {
let parent_scope = map let mut visitor2 = InitializeVisitor {
.get_enclosing_scope(expr_node_id) cx,
.and_then(|id| map.get_enclosing_scope(id)); end_expr: expr,
if let Some(parent_id) = parent_scope { var_id: *id,
if let Node::Block(block) = map.get_by_hir_id(parent_id) { state: VarState::IncrOnce,
for (id, _) in visitor.states.iter().filter(|&(_, v)| *v == VarState::IncrOnce) { name: None,
let mut visitor2 = InitializeVisitor { depth: 0,
cx, past_loop: false,
end_expr: expr, };
var_id: *id, walk_block(&mut visitor2, block);
state: VarState::IncrOnce,
name: None,
depth: 0,
past_loop: false,
};
walk_block(&mut visitor2, block);
if visitor2.state == VarState::Warn { if visitor2.state == VarState::Warn {
if let Some(name) = visitor2.name { if let Some(name) = visitor2.name {
let mut applicability = Applicability::MachineApplicable; let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
EXPLICIT_COUNTER_LOOP, EXPLICIT_COUNTER_LOOP,
expr.span, expr.span,
&format!("the variable `{}` is used as a loop counter.", name), &format!("the variable `{}` is used as a loop counter.", name),
"consider using", "consider using",
format!( format!(
"for ({}, {}) in {}.enumerate()", "for ({}, {}) in {}.enumerate()",
name, name,
snippet_with_applicability(cx, pat.span, "item", &mut applicability), snippet_with_applicability(cx, pat.span, "item", &mut applicability),
if higher::range(cx, arg).is_some() { if higher::range(cx, arg).is_some() {
format!( format!(
"({})", "({})",
snippet_with_applicability(cx, arg.span, "_", &mut applicability) snippet_with_applicability(cx, arg.span, "_", &mut applicability)
) )
} else { } else {
format!( format!(
"{}", "{}",
sugg::Sugg::hir_with_applicability(cx, arg, "_", &mut applicability) sugg::Sugg::hir_with_applicability(cx, arg, "_", &mut applicability).maybe_par()
.maybe_par() )
) }
} ),
), applicability,
applicability, );
);
}
} }
} }
} }
@ -2042,7 +2035,7 @@ fn is_simple_break_expr(expr: &Expr) -> bool {
// To trigger the EXPLICIT_COUNTER_LOOP lint, a variable must be // To trigger the EXPLICIT_COUNTER_LOOP lint, a variable must be
// incremented exactly once in the loop body, and initialized to zero // incremented exactly once in the loop body, and initialized to zero
// at the start of the loop. // at the start of the loop.
#[derive(PartialEq)] #[derive(Debug, PartialEq)]
enum VarState { enum VarState {
Initial, // Not examined yet Initial, // Not examined yet
IncrOnce, // Incremented exactly once, may be a loop counter IncrOnce, // Incremented exactly once, may be a loop counter

View file

@ -495,6 +495,12 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
ExprKind::Err => { ExprKind::Err => {
println!("Err = {}", current); 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);
},
} }
} }

View file

@ -156,6 +156,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
&& self.eq_block(lb, rb) && self.eq_block(lb, rb)
&& both(ll, rl, |l, r| l.ident.as_str() == r.ident.as_str()) && 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, _ => false,
} }
} }
@ -606,6 +607,11 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
} }
}, },
ExprKind::Err => {}, ExprKind::Err => {},
ExprKind::Use(ref e) => {
let c: fn(_) -> _ = ExprKind::Use;
c.hash(&mut self.s);
self.hash_expr(e);
},
} }
} }

View file

@ -330,6 +330,10 @@ fn print_expr(cx: &LateContext<'_, '_>, expr: &hir::Expr, indent: usize) {
hir::ExprKind::Err => { hir::ExprKind::Err => {
println!("{}Err", ind); println!("{}Err", ind);
}, },
hir::ExprKind::Use(ref e) => {
println!("{}Use", ind);
print_expr(cx, e, indent + 1);
},
} }
} }

View file

@ -115,6 +115,7 @@ impl<'a> Sugg<'a> {
| hir::ExprKind::Struct(..) | hir::ExprKind::Struct(..)
| hir::ExprKind::Tup(..) | hir::ExprKind::Tup(..)
| hir::ExprKind::While(..) | hir::ExprKind::While(..)
| hir::ExprKind::Use(_)
| hir::ExprKind::Err => Sugg::NonParen(snippet), | hir::ExprKind::Err => Sugg::NonParen(snippet),
hir::ExprKind::Assign(..) => Sugg::BinOp(AssocOp::Assign, snippet), hir::ExprKind::Assign(..) => Sugg::BinOp(AssocOp::Assign, snippet),
hir::ExprKind::AssignOp(op, ..) => Sugg::BinOp(hirbinop2assignop(op), snippet), hir::ExprKind::AssignOp(op, ..) => Sugg::BinOp(hirbinop2assignop(op), snippet),

View file

@ -1,9 +1,7 @@
if_chain! { if_chain! {
if let ExprKind::Block(ref block) = expr.node; if let ExprKind::Use(ref expr) = expr.node;
if let StmtKind::Local(ref local) = block.node; if let ExprKind::Match(ref expr1, ref arms, MatchSource::ForLoopDesugar) = expr.node;
if let Some(ref init) = local.init; if let ExprKind::Call(ref func, ref args) = expr1.node;
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::Path(ref path) = func.node; if let ExprKind::Path(ref path) = func.node;
if match_qpath(path, &["{{root}}", "std", "iter", "IntoIterator", "into_iter"]); if match_qpath(path, &["{{root}}", "std", "iter", "IntoIterator", "into_iter"]);
if args.len() == 1; if args.len() == 1;
@ -13,12 +11,12 @@ if_chain! {
// unimplemented: field checks // unimplemented: field checks
if arms.len() == 1; if arms.len() == 1;
if let ExprKind::Loop(ref body, ref label, LoopSource::ForLoop) = arms[0].body.node; if let ExprKind::Loop(ref body, ref label, LoopSource::ForLoop) = arms[0].body.node;
if let StmtKind::Local(ref local1) = body.node; if let StmtKind::Local(ref local) = body.node;
if let PatKind::Binding(BindingAnnotation::Mutable, _, name, None) = local1.pat.node; if let PatKind::Binding(BindingAnnotation::Mutable, _, name, None) = local.pat.node;
if name.node.as_str() == "__next"; if name.node.as_str() == "__next";
if let StmtKind::Expr(ref e, _) = local1.pat.node if let StmtKind::Expr(ref e, _) = local.pat.node
if let ExprKind::Match(ref expr1, ref arms1, MatchSource::ForLoopDesugar) = e.node; if let ExprKind::Match(ref expr2, ref arms1, MatchSource::ForLoopDesugar) = e.node;
if let ExprKind::Call(ref func1, ref args1) = expr1.node; if let ExprKind::Call(ref func1, ref args1) = expr2.node;
if let ExprKind::Path(ref path2) = func1.node; if let ExprKind::Path(ref path2) = func1.node;
if match_qpath(path2, &["{{root}}", "std", "iter", "Iterator", "next"]); if match_qpath(path2, &["{{root}}", "std", "iter", "Iterator", "next"]);
if args1.len() == 1; if args1.len() == 1;
@ -40,27 +38,23 @@ if_chain! {
if arms1[1].pats.len() == 1; if arms1[1].pats.len() == 1;
if let PatKind::Path(ref path7) = arms1[1].pats[0].node; if let PatKind::Path(ref path7) = arms1[1].pats[0].node;
if match_qpath(path7, &["{{root}}", "std", "option", "Option", "None"]); if match_qpath(path7, &["{{root}}", "std", "option", "Option", "None"]);
if let StmtKind::Local(ref local2) = path7.node; if let StmtKind::Local(ref local1) = path7.node;
if let Some(ref init1) = local2.init; if let Some(ref init) = local1.init;
if let ExprKind::Path(ref path8) = init1.node; if let ExprKind::Path(ref path8) = init.node;
if match_qpath(path8, &["__next"]); 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 name1.node.as_str() == "y";
if let StmtKind::Expr(ref e1, _) = local2.pat.node if let StmtKind::Expr(ref e1, _) = local1.pat.node
if let ExprKind::Block(ref block1) = e1.node; if let ExprKind::Block(ref block) = e1.node;
if let StmtKind::Local(ref local3) = block1.node; if let StmtKind::Local(ref local2) = block.node;
if let Some(ref init2) = local3.init; if let Some(ref init1) = local2.init;
if let ExprKind::Path(ref path9) = init2.node; if let ExprKind::Path(ref path9) = init1.node;
if match_qpath(path9, &["y"]); 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 name2.node.as_str() == "z";
if arms[0].pats.len() == 1; if arms[0].pats.len() == 1;
if let PatKind::Binding(BindingAnnotation::Mutable, _, name3, None) = arms[0].pats[0].node; if let PatKind::Binding(BindingAnnotation::Mutable, _, name3, None) = arms[0].pats[0].node;
if name3.node.as_str() == "iter"; 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 { then {
// report your lint here // report your lint here
} }