From 41840ae3c41483745ff92f3ba8e99f9ffcb3dff8 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Mon, 23 Oct 2017 15:18:02 -0400 Subject: [PATCH 1/3] mechanically swap if_let_chain -> if_chain --- clippy_lints/src/attrs.rs | 15 +- clippy_lints/src/bit_mask.rs | 43 ++-- clippy_lints/src/bytecount.rs | 98 ++++---- clippy_lints/src/collapsible_if.rs | 64 ++--- clippy_lints/src/derive.rs | 75 +++--- clippy_lints/src/drop_forget_ref.rs | 87 +++---- clippy_lints/src/entry.rs | 90 +++---- clippy_lints/src/eval_order_dependence.rs | 33 +-- clippy_lints/src/explicit_write.rs | 109 +++++---- clippy_lints/src/fallible_impl_from.rs | 91 +++---- clippy_lints/src/format.rs | 79 +++--- clippy_lints/src/invalid_ref.rs | 33 +-- clippy_lints/src/let_if_seq.rs | 181 +++++++------- clippy_lints/src/literal_digit_grouping.rs | 102 ++++---- clippy_lints/src/loops.rs | 231 +++++++++--------- clippy_lints/src/map_clone.rs | 55 +++-- clippy_lints/src/matches.rs | 67 ++--- clippy_lints/src/methods.rs | 229 ++++++++--------- clippy_lints/src/misc.rs | 125 +++++----- clippy_lints/src/misc_early.rs | 167 ++++++------- clippy_lints/src/needless_borrow.rs | 37 +-- clippy_lints/src/needless_borrowed_ref.rs | 23 +- clippy_lints/src/needless_pass_by_value.rs | 226 ++++++++--------- clippy_lints/src/neg_multiply.rs | 25 +- clippy_lints/src/new_without_default.rs | 67 ++--- clippy_lints/src/ok_if_let.rs | 31 +-- .../src/overflow_check_conditional.rs | 86 +++---- clippy_lints/src/panic.rs | 37 +-- clippy_lints/src/partialeq_ne_impl.rs | 27 +- clippy_lints/src/print.rs | 149 +++++------ clippy_lints/src/ptr.rs | 15 +- clippy_lints/src/ranges.rs | 109 +++++---- clippy_lints/src/regex.rs | 81 +++--- clippy_lints/src/returns.rs | 43 ++-- clippy_lints/src/swap.rs | 196 +++++++-------- clippy_lints/src/transmute.rs | 17 +- clippy_lints/src/types.rs | 180 +++++++------- clippy_lints/src/use_self.rs | 35 +-- clippy_lints/src/utils/higher.rs | 105 ++++---- clippy_lints/src/utils/mod.rs | 55 +++-- clippy_lints/src/vec.rs | 36 +-- clippy_lints/src/zero_div_zero.rs | 39 +-- 42 files changed, 1843 insertions(+), 1750 deletions(-) diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 83ec32615d10..8baff5519104 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -94,13 +94,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AttrPass { return; } for item in items { - if_let_chain! {[ - let NestedMetaItemKind::MetaItem(ref mi) = item.node, - let MetaItemKind::NameValue(ref lit) = mi.node, - mi.name() == "since", - ], { - check_semver(cx, item.span, lit); - }} + if_chain! { + if let NestedMetaItemKind::MetaItem(ref mi) = item.node; + if let MetaItemKind::NameValue(ref lit) = mi.node; + if mi.name() == "since"; + then { + check_semver(cx, item.span, lit); + } + } } } } diff --git a/clippy_lints/src/bit_mask.rs b/clippy_lints/src/bit_mask.rs index 6372221fd440..f0ff27d4d637 100644 --- a/clippy_lints/src/bit_mask.rs +++ b/clippy_lints/src/bit_mask.rs @@ -119,27 +119,28 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BitMask { } } } - if_let_chain!{[ - let Expr_::ExprBinary(ref op, ref left, ref right) = e.node, - BinOp_::BiEq == op.node, - let Expr_::ExprBinary(ref op1, ref left1, ref right1) = left.node, - BinOp_::BiBitAnd == op1.node, - let Expr_::ExprLit(ref lit) = right1.node, - let LitKind::Int(n, _) = lit.node, - let Expr_::ExprLit(ref lit1) = right.node, - let LitKind::Int(0, _) = lit1.node, - n.leading_zeros() == n.count_zeros(), - n > u128::from(self.verbose_bit_mask_threshold), - ], { - span_lint_and_then(cx, - VERBOSE_BIT_MASK, - e.span, - "bit mask could be simplified with a call to `trailing_zeros`", - |db| { - let sugg = Sugg::hir(cx, left1, "...").maybe_par(); - db.span_suggestion(e.span, "try", format!("{}.trailing_zeros() >= {}", sugg, n.count_ones())); - }); - }} + if_chain! { + if let Expr_::ExprBinary(ref op, ref left, ref right) = e.node; + if BinOp_::BiEq == op.node; + if let Expr_::ExprBinary(ref op1, ref left1, ref right1) = left.node; + if BinOp_::BiBitAnd == op1.node; + if let Expr_::ExprLit(ref lit) = right1.node; + if let LitKind::Int(n, _) = lit.node; + if let Expr_::ExprLit(ref lit1) = right.node; + if let LitKind::Int(0, _) = lit1.node; + if n.leading_zeros() == n.count_zeros(); + if n > u128::from(self.verbose_bit_mask_threshold); + then { + span_lint_and_then(cx, + VERBOSE_BIT_MASK, + e.span, + "bit mask could be simplified with a call to `trailing_zeros`", + |db| { + let sugg = Sugg::hir(cx, left1, "...").maybe_par(); + db.span_suggestion(e.span, "try", format!("{}.trailing_zeros() >= {}", sugg, n.count_ones())); + }); + } + } } } diff --git a/clippy_lints/src/bytecount.rs b/clippy_lints/src/bytecount.rs index 58f1227d91ed..e0ce4bbc93bf 100644 --- a/clippy_lints/src/bytecount.rs +++ b/clippy_lints/src/bytecount.rs @@ -37,56 +37,58 @@ impl LintPass for ByteCount { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ByteCount { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { - if_let_chain!([ - let ExprMethodCall(ref count, _, ref count_args) = expr.node, - count.name == "count", - count_args.len() == 1, - let ExprMethodCall(ref filter, _, ref filter_args) = count_args[0].node, - filter.name == "filter", - filter_args.len() == 2, - let ExprClosure(_, _, body_id, _, _) = filter_args[1].node, - ], { - let body = cx.tcx.hir.body(body_id); - if_let_chain!([ - body.arguments.len() == 1, - let Some(argname) = get_pat_name(&body.arguments[0].pat), - let ExprBinary(ref op, ref l, ref r) = body.value.node, - op.node == BiEq, - match_type(cx, - walk_ptrs_ty(cx.tables.expr_ty(&filter_args[0])), - &paths::SLICE_ITER), - ], { - let needle = match get_path_name(l) { - Some(name) if check_arg(name, argname, r) => r, - _ => match get_path_name(r) { - Some(name) if check_arg(name, argname, l) => l, - _ => { return; } + if_chain! { + if let ExprMethodCall(ref count, _, ref count_args) = expr.node; + if count.name == "count"; + if count_args.len() == 1; + if let ExprMethodCall(ref filter, _, ref filter_args) = count_args[0].node; + if filter.name == "filter"; + if filter_args.len() == 2; + if let ExprClosure(_, _, body_id, _, _) = filter_args[1].node; + then { + let body = cx.tcx.hir.body(body_id); + if_chain! { + if body.arguments.len() == 1; + if let Some(argname) = get_pat_name(&body.arguments[0].pat); + if let ExprBinary(ref op, ref l, ref r) = body.value.node; + if op.node == BiEq; + if match_type(cx, + walk_ptrs_ty(cx.tables.expr_ty(&filter_args[0])), + &paths::SLICE_ITER); + then { + let needle = match get_path_name(l) { + Some(name) if check_arg(name, argname, r) => r, + _ => match get_path_name(r) { + Some(name) if check_arg(name, argname, l) => l, + _ => { return; } + } + }; + if ty::TyUint(UintTy::U8) != walk_ptrs_ty(cx.tables.expr_ty(needle)).sty { + return; + } + let haystack = if let ExprMethodCall(ref path, _, ref args) = + filter_args[0].node { + let p = path.name; + if (p == "iter" || p == "iter_mut") && args.len() == 1 { + &args[0] + } else { + &filter_args[0] + } + } else { + &filter_args[0] + }; + span_lint_and_sugg(cx, + NAIVE_BYTECOUNT, + expr.span, + "You appear to be counting bytes the naive way", + "Consider using the bytecount crate", + format!("bytecount::count({}, {})", + snippet(cx, haystack.span, ".."), + snippet(cx, needle.span, ".."))); } }; - if ty::TyUint(UintTy::U8) != walk_ptrs_ty(cx.tables.expr_ty(needle)).sty { - return; - } - let haystack = if let ExprMethodCall(ref path, _, ref args) = - filter_args[0].node { - let p = path.name; - if (p == "iter" || p == "iter_mut") && args.len() == 1 { - &args[0] - } else { - &filter_args[0] - } - } else { - &filter_args[0] - }; - span_lint_and_sugg(cx, - NAIVE_BYTECOUNT, - expr.span, - "You appear to be counting bytes the naive way", - "Consider using the bytecount crate", - format!("bytecount::count({}, {})", - snippet(cx, haystack.span, ".."), - snippet(cx, needle.span, ".."))); - }); - }); + } + }; } } diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index fb0ff23cc632..3ac19980a6d3 100644 --- a/clippy_lints/src/collapsible_if.rs +++ b/clippy_lints/src/collapsible_if.rs @@ -100,43 +100,45 @@ fn check_if(cx: &EarlyContext, expr: &ast::Expr) { } fn check_collapsible_maybe_if_let(cx: &EarlyContext, else_: &ast::Expr) { - if_let_chain! {[ - let ast::ExprKind::Block(ref block) = else_.node, - let Some(else_) = expr_block(block), - !in_macro(else_.span), - ], { - match else_.node { - ast::ExprKind::If(..) | ast::ExprKind::IfLet(..) => { - span_lint_and_sugg(cx, - COLLAPSIBLE_IF, - block.span, - "this `else { if .. }` block can be collapsed", - "try", - snippet_block(cx, else_.span, "..").into_owned()); + if_chain! { + if let ast::ExprKind::Block(ref block) = else_.node; + if let Some(else_) = expr_block(block); + if !in_macro(else_.span); + then { + match else_.node { + ast::ExprKind::If(..) | ast::ExprKind::IfLet(..) => { + span_lint_and_sugg(cx, + COLLAPSIBLE_IF, + block.span, + "this `else { if .. }` block can be collapsed", + "try", + snippet_block(cx, else_.span, "..").into_owned()); + } + _ => (), } - _ => (), } - }} + } } fn check_collapsible_no_if_let(cx: &EarlyContext, expr: &ast::Expr, check: &ast::Expr, then: &ast::Block) { - if_let_chain! {[ - let Some(inner) = expr_block(then), - let ast::ExprKind::If(ref check_inner, ref content, None) = inner.node, - ], { - if expr.span.ctxt() != inner.span.ctxt() { - return; + if_chain! { + if let Some(inner) = expr_block(then); + if let ast::ExprKind::If(ref check_inner, ref content, None) = inner.node; + then { + if expr.span.ctxt() != inner.span.ctxt() { + return; + } + span_lint_and_then(cx, COLLAPSIBLE_IF, expr.span, "this if statement can be collapsed", |db| { + let lhs = Sugg::ast(cx, check, ".."); + let rhs = Sugg::ast(cx, check_inner, ".."); + db.span_suggestion(expr.span, + "try", + format!("if {} {}", + lhs.and(rhs), + snippet_block(cx, content.span, ".."))); + }); } - span_lint_and_then(cx, COLLAPSIBLE_IF, expr.span, "this if statement can be collapsed", |db| { - let lhs = Sugg::ast(cx, check, ".."); - let rhs = Sugg::ast(cx, check_inner, ".."); - db.span_suggestion(expr.span, - "try", - format!("if {} {}", - lhs.and(rhs), - snippet_block(cx, content.span, ".."))); - }); - }} + } } /// If the block contains only one expression, return it. diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index a891d7721c3f..2c45aaf6ac93 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -91,43 +91,44 @@ fn check_hash_peq<'a, 'tcx>( ty: Ty<'tcx>, hash_is_automatically_derived: bool, ) { - if_let_chain! {[ - match_path(&trait_ref.path, &paths::HASH), - let Some(peq_trait_def_id) = cx.tcx.lang_items().eq_trait() - ], { - // Look for the PartialEq implementations for `ty` - cx.tcx.for_each_relevant_impl(peq_trait_def_id, ty, |impl_id| { - let peq_is_automatically_derived = is_automatically_derived(&cx.tcx.get_attrs(impl_id)); - - if peq_is_automatically_derived == hash_is_automatically_derived { - return; - } - - let trait_ref = cx.tcx.impl_trait_ref(impl_id).expect("must be a trait implementation"); - - // Only care about `impl PartialEq for Foo` - // For `impl PartialEq for A, input_types is [A, B] - if trait_ref.substs.type_at(1) == ty { - let mess = if peq_is_automatically_derived { - "you are implementing `Hash` explicitly but have derived `PartialEq`" - } else { - "you are deriving `Hash` but have implemented `PartialEq` explicitly" - }; - - span_lint_and_then( - cx, DERIVE_HASH_XOR_EQ, span, - mess, - |db| { - if let Some(node_id) = cx.tcx.hir.as_local_node_id(impl_id) { - db.span_note( - cx.tcx.hir.span(node_id), - "`PartialEq` implemented here" - ); - } - }); - } - }); - }} + if_chain! { + if match_path(&trait_ref.path, &paths::HASH); + if let Some(peq_trait_def_id) = cx.tcx.lang_items().eq_trait(); + then { + // Look for the PartialEq implementations for `ty` + cx.tcx.for_each_relevant_impl(peq_trait_def_id, ty, |impl_id| { + let peq_is_automatically_derived = is_automatically_derived(&cx.tcx.get_attrs(impl_id)); + + if peq_is_automatically_derived == hash_is_automatically_derived { + return; + } + + let trait_ref = cx.tcx.impl_trait_ref(impl_id).expect("must be a trait implementation"); + + // Only care about `impl PartialEq for Foo` + // For `impl PartialEq for A, input_types is [A, B] + if trait_ref.substs.type_at(1) == ty { + let mess = if peq_is_automatically_derived { + "you are implementing `Hash` explicitly but have derived `PartialEq`" + } else { + "you are deriving `Hash` but have implemented `PartialEq` explicitly" + }; + + span_lint_and_then( + cx, DERIVE_HASH_XOR_EQ, span, + mess, + |db| { + if let Some(node_id) = cx.tcx.hir.as_local_node_id(impl_id) { + db.span_note( + cx.tcx.hir.span(node_id), + "`PartialEq` implemented here" + ); + } + }); + } + }); + } + } } /// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint. diff --git a/clippy_lints/src/drop_forget_ref.rs b/clippy_lints/src/drop_forget_ref.rs index 46b228e70abf..1601c276e2b2 100644 --- a/clippy_lints/src/drop_forget_ref.rs +++ b/clippy_lints/src/drop_forget_ref.rs @@ -115,50 +115,51 @@ impl LintPass for Pass { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { - if_let_chain!{[ - let ExprCall(ref path, ref args) = expr.node, - let ExprPath(ref qpath) = path.node, - args.len() == 1, - let Some(def_id) = opt_def_id(cx.tables.qpath_def(qpath, path.hir_id)), - ], { - let lint; - let msg; - let arg = &args[0]; - let arg_ty = cx.tables.expr_ty(arg); - - if let ty::TyRef(..) = arg_ty.sty { - if match_def_path(cx.tcx, def_id, &paths::DROP) { - lint = DROP_REF; - msg = DROP_REF_SUMMARY.to_string(); - } else if match_def_path(cx.tcx, def_id, &paths::MEM_FORGET) { - lint = FORGET_REF; - msg = FORGET_REF_SUMMARY.to_string(); - } else { - return; + if_chain! { + if let ExprCall(ref path, ref args) = expr.node; + if let ExprPath(ref qpath) = path.node; + if args.len() == 1; + if let Some(def_id) = opt_def_id(cx.tables.qpath_def(qpath, path.hir_id)); + then { + let lint; + let msg; + let arg = &args[0]; + let arg_ty = cx.tables.expr_ty(arg); + + if let ty::TyRef(..) = arg_ty.sty { + if match_def_path(cx.tcx, def_id, &paths::DROP) { + lint = DROP_REF; + msg = DROP_REF_SUMMARY.to_string(); + } else if match_def_path(cx.tcx, def_id, &paths::MEM_FORGET) { + lint = FORGET_REF; + msg = FORGET_REF_SUMMARY.to_string(); + } else { + return; + } + span_note_and_lint(cx, + lint, + expr.span, + &msg, + arg.span, + &format!("argument has type {}", arg_ty)); + } else if is_copy(cx, arg_ty) { + if match_def_path(cx.tcx, def_id, &paths::DROP) { + lint = DROP_COPY; + msg = DROP_COPY_SUMMARY.to_string(); + } else if match_def_path(cx.tcx, def_id, &paths::MEM_FORGET) { + lint = FORGET_COPY; + msg = FORGET_COPY_SUMMARY.to_string(); + } else { + return; + } + span_note_and_lint(cx, + lint, + expr.span, + &msg, + arg.span, + &format!("argument has type {}", arg_ty)); } - span_note_and_lint(cx, - lint, - expr.span, - &msg, - arg.span, - &format!("argument has type {}", arg_ty)); - } else if is_copy(cx, arg_ty) { - if match_def_path(cx.tcx, def_id, &paths::DROP) { - lint = DROP_COPY; - msg = DROP_COPY_SUMMARY.to_string(); - } else if match_def_path(cx.tcx, def_id, &paths::MEM_FORGET) { - lint = FORGET_COPY; - msg = FORGET_COPY_SUMMARY.to_string(); - } else { - return; - } - span_note_and_lint(cx, - lint, - expr.span, - &msg, - arg.span, - &format!("argument has type {}", arg_ty)); } - }} + } } } diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs index a3558a189e2a..6c7a5fec03c9 100644 --- a/clippy_lints/src/entry.rs +++ b/clippy_lints/src/entry.rs @@ -87,25 +87,26 @@ fn check_cond<'a, 'tcx, 'b>( cx: &'a LateContext<'a, 'tcx>, check: &'b Expr, ) -> Option<(&'static str, &'b Expr, &'b Expr)> { - if_let_chain! {[ - let ExprMethodCall(ref path, _, ref params) = check.node, - params.len() >= 2, - path.name == "contains_key", - let ExprAddrOf(_, ref key) = params[1].node - ], { - let map = ¶ms[0]; - let obj_ty = walk_ptrs_ty(cx.tables.expr_ty(map)); - - return if match_type(cx, obj_ty, &paths::BTREEMAP) { - Some(("BTreeMap", map, key)) + if_chain! { + if let ExprMethodCall(ref path, _, ref params) = check.node; + if params.len() >= 2; + if path.name == "contains_key"; + if let ExprAddrOf(_, ref key) = params[1].node; + then { + let map = ¶ms[0]; + let obj_ty = walk_ptrs_ty(cx.tables.expr_ty(map)); + + return if match_type(cx, obj_ty, &paths::BTREEMAP) { + Some(("BTreeMap", map, key)) + } + else if match_type(cx, obj_ty, &paths::HASHMAP) { + Some(("HashMap", map, key)) + } + else { + None + }; } - else if match_type(cx, obj_ty, &paths::HASHMAP) { - Some(("HashMap", map, key)) - } - else { - None - }; - }} + } None } @@ -121,32 +122,33 @@ struct InsertVisitor<'a, 'tcx: 'a, 'b> { impl<'a, 'tcx, 'b> Visitor<'tcx> for InsertVisitor<'a, 'tcx, 'b> { fn visit_expr(&mut self, expr: &'tcx Expr) { - if_let_chain! {[ - let ExprMethodCall(ref path, _, ref params) = expr.node, - params.len() == 3, - path.name == "insert", - get_item_name(self.cx, self.map) == get_item_name(self.cx, ¶ms[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 a `{}`", self.ty), |db| { - if self.sole_expr { - let help = format!("{}.entry({}).or_insert({})", - snippet(self.cx, self.map.span, "map"), - snippet(self.cx, params[1].span, ".."), - snippet(self.cx, params[2].span, "..")); - - db.span_suggestion(self.span, "consider using", help); - } - else { - let help = format!("{}.entry({})", - snippet(self.cx, self.map.span, "map"), - snippet(self.cx, params[1].span, "..")); - - db.span_suggestion(self.span, "consider using", help); - } - }); - }} + if_chain! { + if let ExprMethodCall(ref path, _, ref params) = expr.node; + if params.len() == 3; + if path.name == "insert"; + if get_item_name(self.cx, self.map) == get_item_name(self.cx, ¶ms[0]); + if SpanlessEq::new(self.cx).eq_expr(self.key, ¶ms[1]); + then { + span_lint_and_then(self.cx, MAP_ENTRY, self.span, + &format!("usage of `contains_key` followed by `insert` on a `{}`", self.ty), |db| { + if self.sole_expr { + let help = format!("{}.entry({}).or_insert({})", + snippet(self.cx, self.map.span, "map"), + snippet(self.cx, params[1].span, ".."), + snippet(self.cx, params[2].span, "..")); + + db.span_suggestion(self.span, "consider using", help); + } + else { + let help = format!("{}.entry({})", + snippet(self.cx, self.map.span, "map"), + snippet(self.cx, params[1].span, "..")); + + db.span_suggestion(self.span, "consider using", help); + } + }); + } + } if !self.sole_expr { walk_expr(self, expr); diff --git a/clippy_lints/src/eval_order_dependence.rs b/clippy_lints/src/eval_order_dependence.rs index 42af597125d1..847aec415002 100644 --- a/clippy_lints/src/eval_order_dependence.rs +++ b/clippy_lints/src/eval_order_dependence.rs @@ -298,23 +298,24 @@ impl<'a, 'tcx> Visitor<'tcx> for ReadVisitor<'a, 'tcx> { match expr.node { ExprPath(ref qpath) => { - if_let_chain! {[ - let QPath::Resolved(None, ref path) = *qpath, - path.segments.len() == 1, - let def::Def::Local(local_id) = self.cx.tables.qpath_def(qpath, expr.hir_id), - local_id == self.var, + if_chain! { + if let QPath::Resolved(None, ref path) = *qpath; + if path.segments.len() == 1; + if let def::Def::Local(local_id) = self.cx.tables.qpath_def(qpath, expr.hir_id); + if local_id == self.var; // Check that this is a read, not a write. - !is_in_assignment_position(self.cx, expr), - ], { - span_note_and_lint( - self.cx, - EVAL_ORDER_DEPENDENCE, - expr.span, - "unsequenced read of a variable", - self.write_expr.span, - "whether read occurs before this write depends on evaluation order" - ); - }} + if !is_in_assignment_position(self.cx, expr); + then { + span_note_and_lint( + self.cx, + EVAL_ORDER_DEPENDENCE, + expr.span, + "unsequenced read of a variable", + self.write_expr.span, + "whether read occurs before this write depends on evaluation order" + ); + } + } } // We're about to descend a closure. Since we don't know when (or // if) the closure will be evaluated, any reads in it might not diff --git a/clippy_lints/src/explicit_write.rs b/clippy_lints/src/explicit_write.rs index 9650dd0909ca..7ea96cabfac8 100644 --- a/clippy_lints/src/explicit_write.rs +++ b/clippy_lints/src/explicit_write.rs @@ -33,69 +33,70 @@ impl LintPass for Pass { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { - if_let_chain! {[ + if_chain! { // match call to unwrap - let ExprMethodCall(ref unwrap_fun, _, ref unwrap_args) = expr.node, - unwrap_fun.name == "unwrap", + if let ExprMethodCall(ref unwrap_fun, _, ref unwrap_args) = expr.node; + if unwrap_fun.name == "unwrap"; // match call to write_fmt - unwrap_args.len() > 0, - let ExprMethodCall(ref write_fun, _, ref write_args) = - unwrap_args[0].node, - write_fun.name == "write_fmt", + if unwrap_args.len() > 0; + if let ExprMethodCall(ref write_fun, _, ref write_args) = + unwrap_args[0].node; + if write_fun.name == "write_fmt"; // match calls to std::io::stdout() / std::io::stderr () - write_args.len() > 0, - let ExprCall(ref dest_fun, _) = write_args[0].node, - let ExprPath(ref qpath) = dest_fun.node, - let Some(dest_fun_id) = - opt_def_id(resolve_node(cx, qpath, dest_fun.hir_id)), - let Some(dest_name) = if match_def_path(cx.tcx, dest_fun_id, &["std", "io", "stdio", "stdout"]) { + if write_args.len() > 0; + if let ExprCall(ref dest_fun, _) = write_args[0].node; + if let ExprPath(ref qpath) = dest_fun.node; + if let Some(dest_fun_id) = + opt_def_id(resolve_node(cx, qpath, dest_fun.hir_id)); + if let Some(dest_name) = if match_def_path(cx.tcx, dest_fun_id, &["std", "io", "stdio", "stdout"]) { Some("stdout") } else if match_def_path(cx.tcx, dest_fun_id, &["std", "io", "stdio", "stderr"]) { Some("stderr") } else { None - }, - ], { - let write_span = unwrap_args[0].span; - let calling_macro = - // ordering is important here, since `writeln!` uses `write!` internally - if is_expn_of(write_span, "writeln").is_some() { - Some("writeln") - } else if is_expn_of(write_span, "write").is_some() { - Some("write") - } else { - None - }; - let prefix = if dest_name == "stderr" { - "e" - } else { - "" }; - if let Some(macro_name) = calling_macro { - span_lint( - cx, - EXPLICIT_WRITE, - expr.span, - &format!( - "use of `{}!({}(), ...).unwrap()`. Consider using `{}{}!` instead", - macro_name, - dest_name, - prefix, - macro_name.replace("write", "print") - ) - ); - } else { - span_lint( - cx, - EXPLICIT_WRITE, - expr.span, - &format!( - "use of `{}().write_fmt(...).unwrap()`. Consider using `{}print!` instead", - dest_name, - prefix, - ) - ); + then { + let write_span = unwrap_args[0].span; + let calling_macro = + // ordering is important here, since `writeln!` uses `write!` internally + if is_expn_of(write_span, "writeln").is_some() { + Some("writeln") + } else if is_expn_of(write_span, "write").is_some() { + Some("write") + } else { + None + }; + let prefix = if dest_name == "stderr" { + "e" + } else { + "" + }; + if let Some(macro_name) = calling_macro { + span_lint( + cx, + EXPLICIT_WRITE, + expr.span, + &format!( + "use of `{}!({}(), ...).unwrap()`. Consider using `{}{}!` instead", + macro_name, + dest_name, + prefix, + macro_name.replace("write", "print") + ) + ); + } else { + span_lint( + cx, + EXPLICIT_WRITE, + expr.span, + &format!( + "use of `{}().write_fmt(...).unwrap()`. Consider using `{}print!` instead", + dest_name, + prefix, + ) + ); + } } - }} + } } } diff --git a/clippy_lints/src/fallible_impl_from.rs b/clippy_lints/src/fallible_impl_from.rs index bdcda99124c6..e6efd41e6fb9 100644 --- a/clippy_lints/src/fallible_impl_from.rs +++ b/clippy_lints/src/fallible_impl_from.rs @@ -37,13 +37,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FallibleImplFrom { fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item) { // check for `impl From for ..` let impl_def_id = cx.tcx.hir.local_def_id(item.id); - if_let_chain!{[ - let hir::ItemImpl(.., ref impl_items) = item.node, - let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(impl_def_id), - match_def_path(cx.tcx, impl_trait_ref.def_id, &FROM_TRAIT), - ], { - lint_impl_body(cx, item.span, impl_items); - }} + if_chain! { + if let hir::ItemImpl(.., ref impl_items) = item.node; + if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(impl_def_id); + if match_def_path(cx.tcx, impl_trait_ref.def_id, &FROM_TRAIT); + then { + lint_impl_body(cx, item.span, impl_items); + } + } } } @@ -60,14 +61,15 @@ fn lint_impl_body<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, impl_span: Span, impl_it impl<'a, 'tcx: 'a> Visitor<'tcx> for FindPanicUnwrap<'a, 'tcx> { fn visit_expr(&mut self, expr: &'tcx Expr) { // check for `begin_panic` - if_let_chain!{[ - let ExprCall(ref func_expr, _) = expr.node, - let ExprPath(QPath::Resolved(_, ref path)) = func_expr.node, - match_def_path(self.tcx, path.def.def_id(), &BEGIN_PANIC) || - match_def_path(self.tcx, path.def.def_id(), &BEGIN_PANIC_FMT), - ], { - self.result.push(expr.span); - }} + if_chain! { + if let ExprCall(ref func_expr, _) = expr.node; + if let ExprPath(QPath::Resolved(_, ref path)) = func_expr.node; + if match_def_path(self.tcx, path.def.def_id(), &BEGIN_PANIC) || + match_def_path(self.tcx, path.def.def_id(), &BEGIN_PANIC_FMT); + then { + self.result.push(expr.span); + } + } // check for `unwrap` if let Some(arglists) = method_chain_args(expr, &["unwrap"]) { @@ -89,36 +91,37 @@ fn lint_impl_body<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, impl_span: Span, impl_it } for impl_item in impl_items { - if_let_chain!{[ - impl_item.name == "from", - let ImplItemKind::Method(_, body_id) = - cx.tcx.hir.impl_item(impl_item.id).node, - ], { - // check the body for `begin_panic` or `unwrap` - let body = cx.tcx.hir.body(body_id); - let impl_item_def_id = cx.tcx.hir.local_def_id(impl_item.id.node_id); - let mut fpu = FindPanicUnwrap { - tcx: cx.tcx, - tables: cx.tcx.typeck_tables_of(impl_item_def_id), - result: Vec::new(), - }; - fpu.visit_expr(&body.value); - - // if we've found one, lint - if !fpu.result.is_empty() { - span_lint_and_then( - cx, - FALLIBLE_IMPL_FROM, - impl_span, - "consider implementing `TryFrom` instead", - move |db| { - db.help( - "`From` is intended for infallible conversions only. \ - Use `TryFrom` if there's a possibility for the conversion to fail."); - db.span_note(fpu.result, "potential failure(s)"); - }); + if_chain! { + if impl_item.name == "from"; + if let ImplItemKind::Method(_, body_id) = + cx.tcx.hir.impl_item(impl_item.id).node; + then { + // check the body for `begin_panic` or `unwrap` + let body = cx.tcx.hir.body(body_id); + let impl_item_def_id = cx.tcx.hir.local_def_id(impl_item.id.node_id); + let mut fpu = FindPanicUnwrap { + tcx: cx.tcx, + tables: cx.tcx.typeck_tables_of(impl_item_def_id), + result: Vec::new(), + }; + fpu.visit_expr(&body.value); + + // if we've found one, lint + if !fpu.result.is_empty() { + span_lint_and_then( + cx, + FALLIBLE_IMPL_FROM, + impl_span, + "consider implementing `TryFrom` instead", + move |db| { + db.help( + "`From` is intended for infallible conversions only. \ + Use `TryFrom` if there's a possibility for the conversion to fail."); + db.span_note(fpu.result, "potential failure(s)"); + }); + } } - }} + } } } diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index 6a6cbadb6fa3..8004dc170835 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -42,19 +42,20 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { match expr.node { // `format!("{}", foo)` expansion ExprCall(ref fun, ref args) => { - if_let_chain!{[ - let ExprPath(ref qpath) = fun.node, - args.len() == 2, - let Some(fun_def_id) = opt_def_id(resolve_node(cx, qpath, fun.hir_id)), - match_def_path(cx.tcx, fun_def_id, &paths::FMT_ARGUMENTS_NEWV1), + if_chain! { + if let ExprPath(ref qpath) = fun.node; + if args.len() == 2; + if let Some(fun_def_id) = opt_def_id(resolve_node(cx, qpath, fun.hir_id)); + if match_def_path(cx.tcx, fun_def_id, &paths::FMT_ARGUMENTS_NEWV1); // ensure the format string is `"{..}"` with only one argument and no text - check_static_str(&args[0]), + if check_static_str(&args[0]); // ensure the format argument is `{}` ie. Display with no fancy option // and that the argument is a string - check_arg_is_display(cx, &args[1]) - ], { - span_lint(cx, USELESS_FORMAT, span, "useless use of `format!`"); - }} + if check_arg_is_display(cx, &args[1]); + then { + span_lint(cx, USELESS_FORMAT, span, "useless use of `format!`"); + } + } }, // `format!("foo")` expansion contains `match () { () => [], }` ExprMatch(ref matchee, _, _) => if let ExprTup(ref tup) = matchee.node { @@ -70,15 +71,16 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { /// Checks if the expressions matches `&[""]` fn check_static_str(expr: &Expr) -> bool { - if_let_chain! {[ - let ExprAddrOf(_, ref expr) = expr.node, // &[""] - let ExprArray(ref exprs) = expr.node, // [""] - exprs.len() == 1, - let ExprLit(ref lit) = exprs[0].node, - let LitKind::Str(ref lit, _) = lit.node, - ], { - return lit.as_str().is_empty(); - }} + if_chain! { + if let ExprAddrOf(_, ref expr) = expr.node; // &[""] + if let ExprArray(ref exprs) = expr.node; // [""] + if exprs.len() == 1; + if let ExprLit(ref lit) = exprs[0].node; + if let LitKind::Str(ref lit, _) = lit.node; + then { + return lit.as_str().is_empty(); + } + } false } @@ -91,25 +93,26 @@ fn check_static_str(expr: &Expr) -> bool { /// } /// ``` fn check_arg_is_display(cx: &LateContext, expr: &Expr) -> bool { - if_let_chain! {[ - let ExprAddrOf(_, ref expr) = expr.node, - let ExprMatch(_, ref arms, _) = expr.node, - arms.len() == 1, - arms[0].pats.len() == 1, - let PatKind::Tuple(ref pat, None) = arms[0].pats[0].node, - pat.len() == 1, - let ExprArray(ref exprs) = arms[0].body.node, - exprs.len() == 1, - let ExprCall(_, ref args) = exprs[0].node, - args.len() == 2, - let ExprPath(ref qpath) = args[1].node, - let Some(fun_def_id) = opt_def_id(resolve_node(cx, qpath, args[1].hir_id)), - match_def_path(cx.tcx, fun_def_id, &paths::DISPLAY_FMT_METHOD), - ], { - let ty = walk_ptrs_ty(cx.tables.pat_ty(&pat[0])); - - return ty.sty == ty::TyStr || match_type(cx, ty, &paths::STRING); - }} + if_chain! { + if let ExprAddrOf(_, ref expr) = expr.node; + if let ExprMatch(_, ref arms, _) = expr.node; + if arms.len() == 1; + if arms[0].pats.len() == 1; + if let PatKind::Tuple(ref pat, None) = arms[0].pats[0].node; + if pat.len() == 1; + if let ExprArray(ref exprs) = arms[0].body.node; + if exprs.len() == 1; + if let ExprCall(_, ref args) = exprs[0].node; + if args.len() == 2; + if let ExprPath(ref qpath) = args[1].node; + if let Some(fun_def_id) = opt_def_id(resolve_node(cx, qpath, args[1].hir_id)); + if match_def_path(cx.tcx, fun_def_id, &paths::DISPLAY_FMT_METHOD); + then { + let ty = walk_ptrs_ty(cx.tables.pat_ty(&pat[0])); + + return ty.sty == ty::TyStr || match_type(cx, ty, &paths::STRING); + } + } false } diff --git a/clippy_lints/src/invalid_ref.rs b/clippy_lints/src/invalid_ref.rs index ad3398cb0782..649e1f7ac78b 100644 --- a/clippy_lints/src/invalid_ref.rs +++ b/clippy_lints/src/invalid_ref.rs @@ -34,22 +34,23 @@ impl LintPass for InvalidRef { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidRef { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { - if_let_chain!{[ - let ExprCall(ref path, ref args) = expr.node, - let ExprPath(ref qpath) = path.node, - args.len() == 0, - let ty::TyRef(..) = cx.tables.expr_ty(expr).sty, - let Some(def_id) = opt_def_id(cx.tables.qpath_def(qpath, path.hir_id)), - ], { - let msg = if match_def_path(cx.tcx, def_id, &paths::MEM_ZEROED) | match_def_path(cx.tcx, def_id, &paths::INIT) { - ZERO_REF_SUMMARY - } else if match_def_path(cx.tcx, def_id, &paths::MEM_UNINIT) | match_def_path(cx.tcx, def_id, &paths::UNINIT) { - UNINIT_REF_SUMMARY - } else { - return; - }; - span_help_and_lint(cx, INVALID_REF, expr.span, msg, HELP); - }} + if_chain! { + if let ExprCall(ref path, ref args) = expr.node; + if let ExprPath(ref qpath) = path.node; + if args.len() == 0; + if let ty::TyRef(..) = cx.tables.expr_ty(expr).sty; + if let Some(def_id) = opt_def_id(cx.tables.qpath_def(qpath, path.hir_id)); + then { + let msg = if match_def_path(cx.tcx, def_id, &paths::MEM_ZEROED) | match_def_path(cx.tcx, def_id, &paths::INIT) { + ZERO_REF_SUMMARY + } else if match_def_path(cx.tcx, def_id, &paths::MEM_UNINIT) | match_def_path(cx.tcx, def_id, &paths::UNINIT) { + UNINIT_REF_SUMMARY + } else { + return; + }; + span_help_and_lint(cx, INVALID_REF, expr.span, msg, HELP); + } + } return; } } diff --git a/clippy_lints/src/let_if_seq.rs b/clippy_lints/src/let_if_seq.rs index 789dee6b05d2..931b872e036a 100644 --- a/clippy_lints/src/let_if_seq.rs +++ b/clippy_lints/src/let_if_seq.rs @@ -63,69 +63,70 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetIfSeq { fn check_block(&mut self, cx: &LateContext<'a, 'tcx>, block: &'tcx hir::Block) { let mut it = block.stmts.iter().peekable(); while let Some(stmt) = it.next() { - if_let_chain! {[ - let Some(expr) = it.peek(), - let hir::StmtDecl(ref decl, _) = stmt.node, - let hir::DeclLocal(ref decl) = decl.node, - let hir::PatKind::Binding(mode, canonical_id, ref name, None) = decl.pat.node, - let hir::StmtExpr(ref if_, _) = expr.node, - let hir::ExprIf(ref cond, ref then, ref else_) = if_.node, - !used_in_expr(cx, canonical_id, cond), - let hir::ExprBlock(ref then) = then.node, - let Some(value) = check_assign(cx, canonical_id, &*then), - !used_in_expr(cx, canonical_id, value), - ], { - let span = stmt.span.to(if_.span); - - let (default_multi_stmts, default) = if let Some(ref else_) = *else_ { - if let hir::ExprBlock(ref else_) = else_.node { - if let Some(default) = check_assign(cx, canonical_id, else_) { - (else_.stmts.len() > 1, default) - } else if let Some(ref default) = decl.init { - (true, &**default) + if_chain! { + if let Some(expr) = it.peek(); + if let hir::StmtDecl(ref decl, _) = stmt.node; + if let hir::DeclLocal(ref decl) = decl.node; + if let hir::PatKind::Binding(mode, canonical_id, ref name, None) = decl.pat.node; + if let hir::StmtExpr(ref if_, _) = expr.node; + if let hir::ExprIf(ref cond, ref then, ref else_) = if_.node; + if !used_in_expr(cx, canonical_id, cond); + if let hir::ExprBlock(ref then) = then.node; + if let Some(value) = check_assign(cx, canonical_id, &*then); + if !used_in_expr(cx, canonical_id, value); + then { + let span = stmt.span.to(if_.span); + + let (default_multi_stmts, default) = if let Some(ref else_) = *else_ { + if let hir::ExprBlock(ref else_) = else_.node { + if let Some(default) = check_assign(cx, canonical_id, else_) { + (else_.stmts.len() > 1, default) + } else if let Some(ref default) = decl.init { + (true, &**default) + } else { + continue; + } } else { continue; } + } else if let Some(ref default) = decl.init { + (false, &**default) } else { continue; - } - } else if let Some(ref default) = decl.init { - (false, &**default) - } else { - continue; - }; - - let mutability = match mode { - BindingAnnotation::RefMut | BindingAnnotation::Mutable => " ", - _ => "", - }; - - // FIXME: this should not suggest `mut` if we can detect that the variable is not - // use mutably after the `if` - - let sug = format!( - "let {mut}{name} = if {cond} {{{then} {value} }} else {{{else} {default} }};", - mut=mutability, - name=name.node, - cond=snippet(cx, cond.span, "_"), - then=if then.stmts.len() > 1 { " ..;" } else { "" }, - else=if default_multi_stmts { " ..;" } else { "" }, - value=snippet(cx, value.span, ""), - default=snippet(cx, default.span, ""), - ); - span_lint_and_then(cx, - USELESS_LET_IF_SEQ, - span, - "`if _ { .. } else { .. }` is an expression", - |db| { - db.span_suggestion(span, - "it is more idiomatic to write", - sug); - if !mutability.is_empty() { - db.note("you might not need `mut` at all"); - } - }); - }} + }; + + let mutability = match mode { + BindingAnnotation::RefMut | BindingAnnotation::Mutable => " ", + _ => "", + }; + + // FIXME: this should not suggest `mut` if we can detect that the variable is not + // use mutably after the `if` + + let sug = format!( + "let {mut}{name} = if {cond} {{{then} {value} }} else {{{else} {default} }};", + mut=mutability, + name=name.node, + cond=snippet(cx, cond.span, "_"), + then=if then.stmts.len() > 1 { " ..;" } else { "" }, + else=if default_multi_stmts { " ..;" } else { "" }, + value=snippet(cx, value.span, ""), + default=snippet(cx, default.span, ""), + ); + span_lint_and_then(cx, + USELESS_LET_IF_SEQ, + span, + "`if _ { .. } else { .. }` is an expression", + |db| { + db.span_suggestion(span, + "it is more idiomatic to write", + sug); + if !mutability.is_empty() { + db.note("you might not need `mut` at all"); + } + }); + } + } } } } @@ -138,14 +139,15 @@ struct UsedVisitor<'a, 'tcx: 'a> { impl<'a, 'tcx> hir::intravisit::Visitor<'tcx> for UsedVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'tcx hir::Expr) { - if_let_chain! {[ - let hir::ExprPath(ref qpath) = expr.node, - let Def::Local(local_id) = self.cx.tables.qpath_def(qpath, expr.hir_id), - self.id == local_id, - ], { - self.used = true; - return; - }} + if_chain! { + if let hir::ExprPath(ref qpath) = expr.node; + if let Def::Local(local_id) = self.cx.tables.qpath_def(qpath, expr.hir_id); + if self.id == local_id; + then { + self.used = true; + return; + } + } hir::intravisit::walk_expr(self, expr); } fn nested_visit_map<'this>(&'this mut self) -> hir::intravisit::NestedVisitorMap<'this, 'tcx> { @@ -158,31 +160,32 @@ fn check_assign<'a, 'tcx>( decl: ast::NodeId, block: &'tcx hir::Block, ) -> Option<&'tcx hir::Expr> { - if_let_chain! {[ - block.expr.is_none(), - let Some(expr) = block.stmts.iter().last(), - let hir::StmtSemi(ref expr, _) = expr.node, - let hir::ExprAssign(ref var, ref value) = expr.node, - let hir::ExprPath(ref qpath) = var.node, - let Def::Local(local_id) = cx.tables.qpath_def(qpath, var.hir_id), - decl == local_id, - ], { - let mut v = UsedVisitor { - cx: cx, - id: decl, - used: false, - }; - - for s in block.stmts.iter().take(block.stmts.len()-1) { - hir::intravisit::walk_stmt(&mut v, s); - - if v.used { - return None; + if_chain! { + if block.expr.is_none(); + if let Some(expr) = block.stmts.iter().last(); + if let hir::StmtSemi(ref expr, _) = expr.node; + if let hir::ExprAssign(ref var, ref value) = expr.node; + if let hir::ExprPath(ref qpath) = var.node; + if let Def::Local(local_id) = cx.tables.qpath_def(qpath, var.hir_id); + if decl == local_id; + then { + let mut v = UsedVisitor { + cx: cx, + id: decl, + used: false, + }; + + for s in block.stmts.iter().take(block.stmts.len()-1) { + hir::intravisit::walk_stmt(&mut v, s); + + if v.used { + return None; + } } + + return Some(value); } - - return Some(value); - }} + } None } diff --git a/clippy_lints/src/literal_digit_grouping.rs b/clippy_lints/src/literal_digit_grouping.rs index b656fed1cfbc..91e4c5674885 100644 --- a/clippy_lints/src/literal_digit_grouping.rs +++ b/clippy_lints/src/literal_digit_grouping.rs @@ -244,58 +244,60 @@ impl EarlyLintPass for LiteralDigitGrouping { impl LiteralDigitGrouping { fn check_lit(&self, cx: &EarlyContext, lit: &Lit) { // Lint integral literals. - if_let_chain! {[ - let LitKind::Int(..) = lit.node, - let Some(src) = snippet_opt(cx, lit.span), - let Some(firstch) = src.chars().next(), - char::to_digit(firstch, 10).is_some() - ], { - let digit_info = DigitInfo::new(&src, false); - let _ = Self::do_lint(digit_info.digits).map_err(|warning_type| { - warning_type.display(&digit_info.grouping_hint(), cx, &lit.span) - }); - }} + if_chain! { + if let LitKind::Int(..) = lit.node; + if let Some(src) = snippet_opt(cx, lit.span); + if let Some(firstch) = src.chars().next(); + if char::to_digit(firstch, 10).is_some(); + then { + let digit_info = DigitInfo::new(&src, false); + let _ = Self::do_lint(digit_info.digits).map_err(|warning_type| { + warning_type.display(&digit_info.grouping_hint(), cx, &lit.span) + }); + } + } // Lint floating-point literals. - if_let_chain! {[ - let LitKind::Float(..) = lit.node, - let Some(src) = snippet_opt(cx, lit.span), - let Some(firstch) = src.chars().next(), - char::to_digit(firstch, 10).is_some() - ], { - let digit_info = DigitInfo::new(&src, true); - // Separate digits into integral and fractional parts. - let parts: Vec<&str> = digit_info - .digits - .split_terminator('.') - .collect(); - - // Lint integral and fractional parts separately, and then check consistency of digit - // groups if both pass. - let _ = Self::do_lint(parts[0]) - .map(|integral_group_size| { - if parts.len() > 1 { - // Lint the fractional part of literal just like integral part, but reversed. - let fractional_part = &parts[1].chars().rev().collect::(); - let _ = Self::do_lint(fractional_part) - .map(|fractional_group_size| { - let consistent = Self::parts_consistent(integral_group_size, - fractional_group_size, - parts[0].len(), - parts[1].len()); - if !consistent { - WarningType::InconsistentDigitGrouping.display(&digit_info.grouping_hint(), - cx, - &lit.span); - } - }) - .map_err(|warning_type| warning_type.display(&digit_info.grouping_hint(), - cx, - &lit.span)); - } - }) - .map_err(|warning_type| warning_type.display(&digit_info.grouping_hint(), cx, &lit.span)); - }} + if_chain! { + if let LitKind::Float(..) = lit.node; + if let Some(src) = snippet_opt(cx, lit.span); + if let Some(firstch) = src.chars().next(); + if char::to_digit(firstch, 10).is_some(); + then { + let digit_info = DigitInfo::new(&src, true); + // Separate digits into integral and fractional parts. + let parts: Vec<&str> = digit_info + .digits + .split_terminator('.') + .collect(); + + // Lint integral and fractional parts separately, and then check consistency of digit + // groups if both pass. + let _ = Self::do_lint(parts[0]) + .map(|integral_group_size| { + if parts.len() > 1 { + // Lint the fractional part of literal just like integral part, but reversed. + let fractional_part = &parts[1].chars().rev().collect::(); + let _ = Self::do_lint(fractional_part) + .map(|fractional_group_size| { + let consistent = Self::parts_consistent(integral_group_size, + fractional_group_size, + parts[0].len(), + parts[1].len()); + if !consistent { + WarningType::InconsistentDigitGrouping.display(&digit_info.grouping_hint(), + cx, + &lit.span); + } + }) + .map_err(|warning_type| warning_type.display(&digit_info.grouping_hint(), + cx, + &lit.span)); + } + }) + .map_err(|warning_type| warning_type.display(&digit_info.grouping_hint(), cx, &lit.span)); + } + } } /// Given the sizes of the digit groups of both integral and fractional diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index cea0cfb7028f..0c3fd915399f 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -611,16 +611,17 @@ fn check_for_loop<'a, 'tcx>( } fn same_var<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &Expr, var: ast::NodeId) -> bool { - if_let_chain! {[ - let ExprPath(ref qpath) = expr.node, - let QPath::Resolved(None, ref path) = *qpath, - path.segments.len() == 1, - let Def::Local(local_id) = cx.tables.qpath_def(qpath, expr.hir_id), + if_chain! { + if let ExprPath(ref qpath) = expr.node; + if let QPath::Resolved(None, ref path) = *qpath; + if path.segments.len() == 1; + if let Def::Local(local_id) = cx.tables.qpath_def(qpath, expr.hir_id); // our variable! - local_id == var - ], { - return true; - }} + if local_id == var; + then { + return true; + } + } false } @@ -725,14 +726,15 @@ fn fetch_cloned_fixed_offset_var<'a, 'tcx>( expr: &Expr, var: ast::NodeId, ) -> Option { - if_let_chain! {[ - let ExprMethodCall(ref method, _, ref args) = expr.node, - method.name == "clone", - args.len() == 1, - let Some(arg) = args.get(0), - ], { - return get_fixed_offset_var(cx, arg, var); - }} + if_chain! { + if let ExprMethodCall(ref method, _, ref args) = expr.node; + if method.name == "clone"; + if args.len() == 1; + if let Some(arg) = args.get(0); + then { + return get_fixed_offset_var(cx, arg, var); + } + } get_fixed_offset_var(cx, expr, var) } @@ -821,19 +823,20 @@ fn detect_manual_memcpy<'a, 'tcx>( }; let print_limit = |end: &Option<&Expr>, offset: Offset, var_name: &str| if let Some(end) = *end { - if_let_chain! {[ - let ExprMethodCall(ref method, _, ref len_args) = end.node, - method.name == "len", - len_args.len() == 1, - let Some(arg) = len_args.get(0), - snippet(cx, arg.span, "??") == var_name, - ], { - return if offset.negate { - format!("({} - {})", snippet(cx, end.span, ".len()"), offset.value) - } else { - "".to_owned() - }; - }} + if_chain! { + if let ExprMethodCall(ref method, _, ref len_args) = end.node; + if method.name == "len"; + if len_args.len() == 1; + if let Some(arg) = len_args.get(0); + if snippet(cx, arg.span, "??") == var_name; + then { + return if offset.negate { + format!("({} - {})", snippet(cx, end.span, ".len()"), offset.value) + } else { + "".to_owned() + }; + } + } let end_str = match limits { ast::RangeLimits::Closed => { @@ -1003,16 +1006,17 @@ fn check_for_loop_range<'a, 'tcx>( } fn is_len_call(expr: &Expr, var: &Name) -> bool { - if_let_chain! {[ - let ExprMethodCall(ref method, _, ref len_args) = expr.node, - len_args.len() == 1, - method.name == "len", - let ExprPath(QPath::Resolved(_, ref path)) = len_args[0].node, - path.segments.len() == 1, - path.segments[0].name == *var - ], { - return true; - }} + if_chain! { + if let ExprMethodCall(ref method, _, ref len_args) = expr.node; + if len_args.len() == 1; + if method.name == "len"; + if let ExprPath(QPath::Resolved(_, ref path)) = len_args[0].node; + if path.segments.len() == 1; + if path.segments[0].name == *var; + then { + return true; + } + } false } @@ -1374,22 +1378,24 @@ fn mut_warn_with_span(cx: &LateContext, span: Option) { } fn check_for_mutability(cx: &LateContext, bound: &Expr) -> Option { - if_let_chain! {[ - let ExprPath(ref qpath) = bound.node, - let QPath::Resolved(None, _) = *qpath, - ], { - let def = cx.tables.qpath_def(qpath, bound.hir_id); - if let Def::Local(node_id) = def { - let node_str = cx.tcx.hir.get(node_id); - if_let_chain! {[ - let map::Node::NodeBinding(pat) = node_str, - let PatKind::Binding(bind_ann, _, _, _) = pat.node, - let BindingAnnotation::Mutable = bind_ann, - ], { - return Some(node_id); - }} + if_chain! { + if let ExprPath(ref qpath) = bound.node; + if let QPath::Resolved(None, _) = *qpath; + then { + let def = cx.tables.qpath_def(qpath, bound.hir_id); + if let Def::Local(node_id) = def { + let node_str = cx.tcx.hir.get(node_id); + if_chain! { + if let map::Node::NodeBinding(pat) = node_str; + if let PatKind::Binding(bind_ann, _, _, _) = pat.node; + if let BindingAnnotation::Mutable = bind_ann; + then { + return Some(node_id); + } + } + } } - }} + } None } @@ -1476,67 +1482,69 @@ struct VarVisitor<'a, 'tcx: 'a> { impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'tcx Expr) { - if_let_chain! {[ + if_chain! { // an index op - let ExprIndex(ref seqexpr, ref idx) = expr.node, + if let ExprIndex(ref seqexpr, ref idx) = expr.node; // the indexed container is referenced by a name - let ExprPath(ref seqpath) = seqexpr.node, - let QPath::Resolved(None, ref seqvar) = *seqpath, - seqvar.segments.len() == 1, - ], { - let index_used_directly = same_var(self.cx, idx, self.var); - let index_used = index_used_directly || { - let mut used_visitor = LocalUsedVisitor { - cx: self.cx, - local: self.var, - used: false, + if let ExprPath(ref seqpath) = seqexpr.node; + if let QPath::Resolved(None, ref seqvar) = *seqpath; + if seqvar.segments.len() == 1; + then { + let index_used_directly = same_var(self.cx, idx, self.var); + let index_used = index_used_directly || { + let mut used_visitor = LocalUsedVisitor { + cx: self.cx, + local: self.var, + used: false, + }; + walk_expr(&mut used_visitor, idx); + used_visitor.used }; - walk_expr(&mut used_visitor, idx); - used_visitor.used - }; - - if index_used { - let def = self.cx.tables.qpath_def(seqpath, seqexpr.hir_id); - match def { - Def::Local(node_id) | Def::Upvar(node_id, ..) => { - let hir_id = self.cx.tcx.hir.node_to_hir_id(node_id); - - let parent_id = self.cx.tcx.hir.get_parent(expr.id); - let parent_def_id = self.cx.tcx.hir.local_def_id(parent_id); - let extent = self.cx.tcx.region_scope_tree(parent_def_id).var_scope(hir_id.local_id); - self.indexed.insert(seqvar.segments[0].name, Some(extent)); - if index_used_directly { - self.indexed_directly.insert(seqvar.segments[0].name, Some(extent)); + + if index_used { + let def = self.cx.tables.qpath_def(seqpath, seqexpr.hir_id); + match def { + Def::Local(node_id) | Def::Upvar(node_id, ..) => { + let hir_id = self.cx.tcx.hir.node_to_hir_id(node_id); + + let parent_id = self.cx.tcx.hir.get_parent(expr.id); + let parent_def_id = self.cx.tcx.hir.local_def_id(parent_id); + let extent = self.cx.tcx.region_scope_tree(parent_def_id).var_scope(hir_id.local_id); + self.indexed.insert(seqvar.segments[0].name, Some(extent)); + if index_used_directly { + self.indexed_directly.insert(seqvar.segments[0].name, Some(extent)); + } + return; // no need to walk further *on the variable* } - return; // no need to walk further *on the variable* - } - Def::Static(..) | Def::Const(..) => { - self.indexed.insert(seqvar.segments[0].name, None); - if index_used_directly { - self.indexed_directly.insert(seqvar.segments[0].name, None); + Def::Static(..) | Def::Const(..) => { + self.indexed.insert(seqvar.segments[0].name, None); + if index_used_directly { + self.indexed_directly.insert(seqvar.segments[0].name, None); + } + return; // no need to walk further *on the variable* } - return; // no need to walk further *on the variable* + _ => (), } - _ => (), } } - }} + } - if_let_chain! {[ + if_chain! { // directly using a variable - let ExprPath(ref qpath) = expr.node, - let QPath::Resolved(None, ref path) = *qpath, - path.segments.len() == 1, - let Def::Local(local_id) = self.cx.tables.qpath_def(qpath, expr.hir_id), - ], { - if local_id == self.var { - // we are not indexing anything, record that - self.nonindex = true; - } else { - // not the correct variable, but still a variable - self.referenced.insert(path.segments[0].name); + if let ExprPath(ref qpath) = expr.node; + if let QPath::Resolved(None, ref path) = *qpath; + if path.segments.len() == 1; + if let Def::Local(local_id) = self.cx.tables.qpath_def(qpath, expr.hir_id); + then { + if local_id == self.var { + // we are not indexing anything, record that + self.nonindex = true; + } else { + // not the correct variable, but still a variable + self.referenced.insert(path.segments[0].name); + } } - }} + } walk_expr(self, expr); } @@ -1845,12 +1853,13 @@ fn is_conditional(expr: &Expr) -> bool { } fn is_nested(cx: &LateContext, match_expr: &Expr, iter_expr: &Expr) -> bool { - if_let_chain! {[ - let Some(loop_block) = get_enclosing_block(cx, match_expr.id), - let Some(map::Node::NodeExpr(loop_expr)) = cx.tcx.hir.find(cx.tcx.hir.get_parent_node(loop_block.id)), - ], { - return is_loop_nested(cx, loop_expr, iter_expr) - }} + if_chain! { + if let Some(loop_block) = get_enclosing_block(cx, match_expr.id); + if let Some(map::Node::NodeExpr(loop_expr)) = cx.tcx.hir.find(cx.tcx.hir.get_parent_node(loop_block.id)); + then { + return is_loop_nested(cx, loop_expr, iter_expr) + } + } false } diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs index e35e1ab477c0..e126d5c07d70 100644 --- a/clippy_lints/src/map_clone.rs +++ b/clippy_lints/src/map_clone.rs @@ -35,22 +35,36 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { let body = cx.tcx.hir.body(closure_eid); let closure_expr = remove_blocks(&body.value); let ty = cx.tables.pat_ty(&body.arguments[0].pat); - if_let_chain! {[ + if_chain! { // nothing special in the argument, besides reference bindings // (e.g. .map(|&x| x) ) - let Some(first_arg) = iter_input_pats(decl, body).next(), - let Some(arg_ident) = get_arg_name(&first_arg.pat), + if let Some(first_arg) = iter_input_pats(decl, body).next(); + if let Some(arg_ident) = get_arg_name(&first_arg.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.tables.pat_ty(&first_arg.pat)).1 == 1 - { - // the argument is not an &mut T - if let ty::TyRef(_, tam) = ty.sty { - if tam.mutbl == MutImmutable { + if let Some(type_name) = get_type_name(cx, expr, &args[0]); + then { + // 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.tables.pat_ty(&first_arg.pat)).1 == 1 + { + // the argument is not an &mut T + if let ty::TyRef(_, tam) = ty.sty { + if tam.mutbl == MutImmutable { + 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(ref clone_call, _, ref clone_args) = closure_expr.node { + if clone_call.name == "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), @@ -58,20 +72,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { } } } - // explicit clone() calls ( .map(|x| x.clone()) ) - else if let ExprMethodCall(ref clone_call, _, ref clone_args) = closure_expr.node { - if clone_call.name == "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_qpath(path, &paths::CLONE) { let type_name = get_type_name(cx, expr, &args[0]).unwrap_or("_"); diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 78a85c4a6864..18ba34f86212 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -343,21 +343,22 @@ fn check_wild_err_arm(cx: &LateContext, ex: &Expr, arms: &[Arm]) { for arm in arms { if let PatKind::TupleStruct(ref path, ref inner, _) = arm.pats[0].node { let path_str = print::to_string(print::NO_ANN, |s| s.print_qpath(path, false)); - if_let_chain! {[ - path_str == "Err", - inner.iter().any(|pat| pat.node == PatKind::Wild), - let ExprBlock(ref block) = arm.body.node, - is_panic_block(block) - ], { - // `Err(_)` arm with `panic!` found - span_note_and_lint(cx, - MATCH_WILD_ERR_ARM, - arm.pats[0].span, - "Err(_) will match all errors, maybe not a good idea", - arm.pats[0].span, - "to remove this warning, match each error seperately \ - or use unreachable macro"); - }} + if_chain! { + if path_str == "Err"; + if inner.iter().any(|pat| pat.node == PatKind::Wild); + if let ExprBlock(ref block) = arm.body.node; + if is_panic_block(block); + then { + // `Err(_)` arm with `panic!` found + span_note_and_lint(cx, + MATCH_WILD_ERR_ARM, + arm.pats[0].span, + "Err(_) will match all errors, maybe not a good idea", + arm.pats[0].span, + "to remove this warning, match each error seperately \ + or use unreachable macro"); + } + } } } } @@ -428,24 +429,26 @@ fn all_ranges<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arms: &'tcx [Arm], id: NodeI } else { [].iter() }.filter_map(|pat| { - if_let_chain! {[ - let PatKind::Range(ref lhs, ref rhs, ref range_end) = pat.node, - let Ok(lhs) = constcx.eval(lhs), - let Ok(rhs) = constcx.eval(rhs) - ], { - let rhs = match *range_end { - RangeEnd::Included => Bound::Included(rhs), - RangeEnd::Excluded => Bound::Excluded(rhs), - }; - return Some(SpannedRange { span: pat.span, node: (lhs, rhs) }); - }} + if_chain! { + if let PatKind::Range(ref lhs, ref rhs, ref range_end) = pat.node; + if let Ok(lhs) = constcx.eval(lhs); + if let Ok(rhs) = constcx.eval(rhs); + then { + let rhs = match *range_end { + RangeEnd::Included => Bound::Included(rhs), + RangeEnd::Excluded => Bound::Excluded(rhs), + }; + return Some(SpannedRange { span: pat.span, node: (lhs, rhs) }); + } + } - if_let_chain! {[ - let PatKind::Lit(ref value) = pat.node, - let Ok(value) = constcx.eval(value) - ], { - return Some(SpannedRange { span: pat.span, node: (value, Bound::Included(value)) }); - }} + if_chain! { + if let PatKind::Lit(ref value) = pat.node; + if let Ok(value) = constcx.eval(value); + then { + return Some(SpannedRange { span: pat.span, node: (value, Bound::Included(value)) }); + } + } None }) diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index 4a0d1cf19cb4..8b4520a5e039 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -735,60 +735,62 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { let name = implitem.name; let parent = cx.tcx.hir.get_parent(implitem.id); let item = cx.tcx.hir.expect_item(parent); - if_let_chain! {[ - let hir::ImplItemKind::Method(ref sig, id) = implitem.node, - let Some(first_arg_ty) = sig.decl.inputs.get(0), - let Some(first_arg) = iter_input_pats(&sig.decl, cx.tcx.hir.body(id)).next(), - let hir::ItemImpl(_, _, _, _, None, ref self_ty, _) = item.node, - ], { - // check missing trait implementations - for &(method_name, n_args, self_kind, out_type, trait_name) in &TRAIT_METHODS { - if name == method_name && - sig.decl.inputs.len() == n_args && - out_type.matches(&sig.decl.output) && - self_kind.matches(first_arg_ty, first_arg, self_ty, false, &sig.generics) { - span_lint(cx, SHOULD_IMPLEMENT_TRAIT, implitem.span, &format!( - "defining a method called `{}` on this type; consider implementing \ - the `{}` trait or choosing a less ambiguous name", name, trait_name)); + if_chain! { + if let hir::ImplItemKind::Method(ref sig, id) = implitem.node; + if let Some(first_arg_ty) = sig.decl.inputs.get(0); + if let Some(first_arg) = iter_input_pats(&sig.decl, cx.tcx.hir.body(id)).next(); + if let hir::ItemImpl(_, _, _, _, None, ref self_ty, _) = item.node; + then { + // check missing trait implementations + for &(method_name, n_args, self_kind, out_type, trait_name) in &TRAIT_METHODS { + if name == method_name && + sig.decl.inputs.len() == n_args && + out_type.matches(&sig.decl.output) && + self_kind.matches(first_arg_ty, first_arg, self_ty, false, &sig.generics) { + span_lint(cx, SHOULD_IMPLEMENT_TRAIT, implitem.span, &format!( + "defining a method called `{}` on this type; consider implementing \ + the `{}` trait or choosing a less ambiguous name", name, trait_name)); + } + } + + // check conventions w.r.t. conversion method names and predicates + let def_id = cx.tcx.hir.local_def_id(item.id); + let ty = cx.tcx.type_of(def_id); + let is_copy = is_copy(cx, ty); + for &(ref conv, self_kinds) in &CONVENTIONS { + if_chain! { + if conv.check(&name.as_str()); + if !self_kinds.iter().any(|k| k.matches(first_arg_ty, first_arg, self_ty, is_copy, &sig.generics)); + then { + let lint = if item.vis == hir::Visibility::Public { + WRONG_PUB_SELF_CONVENTION + } else { + WRONG_SELF_CONVENTION + }; + span_lint(cx, + lint, + first_arg.pat.span, + &format!("methods called `{}` usually take {}; consider choosing a less \ + ambiguous name", + conv, + &self_kinds.iter() + .map(|k| k.description()) + .collect::>() + .join(" or "))); + } + } + } + + let ret_ty = return_ty(cx, implitem.id); + if name == "new" && + !ret_ty.walk().any(|t| same_tys(cx, t, ty)) { + span_lint(cx, + NEW_RET_NO_SELF, + implitem.span, + "methods called `new` usually return `Self`"); } } - - // check conventions w.r.t. conversion method names and predicates - let def_id = cx.tcx.hir.local_def_id(item.id); - let ty = cx.tcx.type_of(def_id); - let is_copy = is_copy(cx, ty); - for &(ref conv, self_kinds) in &CONVENTIONS { - if_let_chain! {[ - conv.check(&name.as_str()), - !self_kinds.iter().any(|k| k.matches(first_arg_ty, first_arg, self_ty, is_copy, &sig.generics)), - ], { - let lint = if item.vis == hir::Visibility::Public { - WRONG_PUB_SELF_CONVENTION - } else { - WRONG_SELF_CONVENTION - }; - span_lint(cx, - lint, - first_arg.pat.span, - &format!("methods called `{}` usually take {}; consider choosing a less \ - ambiguous name", - conv, - &self_kinds.iter() - .map(|k| k.description()) - .collect::>() - .join(" or "))); - }} - } - - let ret_ty = return_ty(cx, implitem.id); - if name == "new" && - !ret_ty.walk().any(|t| same_tys(cx, t, ty)) { - span_lint(cx, - NEW_RET_NO_SELF, - implitem.span, - "methods called `new` usually return `Self`"); - } - }} + } } } @@ -1014,20 +1016,21 @@ fn lint_extend(cx: &LateContext, expr: &hir::Expr, args: &[hir::Expr]) { } fn lint_cstring_as_ptr(cx: &LateContext, expr: &hir::Expr, new: &hir::Expr, unwrap: &hir::Expr) { - if_let_chain!{[ - let hir::ExprCall(ref fun, ref args) = new.node, - args.len() == 1, - let hir::ExprPath(ref path) = fun.node, - let Def::Method(did) = cx.tables.qpath_def(path, fun.hir_id), - match_def_path(cx.tcx, did, &paths::CSTRING_NEW) - ], { - span_lint_and_then(cx, TEMPORARY_CSTRING_AS_PTR, expr.span, - "you are getting the inner pointer of a temporary `CString`", - |db| { - db.note("that pointer will be invalid outside this expression"); - db.span_help(unwrap.span, "assign the `CString` to a variable to extend its lifetime"); - }); - }} + if_chain! { + if let hir::ExprCall(ref fun, ref args) = new.node; + if args.len() == 1; + if let hir::ExprPath(ref path) = fun.node; + if let Def::Method(did) = cx.tables.qpath_def(path, fun.hir_id); + if match_def_path(cx.tcx, did, &paths::CSTRING_NEW); + then { + span_lint_and_then(cx, TEMPORARY_CSTRING_AS_PTR, expr.span, + "you are getting the inner pointer of a temporary `CString`", + |db| { + db.note("that pointer will be invalid outside this expression"); + db.span_help(unwrap.span, "assign the `CString` to a variable to extend its lifetime"); + }); + } + } } fn lint_iter_cloned_collect(cx: &LateContext, expr: &hir::Expr, iter_args: &[hir::Expr]) { @@ -1427,33 +1430,34 @@ fn lint_binary_expr_with_method_call<'a, 'tcx: 'a>(cx: &LateContext<'a, 'tcx>, i /// Wrapper fn for `CHARS_NEXT_CMP` and `CHARS_NEXT_CMP` lints. fn lint_chars_cmp<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, info: &BinaryExprInfo, chain_methods: &[&str], lint: &'static Lint, suggest: &str) -> bool { - if_let_chain! {[ - let Some(args) = method_chain_args(info.chain, chain_methods), - let hir::ExprCall(ref fun, ref arg_char) = info.other.node, - arg_char.len() == 1, - let hir::ExprPath(ref qpath) = fun.node, - let Some(segment) = single_segment_path(qpath), - segment.name == "Some" - ], { - let self_ty = walk_ptrs_ty(cx.tables.expr_ty_adjusted(&args[0][0])); - - if self_ty.sty != ty::TyStr { - return false; + if_chain! { + if let Some(args) = method_chain_args(info.chain, chain_methods); + if let hir::ExprCall(ref fun, ref arg_char) = info.other.node; + if arg_char.len() == 1; + if let hir::ExprPath(ref qpath) = fun.node; + if let Some(segment) = single_segment_path(qpath); + if segment.name == "Some"; + then { + let self_ty = walk_ptrs_ty(cx.tables.expr_ty_adjusted(&args[0][0])); + + if self_ty.sty != ty::TyStr { + return false; + } + + span_lint_and_sugg(cx, + lint, + info.expr.span, + &format!("you should use the `{}` method", suggest), + "like this", + format!("{}{}.{}({})", + if info.eq { "" } else { "!" }, + snippet(cx, args[0][0].span, "_"), + suggest, + snippet(cx, arg_char[0].span, "_"))); + + return true; } - - span_lint_and_sugg(cx, - lint, - info.expr.span, - &format!("you should use the `{}` method", suggest), - "like this", - format!("{}{}.{}({})", - if info.eq { "" } else { "!" }, - snippet(cx, args[0][0].span, "_"), - suggest, - snippet(cx, arg_char[0].span, "_"))); - - return true; - }} + } false } @@ -1474,26 +1478,27 @@ fn lint_chars_last_cmp<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, info: &BinaryExprIn /// Wrapper fn for `CHARS_NEXT_CMP` and `CHARS_LAST_CMP` lints with `unwrap()`. fn lint_chars_cmp_with_unwrap<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, info: &BinaryExprInfo, chain_methods: &[&str], lint: &'static Lint, suggest: &str) -> bool { - if_let_chain! {[ - let Some(args) = method_chain_args(info.chain, chain_methods), - let hir::ExprLit(ref lit) = info.other.node, - let ast::LitKind::Char(c) = lit.node, - ], { - span_lint_and_sugg( - cx, - lint, - info.expr.span, - &format!("you should use the `{}` method", suggest), - "like this", - format!("{}{}.{}('{}')", - if info.eq { "" } else { "!" }, - snippet(cx, args[0][0].span, "_"), - suggest, - c) - ); - - return true; - }} + if_chain! { + if let Some(args) = method_chain_args(info.chain, chain_methods); + if let hir::ExprLit(ref lit) = info.other.node; + if let ast::LitKind::Char(c) = lit.node; + then { + span_lint_and_sugg( + cx, + lint, + info.expr.span, + &format!("you should use the `{}` method", suggest), + "like this", + format!("{}{}.{}('{}')", + if info.eq { "" } else { "!" }, + snippet(cx, args[0][0].span, "_"), + suggest, + c) + ); + + return true; + } + } false } diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 98fb90f57c1f..18d7f7230a82 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -251,55 +251,57 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { } fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, s: &'tcx Stmt) { - if_let_chain! {[ - let StmtDecl(ref d, _) = s.node, - let DeclLocal(ref l) = d.node, - let PatKind::Binding(an, _, i, None) = l.pat.node, - let Some(ref init) = l.init - ], { - if an == BindingAnnotation::Ref || an == BindingAnnotation::RefMut { - let init = Sugg::hir(cx, init, ".."); - let (mutopt,initref) = if an == BindingAnnotation::RefMut { - ("mut ", init.mut_addr()) - } else { - ("", init.addr()) - }; - let tyopt = if let Some(ref ty) = l.ty { - format!(": &{mutopt}{ty}", mutopt=mutopt, ty=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 {name}{tyopt} = {initref};", - name=snippet(cx, i.span, "_"), - tyopt=tyopt, - initref=initref)); - } - ); + if_chain! { + if let StmtDecl(ref d, _) = s.node; + if let DeclLocal(ref l) = d.node; + if let PatKind::Binding(an, _, i, None) = l.pat.node; + if let Some(ref init) = l.init; + then { + if an == BindingAnnotation::Ref || an == BindingAnnotation::RefMut { + let init = Sugg::hir(cx, init, ".."); + let (mutopt,initref) = if an == BindingAnnotation::RefMut { + ("mut ", init.mut_addr()) + } else { + ("", init.addr()) + }; + let tyopt = if let Some(ref ty) = l.ty { + format!(": &{mutopt}{ty}", mutopt=mutopt, ty=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 {name}{tyopt} = {initref};", + name=snippet(cx, i.span, "_"), + tyopt=tyopt, + initref=initref)); + } + ); + } } - }}; - if_let_chain! {[ - let StmtSemi(ref expr, _) = s.node, - let Expr_::ExprBinary(ref binop, ref a, ref b) = expr.node, - binop.node == BiAnd || binop.node == BiOr, - let Some(sugg) = Sugg::hir_opt(cx, a), - ], { - span_lint_and_then(cx, - SHORT_CIRCUIT_STATEMENT, - s.span, - "boolean short circuit operator in statement may be clearer using an explicit test", - |db| { - let sugg = if binop.node == BiOr { !sugg } else { sugg }; - db.span_suggestion(s.span, "replace it with", - format!("if {} {{ {}; }}", sugg, &snippet(cx, b.span, ".."))); - }); - }}; + }; + if_chain! { + if let StmtSemi(ref expr, _) = s.node; + if let Expr_::ExprBinary(ref binop, ref a, ref b) = expr.node; + if binop.node == BiAnd || binop.node == BiOr; + if let Some(sugg) = Sugg::hir_opt(cx, a); + then { + span_lint_and_then(cx, + SHORT_CIRCUIT_STATEMENT, + s.span, + "boolean short circuit operator in statement may be clearer using an explicit test", + |db| { + let sugg = if binop.node == BiOr { !sugg } else { sugg }; + db.span_suggestion(s.span, "replace it with", + format!("if {} {{ {}; }}", sugg, &snippet(cx, b.span, ".."))); + }); + } + }; } fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { @@ -582,17 +584,18 @@ fn non_macro_local(cx: &LateContext, def: &def::Def) -> bool { } fn check_cast(cx: &LateContext, span: Span, e: &Expr, ty: &Ty) { - if_let_chain! {[ - let TyPtr(MutTy { mutbl, .. }) = ty.node, - let ExprLit(ref lit) = e.node, - let LitKind::Int(value, ..) = lit.node, - value == 0, - !in_constant(cx, e.id) - ], { - let msg = match mutbl { - Mutability::MutMutable => "`0 as *mut _` detected. Consider using `ptr::null_mut()`", - Mutability::MutImmutable => "`0 as *const _` detected. Consider using `ptr::null()`", - }; - span_lint(cx, ZERO_PTR, span, msg); - }} + if_chain! { + if let TyPtr(MutTy { mutbl, .. }) = ty.node; + if let ExprLit(ref lit) = e.node; + if let LitKind::Int(value, ..) = lit.node; + if value == 0; + if !in_constant(cx, e.id); + then { + let msg = match mutbl { + Mutability::MutMutable => "`0 as *mut _` detected. Consider using `ptr::null_mut()`", + Mutability::MutImmutable => "`0 as *const _` detected. Consider using `ptr::null()`", + }; + span_lint(cx, ZERO_PTR, span, msg); + } + } } diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index bf4df6e28730..160ccd8e9b3a 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -322,100 +322,103 @@ impl EarlyLintPass for MiscEarly { fn check_block(&mut self, cx: &EarlyContext, block: &Block) { for w in block.stmts.windows(2) { - if_let_chain! {[ - let StmtKind::Local(ref local) = w[0].node, - let Option::Some(ref t) = local.init, - let ExprKind::Closure(_, _, _, _) = t.node, - let PatKind::Ident(_, sp_ident, _) = local.pat.node, - let StmtKind::Semi(ref second) = w[1].node, - let ExprKind::Assign(_, ref call) = second.node, - let ExprKind::Call(ref closure, _) = call.node, - let ExprKind::Path(_, ref path) = closure.node - ], { - if sp_ident.node == (&path.segments[0]).identifier { - span_lint( - cx, - REDUNDANT_CLOSURE_CALL, - second.span, - "Closure called just once immediately after it was declared", - ); + if_chain! { + if let StmtKind::Local(ref local) = w[0].node; + if let Option::Some(ref t) = local.init; + if let ExprKind::Closure(_, _, _, _) = t.node; + if let PatKind::Ident(_, sp_ident, _) = local.pat.node; + if let StmtKind::Semi(ref second) = w[1].node; + if let ExprKind::Assign(_, ref call) = second.node; + if let ExprKind::Call(ref closure, _) = call.node; + if let ExprKind::Path(_, ref path) = closure.node; + then { + if sp_ident.node == (&path.segments[0]).identifier { + span_lint( + cx, + REDUNDANT_CLOSURE_CALL, + second.span, + "Closure called just once immediately after it was declared", + ); + } } - }} + } } } } impl MiscEarly { fn check_lit(&self, cx: &EarlyContext, lit: &Lit) { - if_let_chain! {[ - let LitKind::Int(value, ..) = lit.node, - let Some(src) = snippet_opt(cx, lit.span), - let Some(firstch) = src.chars().next(), - char::to_digit(firstch, 10).is_some() - ], { - let mut prev = '\0'; - for ch in src.chars() { - if ch == 'i' || ch == 'u' { - if prev != '_' { - span_lint(cx, UNSEPARATED_LITERAL_SUFFIX, lit.span, - "integer type suffix should be separated by an underscore"); - } - break; - } - prev = ch; - } - if src.starts_with("0x") { - let mut seen = (false, false); + if_chain! { + if let LitKind::Int(value, ..) = lit.node; + if let Some(src) = snippet_opt(cx, lit.span); + if let Some(firstch) = src.chars().next(); + if char::to_digit(firstch, 10).is_some(); + then { + let mut prev = '\0'; for ch in src.chars() { - match ch { - 'a' ... 'f' => seen.0 = true, - 'A' ... 'F' => seen.1 = true, - 'i' | 'u' => break, // start of suffix already - _ => () + if ch == 'i' || ch == 'u' { + if prev != '_' { + span_lint(cx, UNSEPARATED_LITERAL_SUFFIX, lit.span, + "integer type suffix should be separated by an underscore"); + } + break; } + prev = ch; } - if seen.0 && seen.1 { - span_lint(cx, MIXED_CASE_HEX_LITERALS, lit.span, - "inconsistent casing in hexadecimal literal"); - } - } else if src.starts_with("0b") || src.starts_with("0o") { - /* nothing to do */ - } else if value != 0 && src.starts_with('0') { - span_lint_and_then(cx, - ZERO_PREFIXED_LITERAL, - lit.span, - "this is a decimal constant", - |db| { - db.span_suggestion( - lit.span, - "if you mean to use a decimal constant, remove the `0` to remove confusion", - src.trim_left_matches(|c| c == '_' || c == '0').to_string(), - ); - db.span_suggestion( - lit.span, - "if you mean to use an octal constant, use `0o`", - format!("0o{}", src.trim_left_matches(|c| c == '_' || c == '0')), - ); - }); - } - }} - if_let_chain! {[ - let LitKind::Float(..) = lit.node, - let Some(src) = snippet_opt(cx, lit.span), - let Some(firstch) = src.chars().next(), - char::to_digit(firstch, 10).is_some() - ], { - let mut prev = '\0'; - for ch in src.chars() { - if ch == 'f' { - if prev != '_' { - span_lint(cx, UNSEPARATED_LITERAL_SUFFIX, lit.span, - "float type suffix should be separated by an underscore"); + if src.starts_with("0x") { + let mut seen = (false, false); + for ch in src.chars() { + match ch { + 'a' ... 'f' => seen.0 = true, + 'A' ... 'F' => seen.1 = true, + 'i' | 'u' => break, // start of suffix already + _ => () + } } - break; + if seen.0 && seen.1 { + span_lint(cx, MIXED_CASE_HEX_LITERALS, lit.span, + "inconsistent casing in hexadecimal literal"); + } + } else if src.starts_with("0b") || src.starts_with("0o") { + /* nothing to do */ + } else if value != 0 && src.starts_with('0') { + span_lint_and_then(cx, + ZERO_PREFIXED_LITERAL, + lit.span, + "this is a decimal constant", + |db| { + db.span_suggestion( + lit.span, + "if you mean to use a decimal constant, remove the `0` to remove confusion", + src.trim_left_matches(|c| c == '_' || c == '0').to_string(), + ); + db.span_suggestion( + lit.span, + "if you mean to use an octal constant, use `0o`", + format!("0o{}", src.trim_left_matches(|c| c == '_' || c == '0')), + ); + }); } - prev = ch; } - }} + } + if_chain! { + if let LitKind::Float(..) = lit.node; + if let Some(src) = snippet_opt(cx, lit.span); + if let Some(firstch) = src.chars().next(); + if char::to_digit(firstch, 10).is_some(); + then { + let mut prev = '\0'; + for ch in src.chars() { + if ch == 'f' { + if prev != '_' { + span_lint(cx, UNSEPARATED_LITERAL_SUFFIX, lit.span, + "float type suffix should be separated by an underscore"); + } + break; + } + prev = ch; + } + } + } } } diff --git a/clippy_lints/src/needless_borrow.rs b/clippy_lints/src/needless_borrow.rs index d8f892d40735..be1fd1dc5256 100644 --- a/clippy_lints/src/needless_borrow.rs +++ b/clippy_lints/src/needless_borrow.rs @@ -75,25 +75,26 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBorrow { if in_macro(pat.span) { return; } - if_let_chain! {[ - let PatKind::Binding(BindingAnnotation::Ref, _, name, _) = pat.node, - let ty::TyRef(_, ref tam) = cx.tables.pat_ty(pat).sty, - tam.mutbl == MutImmutable, - let ty::TyRef(_, ref tam) = tam.ty.sty, + if_chain! { + if let PatKind::Binding(BindingAnnotation::Ref, _, name, _) = pat.node; + if let ty::TyRef(_, ref tam) = cx.tables.pat_ty(pat).sty; + if tam.mutbl == MutImmutable; + if let ty::TyRef(_, ref tam) = tam.ty.sty; // only lint immutable refs, because borrowed `&mut T` cannot be moved out - tam.mutbl == MutImmutable, - ], { - span_lint_and_then( - cx, - NEEDLESS_BORROW, - pat.span, - "this pattern creates a reference to a reference", - |db| { - if let Some(snippet) = snippet_opt(cx, name.span) { - db.span_suggestion(pat.span, "change this to", snippet); + if tam.mutbl == MutImmutable; + then { + span_lint_and_then( + cx, + NEEDLESS_BORROW, + pat.span, + "this pattern creates a reference to a reference", + |db| { + if let Some(snippet) = snippet_opt(cx, name.span) { + db.span_suggestion(pat.span, "change this to", snippet); + } } - } - ) - }} + ) + } + } } } diff --git a/clippy_lints/src/needless_borrowed_ref.rs b/clippy_lints/src/needless_borrowed_ref.rs index 1c00263cbc2d..c9a2e6b0935e 100644 --- a/clippy_lints/src/needless_borrowed_ref.rs +++ b/clippy_lints/src/needless_borrowed_ref.rs @@ -64,19 +64,20 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBorrowedRef { return; } - if_let_chain! {[ + if_chain! { // Only lint immutable refs, because `&mut ref T` may be useful. - let PatKind::Ref(ref sub_pat, MutImmutable) = pat.node, + if let PatKind::Ref(ref sub_pat, MutImmutable) = pat.node; // Check sub_pat got a `ref` keyword (excluding `ref mut`). - let PatKind::Binding(BindingAnnotation::Ref, _, spanned_name, ..) = sub_pat.node, - ], { - span_lint_and_then(cx, NEEDLESS_BORROWED_REFERENCE, pat.span, - "this pattern takes a reference on something that is being de-referenced", - |db| { - let hint = snippet(cx, spanned_name.span, "..").into_owned(); - db.span_suggestion(pat.span, "try removing the `&ref` part and just keep", hint); - }); - }} + if let PatKind::Binding(BindingAnnotation::Ref, _, spanned_name, ..) = sub_pat.node; + then { + span_lint_and_then(cx, NEEDLESS_BORROWED_REFERENCE, pat.span, + "this pattern takes a reference on something that is being de-referenced", + |db| { + let hint = snippet(cx, spanned_name.span, "..").into_owned(); + db.span_suggestion(pat.span, "try removing the `&ref` part and just keep", hint); + }); + } + } } } diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index e67dca5d00d2..e00938fb3ef1 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -64,13 +64,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue { match kind { FnKind::ItemFn(.., attrs) => for a in attrs { - if_let_chain!{[ - a.meta_item_list().is_some(), - let Some(name) = a.name(), - name == "proc_macro_derive", - ], { - return; - }} + if_chain! { + if a.meta_item_list().is_some(); + if let Some(name) = a.name(); + if name == "proc_macro_derive"; + then { + return; + } + } }, _ => return, } @@ -148,101 +149,103 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue { ) }; - if_let_chain! {[ - !is_self(arg), - !ty.is_mutable_pointer(), - !is_copy(cx, ty), - !fn_traits.iter().any(|&t| implements_trait(cx, ty, t, &[])), - !implements_borrow_trait, - !all_borrowable_trait, + if_chain! { + if !is_self(arg); + if !ty.is_mutable_pointer(); + if !is_copy(cx, ty); + if !fn_traits.iter().any(|&t| implements_trait(cx, ty, t, &[])); + if !implements_borrow_trait; + if !all_borrowable_trait; - let PatKind::Binding(mode, canonical_id, ..) = arg.pat.node, - !moved_vars.contains(&canonical_id), - ], { - if mode == BindingAnnotation::Mutable || mode == BindingAnnotation::RefMut { - continue; - } - - // Dereference suggestion - let sugg = |db: &mut DiagnosticBuilder| { - let deref_span = spans_need_deref.get(&canonical_id); - if_let_chain! {[ - match_type(cx, ty, &paths::VEC), - let Some(clone_spans) = - get_spans(cx, Some(body.id()), idx, &[("clone", ".to_owned()")]), - let TyPath(QPath::Resolved(_, ref path)) = input.node, - let Some(elem_ty) = path.segments.iter() - .find(|seg| seg.name == "Vec") - .and_then(|ps| ps.parameters.as_ref()) - .map(|params| ¶ms.types[0]), - ], { - let slice_ty = format!("&[{}]", snippet(cx, elem_ty.span, "_")); - db.span_suggestion(input.span, - "consider changing the type to", - slice_ty); - - for (span, suggestion) in clone_spans { - db.span_suggestion( - span, - &snippet_opt(cx, span) - .map_or( - "change the call to".into(), - |x| Cow::from(format!("change `{}` to", x)), - ), - suggestion.into() - ); - } - - // cannot be destructured, no need for `*` suggestion - assert!(deref_span.is_none()); - return; - }} - - if match_type(cx, ty, &paths::STRING) { - if let Some(clone_spans) = - get_spans(cx, Some(body.id()), idx, &[("clone", ".to_string()"), ("as_str", "")]) { - db.span_suggestion(input.span, "consider changing the type to", "&str".to_string()); - - for (span, suggestion) in clone_spans { - db.span_suggestion( - span, - &snippet_opt(cx, span) - .map_or( - "change the call to".into(), - |x| Cow::from(format!("change `{}` to", x)) - ), - suggestion.into(), - ); + if let PatKind::Binding(mode, canonical_id, ..) = arg.pat.node; + if !moved_vars.contains(&canonical_id); + then { + if mode == BindingAnnotation::Mutable || mode == BindingAnnotation::RefMut { + continue; + } + + // Dereference suggestion + let sugg = |db: &mut DiagnosticBuilder| { + let deref_span = spans_need_deref.get(&canonical_id); + if_chain! { + if match_type(cx, ty, &paths::VEC); + if let Some(clone_spans) = + get_spans(cx, Some(body.id()), idx, &[("clone", ".to_owned()")]); + if let TyPath(QPath::Resolved(_, ref path)) = input.node; + if let Some(elem_ty) = path.segments.iter() + .find(|seg| seg.name == "Vec") + .and_then(|ps| ps.parameters.as_ref()) + .map(|params| ¶ms.types[0]); + then { + let slice_ty = format!("&[{}]", snippet(cx, elem_ty.span, "_")); + db.span_suggestion(input.span, + "consider changing the type to", + slice_ty); + + for (span, suggestion) in clone_spans { + db.span_suggestion( + span, + &snippet_opt(cx, span) + .map_or( + "change the call to".into(), + |x| Cow::from(format!("change `{}` to", x)), + ), + suggestion.into() + ); + } + + // cannot be destructured, no need for `*` suggestion + assert!(deref_span.is_none()); + return; } - - assert!(deref_span.is_none()); - return; } - } - - let mut spans = vec![(input.span, format!("&{}", snippet(cx, input.span, "_")))]; - - // Suggests adding `*` to dereference the added reference. - if let Some(deref_span) = deref_span { - spans.extend( - deref_span - .iter() - .cloned() - .map(|span| (span, format!("*{}", snippet(cx, span, "")))), - ); - spans.sort_by_key(|&(span, _)| span); - } - multispan_sugg(db, "consider taking a reference instead".to_string(), spans); - }; - - span_lint_and_then( - cx, - NEEDLESS_PASS_BY_VALUE, - input.span, - "this argument is passed by value, but not consumed in the function body", - sugg, - ); - }} + + if match_type(cx, ty, &paths::STRING) { + if let Some(clone_spans) = + get_spans(cx, Some(body.id()), idx, &[("clone", ".to_string()"), ("as_str", "")]) { + db.span_suggestion(input.span, "consider changing the type to", "&str".to_string()); + + for (span, suggestion) in clone_spans { + db.span_suggestion( + span, + &snippet_opt(cx, span) + .map_or( + "change the call to".into(), + |x| Cow::from(format!("change `{}` to", x)) + ), + suggestion.into(), + ); + } + + assert!(deref_span.is_none()); + return; + } + } + + let mut spans = vec![(input.span, format!("&{}", snippet(cx, input.span, "_")))]; + + // Suggests adding `*` to dereference the added reference. + if let Some(deref_span) = deref_span { + spans.extend( + deref_span + .iter() + .cloned() + .map(|span| (span, format!("*{}", snippet(cx, span, "")))), + ); + spans.sort_by_key(|&(span, _)| span); + } + multispan_sugg(db, "consider taking a reference instead".to_string(), spans); + }; + + span_lint_and_then( + cx, + NEEDLESS_PASS_BY_VALUE, + input.span, + "this argument is passed by value, but not consumed in the function body", + sugg, + ); + } + } } } } @@ -299,18 +302,19 @@ impl<'a, 'tcx> MovedVariablesCtxt<'a, 'tcx> { map::Node::NodeStmt(s) => { // `let = x;` - if_let_chain! {[ - let StmtDecl(ref decl, _) = s.node, - let DeclLocal(ref local) = decl.node, - ], { - self.spans_need_deref - .entry(vid) - .or_insert_with(HashSet::new) - .insert(local.init - .as_ref() - .map(|e| e.span) - .expect("`let` stmt without init aren't caught by match_pat")); - }} + if_chain! { + if let StmtDecl(ref decl, _) = s.node; + if let DeclLocal(ref local) = decl.node; + then { + self.spans_need_deref + .entry(vid) + .or_insert_with(HashSet::new) + .insert(local.init + .as_ref() + .map(|e| e.span) + .expect("`let` stmt without init aren't caught by match_pat")); + } + } }, _ => {}, diff --git a/clippy_lints/src/neg_multiply.rs b/clippy_lints/src/neg_multiply.rs index d7437f34cff3..e34136face9b 100644 --- a/clippy_lints/src/neg_multiply.rs +++ b/clippy_lints/src/neg_multiply.rs @@ -45,16 +45,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NegMultiply { } fn check_mul(cx: &LateContext, span: Span, lit: &Expr, exp: &Expr) { - if_let_chain!([ - let ExprLit(ref l) = lit.node, - let Constant::Int(ref ci) = consts::lit_to_constant(&l.node, cx.tcx, cx.tables.expr_ty(lit)), - let Some(val) = ci.to_u64(), - val == 1, - cx.tables.expr_ty(exp).is_integral() - ], { - span_lint(cx, - NEG_MULTIPLY, - span, - "Negation by multiplying with -1"); - }) + if_chain! { + if let ExprLit(ref l) = lit.node; + if let Constant::Int(ref ci) = consts::lit_to_constant(&l.node, cx.tcx, cx.tables.expr_ty(lit)); + if let Some(val) = ci.to_u64(); + if val == 1; + if cx.tables.expr_ty(exp).is_integral(); + then { + span_lint(cx, + NEG_MULTIPLY, + span, + "Negation by multiplying with -1"); + } + } } diff --git a/clippy_lints/src/new_without_default.rs b/clippy_lints/src/new_without_default.rs index 1c5524af68e7..53d6d3f2fb84 100644 --- a/clippy_lints/src/new_without_default.rs +++ b/clippy_lints/src/new_without_default.rs @@ -117,40 +117,41 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault { if decl.inputs.is_empty() && name == "new" && cx.access_levels.is_reachable(id) { let self_ty = cx.tcx .type_of(cx.tcx.hir.local_def_id(cx.tcx.hir.get_parent(id))); - if_let_chain!{[ - same_tys(cx, self_ty, return_ty(cx, id)), - let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT), - !implements_trait(cx, self_ty, default_trait_id, &[]) - ], { - if let Some(sp) = can_derive_default(self_ty, cx, default_trait_id) { - span_lint_and_then(cx, - NEW_WITHOUT_DEFAULT_DERIVE, span, - &format!("you should consider deriving a \ - `Default` implementation for `{}`", - self_ty), - |db| { - db.suggest_item_with_attr(cx, sp, "try this", "#[derive(Default)]"); - }); - } else { - span_lint_and_then(cx, - NEW_WITHOUT_DEFAULT, span, - &format!("you should consider adding a \ - `Default` implementation for `{}`", - self_ty), - |db| { - db.suggest_prepend_item(cx, - span, - "try this", - &format!( -"impl Default for {} {{ - fn default() -> Self {{ - Self::new() - }} -}}", - self_ty)); - }); + if_chain! { + if same_tys(cx, self_ty, return_ty(cx, id)); + if let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT); + if !implements_trait(cx, self_ty, default_trait_id, &[]); + then { + if let Some(sp) = can_derive_default(self_ty, cx, default_trait_id) { + span_lint_and_then(cx, + NEW_WITHOUT_DEFAULT_DERIVE, span, + &format!("you should consider deriving a \ + `Default` implementation for `{}`", + self_ty), + |db| { + db.suggest_item_with_attr(cx, sp, "try this", "#[derive(Default)]"); + }); + } else { + span_lint_and_then(cx, + NEW_WITHOUT_DEFAULT, span, + &format!("you should consider adding a \ + `Default` implementation for `{}`", + self_ty), + |db| { + db.suggest_prepend_item(cx, + span, + "try this", + &format!( + "impl Default for {} {{ + fn default() -> Self {{ + Self::new() + }} + }}", + self_ty)); + }); + } } - }} + } } } } diff --git a/clippy_lints/src/ok_if_let.rs b/clippy_lints/src/ok_if_let.rs index 67d39333ff98..e6fe0631a63b 100644 --- a/clippy_lints/src/ok_if_let.rs +++ b/clippy_lints/src/ok_if_let.rs @@ -43,21 +43,22 @@ impl LintPass for Pass { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { - if_let_chain! {[ //begin checking variables - let ExprMatch(ref op, ref body, ref source) = expr.node, //test if expr is a match - let MatchSource::IfLetDesugar { .. } = *source, //test if it is an If Let - let ExprMethodCall(_, _, ref result_types) = op.node, //check is expr.ok() has type Result.ok() - let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = body[0].pats[0].node, //get operation - method_chain_args(op, &["ok"]).is_some() //test to see if using ok() methoduse std::marker::Sized; - - ], { - let is_result_type = match_type(cx, cx.tables.expr_ty(&result_types[0]), &paths::RESULT); - let some_expr_string = snippet(cx, y[0].span, ""); - if print::to_string(print::NO_ANN, |s| s.print_path(x, false)) == "Some" && is_result_type { - span_help_and_lint(cx, IF_LET_SOME_RESULT, expr.span, - "Matching on `Some` with `ok()` is redundant", - &format!("Consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string)); + if_chain! { //begin checking variables + if let ExprMatch(ref op, ref body, ref source) = expr.node; //test if expr is a match + if let MatchSource::IfLetDesugar { .. } = *source; //test if it is an If Let + if let ExprMethodCall(_, _, ref result_types) = op.node; //check is expr.ok() has type Result.ok() + if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = body[0].pats[0].node; //get operation + if method_chain_args(op, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized; + + then { + let is_result_type = match_type(cx, cx.tables.expr_ty(&result_types[0]), &paths::RESULT); + let some_expr_string = snippet(cx, y[0].span, ""); + if print::to_string(print::NO_ANN, |s| s.print_path(x, false)) == "Some" && is_result_type { + span_help_and_lint(cx, IF_LET_SOME_RESULT, expr.span, + "Matching on `Some` with `ok()` is redundant", + &format!("Consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string)); + } } - }} + } } } diff --git a/clippy_lints/src/overflow_check_conditional.rs b/clippy_lints/src/overflow_check_conditional.rs index ee44cc70b435..4177f4a3e73c 100644 --- a/clippy_lints/src/overflow_check_conditional.rs +++ b/clippy_lints/src/overflow_check_conditional.rs @@ -31,52 +31,54 @@ impl LintPass for OverflowCheckConditional { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OverflowCheckConditional { // a + b < a, a > a + b, a < a - b, a - b > a fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx 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(QPath::Resolved(_, ref path1)) = ident1.node, - let Expr_::ExprPath(QPath::Resolved(_, ref path2)) = ident2.node, - let Expr_::ExprPath(QPath::Resolved(_, ref path3)) = second.node, - path1.segments[0] == path3.segments[0] || path2.segments[0] == path3.segments[0], - cx.tables.expr_ty(ident1).is_integral(), - cx.tables.expr_ty(ident2).is_integral() - ], { - if let BinOp_::BiLt = op.node { - if let BinOp_::BiAdd = op2.node { - span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, - "You are trying to use classic C overflow conditions that will fail in Rust."); + if_chain! { + if let Expr_::ExprBinary(ref op, ref first, ref second) = expr.node; + if let Expr_::ExprBinary(ref op2, ref ident1, ref ident2) = first.node; + if let Expr_::ExprPath(QPath::Resolved(_, ref path1)) = ident1.node; + if let Expr_::ExprPath(QPath::Resolved(_, ref path2)) = ident2.node; + if let Expr_::ExprPath(QPath::Resolved(_, ref path3)) = second.node; + if path1.segments[0] == path3.segments[0] || path2.segments[0] == path3.segments[0]; + if cx.tables.expr_ty(ident1).is_integral(); + if cx.tables.expr_ty(ident2).is_integral(); + then { + if let BinOp_::BiLt = op.node { + if let BinOp_::BiAdd = op2.node { + span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, + "You are trying to use classic C overflow conditions that will fail in Rust."); + } + } + if let BinOp_::BiGt = op.node { + if let BinOp_::BiSub = op2.node { + span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, + "You are trying to use classic C underflow conditions that will fail in Rust."); + } } } - if let BinOp_::BiGt = op.node { - if let BinOp_::BiSub = op2.node { - span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, - "You are trying to use classic C underflow conditions that will fail in Rust."); - } - } - }} + } - 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(QPath::Resolved(_, ref path1)) = ident1.node, - let Expr_::ExprPath(QPath::Resolved(_, ref path2)) = ident2.node, - let Expr_::ExprPath(QPath::Resolved(_, ref path3)) = first.node, - path1.segments[0] == path3.segments[0] || path2.segments[0] == path3.segments[0], - cx.tables.expr_ty(ident1).is_integral(), - cx.tables.expr_ty(ident2).is_integral() - ], { - if let BinOp_::BiGt = op.node { - if let BinOp_::BiAdd = op2.node { - span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, - "You are trying to use classic C overflow conditions that will fail in Rust."); + if_chain! { + if let Expr_::ExprBinary(ref op, ref first, ref second) = expr.node; + if let Expr_::ExprBinary(ref op2, ref ident1, ref ident2) = second.node; + if let Expr_::ExprPath(QPath::Resolved(_, ref path1)) = ident1.node; + if let Expr_::ExprPath(QPath::Resolved(_, ref path2)) = ident2.node; + if let Expr_::ExprPath(QPath::Resolved(_, ref path3)) = first.node; + if path1.segments[0] == path3.segments[0] || path2.segments[0] == path3.segments[0]; + if cx.tables.expr_ty(ident1).is_integral(); + if cx.tables.expr_ty(ident2).is_integral(); + then { + if let BinOp_::BiGt = op.node { + if let BinOp_::BiAdd = op2.node { + span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, + "You are trying to use classic C overflow conditions that will fail in Rust."); + } + } + if let BinOp_::BiLt = op.node { + if let BinOp_::BiSub = op2.node { + span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, + "You are trying to use classic C underflow conditions that will fail in Rust."); + } } } - if let BinOp_::BiLt = op.node { - if let BinOp_::BiSub = op2.node { - span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, - "You are trying to use classic C underflow conditions that will fail in Rust."); - } - } - }} + } } } diff --git a/clippy_lints/src/panic.rs b/clippy_lints/src/panic.rs index f0428534456a..1a14a0bc45da 100644 --- a/clippy_lints/src/panic.rs +++ b/clippy_lints/src/panic.rs @@ -34,23 +34,24 @@ impl LintPass for Pass { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { - if_let_chain! {[ - let ExprBlock(ref block) = expr.node, - let Some(ref ex) = block.expr, - let ExprCall(ref fun, ref params) = ex.node, - params.len() == 2, - let ExprPath(ref qpath) = fun.node, - let Some(fun_def_id) = opt_def_id(resolve_node(cx, qpath, fun.hir_id)), - match_def_path(cx.tcx, fun_def_id, &paths::BEGIN_PANIC), - let ExprLit(ref lit) = params[0].node, - is_direct_expn_of(expr.span, "panic").is_some(), - let LitKind::Str(ref string, _) = lit.node, - let Some(par) = string.as_str().find('{'), - string.as_str()[par..].contains('}'), - params[0].span.source_callee().is_none() - ], { - span_lint(cx, PANIC_PARAMS, params[0].span, - "you probably are missing some parameter in your format string"); - }} + if_chain! { + if let ExprBlock(ref block) = expr.node; + if let Some(ref ex) = block.expr; + if let ExprCall(ref fun, ref params) = ex.node; + if params.len() == 2; + if let ExprPath(ref qpath) = fun.node; + if let Some(fun_def_id) = opt_def_id(resolve_node(cx, qpath, fun.hir_id)); + if match_def_path(cx.tcx, fun_def_id, &paths::BEGIN_PANIC); + if let ExprLit(ref lit) = params[0].node; + if is_direct_expn_of(expr.span, "panic").is_some(); + if let LitKind::Str(ref string, _) = lit.node; + if let Some(par) = string.as_str().find('{'); + if string.as_str()[par..].contains('}'); + if params[0].span.source_callee().is_none(); + then { + span_lint(cx, PANIC_PARAMS, params[0].span, + "you probably are missing some parameter in your format string"); + } + } } } diff --git a/clippy_lints/src/partialeq_ne_impl.rs b/clippy_lints/src/partialeq_ne_impl.rs index 34f1e4bc493a..22dc43eb6c58 100644 --- a/clippy_lints/src/partialeq_ne_impl.rs +++ b/clippy_lints/src/partialeq_ne_impl.rs @@ -37,20 +37,21 @@ impl LintPass for Pass { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) { - if_let_chain! {[ - let ItemImpl(_, _, _, _, Some(ref trait_ref), _, ref impl_items) = item.node, - !is_automatically_derived(&*item.attrs), - let Some(eq_trait) = cx.tcx.lang_items().eq_trait(), - trait_ref.path.def.def_id() == eq_trait - ], { - for impl_item in impl_items { - if impl_item.name == "ne" { - span_lint(cx, - PARTIALEQ_NE_IMPL, - impl_item.span, - "re-implementing `PartialEq::ne` is unnecessary") + if_chain! { + if let ItemImpl(_, _, _, _, Some(ref trait_ref), _, ref impl_items) = item.node; + if !is_automatically_derived(&*item.attrs); + if let Some(eq_trait) = cx.tcx.lang_items().eq_trait(); + if trait_ref.path.def.def_id() == eq_trait; + then { + for impl_item in impl_items { + if impl_item.name == "ne" { + span_lint(cx, + PARTIALEQ_NE_IMPL, + impl_item.span, + "re-implementing `PartialEq::ne` is unnecessary") + } } } - }}; + }; } } diff --git a/clippy_lints/src/print.rs b/clippy_lints/src/print.rs index b7ccf19a313b..ce6b96108a40 100644 --- a/clippy_lints/src/print.rs +++ b/clippy_lints/src/print.rs @@ -89,58 +89,60 @@ impl LintPass for Pass { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { - if_let_chain! {[ - let ExprCall(ref fun, ref args) = expr.node, - let ExprPath(ref qpath) = fun.node, - let Some(fun_id) = opt_def_id(resolve_node(cx, qpath, fun.hir_id)), - ], { - - // Search for `std::io::_print(..)` which is unique in a - // `print!` expansion. - if match_def_path(cx.tcx, fun_id, &paths::IO_PRINT) { - if let Some(span) = is_expn_of(expr.span, "print") { - // `println!` uses `print!`. - let (span, name) = match is_expn_of(span, "println") { - Some(span) => (span, "println"), - None => (span, "print"), - }; - - span_lint(cx, PRINT_STDOUT, span, &format!("use of `{}!`", name)); - - if_let_chain!{[ - // ensure we're calling Arguments::new_v1 - args.len() == 1, - let ExprCall(ref args_fun, ref args_args) = args[0].node, - let ExprPath(ref qpath) = args_fun.node, - let Some(const_def_id) = opt_def_id(resolve_node(cx, qpath, args_fun.hir_id)), - match_def_path(cx.tcx, const_def_id, &paths::FMT_ARGUMENTS_NEWV1), - args_args.len() == 2, - let ExprAddrOf(_, ref match_expr) = args_args[1].node, - let ExprMatch(ref args, _, _) = match_expr.node, - let ExprTup(ref args) = args.node, - let Some((fmtstr, fmtlen)) = get_argument_fmtstr_parts(&args_args[0]), - ], { - match name { - "print" => check_print(cx, span, args, fmtstr, fmtlen), - "println" => check_println(cx, span, fmtstr, fmtlen), - _ => (), + if_chain! { + if let ExprCall(ref fun, ref args) = expr.node; + if let ExprPath(ref qpath) = fun.node; + if let Some(fun_id) = opt_def_id(resolve_node(cx, qpath, fun.hir_id)); + then { + + // Search for `std::io::_print(..)` which is unique in a + // `print!` expansion. + if match_def_path(cx.tcx, fun_id, &paths::IO_PRINT) { + if let Some(span) = is_expn_of(expr.span, "print") { + // `println!` uses `print!`. + let (span, name) = match is_expn_of(span, "println") { + Some(span) => (span, "println"), + None => (span, "print"), + }; + + span_lint(cx, PRINT_STDOUT, span, &format!("use of `{}!`", name)); + + if_chain! { + // ensure we're calling Arguments::new_v1 + if args.len() == 1; + if let ExprCall(ref args_fun, ref args_args) = args[0].node; + if let ExprPath(ref qpath) = args_fun.node; + if let Some(const_def_id) = opt_def_id(resolve_node(cx, qpath, args_fun.hir_id)); + if match_def_path(cx.tcx, const_def_id, &paths::FMT_ARGUMENTS_NEWV1); + if args_args.len() == 2; + if let ExprAddrOf(_, ref match_expr) = args_args[1].node; + if let ExprMatch(ref args, _, _) = match_expr.node; + if let ExprTup(ref args) = args.node; + if let Some((fmtstr, fmtlen)) = get_argument_fmtstr_parts(&args_args[0]); + then { + match name { + "print" => check_print(cx, span, args, fmtstr, fmtlen), + "println" => check_println(cx, span, fmtstr, fmtlen), + _ => (), + } + } } - }} + } } - } - // Search for something like - // `::std::fmt::ArgumentV1::new(__arg0, ::std::fmt::Debug::fmt)` - else if args.len() == 2 && match_def_path(cx.tcx, fun_id, &paths::FMT_ARGUMENTV1_NEW) { - if let ExprPath(ref qpath) = args[1].node { - if let Some(def_id) = opt_def_id(cx.tables.qpath_def(qpath, args[1].hir_id)) { - if match_def_path(cx.tcx, def_id, &paths::DEBUG_FMT_METHOD) - && !is_in_debug_impl(cx, expr) && is_expn_of(expr.span, "panic").is_none() { - span_lint(cx, USE_DEBUG, args[0].span, "use of `Debug`-based formatting"); + // Search for something like + // `::std::fmt::ArgumentV1::new(__arg0, ::std::fmt::Debug::fmt)` + else if args.len() == 2 && match_def_path(cx.tcx, fun_id, &paths::FMT_ARGUMENTV1_NEW) { + if let ExprPath(ref qpath) = args[1].node { + if let Some(def_id) = opt_def_id(cx.tables.qpath_def(qpath, args[1].hir_id)) { + if match_def_path(cx.tcx, def_id, &paths::DEBUG_FMT_METHOD) + && !is_in_debug_impl(cx, expr) && is_expn_of(expr.span, "panic").is_none() { + span_lint(cx, USE_DEBUG, args[0].span, "use of `Debug`-based formatting"); + } } } } } - }} + } } } @@ -152,36 +154,38 @@ fn check_print<'a, 'tcx>( fmtstr: InternedString, fmtlen: usize, ) { - if_let_chain!{[ + if_chain! { // check the final format string part - let Some('\n') = fmtstr.chars().last(), + if let Some('\n') = fmtstr.chars().last(); // "foo{}bar" is made into two strings + one argument, // if the format string starts with `{}` (eg. "{}foo"), // the string array is prepended an empty string "". // We only want to check the last string after any `{}`: - args.len() < fmtlen, - ], { - span_lint(cx, PRINT_WITH_NEWLINE, span, - "using `print!()` with a format string that ends in a \ - newline, consider using `println!()` instead"); - }} + if args.len() < fmtlen; + then { + span_lint(cx, PRINT_WITH_NEWLINE, span, + "using `print!()` with a format string that ends in a \ + newline, consider using `println!()` instead"); + } + } } /// Check for println!("") fn check_println<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, span: Span, fmtstr: InternedString, fmtlen: usize) { - if_let_chain!{[ + if_chain! { // check that the string is empty - fmtlen == 1, - fmtstr.deref() == "\n", + if fmtlen == 1; + if fmtstr.deref() == "\n"; // check the presence of that string - let Ok(snippet) = cx.sess().codemap().span_to_snippet(span), - snippet.contains("\"\""), - ], { - span_lint(cx, PRINT_WITH_NEWLINE, span, - "using `println!(\"\")`, consider using `println!()` instead"); - }} + if let Ok(snippet) = cx.sess().codemap().span_to_snippet(span); + if snippet.contains("\"\""); + then { + span_lint(cx, PRINT_WITH_NEWLINE, span, + "using `println!(\"\")`, consider using `println!()` instead"); + } + } } fn is_in_debug_impl(cx: &LateContext, expr: &Expr) -> bool { @@ -202,14 +206,15 @@ fn is_in_debug_impl(cx: &LateContext, expr: &Expr) -> bool { /// Returns the slice of format string parts in an `Arguments::new_v1` call. fn get_argument_fmtstr_parts(expr: &Expr) -> Option<(InternedString, usize)> { - if_let_chain! {[ - let ExprAddrOf(_, ref expr) = expr.node, // &["…", "…", …] - let ExprArray(ref exprs) = expr.node, - let Some(expr) = exprs.last(), - let ExprLit(ref lit) = expr.node, - let LitKind::Str(ref lit, _) = lit.node, - ], { - return Some((lit.as_str(), exprs.len())); - }} + if_chain! { + if let ExprAddrOf(_, ref expr) = expr.node; // &["…", "…", …] + if let ExprArray(ref exprs) = expr.node; + if let Some(expr) = exprs.last(); + if let ExprLit(ref lit) = expr.node; + if let LitKind::Str(ref lit, _) = lit.node; + then { + return Some((lit.as_str(), exprs.len())); + } + } None } diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index 8f42750e19b1..916132daeff2 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -156,13 +156,14 @@ fn check_fn(cx: &LateContext, decl: &FnDecl, fn_id: NodeId, opt_body_id: Option< { if match_type(cx, ty, &paths::VEC) { let mut ty_snippet = None; - if_let_chain!([ - let TyPath(QPath::Resolved(_, ref path)) = walk_ptrs_hir_ty(arg).node, - let Some(&PathSegment{parameters: Some(ref parameters), ..}) = path.segments.last(), - parameters.types.len() == 1, - ], { - ty_snippet = snippet_opt(cx, parameters.types[0].span); - }); + if_chain! { + if let TyPath(QPath::Resolved(_, ref path)) = walk_ptrs_hir_ty(arg).node; + if let Some(&PathSegment{parameters: Some(ref parameters), ..}) = path.segments.last(); + if parameters.types.len() == 1; + then { + ty_snippet = snippet_opt(cx, parameters.types[0].span); + } + }; if let Some(spans) = get_spans(cx, opt_body_id, idx, &[("clone", ".to_owned()")]) { span_lint_and_then( cx, diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index aff0c4b08aba..6e9bebca7570 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -113,69 +113,72 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { } else if name == "zip" && args.len() == 2 { let iter = &args[0].node; let zip_arg = &args[1]; - if_let_chain! {[ + if_chain! { // .iter() call - let ExprMethodCall(ref iter_path, _, ref iter_args ) = *iter, - iter_path.name == "iter", + if let ExprMethodCall(ref iter_path, _, ref iter_args ) = *iter; + if iter_path.name == "iter"; // range expression in .zip() call: 0..x.len() - let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::range(zip_arg), - is_integer_literal(start, 0), + if let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::range(zip_arg); + if is_integer_literal(start, 0); // .len() call - let ExprMethodCall(ref len_path, _, ref len_args) = end.node, - len_path.name == "len" && len_args.len() == 1, + if let ExprMethodCall(ref len_path, _, ref len_args) = end.node; + if len_path.name == "len" && len_args.len() == 1; // .iter() and .len() called on same Path - let ExprPath(QPath::Resolved(_, ref iter_path)) = iter_args[0].node, - let ExprPath(QPath::Resolved(_, ref len_path)) = len_args[0].node, - iter_path.segments == len_path.segments - ], { - 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 ExprPath(QPath::Resolved(_, ref iter_path)) = iter_args[0].node; + if let ExprPath(QPath::Resolved(_, ref len_path)) = len_args[0].node; + if iter_path.segments == len_path.segments; + then { + span_lint(cx, + RANGE_ZIP_WITH_LEN, + expr.span, + &format!("It is more idiomatic to use {}.iter().enumerate()", + snippet(cx, iter_args[0].span, "_"))); + } + } } } // exclusive range plus one: x..(y+1) - if_let_chain! {[ - let Some(higher::Range { start, end: Some(end), limits: RangeLimits::HalfOpen }) = higher::range(expr), - let Some(y) = y_plus_one(end), - ], { - span_lint_and_then( - cx, - RANGE_PLUS_ONE, - expr.span, - "an inclusive range would be more readable", - |db| { - let start = start.map_or("".to_owned(), |x| Sugg::hir(cx, x, "x").to_string()); - let end = Sugg::hir(cx, y, "y"); - db.span_suggestion(expr.span, - "use", - format!("{}..={}", start, end)); - }, - ); - }} + if_chain! { + if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::HalfOpen }) = higher::range(expr); + if let Some(y) = y_plus_one(end); + then { + span_lint_and_then( + cx, + RANGE_PLUS_ONE, + expr.span, + "an inclusive range would be more readable", + |db| { + let start = start.map_or("".to_owned(), |x| Sugg::hir(cx, x, "x").to_string()); + let end = Sugg::hir(cx, y, "y"); + db.span_suggestion(expr.span, + "use", + format!("{}..={}", start, end)); + }, + ); + } + } // inclusive range minus one: x..=(y-1) - if_let_chain! {[ - let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::range(expr), - let Some(y) = y_minus_one(end), - ], { - span_lint_and_then( - cx, - RANGE_MINUS_ONE, - expr.span, - "an exclusive range would be more readable", - |db| { - let start = start.map_or("".to_owned(), |x| Sugg::hir(cx, x, "x").to_string()); - let end = Sugg::hir(cx, y, "y"); - db.span_suggestion(expr.span, - "use", - format!("{}..{}", start, end)); - }, - ); - }} + if_chain! { + if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::range(expr); + if let Some(y) = y_minus_one(end); + then { + span_lint_and_then( + cx, + RANGE_MINUS_ONE, + expr.span, + "an exclusive range would be more readable", + |db| { + let start = start.map_or("".to_owned(), |x| Sugg::hir(cx, x, "x").to_string()); + let end = Sugg::hir(cx, y, "y"); + db.span_suggestion(expr.span, + "use", + format!("{}..{}", start, end)); + }, + ); + } + } } } diff --git a/clippy_lints/src/regex.rs b/clippy_lints/src/regex.rs index dc284001b1d0..0c125825d8a6 100644 --- a/clippy_lints/src/regex.rs +++ b/clippy_lints/src/regex.rs @@ -86,22 +86,23 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { } fn check_block(&mut self, cx: &LateContext<'a, 'tcx>, block: &'tcx Block) { - if_let_chain!{[ - self.last.is_none(), - let Some(ref expr) = block.expr, - match_type(cx, cx.tables.expr_ty(expr), &paths::REGEX), - let Some(span) = is_expn_of(expr.span, "regex"), - ], { - if !self.spans.contains(&span) { - span_lint(cx, - REGEX_MACRO, - span, - "`regex!(_)` found. \ - Please use `Regex::new(_)`, which is faster for now."); - self.spans.insert(span); + if_chain! { + if self.last.is_none(); + if let Some(ref expr) = block.expr; + if match_type(cx, cx.tables.expr_ty(expr), &paths::REGEX); + if let Some(span) = is_expn_of(expr.span, "regex"); + then { + if !self.spans.contains(&span) { + span_lint(cx, + REGEX_MACRO, + span, + "`regex!(_)` found. \ + Please use `Regex::new(_)`, which is faster for now."); + self.spans.insert(span); + } + self.last = Some(block.id); } - self.last = Some(block.id); - }} + } } fn check_block_post(&mut self, _: &LateContext<'a, 'tcx>, block: &'tcx Block) { @@ -111,24 +112,25 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { } fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { - if_let_chain!{[ - let ExprCall(ref fun, ref args) = expr.node, - let ExprPath(ref qpath) = fun.node, - args.len() == 1, - let Some(def_id) = opt_def_id(cx.tables.qpath_def(qpath, fun.hir_id)), - ], { - if match_def_path(cx.tcx, def_id, &paths::REGEX_NEW) || - match_def_path(cx.tcx, def_id, &paths::REGEX_BUILDER_NEW) { - check_regex(cx, &args[0], true); - } else if match_def_path(cx.tcx, def_id, &paths::REGEX_BYTES_NEW) || - match_def_path(cx.tcx, def_id, &paths::REGEX_BYTES_BUILDER_NEW) { - check_regex(cx, &args[0], false); - } else if match_def_path(cx.tcx, def_id, &paths::REGEX_SET_NEW) { - check_set(cx, &args[0], true); - } else if match_def_path(cx.tcx, def_id, &paths::REGEX_BYTES_SET_NEW) { - check_set(cx, &args[0], false); + if_chain! { + if let ExprCall(ref fun, ref args) = expr.node; + if let ExprPath(ref qpath) = fun.node; + if args.len() == 1; + if let Some(def_id) = opt_def_id(cx.tables.qpath_def(qpath, fun.hir_id)); + then { + if match_def_path(cx.tcx, def_id, &paths::REGEX_NEW) || + match_def_path(cx.tcx, def_id, &paths::REGEX_BUILDER_NEW) { + check_regex(cx, &args[0], true); + } else if match_def_path(cx.tcx, def_id, &paths::REGEX_BYTES_NEW) || + match_def_path(cx.tcx, def_id, &paths::REGEX_BYTES_BUILDER_NEW) { + check_regex(cx, &args[0], false); + } else if match_def_path(cx.tcx, def_id, &paths::REGEX_SET_NEW) { + check_set(cx, &args[0], true); + } else if match_def_path(cx.tcx, def_id, &paths::REGEX_BYTES_SET_NEW) { + check_set(cx, &args[0], false); + } } - }} + } } } @@ -179,14 +181,15 @@ fn is_trivial_regex(s: ®ex_syntax::Expr) -> Option<&'static str> { } fn check_set<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, utf8: bool) { - if_let_chain! {[ - let ExprAddrOf(_, ref expr) = expr.node, - let ExprArray(ref exprs) = expr.node, - ], { - for expr in exprs { - check_regex(cx, expr, utf8); + if_chain! { + if let ExprAddrOf(_, ref expr) = expr.node; + if let ExprArray(ref exprs) = expr.node; + then { + for expr in exprs { + check_regex(cx, expr, utf8); + } } - }} + } } fn check_regex<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, utf8: bool) { diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 0884ebbf5cf0..98027885ccf1 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -103,28 +103,29 @@ impl ReturnPass { let mut it = block.stmts.iter(); // we need both a let-binding stmt and an expr - if_let_chain! {[ - let Some(retexpr) = it.next_back(), - let ast::StmtKind::Expr(ref retexpr) = retexpr.node, - let Some(stmt) = it.next_back(), - let ast::StmtKind::Local(ref local) = stmt.node, + if_chain! { + if let Some(retexpr) = it.next_back(); + if let ast::StmtKind::Expr(ref retexpr) = retexpr.node; + if let Some(stmt) = it.next_back(); + if let ast::StmtKind::Local(ref local) = stmt.node; // don't lint in the presence of type inference - local.ty.is_none(), - !local.attrs.iter().any(attr_is_cfg), - let Some(ref initexpr) = local.init, - let ast::PatKind::Ident(_, Spanned { node: id, .. }, _) = local.pat.node, - let ast::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, - "returning the result of a let binding from a block. \ - Consider returning the expression directly.", - initexpr.span, - "this expression can be directly returned"); - }} + if local.ty.is_none(); + if !local.attrs.iter().any(attr_is_cfg); + if let Some(ref initexpr) = local.init; + if let ast::PatKind::Ident(_, Spanned { node: id, .. }, _) = local.pat.node; + if let ast::ExprKind::Path(_, ref path) = retexpr.node; + if match_path_ast(path, &[&id.name.as_str()]); + if !in_external_macro(cx, initexpr.span); + then { + span_note_and_lint(cx, + LET_AND_RETURN, + retexpr.span, + "returning the result of a let binding from a block. \ + Consider returning the expression directly.", + initexpr.span, + "this expression can be directly returned"); + } + } } } diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs index 6119e5008f3c..6497bb9b443a 100644 --- a/clippy_lints/src/swap.rs +++ b/clippy_lints/src/swap.rs @@ -59,120 +59,122 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Swap { /// Implementation of the `MANUAL_SWAP` lint. fn check_manual_swap(cx: &LateContext, block: &Block) { for w in block.stmts.windows(3) { - if_let_chain!{[ + if_chain! { // let t = foo(); - let StmtDecl(ref tmp, _) = w[0].node, - let DeclLocal(ref tmp) = tmp.node, - let Some(ref tmp_init) = tmp.init, - let PatKind::Binding(_, _, ref tmp_name, None) = tmp.pat.node, + if let StmtDecl(ref tmp, _) = w[0].node; + if let DeclLocal(ref tmp) = tmp.node; + if let Some(ref tmp_init) = tmp.init; + if let PatKind::Binding(_, _, ref tmp_name, None) = tmp.pat.node; // foo() = bar(); - let StmtSemi(ref first, _) = w[1].node, - let ExprAssign(ref lhs1, ref rhs1) = first.node, + if let StmtSemi(ref first, _) = w[1].node; + if let ExprAssign(ref lhs1, ref rhs1) = first.node; // bar() = t; - let StmtSemi(ref second, _) = w[2].node, - let ExprAssign(ref lhs2, ref rhs2) = second.node, - let ExprPath(QPath::Resolved(None, ref rhs2)) = rhs2.node, - rhs2.segments.len() == 1, + if let StmtSemi(ref second, _) = w[2].node; + if let ExprAssign(ref lhs2, ref rhs2) = second.node; + if let ExprPath(QPath::Resolved(None, ref rhs2)) = rhs2.node; + if rhs2.segments.len() == 1; - tmp_name.node.as_str() == rhs2.segments[0].name.as_str(), - SpanlessEq::new(cx).ignore_fn().eq_expr(tmp_init, lhs1), - SpanlessEq::new(cx).ignore_fn().eq_expr(rhs1, lhs2) - ], { - fn check_for_slice<'a>( - cx: &LateContext, - lhs1: &'a Expr, - lhs2: &'a Expr, - ) -> Option<(&'a Expr, &'a Expr, &'a Expr)> { - if let ExprIndex(ref lhs1, ref idx1) = lhs1.node { - if let ExprIndex(ref lhs2, ref idx2) = lhs2.node { - if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, lhs2) { - let ty = walk_ptrs_ty(cx.tables.expr_ty(lhs1)); - - if matches!(ty.sty, ty::TySlice(_)) || - matches!(ty.sty, ty::TyArray(_, _)) || - match_type(cx, ty, &paths::VEC) || - match_type(cx, ty, &paths::VEC_DEQUE) { - return Some((lhs1, idx1, idx2)); + if tmp_name.node.as_str() == rhs2.segments[0].name.as_str(); + if SpanlessEq::new(cx).ignore_fn().eq_expr(tmp_init, lhs1); + if SpanlessEq::new(cx).ignore_fn().eq_expr(rhs1, lhs2); + then { + fn check_for_slice<'a>( + cx: &LateContext, + lhs1: &'a Expr, + lhs2: &'a Expr, + ) -> Option<(&'a Expr, &'a Expr, &'a Expr)> { + if let ExprIndex(ref lhs1, ref idx1) = lhs1.node { + if let ExprIndex(ref lhs2, ref idx2) = lhs2.node { + if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, lhs2) { + let ty = walk_ptrs_ty(cx.tables.expr_ty(lhs1)); + + if matches!(ty.sty, ty::TySlice(_)) || + matches!(ty.sty, ty::TyArray(_, _)) || + match_type(cx, ty, &paths::VEC) || + match_type(cx, ty, &paths::VEC_DEQUE) { + return Some((lhs1, idx1, idx2)); + } } } } + + None } - - None - } - - let (replace, what, sugg) = if let Some((slice, idx1, idx2)) = check_for_slice(cx, lhs1, lhs2) { - if let Some(slice) = Sugg::hir_opt(cx, slice) { - (false, - format!(" elements of `{}`", slice), - format!("{}.swap({}, {})", - slice.maybe_par(), - snippet(cx, idx1.span, ".."), - snippet(cx, idx2.span, ".."))) + + let (replace, what, sugg) = if let Some((slice, idx1, idx2)) = check_for_slice(cx, lhs1, lhs2) { + if let Some(slice) = Sugg::hir_opt(cx, slice) { + (false, + format!(" elements of `{}`", slice), + format!("{}.swap({}, {})", + slice.maybe_par(), + snippet(cx, idx1.span, ".."), + snippet(cx, idx2.span, ".."))) + } else { + (false, "".to_owned(), "".to_owned()) + } + } else if let (Some(first), Some(second)) = (Sugg::hir_opt(cx, lhs1), Sugg::hir_opt(cx, rhs1)) { + (true, format!(" `{}` and `{}`", first, second), + format!("std::mem::swap({}, {})", first.mut_addr(), second.mut_addr())) } else { - (false, "".to_owned(), "".to_owned()) - } - } else if let (Some(first), Some(second)) = (Sugg::hir_opt(cx, lhs1), Sugg::hir_opt(cx, rhs1)) { - (true, format!(" `{}` and `{}`", first, second), - format!("std::mem::swap({}, {})", first.mut_addr(), second.mut_addr())) - } else { - (true, "".to_owned(), "".to_owned()) - }; - - let span = w[0].span.to(second.span); - - span_lint_and_then(cx, - MANUAL_SWAP, - span, - &format!("this looks like you are swapping{} manually", what), - |db| { - if !sugg.is_empty() { - db.span_suggestion(span, "try", sugg); - - if replace { - db.note("or maybe you should use `std::mem::replace`?"); + (true, "".to_owned(), "".to_owned()) + }; + + let span = w[0].span.to(second.span); + + span_lint_and_then(cx, + MANUAL_SWAP, + span, + &format!("this looks like you are swapping{} manually", what), + |db| { + if !sugg.is_empty() { + db.span_suggestion(span, "try", sugg); + + if replace { + db.note("or maybe you should use `std::mem::replace`?"); + } } - } - }); - }} + }); + } + } } } /// Implementation of the `ALMOST_SWAPPED` lint. fn check_suspicious_swap(cx: &LateContext, block: &Block) { for w in block.stmts.windows(2) { - if_let_chain!{[ - let StmtSemi(ref first, _) = w[0].node, - let StmtSemi(ref second, _) = w[1].node, - !differing_macro_contexts(first.span, second.span), - let ExprAssign(ref lhs0, ref rhs0) = first.node, - let ExprAssign(ref lhs1, ref rhs1) = second.node, - SpanlessEq::new(cx).ignore_fn().eq_expr(lhs0, rhs1), - SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, rhs0) - ], { - let lhs0 = Sugg::hir_opt(cx, lhs0); - let rhs0 = Sugg::hir_opt(cx, rhs0); - let (what, lhs, rhs) = if let (Some(first), Some(second)) = (lhs0, rhs0) { - (format!(" `{}` and `{}`", first, second), first.mut_addr().to_string(), second.mut_addr().to_string()) - } else { - ("".to_owned(), "".to_owned(), "".to_owned()) - }; - - let span = first.span.to(second.span); - - span_lint_and_then(cx, - ALMOST_SWAPPED, - span, - &format!("this looks like you are trying to swap{}", what), - |db| { - if !what.is_empty() { - db.span_suggestion(span, "try", - format!("std::mem::swap({}, {})", lhs, rhs)); - db.note("or maybe you should use `std::mem::replace`?"); - } - }); - }} + if_chain! { + if let StmtSemi(ref first, _) = w[0].node; + if let StmtSemi(ref second, _) = w[1].node; + if !differing_macro_contexts(first.span, second.span); + if let ExprAssign(ref lhs0, ref rhs0) = first.node; + if let ExprAssign(ref lhs1, ref rhs1) = second.node; + if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs0, rhs1); + if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, rhs0); + then { + let lhs0 = Sugg::hir_opt(cx, lhs0); + let rhs0 = Sugg::hir_opt(cx, rhs0); + let (what, lhs, rhs) = if let (Some(first), Some(second)) = (lhs0, rhs0) { + (format!(" `{}` and `{}`", first, second), first.mut_addr().to_string(), second.mut_addr().to_string()) + } else { + ("".to_owned(), "".to_owned(), "".to_owned()) + }; + + let span = first.span.to(second.span); + + span_lint_and_then(cx, + ALMOST_SWAPPED, + span, + &format!("this looks like you are trying to swap{}", what), + |db| { + if !what.is_empty() { + db.span_suggestion(span, "try", + format!("std::mem::swap({}, {})", lhs, rhs)); + db.note("or maybe you should use `std::mem::replace`?"); + } + }); + } + } } } diff --git a/clippy_lints/src/transmute.rs b/clippy_lints/src/transmute.rs index a97c24166b48..f816ddc464f5 100644 --- a/clippy_lints/src/transmute.rs +++ b/clippy_lints/src/transmute.rs @@ -301,14 +301,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Transmute { /// lifetime, but it should be rare. fn get_type_snippet(cx: &LateContext, path: &QPath, to_rty: Ty) -> String { let seg = last_path_segment(path); - if_let_chain!{[ - let Some(ref params) = seg.parameters, - !params.parenthesized, - let Some(to_ty) = params.types.get(1), - let TyRptr(_, ref to_ty) = to_ty.node, - ], { - return snippet(cx, to_ty.ty.span, &to_rty.to_string()).to_string(); - }} + if_chain! { + if let Some(ref params) = seg.parameters; + if !params.parenthesized; + if let Some(to_ty) = params.types.get(1); + if let TyRptr(_, ref to_ty) = to_ty.node; + then { + return snippet(cx, to_ty.ty.span, &to_rty.to_string()).to_string(); + } + } to_rty.to_string() } diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 44aab3917c78..96caea5c17dd 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -158,21 +158,22 @@ fn check_ty(cx: &LateContext, ast_ty: &hir::Ty, is_local: bool) { if let Some(def_id) = opt_def_id(def) { if Some(def_id) == cx.tcx.lang_items().owned_box() { let last = last_path_segment(qpath); - if_let_chain! {[ - let Some(ref params) = last.parameters, - !params.parenthesized, - let Some(vec) = params.types.get(0), - let TyPath(ref qpath) = vec.node, - let Some(did) = opt_def_id(cx.tables.qpath_def(qpath, cx.tcx.hir.node_to_hir_id(vec.id))), - match_def_path(cx.tcx, did, &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."); - return; // don't recurse into the type - }} + if_chain! { + if let Some(ref params) = last.parameters; + if !params.parenthesized; + if let Some(vec) = params.types.get(0); + if let TyPath(ref qpath) = vec.node; + if let Some(did) = opt_def_id(cx.tables.qpath_def(qpath, cx.tcx.hir.node_to_hir_id(vec.id))); + if match_def_path(cx.tcx, did, &paths::VEC); + then { + 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."); + return; // don't recurse into the type + } + } } else if match_def_path(cx.tcx, def_id, &paths::LINKED_LIST) { span_help_and_lint( cx, @@ -227,39 +228,40 @@ fn check_ty_rptr(cx: &LateContext, ast_ty: &hir::Ty, is_local: bool, lt: &Lifeti TyPath(ref qpath) => { let hir_id = cx.tcx.hir.node_to_hir_id(mut_ty.ty.id); let def = cx.tables.qpath_def(qpath, hir_id); - if_let_chain! {[ - let Some(def_id) = opt_def_id(def), - Some(def_id) == cx.tcx.lang_items().owned_box(), - let QPath::Resolved(None, ref path) = *qpath, - let [ref bx] = *path.segments, - let Some(ref params) = bx.parameters, - !params.parenthesized, - let [ref inner] = *params.types - ], { - if is_any_trait(inner) { - // Ignore `Box` types, see #1884 for details. - return; + if_chain! { + if let Some(def_id) = opt_def_id(def); + if Some(def_id) == cx.tcx.lang_items().owned_box(); + if let QPath::Resolved(None, ref path) = *qpath; + if let [ref bx] = *path.segments; + if let Some(ref params) = bx.parameters; + if !params.parenthesized; + if let [ref inner] = *params.types; + then { + if is_any_trait(inner) { + // Ignore `Box` types, see #1884 for details. + return; + } + + let ltopt = if lt.is_elided() { + "".to_owned() + } else { + format!("{} ", lt.name.name().as_str()) + }; + let mutopt = if mut_ty.mutbl == Mutability::MutMutable { + "mut " + } else { + "" + }; + span_lint_and_sugg(cx, + BORROWED_BOX, + ast_ty.span, + "you seem to be trying to use `&Box`. Consider using just `&T`", + "try", + format!("&{}{}{}", ltopt, mutopt, &snippet(cx, inner.span, "..")) + ); + return; // don't recurse into the type } - - let ltopt = if lt.is_elided() { - "".to_owned() - } else { - format!("{} ", lt.name.name().as_str()) - }; - let mutopt = if mut_ty.mutbl == Mutability::MutMutable { - "mut " - } else { - "" - }; - span_lint_and_sugg(cx, - BORROWED_BOX, - ast_ty.span, - "you seem to be trying to use `&Box`. Consider using just `&T`", - "try", - format!("&{}{}{}", ltopt, mutopt, &snippet(cx, inner.span, "..")) - ); - return; // don't recurse into the type - }}; + }; check_ty(cx, &mut_ty.ty, is_local); }, _ => check_ty(cx, &mut_ty.ty, is_local), @@ -268,15 +270,16 @@ fn check_ty_rptr(cx: &LateContext, ast_ty: &hir::Ty, is_local: bool, lt: &Lifeti // Returns true if given type is `Any` trait. fn is_any_trait(t: &hir::Ty) -> bool { - if_let_chain! {[ - let TyTraitObject(ref traits, _) = t.node, - traits.len() >= 1, + if_chain! { + if let TyTraitObject(ref traits, _) = t.node; + if traits.len() >= 1; // Only Send/Sync can be used as additional traits, so it is enough to // check only the first trait. - match_path(&traits[0].trait_ref.path, &paths::ANY_TRAIT) - ], { - return true; - }} + if match_path(&traits[0].trait_ref.path, &paths::ANY_TRAIT); + then { + return true; + } + } false } @@ -1719,43 +1722,44 @@ impl<'a, 'b, 'tcx: 'a + 'b> Visitor<'tcx> for ImplicitHasherConstructorVisitor<' } fn visit_expr(&mut self, e: &'tcx Expr) { - if_let_chain!{[ - let ExprCall(ref fun, ref args) = e.node, - let ExprPath(QPath::TypeRelative(ref ty, ref method)) = fun.node, - let TyPath(QPath::Resolved(None, ref ty_path)) = ty.node, - ], { - if !same_tys(self.cx, self.target.ty(), self.body.expr_ty(e)) { - return; - } - - if match_path(ty_path, &paths::HASHMAP) { - if method.name == "new" { - self.suggestions - .insert(e.span, "HashMap::default()".to_string()); - } else if method.name == "with_capacity" { - self.suggestions.insert( - e.span, - format!( - "HashMap::with_capacity_and_hasher({}, Default::default())", - snippet(self.cx, args[0].span, "capacity"), - ), - ); + if_chain! { + if let ExprCall(ref fun, ref args) = e.node; + if let ExprPath(QPath::TypeRelative(ref ty, ref method)) = fun.node; + if let TyPath(QPath::Resolved(None, ref ty_path)) = ty.node; + then { + if !same_tys(self.cx, self.target.ty(), self.body.expr_ty(e)) { + return; } - } else if match_path(ty_path, &paths::HASHSET) { - if method.name == "new" { - self.suggestions - .insert(e.span, "HashSet::default()".to_string()); - } else if method.name == "with_capacity" { - self.suggestions.insert( - e.span, - format!( - "HashSet::with_capacity_and_hasher({}, Default::default())", - snippet(self.cx, args[0].span, "capacity"), - ), - ); + + if match_path(ty_path, &paths::HASHMAP) { + if method.name == "new" { + self.suggestions + .insert(e.span, "HashMap::default()".to_string()); + } else if method.name == "with_capacity" { + self.suggestions.insert( + e.span, + format!( + "HashMap::with_capacity_and_hasher({}, Default::default())", + snippet(self.cx, args[0].span, "capacity"), + ), + ); + } + } else if match_path(ty_path, &paths::HASHSET) { + if method.name == "new" { + self.suggestions + .insert(e.span, "HashSet::default()".to_string()); + } else if method.name == "with_capacity" { + self.suggestions.insert( + e.span, + format!( + "HashSet::with_capacity_and_hasher({}, Default::default())", + snippet(self.cx, args[0].span, "capacity"), + ), + ); + } } } - }} + } walk_expr(self, e); } diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index 946df625cb61..985b52b4f535 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -54,26 +54,27 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseSelf { if in_macro(item.span) { return; } - if_let_chain!([ - let ItemImpl(.., ref item_type, ref refs) = item.node, - let Ty_::TyPath(QPath::Resolved(_, ref item_path)) = item_type.node, - ], { - let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).parameters; - let should_check = if let Some(ref params) = *parameters { - !params.parenthesized && params.lifetimes.len() == 0 - } else { - true - }; - if should_check { - let visitor = &mut UseSelfVisitor { - item_path: item_path, - cx: cx, + if_chain! { + if let ItemImpl(.., ref item_type, ref refs) = item.node; + if let Ty_::TyPath(QPath::Resolved(_, ref item_path)) = item_type.node; + then { + let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).parameters; + let should_check = if let Some(ref params) = *parameters { + !params.parenthesized && params.lifetimes.len() == 0 + } else { + true }; - for impl_item_ref in refs { - visitor.visit_impl_item(cx.tcx.hir.impl_item(impl_item_ref.id)); + if should_check { + let visitor = &mut UseSelfVisitor { + item_path: item_path, + cx: cx, + }; + for impl_item_ref in refs { + visitor.visit_impl_item(cx.tcx.hir.impl_item(impl_item_ref.id)); + } } } - }) + } } } diff --git a/clippy_lints/src/utils/higher.rs b/clippy_lints/src/utils/higher.rs index d162dea7f110..93afb449cf64 100644 --- a/clippy_lints/src/utils/higher.rs +++ b/clippy_lints/src/utils/higher.rs @@ -120,13 +120,14 @@ pub fn is_from_for_desugar(decl: &hir::Decl) -> bool { // // do stuff // } // ``` - if_let_chain! {[ - let hir::DeclLocal(ref loc) = decl.node, - let Some(ref expr) = loc.init, - let hir::ExprMatch(_, _, hir::MatchSource::ForLoopDesugar) = expr.node, - ], { - return true; - }} + if_chain! { + if let hir::DeclLocal(ref loc) = decl.node; + if let Some(ref expr) = loc.init; + if let hir::ExprMatch(_, _, hir::MatchSource::ForLoopDesugar) = expr.node; + then { + return true; + } + } // This detects a variable binding in for loop to avoid `let_unit_value` // lint (see issue #1964). @@ -136,12 +137,13 @@ pub fn is_from_for_desugar(decl: &hir::Decl) -> bool { // // anything // } // ``` - if_let_chain! {[ - let hir::DeclLocal(ref loc) = decl.node, - let hir::LocalSource::ForLoopDesugar = loc.source, - ], { - return true; - }} + if_chain! { + if let hir::DeclLocal(ref loc) = decl.node; + if let hir::LocalSource::ForLoopDesugar = loc.source; + then { + return true; + } + } false } @@ -149,19 +151,20 @@ pub fn is_from_for_desugar(decl: &hir::Decl) -> bool { /// Recover the essential nodes of a desugared for loop: /// `for pat in arg { body }` becomes `(pat, arg, body)`. pub fn for_loop(expr: &hir::Expr) -> Option<(&hir::Pat, &hir::Expr, &hir::Expr)> { - if_let_chain! {[ - let hir::ExprMatch(ref iterexpr, ref arms, hir::MatchSource::ForLoopDesugar) = expr.node, - let hir::ExprCall(_, ref iterargs) = iterexpr.node, - iterargs.len() == 1 && arms.len() == 1 && arms[0].guard.is_none(), - let hir::ExprLoop(ref block, _, _) = arms[0].body.node, - block.expr.is_none(), - let [ _, _, ref let_stmt, ref body ] = *block.stmts, - let hir::StmtDecl(ref decl, _) = let_stmt.node, - let hir::DeclLocal(ref decl) = decl.node, - let hir::StmtExpr(ref expr, _) = body.node, - ], { - return Some((&*decl.pat, &iterargs[0], expr)); - }} + if_chain! { + if let hir::ExprMatch(ref iterexpr, ref arms, hir::MatchSource::ForLoopDesugar) = expr.node; + if let hir::ExprCall(_, ref iterargs) = iterexpr.node; + if iterargs.len() == 1 && arms.len() == 1 && arms[0].guard.is_none(); + if let hir::ExprLoop(ref block, _, _) = arms[0].body.node; + if block.expr.is_none(); + if let [ _, _, ref let_stmt, ref body ] = *block.stmts; + if let hir::StmtDecl(ref decl, _) = let_stmt.node; + if let hir::DeclLocal(ref decl) = decl.node; + if let hir::StmtExpr(ref expr, _) = body.node; + then { + return Some((&*decl.pat, &iterargs[0], expr)); + } + } None } @@ -176,31 +179,33 @@ pub enum VecArgs<'a> { /// Returns the arguments of the `vec!` macro if this expression was expanded /// from `vec!`. pub fn vec_macro<'e>(cx: &LateContext, expr: &'e hir::Expr) -> Option> { - if_let_chain!{[ - let hir::ExprCall(ref fun, ref args) = expr.node, - let hir::ExprPath(ref path) = fun.node, - is_expn_of(fun.span, "vec").is_some(), - let Some(fun_def_id) = opt_def_id(resolve_node(cx, path, fun.hir_id)), - ], { - return if match_def_path(cx.tcx, fun_def_id, &paths::VEC_FROM_ELEM) && args.len() == 2 { - // `vec![elem; size]` case - Some(VecArgs::Repeat(&args[0], &args[1])) + if_chain! { + if let hir::ExprCall(ref fun, ref args) = expr.node; + if let hir::ExprPath(ref path) = fun.node; + if is_expn_of(fun.span, "vec").is_some(); + if let Some(fun_def_id) = opt_def_id(resolve_node(cx, path, fun.hir_id)); + then { + return if match_def_path(cx.tcx, fun_def_id, &paths::VEC_FROM_ELEM) && args.len() == 2 { + // `vec![elem; size]` case + Some(VecArgs::Repeat(&args[0], &args[1])) + } + else if match_def_path(cx.tcx, fun_def_id, &paths::SLICE_INTO_VEC) && args.len() == 1 { + // `vec![a, b, c]` case + if_chain! { + if let hir::ExprBox(ref boxed) = args[0].node; + if let hir::ExprArray(ref args) = boxed.node; + then { + return Some(VecArgs::Vec(&*args)); + } + } + + None + } + else { + None + }; } - else if match_def_path(cx.tcx, fun_def_id, &paths::SLICE_INTO_VEC) && args.len() == 1 { - // `vec![a, b, c]` case - if_let_chain!{[ - let hir::ExprBox(ref boxed) = args[0].node, - let hir::ExprArray(ref args) = boxed.node - ], { - return Some(VecArgs::Vec(&*args)); - }} - - None - } - else { - None - }; - }} + } None } diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index cf3bf41a925f..9bc87dee68c6 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -1004,13 +1004,14 @@ pub fn is_self(slf: &Arg) -> bool { } pub fn is_self_ty(slf: &hir::Ty) -> bool { - if_let_chain! {[ - let TyPath(ref qp) = slf.node, - let QPath::Resolved(None, ref path) = *qp, - let Def::SelfTy(..) = path.def, - ], { - return true - }} + if_chain! { + if let TyPath(ref qp) = slf.node; + if let QPath::Resolved(None, ref path) = *qp; + if let Def::SelfTy(..) = path.def; + then { + return true + } + } false } @@ -1022,16 +1023,17 @@ pub fn iter_input_pats<'tcx>(decl: &FnDecl, body: &'tcx Body) -> impl Iterator Option<&Expr> { fn is_ok(arm: &Arm) -> bool { - if_let_chain! {[ - let PatKind::TupleStruct(ref path, ref pat, None) = arm.pats[0].node, - match_qpath(path, &paths::RESULT_OK[1..]), - let PatKind::Binding(_, defid, _, None) = pat[0].node, - let ExprPath(QPath::Resolved(None, ref path)) = arm.body.node, - let Def::Local(lid) = path.def, - lid == defid, - ], { - return true; - }} + if_chain! { + if let PatKind::TupleStruct(ref path, ref pat, None) = arm.pats[0].node; + if match_qpath(path, &paths::RESULT_OK[1..]); + if let PatKind::Binding(_, defid, _, None) = pat[0].node; + if let ExprPath(QPath::Resolved(None, ref path)) = arm.body.node; + if let Def::Local(lid) = path.def; + if lid == defid; + then { + return true; + } + } false } @@ -1049,15 +1051,16 @@ pub fn is_try(expr: &Expr) -> Option<&Expr> { return Some(expr); } - if_let_chain! {[ - arms.len() == 2, - arms[0].pats.len() == 1 && arms[0].guard.is_none(), - arms[1].pats.len() == 1 && arms[1].guard.is_none(), - (is_ok(&arms[0]) && is_err(&arms[1])) || - (is_ok(&arms[1]) && is_err(&arms[0])), - ], { - return Some(expr); - }} + if_chain! { + if arms.len() == 2; + if arms[0].pats.len() == 1 && arms[0].guard.is_none(); + if arms[1].pats.len() == 1 && arms[1].guard.is_none(); + if (is_ok(&arms[0]) && is_err(&arms[1])) || + (is_ok(&arms[1]) && is_err(&arms[0])); + then { + return Some(expr); + } + } } None diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index 6044eaac3769..90a6896b348f 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -35,25 +35,27 @@ impl LintPass for Pass { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { // search for `&vec![_]` expressions where the adjusted type is `&[_]` - if_let_chain!{[ - let ty::TyRef(_, ref ty) = cx.tables.expr_ty_adjusted(expr).sty, - let ty::TySlice(..) = ty.ty.sty, - let ExprAddrOf(_, ref addressee) = expr.node, - let Some(vec_args) = higher::vec_macro(cx, addressee), - ], { - check_vec_macro(cx, &vec_args, expr.span); - }} + if_chain! { + if let ty::TyRef(_, ref ty) = cx.tables.expr_ty_adjusted(expr).sty; + if let ty::TySlice(..) = ty.ty.sty; + if let ExprAddrOf(_, ref addressee) = expr.node; + if let Some(vec_args) = higher::vec_macro(cx, addressee); + then { + check_vec_macro(cx, &vec_args, expr.span); + } + } // search for `for _ in vec![…]` - if_let_chain!{[ - let Some((_, arg, _)) = higher::for_loop(expr), - let Some(vec_args) = higher::vec_macro(cx, arg), - is_copy(cx, vec_type(cx.tables.expr_ty_adjusted(arg))), - ], { - // report the error around the `vec!` not inside `:` - let span = arg.span.ctxt().outer().expn_info().map(|info| info.call_site).expect("unable to get call_site"); - check_vec_macro(cx, &vec_args, span); - }} + if_chain! { + if let Some((_, arg, _)) = higher::for_loop(expr); + if let Some(vec_args) = higher::vec_macro(cx, arg); + if is_copy(cx, vec_type(cx.tables.expr_ty_adjusted(arg))); + then { + // report the error around the `vec!` not inside `:` + let span = arg.span.ctxt().outer().expn_info().map(|info| info.call_site).expect("unable to get call_site"); + check_vec_macro(cx, &vec_args, span); + } + } } } diff --git a/clippy_lints/src/zero_div_zero.rs b/clippy_lints/src/zero_div_zero.rs index 5ff9fb9ffd5d..313af61bf15b 100644 --- a/clippy_lints/src/zero_div_zero.rs +++ b/clippy_lints/src/zero_div_zero.rs @@ -31,27 +31,28 @@ impl LintPass for Pass { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx 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, + if_chain! { + if let ExprBinary(ref op, ref left, ref right) = expr.node; + if 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(cx, left), - let Some(Constant::Float(ref rhs_value, rhs_width)) = constant_simple(cx, right), - Ok(0.0) == lhs_value.parse(), - Ok(0.0) == rhs_value.parse() - ], { - // 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 Some(Constant::Float(ref lhs_value, lhs_width)) = constant_simple(cx, left); + if let Some(Constant::Float(ref rhs_value, rhs_width)) = constant_simple(cx, right); + if Ok(0.0) == lhs_value.parse(); + if Ok(0.0) == rhs_value.parse(); + then { + // 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)); + } + } } } From 2153d1e560a2de06ddd60f29df86c342f1f83a90 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Mon, 23 Oct 2017 15:20:37 -0400 Subject: [PATCH 2/3] manual fixups if_let_chain -> if_chain --- clippy_lints/src/assign_ops.rs | 13 +-- clippy_lints/src/new_without_default.rs | 10 +- clippy_lints/src/utils/author.rs | 136 ++++++++++++------------ tests/ui/trailing_zeros.stdout | 29 ++--- 4 files changed, 96 insertions(+), 92 deletions(-) diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs index 33a1d94f420f..ab9ba4a9327b 100644 --- a/clippy_lints/src/assign_ops.rs +++ b/clippy_lints/src/assign_ops.rs @@ -140,12 +140,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps { let parent_fn = cx.tcx.hir.get_parent(e.id); let parent_impl = cx.tcx.hir.get_parent(parent_fn); // the crate node is the only one that is not in the map - if_let_chain!{[ - parent_impl != ast::CRATE_NODE_ID, - let hir::map::Node::NodeItem(item) = cx.tcx.hir.get(parent_impl), - let hir::Item_::ItemImpl(_, _, _, _, Some(ref trait_ref), _, _) = item.node, - trait_ref.path.def.def_id() == trait_id - ], { return; }} + if_chain! { + if parent_impl != ast::CRATE_NODE_ID; + if let hir::map::Node::NodeItem(item) = cx.tcx.hir.get(parent_impl); + if let hir::Item_::ItemImpl(_, _, _, _, Some(ref trait_ref), _, _) = item.node; + if trait_ref.path.def.def_id() == trait_id; + then { return; } + } implements_trait($cx, $ty, trait_id, &[$rty]) },)* _ => false, diff --git a/clippy_lints/src/new_without_default.rs b/clippy_lints/src/new_without_default.rs index 53d6d3f2fb84..31a9de871b46 100644 --- a/clippy_lints/src/new_without_default.rs +++ b/clippy_lints/src/new_without_default.rs @@ -142,11 +142,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault { span, "try this", &format!( - "impl Default for {} {{ - fn default() -> Self {{ - Self::new() - }} - }}", +"impl Default for {} {{ + fn default() -> Self {{ + Self::new() + }} +}}", self_ty)); }); } diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index fafb6d12d1fd..a4bdcd152904 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -28,15 +28,16 @@ use std::collections::HashMap; /// prints /// /// ``` -/// if_let_chain!{[ -/// let Expr_::ExprIf(ref cond, ref then, None) = item.node, -/// let Expr_::ExprBinary(BinOp::Eq, ref left, ref right) = cond.node, -/// let Expr_::ExprPath(ref path) = left.node, -/// let Expr_::ExprLit(ref lit) = right.node, -/// let LitKind::Int(42, _) = lit.node, -/// ], { -/// // report your lint here -/// }} +/// if_chain!{ +/// if let Expr_::ExprIf(ref cond, ref then, None) = item.node, +/// if let Expr_::ExprBinary(BinOp::Eq, ref left, ref right) = cond.node, +/// if let Expr_::ExprPath(ref path) = left.node, +/// if let Expr_::ExprLit(ref lit) = right.node, +/// if let LitKind::Int(42, _) = lit.node, +/// then { +/// // report your lint here +/// } +/// } /// ``` declare_lint! { pub LINT_AUTHOR, @@ -53,13 +54,14 @@ impl LintPass for Pass { } fn prelude() { - println!("if_let_chain!{{["); + println!("if_chain! {{"); } fn done() { - println!("], {{"); - println!(" // report your lint here"); - println!("}}}}"); + println!(" then {{"); + println!(" // report your lint here"); + println!(" }}"); + println!("}}"); } impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { @@ -181,36 +183,36 @@ struct PrintVisitor { impl<'tcx> Visitor<'tcx> for PrintVisitor { fn visit_expr(&mut self, expr: &Expr) { - print!(" let Expr_::Expr"); + print!(" if let Expr_::Expr"); let current = format!("{}.node", self.current); match expr.node { Expr_::ExprBox(ref inner) => { let inner_pat = self.next("inner"); - println!("Box(ref {}) = {},", inner_pat, current); + println!("Box(ref {}) = {};", inner_pat, current); self.current = inner_pat; self.visit_expr(inner); }, Expr_::ExprArray(ref elements) => { let elements_pat = self.next("elements"); - println!("Array(ref {}) = {},", elements_pat, current); - println!(" {}.len() == {},", elements_pat, elements.len()); + println!("Array(ref {}) = {};", elements_pat, current); + println!(" if {}.len() == {};", elements_pat, elements.len()); for (i, element) in elements.iter().enumerate() { self.current = format!("{}[{}]", elements_pat, i); self.visit_expr(element); } }, Expr_::ExprCall(ref _func, ref _args) => { - println!("Call(ref func, ref args) = {},", current); + println!("Call(ref func, ref args) = {};", current); println!(" // unimplemented: `ExprCall` is not further destructured at the moment"); }, Expr_::ExprMethodCall(ref _method_name, ref _generics, ref _args) => { - println!("MethodCall(ref method_name, ref generics, ref args) = {},", current); + println!("MethodCall(ref method_name, ref generics, ref args) = {};", current); println!(" // unimplemented: `ExprMethodCall` is not further destructured at the moment"); }, Expr_::ExprTup(ref elements) => { let elements_pat = self.next("elements"); - println!("Tup(ref {}) = {},", elements_pat, current); - println!(" {}.len() == {},", elements_pat, elements.len()); + println!("Tup(ref {}) = {};", elements_pat, current); + println!(" if {}.len() == {};", elements_pat, elements.len()); for (i, element) in elements.iter().enumerate() { self.current = format!("{}[{}]", elements_pat, i); self.visit_expr(element); @@ -220,8 +222,8 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { let op_pat = self.next("op"); let left_pat = self.next("left"); let right_pat = self.next("right"); - println!("Binary(ref {}, ref {}, ref {}) = {},", op_pat, left_pat, right_pat, current); - println!(" BinOp_::{:?} == {}.node,", op.node, op_pat); + println!("Binary(ref {}, ref {}, ref {}) = {};", op_pat, left_pat, right_pat, current); + println!(" if BinOp_::{:?} == {}.node;", op.node, op_pat); self.current = left_pat; self.visit_expr(left); self.current = right_pat; @@ -229,42 +231,42 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { }, Expr_::ExprUnary(ref op, ref inner) => { let inner_pat = self.next("inner"); - println!("Unary(UnOp::{:?}, ref {}) = {},", op, inner_pat, current); + println!("Unary(UnOp::{:?}, ref {}) = {};", op, inner_pat, current); self.current = inner_pat; self.visit_expr(inner); }, Expr_::ExprLit(ref lit) => { let lit_pat = self.next("lit"); - println!("Lit(ref {}) = {},", lit_pat, current); + println!("Lit(ref {}) = {};", lit_pat, current); match lit.node { - LitKind::Bool(val) => println!(" let LitKind::Bool({:?}) = {}.node,", val, lit_pat), - LitKind::Char(c) => println!(" let LitKind::Char({:?}) = {}.node,", c, lit_pat), - LitKind::Byte(b) => println!(" let LitKind::Byte({}) = {}.node,", b, lit_pat), + LitKind::Bool(val) => println!(" if let LitKind::Bool({:?}) = {}.node;", val, lit_pat), + LitKind::Char(c) => println!(" if let LitKind::Char({:?}) = {}.node;", c, lit_pat), + LitKind::Byte(b) => println!(" if let LitKind::Byte({}) = {}.node;", b, lit_pat), // FIXME: also check int type - LitKind::Int(i, _) => println!(" let LitKind::Int({}, _) = {}.node,", i, lit_pat), - LitKind::Float(..) => println!(" let LitKind::Float(..) = {}.node,", lit_pat), - LitKind::FloatUnsuffixed(_) => println!(" let LitKind::FloatUnsuffixed(_) = {}.node,", lit_pat), + LitKind::Int(i, _) => println!(" if let LitKind::Int({}, _) = {}.node;", i, lit_pat), + LitKind::Float(..) => println!(" if let LitKind::Float(..) = {}.node;", lit_pat), + LitKind::FloatUnsuffixed(_) => println!(" if let LitKind::FloatUnsuffixed(_) = {}.node;", lit_pat), LitKind::ByteStr(ref vec) => { let vec_pat = self.next("vec"); - println!(" let LitKind::ByteStr(ref {}) = {}.node,", vec_pat, lit_pat); - println!(" let [{:?}] = **{},", vec, vec_pat); + println!(" if let LitKind::ByteStr(ref {}) = {}.node;", vec_pat, lit_pat); + println!(" if let [{:?}] = **{};", vec, vec_pat); }, LitKind::Str(ref text, _) => { let str_pat = self.next("s"); - println!(" let LitKind::Str(ref {}) = {}.node,", str_pat, lit_pat); - println!(" {}.as_str() == {:?}", str_pat, &*text.as_str()) + println!(" if let LitKind::Str(ref {}) = {}.node;", str_pat, lit_pat); + println!(" if {}.as_str() == {:?}", str_pat, &*text.as_str()) }, } }, Expr_::ExprCast(ref expr, ref _ty) => { let cast_pat = self.next("expr"); - println!("Cast(ref {}, _) = {},", cast_pat, current); + println!("Cast(ref {}, _) = {};", cast_pat, current); self.current = cast_pat; self.visit_expr(expr); }, Expr_::ExprType(ref expr, ref _ty) => { let cast_pat = self.next("expr"); - println!("Type(ref {}, _) = {},", cast_pat, current); + println!("Type(ref {}, _) = {};", cast_pat, current); self.current = cast_pat; self.visit_expr(expr); }, @@ -273,11 +275,11 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { let then_pat = self.next("then"); if let Some(ref else_) = *opt_else { let else_pat = self.next("else_"); - println!("If(ref {}, ref {}, Some(ref {})) = {},", cond_pat, then_pat, else_pat, current); + println!("If(ref {}, ref {}, Some(ref {})) = {};", cond_pat, then_pat, else_pat, current); self.current = else_pat; self.visit_expr(else_); } else { - println!("If(ref {}, ref {}, None) = {},", cond_pat, then_pat, current); + println!("If(ref {}, ref {}, None) = {};", cond_pat, then_pat, current); } self.current = cond_pat; self.visit_expr(cond); @@ -285,37 +287,37 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { self.visit_expr(then); }, Expr_::ExprWhile(ref _cond, ref _body, ref _opt_label) => { - println!("While(ref cond, ref body, ref opt_label) = {},", current); + println!("While(ref cond, ref body, ref opt_label) = {};", current); println!(" // unimplemented: `ExprWhile` is not further destructured at the moment"); }, Expr_::ExprLoop(ref _body, ref _opt_label, ref _desuraging) => { - println!("Loop(ref body, ref opt_label, ref desugaring) = {},", current); + println!("Loop(ref body, ref opt_label, ref desugaring) = {};", current); println!(" // unimplemented: `ExprLoop` is not further destructured at the moment"); }, Expr_::ExprMatch(ref _expr, ref _arms, ref _desugaring) => { - println!("Match(ref expr, ref arms, ref desugaring) = {},", current); + println!("Match(ref expr, ref arms, ref desugaring) = {};", current); println!(" // unimplemented: `ExprMatch` is not further destructured at the moment"); }, Expr_::ExprClosure(ref _capture_clause, ref _func, _, _, _) => { - println!("Closure(ref capture_clause, ref func, _, _, _) = {},", current); + println!("Closure(ref capture_clause, ref func, _, _, _) = {};", current); println!(" // unimplemented: `ExprClosure` is not further destructured at the moment"); }, Expr_::ExprYield(ref sub) => { let sub_pat = self.next("sub"); - println!("Yield(ref sub) = {},", current); + println!("Yield(ref sub) = {};", current); self.current = sub_pat; self.visit_expr(sub); }, Expr_::ExprBlock(ref block) => { let block_pat = self.next("block"); - println!("Block(ref {}) = {},", block_pat, current); + println!("Block(ref {}) = {};", block_pat, current); self.current = block_pat; self.visit_block(block); }, Expr_::ExprAssign(ref target, ref value) => { let target_pat = self.next("target"); let value_pat = self.next("value"); - println!("Assign(ref {}, ref {}) = {},", target_pat, value_pat, current); + println!("Assign(ref {}, ref {}) = {};", target_pat, value_pat, current); self.current = target_pat; self.visit_expr(target); self.current = value_pat; @@ -325,8 +327,8 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { let op_pat = self.next("op"); let target_pat = self.next("target"); let value_pat = self.next("value"); - println!("AssignOp(ref {}, ref {}, ref {}) = {},", op_pat, target_pat, value_pat, current); - println!(" BinOp_::{:?} == {}.node,", op.node, op_pat); + println!("AssignOp(ref {}, ref {}, ref {}) = {};", op_pat, target_pat, value_pat, current); + println!(" if BinOp_::{:?} == {}.node;", op.node, op_pat); self.current = target_pat; self.visit_expr(target); self.current = value_pat; @@ -335,23 +337,23 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { Expr_::ExprField(ref object, ref field_name) => { let obj_pat = self.next("object"); let field_name_pat = self.next("field_name"); - println!("Field(ref {}, ref {}) = {},", obj_pat, field_name_pat, current); - println!(" {}.node.as_str() == {:?}", field_name_pat, field_name.node.as_str()); + println!("Field(ref {}, ref {}) = {};", obj_pat, field_name_pat, current); + println!(" if {}.node.as_str() == {:?}", field_name_pat, field_name.node.as_str()); self.current = obj_pat; self.visit_expr(object); }, Expr_::ExprTupField(ref object, ref field_id) => { let obj_pat = self.next("object"); let field_id_pat = self.next("field_id"); - println!("TupField(ref {}, ref {}) = {},", obj_pat, field_id_pat, current); - println!(" {}.node == {}", field_id_pat, field_id.node); + println!("TupField(ref {}, ref {}) = {};", obj_pat, field_id_pat, current); + println!(" if {}.node == {}", field_id_pat, field_id.node); self.current = obj_pat; self.visit_expr(object); }, Expr_::ExprIndex(ref object, ref index) => { let object_pat = self.next("object"); let index_pat = self.next("index"); - println!("Index(ref {}, ref {}) = {},", object_pat, index_pat, current); + println!("Index(ref {}, ref {}) = {};", object_pat, index_pat, current); self.current = object_pat; self.visit_expr(object); self.current = index_pat; @@ -359,13 +361,13 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { }, Expr_::ExprPath(ref path) => { let path_pat = self.next("path"); - println!("Path(ref {}) = {},", path_pat, current); + println!("Path(ref {}) = {};", path_pat, current); self.current = path_pat; self.visit_qpath(path, expr.id, expr.span); }, Expr_::ExprAddrOf(mutability, ref inner) => { let inner_pat = self.next("inner"); - println!("AddrOf({:?}, ref {}) = {},", mutability, inner_pat, current); + println!("AddrOf({:?}, ref {}) = {};", mutability, inner_pat, current); self.current = inner_pat; self.visit_expr(inner); }, @@ -373,29 +375,29 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { let destination_pat = self.next("destination"); if let Some(ref value) = *opt_value { let value_pat = self.next("value"); - println!("Break(ref {}, Some(ref {})) = {},", destination_pat, value_pat, current); + println!("Break(ref {}, Some(ref {})) = {};", destination_pat, value_pat, current); self.current = value_pat; self.visit_expr(value); } else { - println!("Break(ref {}, None) = {},", destination_pat, current); + println!("Break(ref {}, None) = {};", destination_pat, current); } // FIXME: implement label printing }, Expr_::ExprAgain(ref _destination) => { let destination_pat = self.next("destination"); - println!("Again(ref {}) = {},", destination_pat, current); + println!("Again(ref {}) = {};", destination_pat, current); // FIXME: implement label printing }, Expr_::ExprRet(ref opt_value) => if let Some(ref value) = *opt_value { let value_pat = self.next("value"); - println!("Ret(Some(ref {})) = {},", value_pat, current); + println!("Ret(Some(ref {})) = {};", value_pat, current); self.current = value_pat; self.visit_expr(value); } else { - println!("Ret(None) = {},", current); + println!("Ret(None) = {};", current); }, Expr_::ExprInlineAsm(_, ref _input, ref _output) => { - println!("InlineAsm(_, ref input, ref output) = {},", current); + println!("InlineAsm(_, ref input, ref output) = {};", current); println!(" // unimplemented: `ExprInlineAsm` is not further destructured at the moment"); }, Expr_::ExprStruct(ref path, ref fields, ref opt_base) => { @@ -404,7 +406,7 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { if let Some(ref base) = *opt_base { let base_pat = self.next("base"); println!( - "Struct(ref {}, ref {}, Some(ref {})) = {},", + "Struct(ref {}, ref {}, Some(ref {})) = {};", path_pat, fields_pat, base_pat, @@ -413,17 +415,17 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { self.current = base_pat; self.visit_expr(base); } else { - println!("Struct(ref {}, ref {}, None) = {},", path_pat, fields_pat, current); + println!("Struct(ref {}, ref {}, None) = {};", path_pat, fields_pat, current); } self.current = path_pat; self.visit_qpath(path, expr.id, expr.span); - println!(" {}.len() == {},", fields_pat, fields.len()); + println!(" if {}.len() == {};", fields_pat, fields.len()); println!(" // unimplemented: field checks"); }, // FIXME: compute length (needs type info) Expr_::ExprRepeat(ref value, _) => { let value_pat = self.next("value"); - println!("Repeat(ref {}, _) = {},", value_pat, current); + println!("Repeat(ref {}, _) = {};", value_pat, current); println!("// unimplemented: repeat count check"); self.current = value_pat; self.visit_expr(value); @@ -432,9 +434,9 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { } fn visit_qpath(&mut self, path: &QPath, _: NodeId, _: Span) { - print!(" match_qpath({}, &[", self.current); + print!(" if match_qpath({}, &[", self.current); print_path(path, &mut true); - println!("]),"); + println!("]);"); } fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { NestedVisitorMap::None diff --git a/tests/ui/trailing_zeros.stdout b/tests/ui/trailing_zeros.stdout index 52ec01260beb..145c102ed95b 100644 --- a/tests/ui/trailing_zeros.stdout +++ b/tests/ui/trailing_zeros.stdout @@ -1,14 +1,15 @@ -if_let_chain!{[ - let Expr_::ExprBinary(ref op, ref left, ref right) = expr.node, - BinOp_::BiEq == op.node, - let Expr_::ExprBinary(ref op1, ref left1, ref right1) = left.node, - BinOp_::BiBitAnd == op1.node, - let Expr_::ExprPath(ref path) = left1.node, - match_qpath(path, &["x"]), - let Expr_::ExprLit(ref lit) = right1.node, - let LitKind::Int(15, _) = lit.node, - let Expr_::ExprLit(ref lit1) = right.node, - let LitKind::Int(0, _) = lit1.node, -], { - // report your lint here -}} +if_chain! { + if let Expr_::ExprBinary(ref op, ref left, ref right) = expr.node; + if BinOp_::BiEq == op.node; + if let Expr_::ExprBinary(ref op1, ref left1, ref right1) = left.node; + if BinOp_::BiBitAnd == op1.node; + if let Expr_::ExprPath(ref path) = left1.node; + if match_qpath(path, &["x"]); + if let Expr_::ExprLit(ref lit) = right1.node; + if let LitKind::Int(15, _) = lit.node; + if let Expr_::ExprLit(ref lit1) = right.node; + if let LitKind::Int(0, _) = lit1.node; + then { + // report your lint here + } +} From 24a2c14733604132135b2ab598e7a1cf95d8b3ab Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Mon, 23 Oct 2017 15:20:58 -0400 Subject: [PATCH 3/3] remove if_let_chain --- clippy_lints/Cargo.toml | 1 + clippy_lints/src/lib.rs | 5 +++ clippy_lints/src/utils/mod.rs | 57 ----------------------------------- 3 files changed, 6 insertions(+), 57 deletions(-) diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index 414cd68a6601..2dcac941c09f 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -28,6 +28,7 @@ toml = "0.4" unicode-normalization = "0.1" pulldown-cmark = "0.0.15" url = "1.5.0" +if_chain = "0.1" [features] debugging = [] diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 27ef37dd00f9..1cd3479f11d7 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -11,6 +11,8 @@ #![feature(inclusive_range_syntax, range_contains)] #![allow(unknown_lints, indexing_slicing, shadow_reuse, missing_docs_in_private_items)] +#![recursion_limit="256"] + #[macro_use] extern crate rustc; extern crate rustc_typeck; @@ -54,6 +56,9 @@ extern crate itertools; extern crate pulldown_cmark; extern crate url; +#[macro_use] +extern crate if_chain; + macro_rules! declare_restriction_lint { { pub $name:tt, $description:tt } => { declare_lint! { pub $name, Allow, $description } diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 9bc87dee68c6..e0ed66894580 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -37,63 +37,6 @@ pub use self::hir_utils::{SpanlessEq, SpanlessHash}; pub type MethodArgs = HirVec>; -/// Produce a nested chain of if-lets and ifs from the patterns: -/// -/// ```rust,ignore -/// if_let_chain! {[ -/// let Some(y) = x, -/// y.len() == 2, -/// let Some(z) = y, -/// ], { -/// block -/// }} -/// ``` -/// -/// becomes -/// -/// ```rust,ignore -/// if let Some(y) = x { -/// if y.len() == 2 { -/// if let Some(z) = y { -/// block -/// } -/// } -/// } -/// ``` -#[macro_export] -macro_rules! if_let_chain { - ([let $pat:pat = $expr:expr, $($tt:tt)+], $block:block) => { - if let $pat = $expr { - if_let_chain!{ [$($tt)+], $block } - } - }; - ([let $pat:pat = $expr:expr], $block:block) => { - if let $pat = $expr { - $block - } - }; - ([let $pat:pat = $expr:expr,], $block:block) => { - if let $pat = $expr { - $block - } - }; - ([$expr:expr, $($tt:tt)+], $block:block) => { - if $expr { - if_let_chain!{ [$($tt)+], $block } - } - }; - ([$expr:expr], $block:block) => { - if $expr { - $block - } - }; - ([$expr:expr,], $block:block) => { - if $expr { - $block - } - }; -} - pub mod higher; /// Returns true if the two spans come from differing expansions (i.e. one is