From d85b8062e3e5fea82e8eeed06432d2a0157d589b Mon Sep 17 00:00:00 2001 From: mcarton Date: Mon, 6 Jun 2016 02:09:19 +0200 Subject: [PATCH] Format all `if_let_chain` consistently --- clippy_lints/src/entry.rs | 1 - clippy_lints/src/loops.rs | 40 ++++++----- clippy_lints/src/map_clone.rs | 40 ++++++----- clippy_lints/src/misc.rs | 44 ++++++------- .../src/overflow_check_conditional.rs | 32 ++++----- clippy_lints/src/ranges.rs | 44 ++++++------- clippy_lints/src/returns.rs | 27 ++++---- clippy_lints/src/types.rs | 33 +++++----- clippy_lints/src/utils/mod.rs | 66 +++++++++---------- clippy_lints/src/zero_div_zero.rs | 47 +++++++------ 10 files changed, 175 insertions(+), 199 deletions(-) diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs index d63d8c67c..bc209fd48 100644 --- a/clippy_lints/src/entry.rs +++ b/clippy_lints/src/entry.rs @@ -120,7 +120,6 @@ impl<'a, 'tcx, 'v, 'b> Visitor<'v> for InsertVisitor<'a, 'tcx, 'b> { get_item_name(self.cx, self.map) == get_item_name(self.cx, &*params[0]), SpanlessEq::new(self.cx).eq_expr(self.key, ¶ms[1]) ], { - span_lint_and_then(self.cx, MAP_ENTRY, self.span, &format!("usage of `contains_key` followed by `insert` on `{}`", self.ty), |db| { if self.sole_expr { diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 08978e2c5..f3f10fae1 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -667,30 +667,28 @@ impl<'v, 't> Visitor<'v> for VarVisitor<'v, 't> { if let ExprPath(None, ref path) = expr.node { if path.segments.len() == 1 && path.segments[0].name == self.var { // we are referencing our variable! now check if it's as an index - if_let_chain! { - [ - let Some(parexpr) = get_parent_expr(self.cx, expr), - let ExprIndex(ref seqexpr, _) = parexpr.node, - let ExprPath(None, ref seqvar) = seqexpr.node, - seqvar.segments.len() == 1 - ], { - let def_map = self.cx.tcx.def_map.borrow(); - if let Some(def) = def_map.get(&seqexpr.id) { - match def.base_def { - Def::Local(..) | Def::Upvar(..) => { - let extent = self.cx.tcx.region_maps.var_scope(def.base_def.var_id()); - self.indexed.insert(seqvar.segments[0].name, Some(extent)); - return; // no need to walk further - } - Def::Static(..) | Def::Const(..) => { - self.indexed.insert(seqvar.segments[0].name, None); - return; // no need to walk further - } - _ => (), + if_let_chain! {[ + let Some(parexpr) = get_parent_expr(self.cx, expr), + let ExprIndex(ref seqexpr, _) = parexpr.node, + let ExprPath(None, ref seqvar) = seqexpr.node, + seqvar.segments.len() == 1 + ], { + let def_map = self.cx.tcx.def_map.borrow(); + if let Some(def) = def_map.get(&seqexpr.id) { + match def.base_def { + Def::Local(..) | Def::Upvar(..) => { + let extent = self.cx.tcx.region_maps.var_scope(def.base_def.var_id()); + self.indexed.insert(seqvar.segments[0].name, Some(extent)); + return; // no need to walk further } + Def::Static(..) | Def::Const(..) => { + self.indexed.insert(seqvar.segments[0].name, None); + return; // no need to walk further + } + _ => (), } } - } + }} // we are not indexing anything, record that self.nonindex = true; return; diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs index bd81cb231..168aade32 100644 --- a/clippy_lints/src/map_clone.rs +++ b/clippy_lints/src/map_clone.rs @@ -27,8 +27,7 @@ impl LateLintPass for MapClonePass { if name.node.as_str() == "map" && args.len() == 2 { match args[1].node { ExprClosure(_, ref decl, ref blk, _) => { - if_let_chain! { - [ + if_let_chain! {[ // just one expression in the closure blk.stmts.is_empty(), let Some(ref closure_expr) = blk.expr, @@ -37,32 +36,31 @@ impl LateLintPass for MapClonePass { let Some(arg_ident) = get_arg_name(&*decl.inputs[0].pat), // the method is being called on a known type (option or iterator) let Some(type_name) = get_type_name(cx, expr, &args[0]) - ], { - // look for derefs, for .map(|x| *x) - if only_derefs(cx, &*closure_expr, arg_ident) && - // .cloned() only removes one level of indirection, don't lint on more - walk_ptrs_ty_depth(cx.tcx.pat_ty(&*decl.inputs[0].pat)).1 == 1 + ], { + // look for derefs, for .map(|x| *x) + if only_derefs(cx, &*closure_expr, arg_ident) && + // .cloned() only removes one level of indirection, don't lint on more + walk_ptrs_ty_depth(cx.tcx.pat_ty(&*decl.inputs[0].pat)).1 == 1 + { + span_help_and_lint(cx, MAP_CLONE, expr.span, &format!( + "you seem to be using .map() to clone the contents of an {}, consider \ + using `.cloned()`", type_name), + &format!("try\n{}.cloned()", snippet(cx, args[0].span, ".."))); + } + // explicit clone() calls ( .map(|x| x.clone()) ) + else if let ExprMethodCall(clone_call, _, ref clone_args) = closure_expr.node { + if clone_call.node.as_str() == "clone" && + clone_args.len() == 1 && + match_trait_method(cx, closure_expr, &paths::CLONE_TRAIT) && + expr_eq_name(&clone_args[0], arg_ident) { span_help_and_lint(cx, MAP_CLONE, expr.span, &format!( "you seem to be using .map() to clone the contents of an {}, consider \ using `.cloned()`", type_name), &format!("try\n{}.cloned()", snippet(cx, args[0].span, ".."))); } - // explicit clone() calls ( .map(|x| x.clone()) ) - else if let ExprMethodCall(clone_call, _, ref clone_args) = closure_expr.node { - if clone_call.node.as_str() == "clone" && - clone_args.len() == 1 && - match_trait_method(cx, closure_expr, &paths::CLONE_TRAIT) && - expr_eq_name(&clone_args[0], arg_ident) - { - span_help_and_lint(cx, MAP_CLONE, expr.span, &format!( - "you seem to be using .map() to clone the contents of an {}, consider \ - using `.cloned()`", type_name), - &format!("try\n{}.cloned()", snippet(cx, args[0].span, ".."))); - } - } } - } + }} } ExprPath(_, ref path) => { if match_path(path, &paths::CLONE) { diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index c283dc69b..3d9795b0c 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -55,33 +55,31 @@ impl LateLintPass for TopLevelRefPass { } } fn check_stmt(&mut self, cx: &LateContext, s: &Stmt) { - if_let_chain! { - [ + if_let_chain! {[ let StmtDecl(ref d, _) = s.node, let DeclLocal(ref l) = d.node, let PatKind::Binding(BindByRef(_), i, None) = l.pat.node, let Some(ref init) = l.init - ], { - let tyopt = if let Some(ref ty) = l.ty { - format!(": {}", snippet(cx, ty.span, "_")) - } else { - "".to_owned() - }; - span_lint_and_then(cx, - TOPLEVEL_REF_ARG, - l.pat.span, - "`ref` on an entire `let` pattern is discouraged, take a reference with & instead", - |db| { - db.span_suggestion(s.span, - "try", - format!("let {}{} = &{};", - snippet(cx, i.span, "_"), - tyopt, - snippet(cx, init.span, "_"))); - } - ); - } - }; + ], { + let tyopt = if let Some(ref ty) = l.ty { + format!(": {}", snippet(cx, ty.span, "_")) + } else { + "".to_owned() + }; + span_lint_and_then(cx, + TOPLEVEL_REF_ARG, + l.pat.span, + "`ref` on an entire `let` pattern is discouraged, take a reference with & instead", + |db| { + db.span_suggestion(s.span, + "try", + format!("let {}{} = &{};", + snippet(cx, i.span, "_"), + tyopt, + snippet(cx, init.span, "_"))); + } + ); + }} } } diff --git a/clippy_lints/src/overflow_check_conditional.rs b/clippy_lints/src/overflow_check_conditional.rs index 34921bc2c..5e4386fb7 100644 --- a/clippy_lints/src/overflow_check_conditional.rs +++ b/clippy_lints/src/overflow_check_conditional.rs @@ -26,14 +26,14 @@ impl LateLintPass for OverflowCheckConditional { // a + b < a, a > a + b, a < a - b, a - b > a fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { if_let_chain! {[ - let Expr_::ExprBinary(ref op, ref first, ref second) = expr.node, - let Expr_::ExprBinary(ref op2, ref ident1, ref ident2) = first.node, - let Expr_::ExprPath(_,ref path1) = ident1.node, - let Expr_::ExprPath(_, ref path2) = ident2.node, - let Expr_::ExprPath(_, ref path3) = second.node, - &path1.segments[0] == &path3.segments[0] || &path2.segments[0] == &path3.segments[0], - cx.tcx.expr_ty(ident1).is_integral(), - cx.tcx.expr_ty(ident2).is_integral() + let Expr_::ExprBinary(ref op, ref first, ref second) = expr.node, + let Expr_::ExprBinary(ref op2, ref ident1, ref ident2) = first.node, + let Expr_::ExprPath(_,ref path1) = ident1.node, + let Expr_::ExprPath(_, ref path2) = ident2.node, + let Expr_::ExprPath(_, ref path3) = second.node, + &path1.segments[0] == &path3.segments[0] || &path2.segments[0] == &path3.segments[0], + cx.tcx.expr_ty(ident1).is_integral(), + cx.tcx.expr_ty(ident2).is_integral() ], { if let BinOp_::BiLt = op.node { if let BinOp_::BiAdd = op2.node { @@ -48,14 +48,14 @@ impl LateLintPass for OverflowCheckConditional { }} if_let_chain! {[ - let Expr_::ExprBinary(ref op, ref first, ref second) = expr.node, - let Expr_::ExprBinary(ref op2, ref ident1, ref ident2) = second.node, - let Expr_::ExprPath(_,ref path1) = ident1.node, - let Expr_::ExprPath(_, ref path2) = ident2.node, - let Expr_::ExprPath(_, ref path3) = first.node, - &path1.segments[0] == &path3.segments[0] || &path2.segments[0] == &path3.segments[0], - cx.tcx.expr_ty(ident1).is_integral(), - cx.tcx.expr_ty(ident2).is_integral() + let Expr_::ExprBinary(ref op, ref first, ref second) = expr.node, + let Expr_::ExprBinary(ref op2, ref ident1, ref ident2) = second.node, + let Expr_::ExprPath(_,ref path1) = ident1.node, + let Expr_::ExprPath(_, ref path2) = ident2.node, + let Expr_::ExprPath(_, ref path3) = first.node, + &path1.segments[0] == &path3.segments[0] || &path2.segments[0] == &path3.segments[0], + cx.tcx.expr_ty(ident1).is_integral(), + cx.tcx.expr_ty(ident2).is_integral() ], { if let BinOp_::BiGt = op.node { if let BinOp_::BiAdd = op2.node { diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index e96212a9c..8eacbadf8 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -49,29 +49,27 @@ impl LateLintPass for StepByZero { } else if name.as_str() == "zip" && args.len() == 2 { let iter = &args[0].node; let zip_arg = &args[1]; - if_let_chain! { - [ - // .iter() call - let ExprMethodCall( Spanned { node: ref iter_name, .. }, _, ref iter_args ) = *iter, - iter_name.as_str() == "iter", - // range expression in .zip() call: 0..x.len() - let Some(UnsugaredRange { start: Some(ref start), end: Some(ref end), .. }) = unsugar_range(zip_arg), - is_integer_literal(start, 0), - // .len() call - let ExprMethodCall(Spanned { node: ref len_name, .. }, _, ref len_args) = end.node, - len_name.as_str() == "len" && len_args.len() == 1, - // .iter() and .len() called on same Path - let ExprPath(_, Path { segments: ref iter_path, .. }) = iter_args[0].node, - let ExprPath(_, Path { segments: ref len_path, .. }) = len_args[0].node, - iter_path == len_path - ], { - span_lint(cx, - RANGE_ZIP_WITH_LEN, - expr.span, - &format!("It is more idiomatic to use {}.iter().enumerate()", - snippet(cx, iter_args[0].span, "_"))); - } - } + if_let_chain! {[ + // .iter() call + let ExprMethodCall( Spanned { node: ref iter_name, .. }, _, ref iter_args ) = *iter, + iter_name.as_str() == "iter", + // range expression in .zip() call: 0..x.len() + let Some(UnsugaredRange { start: Some(ref start), end: Some(ref end), .. }) = unsugar_range(zip_arg), + is_integer_literal(start, 0), + // .len() call + let ExprMethodCall(Spanned { node: ref len_name, .. }, _, ref len_args) = end.node, + len_name.as_str() == "len" && len_args.len() == 1, + // .iter() and .len() called on same Path + let ExprPath(_, Path { segments: ref iter_path, .. }) = iter_args[0].node, + let ExprPath(_, Path { segments: ref len_path, .. }) = len_args[0].node, + iter_path == len_path + ], { + span_lint(cx, + RANGE_ZIP_WITH_LEN, + expr.span, + &format!("It is more idiomatic to use {}.iter().enumerate()", + snippet(cx, iter_args[0].span, "_"))); + }} } } } diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 7bb468166..6beed822a 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -89,19 +89,17 @@ impl ReturnPass { // Check for "let x = EXPR; x" fn check_let_return(&mut self, cx: &EarlyContext, block: &Block) { // we need both a let-binding stmt and an expr - if_let_chain! { - [ - let Some(stmt) = block.stmts.last(), - let Some(ref retexpr) = block.expr, - let StmtKind::Decl(ref decl, _) = stmt.node, - let DeclKind::Local(ref local) = decl.node, - local.ty.is_none(), - let Some(ref initexpr) = local.init, - let PatKind::Ident(_, Spanned { node: id, .. }, _) = local.pat.node, - let ExprKind::Path(_, ref path) = retexpr.node, - match_path_ast(path, &[&id.name.as_str()]), - !in_external_macro(cx, initexpr.span), - ], { + if_let_chain! {[ + let Some(stmt) = block.stmts.last(), + let Some(ref retexpr) = block.expr, + let StmtKind::Decl(ref decl, _) = stmt.node, + let DeclKind::Local(ref local) = decl.node, + let Some(ref initexpr) = local.init, + let PatKind::Ident(_, Spanned { node: id, .. }, _) = local.pat.node, + let ExprKind::Path(_, ref path) = retexpr.node, + match_path_ast(path, &[&id.name.as_str()]), + !in_external_macro(cx, initexpr.span), + ], { span_note_and_lint(cx, LET_AND_RETURN, retexpr.span, @@ -109,8 +107,7 @@ impl ReturnPass { Consider returning the expression directly.", initexpr.span, "this expression can be directly returned"); - } - } + }} } } diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index c4b810a78..8a1a13187 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -57,24 +57,21 @@ impl LateLintPass for TypePass { if let Some(did) = cx.tcx.def_map.borrow().get(&ast_ty.id) { if let def::Def::Struct(..) = did.full_def() { if Some(did.def_id()) == cx.tcx.lang_items.owned_box() { - if_let_chain! { - [ - let TyPath(_, ref path) = ast_ty.node, - let Some(ref last) = path.segments.last(), - let PathParameters::AngleBracketedParameters(ref ag) = last.parameters, - let Some(ref vec) = ag.types.get(0), - let Some(did) = cx.tcx.def_map.borrow().get(&vec.id), - let def::Def::Struct(..) = did.full_def(), - match_def_path(cx, did.def_id(), &paths::VEC), - ], - { - span_help_and_lint(cx, - BOX_VEC, - ast_ty.span, - "you seem to be trying to use `Box>`. Consider using just `Vec`", - "`Vec` is already on the heap, `Box>` makes an extra allocation."); - } - } + if_let_chain! {[ + let TyPath(_, ref path) = ast_ty.node, + let Some(ref last) = path.segments.last(), + let PathParameters::AngleBracketedParameters(ref ag) = last.parameters, + let Some(ref vec) = ag.types.get(0), + let Some(did) = cx.tcx.def_map.borrow().get(&vec.id), + let def::Def::Struct(..) = did.full_def(), + match_def_path(cx, did.def_id(), &paths::VEC), + ], { + span_help_and_lint(cx, + BOX_VEC, + ast_ty.span, + "you seem to be trying to use `Box>`. Consider using just `Vec`", + "`Vec` is already on the heap, `Box>` makes an extra allocation."); + }} } else if match_def_path(cx, did.def_id(), &paths::LINKED_LIST) { span_help_and_lint(cx, LINKEDLIST, diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 5c61a33ce..a2b2ecfbc 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -30,16 +30,13 @@ pub type MethodArgs = HirVec>; /// Produce a nested chain of if-lets and ifs from the patterns: /// -/// if_let_chain! { -/// [ -/// let Some(y) = x, -/// y.len() == 2, -/// let Some(z) = y, -/// ], -/// { -/// block -/// } -/// } +/// if_let_chain! {[ +/// let Some(y) = x, +/// y.len() == 2, +/// let Some(z) = y, +/// ], { +/// block +/// }} /// /// becomes /// @@ -323,14 +320,13 @@ pub fn get_item_name(cx: &LateContext, expr: &Expr) -> Option { /// Checks if a `let` decl is from a `for` loop desugaring. pub fn is_from_for_desugar(decl: &Decl) -> bool { - if_let_chain! { - [ - let DeclLocal(ref loc) = decl.node, - let Some(ref expr) = loc.init, - let ExprMatch(_, _, MatchSource::ForLoopDesugar) = expr.node - ], - { return true; } - }; + if_let_chain! {[ + let DeclLocal(ref loc) = decl.node, + let Some(ref expr) = loc.init, + let ExprMatch(_, _, MatchSource::ForLoopDesugar) = expr.node + ], { + return true; + }} false } @@ -821,23 +817,21 @@ pub fn same_tys<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, a: ty::Ty<'tcx>, b: ty::Ty /// Recover the essential nodes of a desugared for loop: /// `for pat in arg { body }` becomes `(pat, arg, body)`. pub fn recover_for_loop(expr: &Expr) -> Option<(&Pat, &Expr, &Expr)> { - if_let_chain! { - [ - let ExprMatch(ref iterexpr, ref arms, _) = expr.node, - let ExprCall(_, ref iterargs) = iterexpr.node, - iterargs.len() == 1 && arms.len() == 1 && arms[0].guard.is_none(), - let ExprLoop(ref block, _) = arms[0].body.node, - block.stmts.is_empty(), - let Some(ref loopexpr) = block.expr, - let ExprMatch(_, ref innerarms, MatchSource::ForLoopDesugar) = loopexpr.node, - innerarms.len() == 2 && innerarms[0].pats.len() == 1, - let PatKind::TupleStruct(_, ref somepats, _) = innerarms[0].pats[0].node, - somepats.len() == 1 - ], { - return Some((&somepats[0], - &iterargs[0], - &innerarms[0].body)); - } - } + if_let_chain! {[ + let ExprMatch(ref iterexpr, ref arms, _) = expr.node, + let ExprCall(_, ref iterargs) = iterexpr.node, + iterargs.len() == 1 && arms.len() == 1 && arms[0].guard.is_none(), + let ExprLoop(ref block, _) = arms[0].body.node, + block.stmts.is_empty(), + let Some(ref loopexpr) = block.expr, + let ExprMatch(_, ref innerarms, MatchSource::ForLoopDesugar) = loopexpr.node, + innerarms.len() == 2 && innerarms[0].pats.len() == 1, + let PatKind::TupleStruct(_, ref somepats, _) = innerarms[0].pats[0].node, + somepats.len() == 1 + ], { + return Some((&somepats[0], + &iterargs[0], + &innerarms[0].body)); + }} None } diff --git a/clippy_lints/src/zero_div_zero.rs b/clippy_lints/src/zero_div_zero.rs index 902d84d4d..041ac9483 100644 --- a/clippy_lints/src/zero_div_zero.rs +++ b/clippy_lints/src/zero_div_zero.rs @@ -30,30 +30,27 @@ impl LintPass for ZeroDivZeroPass { impl LateLintPass for ZeroDivZeroPass { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { // check for instances of 0.0/0.0 - if_let_chain! { - [ - let ExprBinary(ref op, ref left, ref right) = expr.node, - let BinOp_::BiDiv = op.node, - // TODO - constant_simple does not fold many operations involving floats. - // That's probably fine for this lint - it's pretty unlikely that someone would - // do something like 0.0/(2.0 - 2.0), but it would be nice to warn on that case too. - let Some(Constant::Float(ref lhs_value, lhs_width)) = constant_simple(left), - let Some(Constant::Float(ref rhs_value, rhs_width)) = constant_simple(right), - let Some(0.0) = lhs_value.parse().ok(), - let Some(0.0) = rhs_value.parse().ok() - ], - { - // since we're about to suggest a use of std::f32::NaN or std::f64::NaN, - // match the precision of the literals that are given. - let float_type = match (lhs_width, rhs_width) { - (FloatWidth::F64, _) - | (_, FloatWidth::F64) => "f64", - _ => "f32" - }; - span_help_and_lint(cx, ZERO_DIVIDED_BY_ZERO, expr.span, - "constant division of 0.0 with 0.0 will always result in NaN", - &format!("Consider using `std::{}::NAN` if you would like a constant representing NaN", float_type)); - } - } + if_let_chain! {[ + let ExprBinary(ref op, ref left, ref right) = expr.node, + let BinOp_::BiDiv = op.node, + // TODO - constant_simple does not fold many operations involving floats. + // That's probably fine for this lint - it's pretty unlikely that someone would + // do something like 0.0/(2.0 - 2.0), but it would be nice to warn on that case too. + let Some(Constant::Float(ref lhs_value, lhs_width)) = constant_simple(left), + let Some(Constant::Float(ref rhs_value, rhs_width)) = constant_simple(right), + let Some(0.0) = lhs_value.parse().ok(), + let Some(0.0) = rhs_value.parse().ok() + ], { + // since we're about to suggest a use of std::f32::NaN or std::f64::NaN, + // match the precision of the literals that are given. + let float_type = match (lhs_width, rhs_width) { + (FloatWidth::F64, _) + | (_, FloatWidth::F64) => "f64", + _ => "f32" + }; + span_help_and_lint(cx, ZERO_DIVIDED_BY_ZERO, expr.span, + "constant division of 0.0 with 0.0 will always result in NaN", + &format!("Consider using `std::{}::NAN` if you would like a constant representing NaN", float_type)); + }} } }