Skip to content

replace if_let_chain with if_chain #2154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 24, 2017
Merged

replace if_let_chain with if_chain #2154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
41840ae
mechanically swap if_let_chain -> if_chain
durka Oct 23, 2017
2153d1e
manual fixups if_let_chain -> if_chain
durka Oct 23, 2017
24a2c14
remove if_let_chain
durka Oct 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clippy_lints/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
13 changes: 7 additions & 6 deletions clippy_lints/src/assign_ops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
15 changes: 8 additions & 7 deletions clippy_lints/src/attrs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
}
}
Expand Down
43 changes: 22 additions & 21 deletions clippy_lints/src/bit_mask.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
});
}
}
}
}

Expand Down
98 changes: 50 additions & 48 deletions clippy_lints/src/bytecount.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, "..")));
});
});
}
};
}
}

Expand Down
64 changes: 33 additions & 31 deletions clippy_lints/src/collapsible_if.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer it if we didn't have to use an additional indentation level. Maybe use something like

if_chain! {
   ...
then {
   ...
}}

Opinions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with the indentation used in the crate's examples: https://docs.rs/if_chain/0.1.2/if_chain/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I can get behind staying with the official style

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.
Expand Down
75 changes: 38 additions & 37 deletions clippy_lints/src/derive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Foo> for Foo`
// For `impl PartialEq<B> 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<Foo> for Foo`
// For `impl PartialEq<B> 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.
Expand Down
Loading