diff --git a/src/bit_mask.rs b/src/bit_mask.rs index 42e3003d3..d84da654e 100644 --- a/src/bit_mask.rs +++ b/src/bit_mask.rs @@ -49,13 +49,11 @@ impl LintPass for BitMask { fn check_expr(&mut self, cx: &Context, e: &Expr) { if let ExprBinary(ref cmp, ref left, ref right) = e.node { if is_comparison_binop(cmp.node) { - let cmp_opt = fetch_int_literal(cx, right); - if cmp_opt.is_some() { - check_compare(cx, left, cmp.node, cmp_opt.unwrap(), &e.span); - } else { - fetch_int_literal(cx, left).map(|cmp_val| - check_compare(cx, right, invert_cmp(cmp.node), cmp_val, &e.span)); - } + fetch_int_literal(cx, right).map(|cmp_opt| + check_compare(cx, left, cmp.node, cmp_opt, &e.span)) + .unwrap_or_else(|| fetch_int_literal(cx, left).map(|cmp_val| + check_compare(cx, right, invert_cmp(cmp.node), cmp_val, + &e.span)).unwrap_or(())) } } } diff --git a/src/eq_op.rs b/src/eq_op.rs index e0b722a0a..022382044 100644 --- a/src/eq_op.rs +++ b/src/eq_op.rs @@ -21,7 +21,9 @@ impl LintPass for EqOp { fn check_expr(&mut self, cx: &Context, e: &Expr) { if let ExprBinary(ref op, ref left, ref right) = e.node { if is_cmp_or_bit(op) && is_exp_equal(left, right) { - cx.span_lint(EQ_OP, e.span, &format!("equal expressions as operands to {}", ast_util::binop_to_string(op.node))); + cx.span_lint(EQ_OP, e.span, &format!( + "equal expressions as operands to {}", + ast_util::binop_to_string(op.node))); } } } @@ -29,32 +31,39 @@ impl LintPass for EqOp { fn is_exp_equal(left : &Expr, right : &Expr) -> bool { match (&left.node, &right.node) { - (&ExprBinary(ref lop, ref ll, ref lr), &ExprBinary(ref rop, ref rl, ref rr)) => + (&ExprBinary(ref lop, ref ll, ref lr), + &ExprBinary(ref rop, ref rl, ref rr)) => lop.node == rop.node && is_exp_equal(ll, rl) && is_exp_equal(lr, rr), (&ExprBox(ref lpl, ref lboxedpl), &ExprBox(ref rpl, ref rboxedpl)) => - both(lpl, rpl, |l, r| is_exp_equal(l, r)) && is_exp_equal(lboxedpl, rboxedpl), + both(lpl, rpl, |l, r| is_exp_equal(l, r)) && + is_exp_equal(lboxedpl, rboxedpl), (&ExprCall(ref lcallee, ref largs), &ExprCall(ref rcallee, ref rargs)) => - is_exp_equal(lcallee, rcallee) && is_exp_vec_equal(largs, rargs), + is_exp_equal(lcallee, rcallee) && is_exps_equal(largs, rargs), (&ExprCast(ref lcast, ref lty), &ExprCast(ref rcast, ref rty)) => is_ty_equal(lty, rty) && is_exp_equal(lcast, rcast), - (&ExprField(ref lfexp, ref lfident), &ExprField(ref rfexp, ref rfident)) => + (&ExprField(ref lfexp, ref lfident), + &ExprField(ref rfexp, ref rfident)) => lfident.node == rfident.node && is_exp_equal(lfexp, rfexp), (&ExprLit(ref llit), &ExprLit(ref rlit)) => llit.node == rlit.node, - (&ExprMethodCall(ref lident, ref lcty, ref lmargs), &ExprMethodCall(ref rident, ref rcty, ref rmargs)) => - lident.node == rident.node && is_ty_vec_equal(lcty, rcty) && is_exp_vec_equal(lmargs, rmargs), - (&ExprParen(ref lparen), &ExprParen(ref rparen)) => is_exp_equal(lparen, rparen), + (&ExprMethodCall(ref lident, ref lcty, ref lmargs), + &ExprMethodCall(ref rident, ref rcty, ref rmargs)) => + lident.node == rident.node && is_tys_equal(lcty, rcty) && + is_exps_equal(lmargs, rmargs), (&ExprParen(ref lparen), _) => is_exp_equal(lparen, right), (_, &ExprParen(ref rparen)) => is_exp_equal(left, rparen), - (&ExprPath(ref lqself, ref lsubpath), &ExprPath(ref rqself, ref rsubpath)) => - both(lqself, rqself, |l, r| is_qself_equal(l, r)) && is_path_equal(lsubpath, rsubpath), - (&ExprTup(ref ltup), &ExprTup(ref rtup)) => is_exp_vec_equal(ltup, rtup), - (&ExprUnary(lunop, ref lparam), &ExprUnary(runop, ref rparam)) => lunop == runop && is_exp_equal(lparam, rparam), - (&ExprVec(ref lvec), &ExprVec(ref rvec)) => is_exp_vec_equal(lvec, rvec), + (&ExprPath(ref lqself, ref lsubpath), + &ExprPath(ref rqself, ref rsubpath)) => + both(lqself, rqself, |l, r| is_qself_equal(l, r)) && + is_path_equal(lsubpath, rsubpath), + (&ExprTup(ref ltup), &ExprTup(ref rtup)) => is_exps_equal(ltup, rtup), + (&ExprUnary(lunop, ref lparam), &ExprUnary(runop, ref rparam)) => + lunop == runop && is_exp_equal(lparam, rparam), + (&ExprVec(ref lvec), &ExprVec(ref rvec)) => is_exps_equal(lvec, rvec), _ => false } } -fn is_exp_vec_equal(left : &Vec>, right : &Vec>) -> bool { +fn is_exps_equal(left : &[P], right : &[P]) -> bool { over(left, right, |l, r| is_exp_equal(l, r)) } @@ -69,19 +78,25 @@ fn is_qself_equal(left : &QSelf, right : &QSelf) -> bool { fn is_ty_equal(left : &Ty, right : &Ty) -> bool { match (&left.node, &right.node) { (&TyVec(ref lvec), &TyVec(ref rvec)) => is_ty_equal(lvec, rvec), - (&TyFixedLengthVec(ref lfvty, ref lfvexp), &TyFixedLengthVec(ref rfvty, ref rfvexp)) => + (&TyFixedLengthVec(ref lfvty, ref lfvexp), + &TyFixedLengthVec(ref rfvty, ref rfvexp)) => is_ty_equal(lfvty, rfvty) && is_exp_equal(lfvexp, rfvexp), (&TyPtr(ref lmut), &TyPtr(ref rmut)) => is_mut_ty_equal(lmut, rmut), (&TyRptr(ref ltime, ref lrmut), &TyRptr(ref rtime, ref rrmut)) => both(ltime, rtime, is_lifetime_equal) && is_mut_ty_equal(lrmut, rrmut), - (&TyBareFn(ref lbare), &TyBareFn(ref rbare)) => is_bare_fn_ty_equal(lbare, rbare), - (&TyTup(ref ltup), &TyTup(ref rtup)) => is_ty_vec_equal(ltup, rtup), - (&TyPath(Option::None, ref lpath), &TyPath(Option::None, ref rpath)) => is_path_equal(lpath, rpath), - (&TyPath(Option::Some(ref lqself), ref lsubpath), &TyPath(Option::Some(ref rqself), ref rsubpath)) => + (&TyBareFn(ref lbare), &TyBareFn(ref rbare)) => + is_bare_fn_ty_equal(lbare, rbare), + (&TyTup(ref ltup), &TyTup(ref rtup)) => is_tys_equal(ltup, rtup), + (&TyPath(Option::None, ref lpath), &TyPath(Option::None, ref rpath)) => + is_path_equal(lpath, rpath), + (&TyPath(Option::Some(ref lqself), ref lsubpath), + &TyPath(Option::Some(ref rqself), ref rsubpath)) => is_qself_equal(lqself, rqself) && is_path_equal(lsubpath, rsubpath), - (&TyObjectSum(ref lsumty, ref lobounds), &TyObjectSum(ref rsumty, ref robounds)) => + (&TyObjectSum(ref lsumty, ref lobounds), + &TyObjectSum(ref rsumty, ref robounds)) => is_ty_equal(lsumty, rsumty) && is_param_bounds_equal(lobounds, robounds), - (&TyPolyTraitRef(ref ltbounds), &TyPolyTraitRef(ref rtbounds)) => is_param_bounds_equal(ltbounds, rtbounds), + (&TyPolyTraitRef(ref ltbounds), &TyPolyTraitRef(ref rtbounds)) => + is_param_bounds_equal(ltbounds, rtbounds), (&TyParen(ref lty), &TyParen(ref rty)) => is_ty_equal(lty, rty), (&TyTypeof(ref lof), &TyTypeof(ref rof)) => is_exp_equal(lof, rof), (&TyInfer, &TyInfer) => true, @@ -91,15 +106,17 @@ fn is_ty_equal(left : &Ty, right : &Ty) -> bool { fn is_param_bound_equal(left : &TyParamBound, right : &TyParamBound) -> bool { match(left, right) { - (&TraitTyParamBound(ref lpoly, ref lmod), &TraitTyParamBound(ref rpoly, ref rmod)) => + (&TraitTyParamBound(ref lpoly, ref lmod), + &TraitTyParamBound(ref rpoly, ref rmod)) => lmod == rmod && is_poly_traitref_equal(lpoly, rpoly), - (&RegionTyParamBound(ref ltime), &RegionTyParamBound(ref rtime)) => is_lifetime_equal(ltime, rtime), + (&RegionTyParamBound(ref ltime), &RegionTyParamBound(ref rtime)) => + is_lifetime_equal(ltime, rtime), _ => false } } fn is_poly_traitref_equal(left : &PolyTraitRef, right : &PolyTraitRef) -> bool { - is_lifetimedef_vec_equal(&left.bound_lifetimes, &right.bound_lifetimes) && + is_lifetimedefs_equal(&left.bound_lifetimes, &right.bound_lifetimes) && is_path_equal(&left.trait_ref.path, &right.trait_ref.path) } @@ -113,11 +130,12 @@ fn is_mut_ty_equal(left : &MutTy, right : &MutTy) -> bool { fn is_bare_fn_ty_equal(left : &BareFnTy, right : &BareFnTy) -> bool { left.unsafety == right.unsafety && left.abi == right.abi && - is_lifetimedef_vec_equal(&left.lifetimes, &right.lifetimes) && is_fndecl_equal(&left.decl, &right.decl) + is_lifetimedefs_equal(&left.lifetimes, &right.lifetimes) && + is_fndecl_equal(&left.decl, &right.decl) } fn is_fndecl_equal(left : &P, right : &P) -> bool { - left.variadic == right.variadic && is_arg_vec_equal(&left.inputs, &right.inputs) && + left.variadic == right.variadic && is_args_equal(&left.inputs, &right.inputs) && is_fnret_ty_equal(&left.output, &right.output) } @@ -133,44 +151,56 @@ fn is_arg_equal(left : &Arg, right : &Arg) -> bool { is_ty_equal(&left.ty, &right.ty) && is_pat_equal(&left.pat, &right.pat) } -fn is_arg_vec_equal(left : &Vec, right : &Vec) -> bool { +fn is_args_equal(left : &[Arg], right : &[Arg]) -> bool { over(left, right, is_arg_equal) } fn is_pat_equal(left : &Pat, right : &Pat) -> bool { match(&left.node, &right.node) { (&PatWild(lwild), &PatWild(rwild)) => lwild == rwild, - (&PatIdent(ref lmode, ref lident, Option::None), &PatIdent(ref rmode, ref rident, Option::None)) => + (&PatIdent(ref lmode, ref lident, Option::None), + &PatIdent(ref rmode, ref rident, Option::None)) => lmode == rmode && is_ident_equal(&lident.node, &rident.node), (&PatIdent(ref lmode, ref lident, Option::Some(ref lpat)), &PatIdent(ref rmode, ref rident, Option::Some(ref rpat))) => - lmode == rmode && is_ident_equal(&lident.node, &rident.node) && is_pat_equal(lpat, rpat), - (&PatEnum(ref lpath, Option::None), &PatEnum(ref rpath, Option::None)) => is_path_equal(lpath, rpath), - (&PatEnum(ref lpath, Option::Some(ref lenum)), &PatEnum(ref rpath, Option::Some(ref renum))) => - is_path_equal(lpath, rpath) && is_pat_vec_equal(lenum, renum), - (&PatStruct(ref lpath, ref lfieldpat, lbool), &PatStruct(ref rpath, ref rfieldpat, rbool)) => - lbool == rbool && is_path_equal(lpath, rpath) && is_spanned_fieldpat_vec_equal(lfieldpat, rfieldpat), - (&PatTup(ref ltup), &PatTup(ref rtup)) => is_pat_vec_equal(ltup, rtup), + lmode == rmode && is_ident_equal(&lident.node, &rident.node) && + is_pat_equal(lpat, rpat), + (&PatEnum(ref lpath, Option::None), &PatEnum(ref rpath, Option::None)) => + is_path_equal(lpath, rpath), + (&PatEnum(ref lpath, Option::Some(ref lenum)), + &PatEnum(ref rpath, Option::Some(ref renum))) => + is_path_equal(lpath, rpath) && is_pats_equal(lenum, renum), + (&PatStruct(ref lpath, ref lfieldpat, lbool), + &PatStruct(ref rpath, ref rfieldpat, rbool)) => + lbool == rbool && is_path_equal(lpath, rpath) && + is_spanned_fieldpats_equal(lfieldpat, rfieldpat), + (&PatTup(ref ltup), &PatTup(ref rtup)) => is_pats_equal(ltup, rtup), (&PatBox(ref lboxed), &PatBox(ref rboxed)) => is_pat_equal(lboxed, rboxed), - (&PatRegion(ref lpat, ref lmut), &PatRegion(ref rpat, ref rmut)) => is_pat_equal(lpat, rpat) && lmut == rmut, + (&PatRegion(ref lpat, ref lmut), &PatRegion(ref rpat, ref rmut)) => + is_pat_equal(lpat, rpat) && lmut == rmut, (&PatLit(ref llit), &PatLit(ref rlit)) => is_exp_equal(llit, rlit), (&PatRange(ref lfrom, ref lto), &PatRange(ref rfrom, ref rto)) => is_exp_equal(lfrom, rfrom) && is_exp_equal(lto, rto), - (&PatVec(ref lfirst, Option::None, ref llast), &PatVec(ref rfirst, Option::None, ref rlast)) => - is_pat_vec_equal(lfirst, rfirst) && is_pat_vec_equal(llast, rlast), - (&PatVec(ref lfirst, Option::Some(ref lpat), ref llast), &PatVec(ref rfirst, Option::Some(ref rpat), ref rlast)) => - is_pat_vec_equal(lfirst, rfirst) && is_pat_equal(lpat, rpat) && is_pat_vec_equal(llast, rlast), + (&PatVec(ref lfirst, Option::None, ref llast), + &PatVec(ref rfirst, Option::None, ref rlast)) => + is_pats_equal(lfirst, rfirst) && is_pats_equal(llast, rlast), + (&PatVec(ref lfirst, Option::Some(ref lpat), ref llast), + &PatVec(ref rfirst, Option::Some(ref rpat), ref rlast)) => + is_pats_equal(lfirst, rfirst) && is_pat_equal(lpat, rpat) && + is_pats_equal(llast, rlast), // I don't match macros for now, the code is slow enough as is ;-) _ => false } } -fn is_spanned_fieldpat_vec_equal(left : &Vec>, right : &Vec>) -> bool { +fn is_spanned_fieldpats_equal(left : &[code::Spanned], + right : &[code::Spanned]) -> bool { over(left, right, |l, r| is_fieldpat_equal(&l.node, &r.node)) } fn is_fieldpat_equal(left : &FieldPat, right : &FieldPat) -> bool { - left.is_shorthand == right.is_shorthand && is_ident_equal(&left.ident, &right.ident) && + left.is_shorthand == right.is_shorthand && + is_ident_equal(&left.ident, &right.ident) && is_pat_equal(&left.pat, &right.pat) } @@ -178,15 +208,16 @@ fn is_ident_equal(left : &Ident, right : &Ident) -> bool { &left.name == &right.name && left.ctxt == right.ctxt } -fn is_pat_vec_equal(left : &Vec>, right : &Vec>) -> bool { +fn is_pats_equal(left : &[P], right : &[P]) -> bool { over(left, right, |l, r| is_pat_equal(l, r)) } fn is_lifetimedef_equal(left : &LifetimeDef, right : &LifetimeDef) -> bool { - is_lifetime_equal(&left.lifetime, &right.lifetime) && over(&left.bounds, &right.bounds, is_lifetime_equal) + is_lifetime_equal(&left.lifetime, &right.lifetime) && + over(&left.bounds, &right.bounds, is_lifetime_equal) } -fn is_lifetimedef_vec_equal(left : &Vec, right : &Vec) -> bool { +fn is_lifetimedefs_equal(left : &[LifetimeDef], right : &[LifetimeDef]) -> bool { over(left, right, is_lifetimedef_equal) } @@ -194,21 +225,25 @@ fn is_lifetime_equal(left : &Lifetime, right : &Lifetime) -> bool { left.name == right.name } -fn is_ty_vec_equal(left : &Vec>, right : &Vec>) -> bool { +fn is_tys_equal(left : &[P], right : &[P]) -> bool { over(left, right, |l, r| is_ty_equal(l, r)) } -fn over(left: &[X], right: &[X], mut eq_fn: F) -> bool where F: FnMut(&X, &X) -> bool { +fn over(left: &[X], right: &[X], mut eq_fn: F) -> bool + where F: FnMut(&X, &X) -> bool { left.len() == right.len() && left.iter().zip(right).all(|(x, y)| eq_fn(x, y)) } -fn both(l: &Option, r: &Option, mut eq_fn : F) -> bool where F: FnMut(&X, &X) -> bool { - if l.is_none() { r.is_none() } else { r.is_some() && eq_fn(l.as_ref().unwrap(), &r.as_ref().unwrap()) } +fn both(l: &Option, r: &Option, mut eq_fn : F) -> bool + where F: FnMut(&X, &X) -> bool { + l.as_ref().map(|x| r.as_ref().map(|y| eq_fn(x, y)).unwrap_or(false)) + .unwrap_or_else(|| r.is_none()) } fn is_cmp_or_bit(op : &BinOp) -> bool { match op.node { - BiEq | BiLt | BiLe | BiGt | BiGe | BiNe | BiAnd | BiOr | BiBitXor | BiBitAnd | BiBitOr => true, + BiEq | BiLt | BiLe | BiGt | BiGe | BiNe | BiAnd | BiOr | BiBitXor | + BiBitAnd | BiBitOr => true, _ => false } } diff --git a/src/misc.rs b/src/misc.rs index 5f5b07a15..c225c83ba 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -239,7 +239,7 @@ fn check_to_owned(cx: &Context, expr: &Expr, other_span: Span) { let name = ident.as_str(); if name == "to_string" || name == "to_owned" { cx.span_lint(CMP_OWNED, expr.span, &format!( - "this creates an owned instance just for comparison. + "this creates an owned instance just for comparison. \ Consider using {}.as_slice() to compare without allocation", cx.sess().codemap().span_to_snippet(other_span).unwrap_or( "..".to_string()))) @@ -250,7 +250,7 @@ fn check_to_owned(cx: &Context, expr: &Expr, other_span: Span) { if path.segments.iter().zip(["String", "from_str"].iter()).all( |(seg, name)| &seg.identifier.as_str() == name) { cx.span_lint(CMP_OWNED, expr.span, &format!( - "this creates an owned instance just for comparison. + "this creates an owned instance just for comparison. \ Consider using {}.as_slice() to compare without allocation", cx.sess().codemap().span_to_snippet(other_span).unwrap_or( "..".to_string()))) diff --git a/src/mut_mut.rs b/src/mut_mut.rs index 2a71b938d..569f7b813 100644 --- a/src/mut_mut.rs +++ b/src/mut_mut.rs @@ -24,23 +24,24 @@ impl LintPass for MutMut { } unwrap_addr(expr).map(|e| { - if unwrap_addr(e).is_some() { + unwrap_addr(e).map(|_| { cx.span_lint(MUT_MUT, expr.span, "Generally you want to avoid &mut &mut _ if possible.") - } else { - if let ty_rptr(_, mt{ty: _, mutbl: MutMutable}) = expr_ty(cx.tcx, e).sty { + }).unwrap_or_else(|| { + if let ty_rptr(_, mt{ty: _, mutbl: MutMutable}) = + expr_ty(cx.tcx, e).sty { cx.span_lint(MUT_MUT, expr.span, - "This expression mutably borrows a mutable reference. Consider reborrowing") + "This expression mutably borrows a mutable reference. \ + Consider reborrowing") } - } - }); + }) + }).unwrap_or(()) } fn check_ty(&mut self, cx: &Context, ty: &Ty) { - if unwrap_mut(ty).and_then(unwrap_mut).is_some() { - cx.span_lint(MUT_MUT, ty.span, - "Generally you want to avoid &mut &mut _ if possible.") - } + unwrap_mut(ty).and_then(unwrap_mut).map(|_| cx.span_lint(MUT_MUT, + ty.span, "Generally you want to avoid &mut &mut _ if possible.")). + unwrap_or(()) } } diff --git a/src/ptr_arg.rs b/src/ptr_arg.rs index 378d30de5..f8f056e79 100644 --- a/src/ptr_arg.rs +++ b/src/ptr_arg.rs @@ -18,7 +18,6 @@ declare_lint! { "Warn on declaration of a &Vec- or &String-typed method argument" } - #[derive(Copy,Clone)] pub struct PtrArg; @@ -58,12 +57,13 @@ fn check_fn(cx: &Context, decl: &FnDecl) { } fn check_ptr_subtype(cx: &Context, span: Span, ty: &Ty) { - if match_ty_unwrap(ty, &["Vec"]).is_some() { - cx.span_lint(PTR_ARG, span, - "Writing '&Vec<_>' instead of '&[_]' involves one more reference and cannot be used with non-vec-based slices. Consider changing the type to &[...]"); - } else { if match_ty_unwrap(ty, &["String"]).is_some() { - cx.span_lint(PTR_ARG, span, - "Writing '&String' instead of '&str' involves a new Object where a slices will do. Consider changing the type to &str"); - } - } + match_ty_unwrap(ty, &["Vec"]).map(|_| { + cx.span_lint(PTR_ARG, span, "Writing '&Vec<_>' instead of '&[_]' \ + involves one more reference and cannot be used with non-vec-based \ + slices. Consider changing the type to &[...]") + }).unwrap_or_else(|| match_ty_unwrap(ty, &["String"]).map(|_| { + cx.span_lint(PTR_ARG, span, + "Writing '&String' instead of '&str' involves a new Object \ + where a slices will do. Consider changing the type to &str") + }).unwrap_or(())); }