Merge pull request #64 from Manishearth/refactoring

refactored Option usage and fn argument types, improved formatting

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);
+				fetch_int_literal(cx, right).map(|cmp_opt| 
-				if cmp_opt.is_some() {
+					check_compare(cx, left, cmp.node, cmp_opt, &e.span))
-					check_compare(cx, left, cmp.node, cmp_opt.unwrap(), &e.span);
+					.unwrap_or_else(|| fetch_int_literal(cx, left).map(|cmp_val| 
-				} else {
+						check_compare(cx, right, invert_cmp(cmp.node), cmp_val,
-					fetch_int_literal(cx, left).map(|cmp_val| 
+						 &e.span)).unwrap_or(()))
-						check_compare(cx, right, invert_cmp(cmp.node), cmp_val, &e.span));
-				}
 			}
 		}
     }

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)) => 
+		(&ExprMethodCall(ref lident, ref lcty, ref lmargs), 
-			lident.node == rident.node && is_ty_vec_equal(lcty, rcty) && is_exp_vec_equal(lmargs, rmargs),
+				&ExprMethodCall(ref rident, ref rcty, ref rmargs)) => 
-		(&ExprParen(ref lparen), &ExprParen(ref rparen)) => is_exp_equal(lparen, rparen),
+			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)) => 
+		(&ExprPath(ref lqself, ref lsubpath), 
-			both(lqself, rqself, |l, r| is_qself_equal(l, r)) && is_path_equal(lsubpath, rsubpath),		
+				&ExprPath(ref rqself, ref rsubpath)) => 
-		(&ExprTup(ref ltup), &ExprTup(ref rtup)) => is_exp_vec_equal(ltup, rtup),
+			both(lqself, rqself, |l, r| is_qself_equal(l, r)) && 
-		(&ExprUnary(lunop, ref lparam), &ExprUnary(runop, ref rparam)) => lunop == runop && is_exp_equal(lparam, rparam), 
+				is_path_equal(lsubpath, rsubpath),		
-		(&ExprVec(ref lvec), &ExprVec(ref rvec)) => is_exp_vec_equal(lvec, rvec),
+		(&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<P<Expr>>, right : &Vec<P<Expr>>) -> bool {
+fn is_exps_equal(left : &[P<Expr>], right : &[P<Expr>]) -> 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),
+	(&TyBareFn(ref lbare), &TyBareFn(ref rbare)) => 
-    (&TyTup(ref ltup), &TyTup(ref rtup)) => is_ty_vec_equal(ltup, rtup),
+		is_bare_fn_ty_equal(lbare, rbare),
-	(&TyPath(Option::None, ref lpath), &TyPath(Option::None, ref rpath)) => is_path_equal(lpath, rpath),
+    (&TyTup(ref ltup), &TyTup(ref rtup)) => is_tys_equal(ltup, rtup),
-	(&TyPath(Option::Some(ref lqself), ref lsubpath), &TyPath(Option::Some(ref rqself), ref rsubpath)) =>
+	(&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<FnDecl>, right : &P<FnDecl>) -> 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<Arg>, right : &Vec<Arg>) -> 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),
+		lmode == rmode && is_ident_equal(&lident.node, &rident.node) && 
-    (&PatEnum(ref lpath, Option::None), &PatEnum(ref rpath, Option::None)) => is_path_equal(lpath, rpath),
+			is_pat_equal(lpat, rpat),
-    (&PatEnum(ref lpath, Option::Some(ref lenum)), &PatEnum(ref rpath, Option::Some(ref renum))) => 
+    (&PatEnum(ref lpath, Option::None), &PatEnum(ref rpath, Option::None)) => 
-		is_path_equal(lpath, rpath) && is_pat_vec_equal(lenum, renum),  
+		is_path_equal(lpath, rpath),
-    (&PatStruct(ref lpath, ref lfieldpat, lbool), &PatStruct(ref rpath, ref rfieldpat, rbool)) =>
+    (&PatEnum(ref lpath, Option::Some(ref lenum)), 
-		lbool == rbool && is_path_equal(lpath, rpath) && is_spanned_fieldpat_vec_equal(lfieldpat, rfieldpat),
+			&PatEnum(ref rpath, Option::Some(ref renum))) => 
-    (&PatTup(ref ltup), &PatTup(ref rtup)) => is_pat_vec_equal(ltup, rtup), 
+		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)) =>
+    (&PatVec(ref lfirst, Option::None, ref llast), 
-		is_pat_vec_equal(lfirst, rfirst) && is_pat_vec_equal(llast, rlast),
+			&PatVec(ref rfirst, Option::None, ref 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_pats_equal(llast, rlast),
-		is_pat_vec_equal(lfirst, rfirst) && is_pat_equal(lpat, rpat) && 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_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<code::Spanned<FieldPat>>, right : &Vec<code::Spanned<FieldPat>>) -> bool {
+fn is_spanned_fieldpats_equal(left : &[code::Spanned<FieldPat>], 
+		right : &[code::Spanned<FieldPat>]) -> 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<P<Pat>>, right : &Vec<P<Pat>>) -> bool {
+fn is_pats_equal(left : &[P<Pat>], right : &[P<Pat>]) -> 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<LifetimeDef>, right : &Vec<LifetimeDef>) -> 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<P<Ty>>, right : &Vec<P<Ty>>) -> bool {
+fn is_tys_equal(left : &[P<Ty>], right : &[P<Ty>]) -> bool {
 	over(left, right, |l, r| is_ty_equal(l, r))
 }
 
-fn over<X, F>(left: &[X], right: &[X], mut eq_fn: F) -> bool where F: FnMut(&X, &X) -> bool {
+fn over<X, F>(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<X, F>(l: &Option<X>, r: &Option<X>, mut eq_fn : F) -> bool where F: FnMut(&X, &X) -> bool {
+fn both<X, F>(l: &Option<X>, r: &Option<X>, mut eq_fn : F) -> bool 
-	if l.is_none() { r.is_none() } else { r.is_some() && eq_fn(l.as_ref().unwrap(), &r.as_ref().unwrap()) }
+		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
     }
 }

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())))

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 {
+			}).unwrap_or_else(|| {
-				if let ty_rptr(_, mt{ty: _, mutbl: MutMutable}) = expr_ty(cx.tcx, e).sty {
+				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() {
+		unwrap_mut(ty).and_then(unwrap_mut).map(|_| cx.span_lint(MUT_MUT, 
-			cx.span_lint(MUT_MUT, ty.span, 
+			ty.span, "Generally you want to avoid &mut &mut _ if possible.")).
-				"Generally you want to avoid &mut &mut _ if possible.")
+			unwrap_or(())
-		}
 	}
 }
 

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() { 
+	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 '&Vec<_>' instead of '&[_]' involves one more reference and cannot be used with non-vec-based slices. Consider changing the type to &[...]");
+			"Writing '&String' instead of '&str' involves a new Object \
-	} else { if match_ty_unwrap(ty, &["String"]).is_some() {
+			where a slices will do. Consider changing the type to &str")
-			cx.span_lint(PTR_ARG, span,
+	}).unwrap_or(()));
-				"Writing '&String' instead of '&str' involves a new Object where a slices will do. Consider changing the type to &str");
-		}
-	}
 }