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

Fix breakage due to rust-lang/rust#60225 #4040

Merged
merged 1 commit into from Apr 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 43 additions & 50 deletions clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
Expand Down Expand Up @@ -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,
);
}
}
}
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions clippy_lints/src/utils/author.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
}
}

Expand Down
6 changes: 6 additions & 0 deletions clippy_lints/src/utils/hir_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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);
},
}
}

Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/utils/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
}
}

Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/sugg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
42 changes: 18 additions & 24 deletions tests/ui/author/for_loop.stdout
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand All @@ -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
}
Expand Down