diff options
| author | Manish Goregaokar <manishsmail@gmail.com> | 2016-01-04 09:56:12 +0530 |
|---|---|---|
| committer | Manish Goregaokar <manishsmail@gmail.com> | 2016-01-04 09:56:12 +0530 |
| commit | c9342d01213ca1663d2cdf23289bae024823ae6a (patch) | |
| tree | 505829eeee2dc85b41f7e2aa7cc44df3d9862a9e /src | |
| parent | 839ad09689e417d1d9e84eb24c627226765f8322 (diff) | |
| download | rust-c9342d01213ca1663d2cdf23289bae024823ae6a.tar.gz rust-c9342d01213ca1663d2cdf23289bae024823ae6a.zip | |
fmt clippy
Diffstat (limited to 'src')
41 files changed, 2186 insertions, 1487 deletions
diff --git a/src/approx_const.rs b/src/approx_const.rs index 05829838903..2c8779ae737 100644 --- a/src/approx_const.rs +++ b/src/approx_const.rs @@ -21,24 +21,22 @@ declare_lint! { } // Tuples are of the form (constant, name, min_digits) -const KNOWN_CONSTS : &'static [(f64, &'static str, usize)] = &[ - (f64::E, "E", 4), - (f64::FRAC_1_PI, "FRAC_1_PI", 4), - (f64::FRAC_1_SQRT_2, "FRAC_1_SQRT_2", 5), - (f64::FRAC_2_PI, "FRAC_2_PI", 5), - (f64::FRAC_2_SQRT_PI, "FRAC_2_SQRT_PI", 5), - (f64::FRAC_PI_2, "FRAC_PI_2", 5), - (f64::FRAC_PI_3, "FRAC_PI_3", 5), - (f64::FRAC_PI_4, "FRAC_PI_4", 5), - (f64::FRAC_PI_6, "FRAC_PI_6", 5), - (f64::FRAC_PI_8, "FRAC_PI_8", 5), - (f64::LN_10, "LN_10", 5), - (f64::LN_2, "LN_2", 5), - (f64::LOG10_E, "LOG10_E", 5), - (f64::LOG2_E, "LOG2_E", 5), - (f64::PI, "PI", 3), - (f64::SQRT_2, "SQRT_2", 5), -]; +const KNOWN_CONSTS: &'static [(f64, &'static str, usize)] = &[(f64::E, "E", 4), + (f64::FRAC_1_PI, "FRAC_1_PI", 4), + (f64::FRAC_1_SQRT_2, "FRAC_1_SQRT_2", 5), + (f64::FRAC_2_PI, "FRAC_2_PI", 5), + (f64::FRAC_2_SQRT_PI, "FRAC_2_SQRT_PI", 5), + (f64::FRAC_PI_2, "FRAC_PI_2", 5), + (f64::FRAC_PI_3, "FRAC_PI_3", 5), + (f64::FRAC_PI_4, "FRAC_PI_4", 5), + (f64::FRAC_PI_6, "FRAC_PI_6", 5), + (f64::FRAC_PI_8, "FRAC_PI_8", 5), + (f64::LN_10, "LN_10", 5), + (f64::LN_2, "LN_2", 5), + (f64::LOG10_E, "LOG10_E", 5), + (f64::LOG2_E, "LOG2_E", 5), + (f64::PI, "PI", 3), + (f64::SQRT_2, "SQRT_2", 5)]; #[derive(Copy,Clone)] pub struct ApproxConstant; @@ -61,9 +59,8 @@ fn check_lit(cx: &LateContext, lit: &Lit, e: &Expr) { match lit.node { LitFloat(ref s, TyF32) => check_known_consts(cx, e, s, "f32"), LitFloat(ref s, TyF64) => check_known_consts(cx, e, s, "f64"), - LitFloatUnsuffixed(ref s) => - check_known_consts(cx, e, s, "f{32, 64}"), - _ => () + LitFloatUnsuffixed(ref s) => check_known_consts(cx, e, s, "f{32, 64}"), + _ => (), } } @@ -71,9 +68,10 @@ fn check_known_consts(cx: &LateContext, e: &Expr, s: &str, module: &str) { if let Ok(_) = s.parse::<f64>() { for &(constant, name, min_digits) in KNOWN_CONSTS { if is_approx_const(constant, s, min_digits) { - span_lint(cx, APPROX_CONSTANT, e.span, &format!( - "approximate value of `{}::{}` found. \ - Consider using it directly", module, &name)); + span_lint(cx, + APPROX_CONSTANT, + e.span, + &format!("approximate value of `{}::{}` found. Consider using it directly", module, &name)); return; } } diff --git a/src/array_indexing.rs b/src/array_indexing.rs index d72adac943f..cfa52f390d2 100644 --- a/src/array_indexing.rs +++ b/src/array_indexing.rs @@ -42,8 +42,7 @@ impl LateLintPass for ArrayIndexing { let index = eval_const_expr_partial(cx.tcx, &index, ExprTypeChecked, None); if let Ok(ConstVal::Uint(index)) = index { if size as u64 <= index { - span_lint(cx, OUT_OF_BOUNDS_INDEXING, e.span, - "const index-expr is out of bounds"); + span_lint(cx, OUT_OF_BOUNDS_INDEXING, e.span, "const index-expr is out of bounds"); } } } diff --git a/src/attrs.rs b/src/attrs.rs index f0dbf390ebb..ec2cfcb0efc 100644 --- a/src/attrs.rs +++ b/src/attrs.rs @@ -57,13 +57,15 @@ impl LateLintPass for AttrPass { fn is_relevant_item(item: &Item) -> bool { if let ItemFn(_, _, _, _, _, ref block) = item.node { is_relevant_block(block) - } else { false } + } else { + false + } } fn is_relevant_impl(item: &ImplItem) -> bool { match item.node { ImplItemKind::Method(_, ref block) => is_relevant_block(block), - _ => false + _ => false, } } @@ -71,7 +73,7 @@ fn is_relevant_trait(item: &TraitItem) -> bool { match item.node { MethodTraitItem(_, None) => true, MethodTraitItem(_, Some(ref block)) => is_relevant_block(block), - _ => false + _ => false, } } @@ -95,25 +97,33 @@ fn is_relevant_expr(expr: &Expr) -> bool { ExprCall(ref path_expr, _) => { if let ExprPath(_, ref path) = path_expr.node { !match_path(path, &BEGIN_UNWIND) - } else { true } + } else { + true + } } - _ => true + _ => true, } } -fn check_attrs(cx: &LateContext, span: Span, name: &Name, - attrs: &[Attribute]) { - if in_macro(cx, span) { return; } +fn check_attrs(cx: &LateContext, span: Span, name: &Name, attrs: &[Attribute]) { + if in_macro(cx, span) { + return; + } for attr in attrs { if let MetaList(ref inline, ref values) = attr.node.value.node { - if values.len() != 1 || inline != &"inline" { continue; } + if values.len() != 1 || inline != &"inline" { + continue; + } if let MetaWord(ref always) = values[0].node { - if always != &"always" { continue; } - span_lint(cx, INLINE_ALWAYS, attr.span, &format!( - "you have declared `#[inline(always)]` on `{}`. This \ - is usually a bad idea", - name)); + if always != &"always" { + continue; + } + span_lint(cx, + INLINE_ALWAYS, + attr.span, + &format!("you have declared `#[inline(always)]` on `{}`. This is usually a bad idea", + name)); } } } diff --git a/src/bit_mask.rs b/src/bit_mask.rs index f4310db0fcb..c2fd3742066 100644 --- a/src/bit_mask.rs +++ b/src/bit_mask.rs @@ -95,18 +95,22 @@ impl LateLintPass for BitMask { fn check_expr(&mut self, cx: &LateContext, e: &Expr) { if let ExprBinary(ref cmp, ref left, ref right) = e.node { if is_comparison_binop(cmp.node) { - fetch_int_literal(cx, right).map_or_else(|| - fetch_int_literal(cx, left).map_or((), |cmp_val| - check_compare(cx, right, invert_cmp(cmp.node), - cmp_val, &e.span)), - |cmp_opt| check_compare(cx, left, cmp.node, cmp_opt, - &e.span)) + fetch_int_literal(cx, right).map_or_else(|| { + fetch_int_literal(cx, left).map_or((), |cmp_val| { + check_compare(cx, + right, + invert_cmp(cmp.node), + cmp_val, + &e.span) + }) + }, + |cmp_opt| check_compare(cx, left, cmp.node, cmp_opt, &e.span)) } } } } -fn invert_cmp(cmp : BinOp_) -> BinOp_ { +fn invert_cmp(cmp: BinOp_) -> BinOp_ { match cmp { BiEq => BiEq, BiNe => BiNe, @@ -114,7 +118,7 @@ fn invert_cmp(cmp : BinOp_) -> BinOp_ { BiGt => BiLt, BiLe => BiGe, BiGe => BiLe, - _ => BiOr // Dummy + _ => BiOr, // Dummy } } @@ -124,114 +128,159 @@ fn check_compare(cx: &LateContext, bit_op: &Expr, cmp_op: BinOp_, cmp_value: u64 if op.node != BiBitAnd && op.node != BiBitOr { return; } - fetch_int_literal(cx, right).or_else(|| fetch_int_literal( - cx, left)).map_or((), |mask| check_bit_mask(cx, op.node, - cmp_op, mask, cmp_value, span)) + fetch_int_literal(cx, right) + .or_else(|| fetch_int_literal(cx, left)) + .map_or((), |mask| check_bit_mask(cx, op.node, cmp_op, mask, cmp_value, span)) } } -fn check_bit_mask(cx: &LateContext, bit_op: BinOp_, cmp_op: BinOp_, - mask_value: u64, cmp_value: u64, span: &Span) { +fn check_bit_mask(cx: &LateContext, bit_op: BinOp_, cmp_op: BinOp_, mask_value: u64, cmp_value: u64, span: &Span) { match cmp_op { - BiEq | BiNe => match bit_op { - BiBitAnd => if mask_value & cmp_value != cmp_value { - if cmp_value != 0 { - span_lint(cx, BAD_BIT_MASK, *span, &format!( - "incompatible bit mask: `_ & {}` can never be equal to `{}`", - mask_value, cmp_value)); + BiEq | BiNe => { + match bit_op { + BiBitAnd => { + if mask_value & cmp_value != cmp_value { + if cmp_value != 0 { + span_lint(cx, + BAD_BIT_MASK, + *span, + &format!("incompatible bit mask: `_ & {}` can never be equal to `{}`", + mask_value, + cmp_value)); + } + } else { + if mask_value == 0 { + span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero"); + } + } } - } else { - if mask_value == 0 { - span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero"); + BiBitOr => { + if mask_value | cmp_value != cmp_value { + span_lint(cx, + BAD_BIT_MASK, + *span, + &format!("incompatible bit mask: `_ | {}` can never be equal to `{}`", + mask_value, + cmp_value)); + } } - }, - BiBitOr => if mask_value | cmp_value != cmp_value { - span_lint(cx, BAD_BIT_MASK, *span, &format!( - "incompatible bit mask: `_ | {}` can never be equal to `{}`", - mask_value, cmp_value)); - }, - _ => () - }, - BiLt | BiGe => match bit_op { - BiBitAnd => if mask_value < cmp_value { - span_lint(cx, BAD_BIT_MASK, *span, &format!( - "incompatible bit mask: `_ & {}` will always be lower than `{}`", - mask_value, cmp_value)); - } else { - if mask_value == 0 { - span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero"); + _ => (), + } + } + BiLt | BiGe => { + match bit_op { + BiBitAnd => { + if mask_value < cmp_value { + span_lint(cx, + BAD_BIT_MASK, + *span, + &format!("incompatible bit mask: `_ & {}` will always be lower than `{}`", + mask_value, + cmp_value)); + } else { + if mask_value == 0 { + span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero"); + } + } } - }, - BiBitOr => if mask_value >= cmp_value { - span_lint(cx, BAD_BIT_MASK, *span, &format!( - "incompatible bit mask: `_ | {}` will never be lower than `{}`", - mask_value, cmp_value)); - } else { - check_ineffective_lt(cx, *span, mask_value, cmp_value, "|"); - }, - BiBitXor => - check_ineffective_lt(cx, *span, mask_value, cmp_value, "^"), - _ => () - }, - BiLe | BiGt => match bit_op { - BiBitAnd => if mask_value <= cmp_value { - span_lint(cx, BAD_BIT_MASK, *span, &format!( - "incompatible bit mask: `_ & {}` will never be higher than `{}`", - mask_value, cmp_value)); - } else { - if mask_value == 0 { - span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero"); + BiBitOr => { + if mask_value >= cmp_value { + span_lint(cx, + BAD_BIT_MASK, + *span, + &format!("incompatible bit mask: `_ | {}` will never be lower than `{}`", + mask_value, + cmp_value)); + } else { + check_ineffective_lt(cx, *span, mask_value, cmp_value, "|"); + } } - }, - BiBitOr => if mask_value > cmp_value { - span_lint(cx, BAD_BIT_MASK, *span, &format!( - "incompatible bit mask: `_ | {}` will always be higher than `{}`", - mask_value, cmp_value)); - } else { - check_ineffective_gt(cx, *span, mask_value, cmp_value, "|"); - }, - BiBitXor => - check_ineffective_gt(cx, *span, mask_value, cmp_value, "^"), - _ => () - }, - _ => () + BiBitXor => check_ineffective_lt(cx, *span, mask_value, cmp_value, "^"), + _ => (), + } + } + BiLe | BiGt => { + match bit_op { + BiBitAnd => { + if mask_value <= cmp_value { + span_lint(cx, + BAD_BIT_MASK, + *span, + &format!("incompatible bit mask: `_ & {}` will never be higher than `{}`", + mask_value, + cmp_value)); + } else { + if mask_value == 0 { + span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero"); + } + } + } + BiBitOr => { + if mask_value > cmp_value { + span_lint(cx, + BAD_BIT_MASK, + *span, + &format!("incompatible bit mask: `_ | {}` will always be higher than `{}`", + mask_value, + cmp_value)); + } else { + check_ineffective_gt(cx, *span, mask_value, cmp_value, "|"); + } + } + BiBitXor => check_ineffective_gt(cx, *span, mask_value, cmp_value, "^"), + _ => (), + } + } + _ => (), } } fn check_ineffective_lt(cx: &LateContext, span: Span, m: u64, c: u64, op: &str) { if c.is_power_of_two() && m < c { - span_lint(cx, INEFFECTIVE_BIT_MASK, span, &format!( - "ineffective bit mask: `x {} {}` compared to `{}`, is the same as x compared directly", - op, m, c)); + span_lint(cx, + INEFFECTIVE_BIT_MASK, + span, + &format!("ineffective bit mask: `x {} {}` compared to `{}`, is the same as x compared directly", + op, + m, + c)); } } fn check_ineffective_gt(cx: &LateContext, span: Span, m: u64, c: u64, op: &str) { if (c + 1).is_power_of_two() && m <= c { - span_lint(cx, INEFFECTIVE_BIT_MASK, span, &format!( - "ineffective bit mask: `x {} {}` compared to `{}`, is the same as x compared directly", - op, m, c)); + span_lint(cx, + INEFFECTIVE_BIT_MASK, + span, + &format!("ineffective bit mask: `x {} {}` compared to `{}`, is the same as x compared directly", + op, + m, + c)); } } -fn fetch_int_literal(cx: &LateContext, lit : &Expr) -> Option<u64> { +fn fetch_int_literal(cx: &LateContext, lit: &Expr) -> Option<u64> { match lit.node { ExprLit(ref lit_ptr) => { if let LitInt(value, _) = lit_ptr.node { Some(value) //TODO: Handle sign - } else { None } + } else { + None + } } ExprPath(_, _) => { - // Important to let the borrow expire before the const lookup to avoid double - // borrowing. - let def_map = cx.tcx.def_map.borrow(); - match def_map.get(&lit.id) { - Some(&PathResolution { base_def: DefConst(def_id), ..}) => Some(def_id), - _ => None + { + // Important to let the borrow expire before the const lookup to avoid double + // borrowing. + let def_map = cx.tcx.def_map.borrow(); + match def_map.get(&lit.id) { + Some(&PathResolution { base_def: DefConst(def_id), ..}) => Some(def_id), + _ => None, + } } + .and_then(|def_id| lookup_const_by_id(cx.tcx, def_id, None)) + .and_then(|l| fetch_int_literal(cx, l)) } - .and_then(|def_id| lookup_const_by_id(cx.tcx, def_id, None)) - .and_then(|l| fetch_int_literal(cx, l)), - _ => None + _ => None, } } diff --git a/src/block_in_if_condition.rs b/src/block_in_if_condition.rs index ce01f591c59..f7d8c95ee76 100644 --- a/src/block_in_if_condition.rs +++ b/src/block_in_if_condition.rs @@ -38,7 +38,7 @@ impl LintPass for BlockInIfCondition { } struct ExVisitor<'v> { - found_block: Option<&'v Expr> + found_block: Option<&'v Expr>, } impl<'v> Visitor<'v> for ExVisitor<'v> { @@ -51,7 +51,7 @@ impl<'v> Visitor<'v> for ExVisitor<'v> { if let Some(ref ex) = block.expr { match ex.node { ExprBlock(_) => true, - _ => false + _ => false, } } else { false @@ -59,7 +59,7 @@ impl<'v> Visitor<'v> for ExVisitor<'v> { } }; if complex { - self.found_block = Some(& expr); + self.found_block = Some(&expr); return; } } @@ -67,8 +67,9 @@ impl<'v> Visitor<'v> for ExVisitor<'v> { } } -const BRACED_EXPR_MESSAGE:&'static str = "omit braces around single expression condition"; -const COMPLEX_BLOCK_MESSAGE:&'static str = "in an 'if' condition, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a 'let'"; +const BRACED_EXPR_MESSAGE: &'static str = "omit braces around single expression condition"; +const COMPLEX_BLOCK_MESSAGE: &'static str = "in an 'if' condition, avoid complex blocks or closures with blocks; \ + instead, move the block or closure higher and bind it with a 'let'"; impl LateLintPass for BlockInIfCondition { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { @@ -82,29 +83,33 @@ impl LateLintPass for BlockInIfCondition { if differing_macro_contexts(expr.span, ex.span) { return; } - span_help_and_lint(cx, BLOCK_IN_IF_CONDITION_EXPR, check.span, - BRACED_EXPR_MESSAGE, - &format!("try\nif {} {} ... ", snippet_block(cx, ex.span, ".."), - snippet_block(cx, then.span, ".."))); + span_help_and_lint(cx, + BLOCK_IN_IF_CONDITION_EXPR, + check.span, + BRACED_EXPR_MESSAGE, + &format!("try\nif {} {} ... ", + snippet_block(cx, ex.span, ".."), + snippet_block(cx, then.span, ".."))); } } else { if differing_macro_contexts(expr.span, block.stmts[0].span) { return; } // move block higher - span_help_and_lint(cx, BLOCK_IN_IF_CONDITION_STMT, check.span, - COMPLEX_BLOCK_MESSAGE, - &format!("try\nlet res = {};\nif res {} ... ", - snippet_block(cx, block.span, ".."), - snippet_block(cx, then.span, ".."))); + span_help_and_lint(cx, + BLOCK_IN_IF_CONDITION_STMT, + check.span, + COMPLEX_BLOCK_MESSAGE, + &format!("try\nlet res = {};\nif res {} ... ", + snippet_block(cx, block.span, ".."), + snippet_block(cx, then.span, ".."))); } } } else { let mut visitor = ExVisitor { found_block: None }; walk_expr(&mut visitor, check); if let Some(ref block) = visitor.found_block { - span_help_and_lint(cx, BLOCK_IN_IF_CONDITION_STMT, block.span, - COMPLEX_BLOCK_MESSAGE, ""); + span_help_and_lint(cx, BLOCK_IN_IF_CONDITION_STMT, block.span, COMPLEX_BLOCK_MESSAGE, ""); } } } diff --git a/src/collapsible_if.rs b/src/collapsible_if.rs index 775e9ec95fc..4a17d3a4608 100644 --- a/src/collapsible_if.rs +++ b/src/collapsible_if.rs @@ -52,23 +52,26 @@ impl LateLintPass for CollapsibleIf { fn check_if(cx: &LateContext, e: &Expr) { if let ExprIf(ref check, ref then, None) = e.node { if let Some(&Expr{ node: ExprIf(ref check_inner, ref content, None), span: sp, ..}) = - single_stmt_of_block(then) { - if e.span.expn_id != sp.expn_id { - return; - } - span_help_and_lint(cx, COLLAPSIBLE_IF, e.span, - "this if statement can be collapsed", - &format!("try\nif {} && {} {}", - check_to_string(cx, check), check_to_string(cx, check_inner), - snippet_block(cx, content.span, ".."))); + single_stmt_of_block(then) { + if e.span.expn_id != sp.expn_id { + return; } + span_help_and_lint(cx, + COLLAPSIBLE_IF, + e.span, + "this if statement can be collapsed", + &format!("try\nif {} && {} {}", + check_to_string(cx, check), + check_to_string(cx, check_inner), + snippet_block(cx, content.span, ".."))); + } } } fn requires_brackets(e: &Expr) -> bool { match e.node { ExprBinary(Spanned {node: n, ..}, _, _) if n == BiEq => false, - _ => true + _ => true, } } @@ -84,16 +87,26 @@ fn single_stmt_of_block(block: &Block) -> Option<&Expr> { if block.stmts.len() == 1 && block.expr.is_none() { if let StmtExpr(ref expr, _) = block.stmts[0].node { single_stmt_of_expr(expr) - } else { None } + } else { + None + } } else { if block.stmts.is_empty() { - if let Some(ref p) = block.expr { Some(p) } else { None } - } else { None } + if let Some(ref p) = block.expr { + Some(p) + } else { + None + } + } else { + None + } } } fn single_stmt_of_expr(expr: &Expr) -> Option<&Expr> { if let ExprBlock(ref block) = expr.node { single_stmt_of_block(block) - } else { Some(expr) } + } else { + Some(expr) + } } diff --git a/src/consts.rs b/src/consts.rs index 9d3a9e7d7c2..171ba6f27f0 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -28,7 +28,7 @@ use syntax::ast::Sign::{self, Plus, Minus}; pub enum FloatWidth { Fw32, Fw64, - FwAny + FwAny, } impl From<FloatTy> for FloatWidth { @@ -85,9 +85,14 @@ impl Constant { match *self { ConstantByte(b) => Some(b as f64), ConstantFloat(ref s, _) => s.parse().ok(), - ConstantInt(i, ty) => Some(if is_negative(ty) { - -(i as f64) } else { i as f64 }), - _ => None + ConstantInt(i, ty) => { + Some(if is_negative(ty) { + -(i as f64) + } else { + i as f64 + }) + } + _ => None, } } } @@ -95,14 +100,14 @@ impl Constant { impl PartialEq for Constant { fn eq(&self, other: &Constant) -> bool { match (self, other) { - (&ConstantStr(ref ls, ref lsty), &ConstantStr(ref rs, ref rsty)) => - ls == rs && lsty == rsty, + (&ConstantStr(ref ls, ref lsty), &ConstantStr(ref rs, ref rsty)) => ls == rs && lsty == rsty, (&ConstantBinary(ref l), &ConstantBinary(ref r)) => l == r, (&ConstantByte(l), &ConstantByte(r)) => l == r, (&ConstantChar(l), &ConstantChar(r)) => l == r, - (&ConstantInt(lv, lty), &ConstantInt(rv, rty)) => lv == rv && - (is_negative(lty) & (lv != 0)) == (is_negative(rty) & (rv != 0)), - (&ConstantFloat(ref ls, lw), &ConstantFloat(ref rs, rw)) => + (&ConstantInt(lv, lty), &ConstantInt(rv, rty)) => { + lv == rv && (is_negative(lty) & (lv != 0)) == (is_negative(rty) & (rv != 0)) + } + (&ConstantFloat(ref ls, lw), &ConstantFloat(ref rs, rw)) => { if match (lw, rw) { (FwAny, _) | (_, FwAny) | (Fw32, Fw32) | (Fw64, Fw64) => true, _ => false, @@ -111,11 +116,13 @@ impl PartialEq for Constant { (Ok(l), Ok(r)) => l.eq(&r), _ => false, } - } else { false }, + } else { + false + } + } (&ConstantBool(l), &ConstantBool(r)) => l == r, (&ConstantVec(ref l), &ConstantVec(ref r)) => l == r, - (&ConstantRepeat(ref lv, ref ls), &ConstantRepeat(ref rv, ref rs)) => - ls == rs && lv == rv, + (&ConstantRepeat(ref lv, ref ls), &ConstantRepeat(ref rv, ref rs)) => ls == rs && lv == rv, (&ConstantTuple(ref l), &ConstantTuple(ref r)) => l == r, _ => false, //TODO: Are there inter-type equalities? } @@ -125,19 +132,24 @@ impl PartialEq for Constant { impl PartialOrd for Constant { fn partial_cmp(&self, other: &Constant) -> Option<Ordering> { match (self, other) { - (&ConstantStr(ref ls, ref lsty), &ConstantStr(ref rs, ref rsty)) => - if lsty != rsty { None } else { Some(ls.cmp(rs)) }, + (&ConstantStr(ref ls, ref lsty), &ConstantStr(ref rs, ref rsty)) => { + if lsty != rsty { + None + } else { + Some(ls.cmp(rs)) + } + } (&ConstantByte(ref l), &ConstantByte(ref r)) => Some(l.cmp(r)), (&ConstantChar(ref l), &ConstantChar(ref r)) => Some(l.cmp(r)), - (&ConstantInt(ref lv, lty), &ConstantInt(ref rv, rty)) => - Some(match (is_negative(lty) && *lv != 0, - is_negative(rty) && *rv != 0) { + (&ConstantInt(ref lv, lty), &ConstantInt(ref rv, rty)) => { + Some(match (is_negative(lty) && *lv != 0, is_negative(rty) && *rv != 0) { (true, true) => rv.cmp(lv), (false, false) => lv.cmp(rv), (true, false) => Less, (false, true) => Greater, - }), - (&ConstantFloat(ref ls, lw), &ConstantFloat(ref rs, rw)) => + }) + } + (&ConstantFloat(ref ls, lw), &ConstantFloat(ref rs, rw)) => { if match (lw, rw) { (FwAny, _) | (_, FwAny) | (Fw32, Fw32) | (Fw64, Fw64) => true, _ => false, @@ -146,17 +158,21 @@ impl PartialOrd for Constant { (Ok(ref l), Ok(ref r)) => l.partial_cmp(r), _ => None, } - } else { None }, + } else { + None + } + } (&ConstantBool(ref l), &ConstantBool(ref r)) => Some(l.cmp(r)), (&ConstantVec(ref l), &ConstantVec(ref r)) => l.partial_cmp(&r), - (&ConstantRepeat(ref lv, ref ls), &ConstantRepeat(ref rv, ref rs)) => + (&ConstantRepeat(ref lv, ref ls), &ConstantRepeat(ref rv, ref rs)) => { match lv.partial_cmp(rv) { Some(Equal) => Some(ls.cmp(rs)), x => x, - }, + } + } (&ConstantTuple(ref l), &ConstantTuple(ref r)) => l.partial_cmp(r), - _ => None, //TODO: Are there any useful inter-type orderings? - } + _ => None, //TODO: Are there any useful inter-type orderings? + } } } @@ -178,9 +194,11 @@ impl fmt::Display for Constant { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { match *self { ConstantStr(ref s, _) => write!(fmt, "{:?}", s), - ConstantByte(ref b) => - write!(fmt, "b'").and_then(|_| format_byte(fmt, *b)) - .and_then(|_| write!(fmt, "'")), + ConstantByte(ref b) => { + write!(fmt, "b'") + .and_then(|_| format_byte(fmt, *b)) + .and_then(|_| write!(fmt, "'")) + } ConstantBinary(ref bs) => { try!(write!(fmt, "b\"")); for b in bs.iter() { @@ -191,14 +209,23 @@ impl fmt::Display for Constant { ConstantChar(ref c) => write!(fmt, "'{}'", c), ConstantInt(ref i, ref ity) => { let (sign, suffix) = match *ity { - LitIntType::SignedIntLit(ref sity, ref sign) => - (if let Sign::Minus = *sign { "-" } else { "" }, - sity.ty_to_string()), - LitIntType::UnsignedIntLit(ref uity) => - ("", uity.ty_to_string()), - LitIntType::UnsuffixedIntLit(ref sign) => - (if let Sign::Minus = *sign { "-" } else { "" }, - "".into()), + LitIntType::SignedIntLit(ref sity, ref sign) => { + (if let Sign::Minus = *sign { + "-" + } else { + "" + }, + sity.ty_to_string()) + } + LitIntType::UnsignedIntLit(ref uity) => ("", uity.ty_to_string()), + LitIntType::UnsuffixedIntLit(ref sign) => { + (if let Sign::Minus = *sign { + "-" + } else { + "" + }, + "".into()) + } }; write!(fmt, "{}{}{}", sign, i, suffix) } @@ -212,12 +239,22 @@ impl fmt::Display for Constant { } ConstantBool(ref b) => write!(fmt, "{}", b), ConstantRepeat(ref c, ref n) => write!(fmt, "[{}; {}]", c, n), - ConstantVec(ref v) => write!(fmt, "[{}]", - v.iter().map(|i| format!("{}", i)) - .collect::<Vec<_>>().join(", ")), - ConstantTuple(ref t) => write!(fmt, "({})", - t.iter().map(|i| format!("{}", i)) - .collect::<Vec<_>>().join(", ")), + ConstantVec(ref v) => { + write!(fmt, + "[{}]", + v.iter() + .map(|i| format!("{}", i)) + .collect::<Vec<_>>() + .join(", ")) + } + ConstantTuple(ref t) => { + write!(fmt, + "({})", + t.iter() + .map(|i| format!("{}", i)) + .collect::<Vec<_>>() + .join(", ")) + } } } } @@ -242,7 +279,9 @@ fn constant_not(o: Constant) -> Option<Constant> { ConstantInt(value, ty) => { let (nvalue, nty) = match ty { SignedIntLit(ity, Plus) => { - if value == ::std::u64::MAX { return None; } + if value == ::std::u64::MAX { + return None; + } (value + 1, SignedIntLit(ity, Minus)) } SignedIntLit(ity, Minus) => { @@ -258,30 +297,40 @@ fn constant_not(o: Constant) -> Option<Constant> { UintTy::TyU16 => ::std::u16::MAX as u64, UintTy::TyU32 => ::std::u32::MAX as u64, UintTy::TyU64 => ::std::u64::MAX, - UintTy::TyUs => { return None; } // refuse to guess + UintTy::TyUs => { + return None; + } // refuse to guess }; (!value & mask, UnsignedIntLit(ity)) } - UnsuffixedIntLit(_) => { return None; } // refuse to guess + UnsuffixedIntLit(_) => { + return None; + } // refuse to guess }; ConstantInt(nvalue, nty) } - _ => { return None; } + _ => { + return None; + } }) } fn constant_negate(o: Constant) -> Option<Constant> { Some(match o { - ConstantInt(value, ty) => - ConstantInt(value, match ty { - SignedIntLit(ity, sign) => - SignedIntLit(ity, neg_sign(sign)), - UnsuffixedIntLit(sign) => UnsuffixedIntLit(neg_sign(sign)), - _ => { return None; } - }), - ConstantFloat(is, ty) => - ConstantFloat(neg_float_str(is), ty), - _ => { return None; } + ConstantInt(value, ty) => { + ConstantInt(value, + match ty { + SignedIntLit(ity, sign) => SignedIntLit(ity, neg_sign(sign)), + UnsuffixedIntLit(sign) => UnsuffixedIntLit(neg_sign(sign)), + _ => { + return None; + } + }) + } + ConstantFloat(is, ty) => ConstantFloat(neg_float_str(is), ty), + _ => { + return None; + } }) } @@ -316,25 +365,42 @@ pub fn is_negative(ty: LitIntType) -> bool { fn unify_int_type(l: LitIntType, r: LitIntType, s: Sign) -> Option<LitIntType> { match (l, r) { - (SignedIntLit(lty, _), SignedIntLit(rty, _)) => if lty == rty { - Some(SignedIntLit(lty, s)) } else { None }, - (UnsignedIntLit(lty), UnsignedIntLit(rty)) => + (SignedIntLit(lty, _), SignedIntLit(rty, _)) => { + if lty == rty { + Some(SignedIntLit(lty, s)) + } else { + None + } + } + (UnsignedIntLit(lty), UnsignedIntLit(rty)) => { if s == Plus && lty == rty { Some(UnsignedIntLit(lty)) - } else { None }, + } else { + None + } + } (UnsuffixedIntLit(_), UnsuffixedIntLit(_)) => Some(UnsuffixedIntLit(s)), (SignedIntLit(lty, _), UnsuffixedIntLit(_)) => Some(SignedIntLit(lty, s)), - (UnsignedIntLit(lty), UnsuffixedIntLit(rs)) => if rs == Plus { - Some(UnsignedIntLit(lty)) } else { None }, + (UnsignedIntLit(lty), UnsuffixedIntLit(rs)) => { + if rs == Plus { + Some(UnsignedIntLit(lty)) + } else { + None + } + } (UnsuffixedIntLit(_), SignedIntLit(rty, _)) => Some(SignedIntLit(rty, s)), - (UnsuffixedIntLit(ls), UnsignedIntLit(rty)) => if ls == Plus { - Some(UnsignedIntLit(rty)) } else { None }, + (UnsuffixedIntLit(ls), UnsignedIntLit(rty)) => { + if ls == Plus { + Some(UnsignedIntLit(rty)) + } else { + None + } + } _ => None, } } -fn add_neg_int(pos: u64, pty: LitIntType, neg: u64, nty: LitIntType) -> - Option<Constant> { +fn add_neg_int(pos: u64, pty: LitIntType, neg: u64, nty: LitIntType) -> Option<Constant> { if neg > pos { unify_int_type(nty, pty, Minus).map(|ty| ConstantInt(neg - pos, ty)) } else { @@ -342,70 +408,80 @@ fn add_neg_int(pos: u64, pty: LitIntType, neg: u64, nty: LitIntType) -> } } -fn sub_int(l: u64, lty: LitIntType, r: u64, rty: LitIntType, neg: bool) -> - Option<Constant> { - unify_int_type(lty, rty, if neg { Minus } else { Plus }).and_then( - |ty| l.checked_sub(r).map(|v| ConstantInt(v, ty))) +fn sub_int(l: u64, lty: LitIntType, r: u64, rty: LitIntType, neg: bool) -> Option<Constant> { + unify_int_type(lty, + rty, + if neg { + Minus + } else { + Plus + }) + .and_then(|ty| l.checked_sub(r).map(|v| ConstantInt(v, ty))) } pub fn constant(lcx: &LateContext, e: &Expr) -> Option<(Constant, bool)> { - let mut cx = ConstEvalLateContext { lcx: Some(lcx), needed_resolution: false }; + let mut cx = ConstEvalLateContext { + lcx: Some(lcx), + needed_resolution: false, + }; cx.expr(e).map(|cst| (cst, cx.needed_resolution)) } pub fn constant_simple(e: &Expr) -> Option<Constant> { - let mut cx = ConstEvalLateContext { lcx: None, needed_resolution: false }; + let mut cx = ConstEvalLateContext { + lcx: None, + needed_resolution: false, + }; cx.expr(e) } struct ConstEvalLateContext<'c, 'cc: 'c> { lcx: Option<&'c LateContext<'c, 'cc>>, - needed_resolution: bool + needed_resolution: bool, } impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { - /// simple constant folding: Insert an expression, get a constant or none. fn expr(&mut self, e: &Expr) -> Option<Constant> { match e.node { ExprPath(_, _) => self.fetch_path(e), ExprBlock(ref block) => self.block(block), - ExprIf(ref cond, ref then, ref otherwise) => - self.ifthenelse(cond, then, otherwise), + ExprIf(ref cond, ref then, ref otherwise) => self.ifthenelse(cond, then, otherwise), ExprLit(ref lit) => Some(lit_to_constant(&lit.node)), ExprVec(ref vec) => self.multi(vec).map(ConstantVec), ExprTup(ref tup) => self.multi(tup).map(ConstantTuple), - ExprRepeat(ref value, ref number) => - self.binop_apply(value, number, |v, n| - Some(ConstantRepeat(Box::new(v), n.as_u64() as usize))), - ExprUnary(op, ref operand) => self.expr(operand).and_then( - |o| match op { - UnNot => constant_not(o), - UnNeg => constant_negate(o), - UnDeref => Some(o), - }), - ExprBinary(op, ref left, ref right) => - self.binop(op, left, right), - //TODO: add other expressions + ExprRepeat(ref value, ref number) => { + self.binop_apply(value, number, |v, n| Some(ConstantRepeat(Box::new(v), n.as_u64() as usize))) + } + ExprUnary(op, ref operand) => { + self.expr(operand).and_then(|o| { + match op { + UnNot => constant_not(o), + UnNeg => constant_negate(o), + UnDeref => Some(o), + } + }) + } + ExprBinary(op, ref left, ref right) => self.binop(op, left, right), + // TODO: add other expressions _ => None, } } /// create `Some(Vec![..])` of all constants, unless there is any /// non-constant part - fn multi<E: Deref<Target=Expr> + Sized>(&mut self, vec: &[E]) -> - Option<Vec<Constant>> { - vec.iter().map(|elem| self.expr(elem)) - .collect::<Option<_>>() + fn multi<E: Deref<Target = Expr> + Sized>(&mut self, vec: &[E]) -> Option<Vec<Constant>> { + vec.iter() + .map(|elem| self.expr(elem)) + .collect::<Option<_>>() } /// lookup a possibly constant expression from a ExprPath fn fetch_path(&mut self, e: &Expr) -> Option<Constant> { if let Some(lcx) = self.lcx { let mut maybe_id = None; - if let Some(&PathResolution { base_def: DefConst(id), ..}) = - lcx.tcx.def_map.borrow().get(&e.id) { + if let Some(&PathResolution { base_def: DefConst(id), ..}) = lcx.tcx.def_map.borrow().get(&e.id) { maybe_id = Some(id); } // separate if lets to avoid doubleborrowing the defmap @@ -426,63 +502,84 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { fn block(&mut self, block: &Block) -> Option<Constant> { if block.stmts.is_empty() { block.expr.as_ref().and_then(|ref b| self.expr(b)) - } else { None } + } else { + None + } } - fn ifthenelse(&mut self, cond: &Expr, then: &Block, otherwise: &Option<P<Expr>>) - -> Option<Constant> { + fn ifthenelse(&mut self, cond: &Expr, then: &Block, otherwise: &Option<P<Expr>>) -> Option<Constant> { if let Some(ConstantBool(b)) = self.expr(cond) { if b { self.block(then) } else { otherwise.as_ref().and_then(|expr| self.expr(expr)) } - } else { None } + } else { + None + } } fn binop(&mut self, op: BinOp, left: &Expr, right: &Expr) -> Option<Constant> { match op.node { - BiAdd => self.binop_apply(left, right, |l, r| - match (l, r) { - (ConstantByte(l8), ConstantByte(r8)) => - l8.checked_add(r8).map(ConstantByte), - (ConstantInt(l64, lty), ConstantInt(r64, rty)) => { - let (ln, rn) = (is_negative(lty), is_negative(rty)); - if ln == rn { - unify_int_type(lty, rty, if ln { Minus } else { Plus }) - .and_then(|ty| l64.checked_add(r64).map( - |v| ConstantInt(v, ty))) - } else { - if ln { - add_neg_int(r64, rty, l64, lty) + BiAdd => { + self.binop_apply(left, right, |l, r| { + match (l, r) { + (ConstantByte(l8), ConstantByte(r8)) => l8.checked_add(r8).map(ConstantByte), + (ConstantInt(l64, lty), ConstantInt(r64, rty)) => { + let (ln, rn) = (is_negative(lty), is_negative(rty)); + if ln == rn { + unify_int_type(lty, + rty, + if ln { + Minus + } else { + Plus + }) + .and_then(|ty| l64.checked_add(r64).map(|v| ConstantInt(v, ty))) } else { - add_neg_int(l64, lty, r64, rty) + if ln { + add_neg_int(r64, rty, l64, lty) + } else { + add_neg_int(l64, lty, r64, rty) + } } } + // TODO: float (would need bignum library?) + _ => None, } - // TODO: float (would need bignum library?) - _ => None - }), - BiSub => self.binop_apply(left, right, |l, r| - match (l, r) { - (ConstantByte(l8), ConstantByte(r8)) => if r8 > l8 { - None } else { Some(ConstantByte(l8 - r8)) }, - (ConstantInt(l64, lty), ConstantInt(r64, rty)) => - match (is_negative(lty), is_negative(rty)) { - (false, false) => sub_int(l64, lty, r64, rty, r64 > l64), - (true, true) => sub_int(l64, lty, r64, rty, l64 > r64), - (true, false) => unify_int_type(lty, rty, Minus) - .and_then(|ty| l64.checked_add(r64).map( - |v| ConstantInt(v, ty))), - (false, true) => unify_int_type(lty, rty, Plus) - .and_then(|ty| l64.checked_add(r64).map( - |v| ConstantInt(v, ty))), - }, - _ => None, - }), + }) + } + BiSub => { + self.binop_apply(left, right, |l, r| { + match (l, r) { + (ConstantByte(l8), ConstantByte(r8)) => { + if r8 > l8 { + None + } else { + Some(ConstantByte(l8 - r8)) + } + } + (ConstantInt(l64, lty), ConstantInt(r64, rty)) => { + match (is_negative(lty), is_negative(rty)) { + (false, false) => sub_int(l64, lty, r64, rty, r64 > l64), + (true, true) => sub_int(l64, lty, r64, rty, l64 > r64), + (true, false) => { + unify_int_type(lty, rty, Minus) + .and_then(|ty| l64.checked_add(r64).map(|v| ConstantInt(v, ty))) + } + (false, true) => { + unify_int_type(lty, rty, Plus) + .and_then(|ty| l64.checked_add(r64).map(|v| ConstantInt(v, ty))) + } + } + } + _ => None, + } + }) + } BiMul => self.divmul(left, right, u64::checked_mul), BiDiv => self.divmul(left, right, u64::checked_div), - //BiRem, + // BiRem, BiAnd => self.short_circuit(left, right, false), BiOr => self.short_circuit(left, right, true), BiBitXor => self.bitop(left, right, |x, y| x ^ y), @@ -490,70 +587,86 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { BiBitOr => self.bitop(left, right, |x, y| (x | y)), BiShl => self.bitop(left, right, |x, y| x << y), BiShr => self.bitop(left, right, |x, y| x >> y), - BiEq => self.binop_apply(left, right, - |l, r| Some(ConstantBool(l == r))), - BiNe => self.binop_apply(left, right, - |l, r| Some(ConstantBool(l != r))), + BiEq => self.binop_apply(left, right, |l, r| Some(ConstantBool(l == r))), + BiNe => self.binop_apply(left, right, |l, r| Some(ConstantBool(l != r))), BiLt => self.cmp(left, right, Less, true), BiLe => self.cmp(left, right, Greater, false), BiGe => self.cmp(left, right, Less, false), BiGt => self.cmp(left, right, Greater, true), - _ => None + _ => None, } } - fn divmul<F>(&mut self, left: &Expr, right: &Expr, f: F) - -> Option<Constant> where F: Fn(u64, u64) -> Option<u64> { - self.binop_apply(left, right, |l, r| + fn divmul<F>(&mut self, left: &Expr, right: &Expr, f: F) -> Option<Constant> + where F: Fn(u64, u64) -> Option<u64> + { + self.binop_apply(left, right, |l, r| { match (l, r) { (ConstantInt(l64, lty), ConstantInt(r64, rty)) => { - f(l64, r64).and_then(|value| - unify_int_type(lty, rty, if is_negative(lty) == - is_negative(rty) { Plus } else { Minus }) - .map(|ty| ConstantInt(value, ty))) + f(l64, r64).and_then(|value| { + unify_int_type(lty, + rty, + if is_negative(lty) == is_negative(rty) { + Plus + } else { + Minus + }) + .map(|ty| ConstantInt(value, ty)) + }) } _ => None, - }) + } + }) } - fn bitop<F>(&mut self, left: &Expr, right: &Expr, f: F) - -> Option<Constant> where F: Fn(u64, u64) -> u64 { - self.binop_apply(left, right, |l, r| match (l, r) { - (ConstantBool(l), ConstantBool(r)) => - Some(ConstantBool(f(l as u64, r as u64) != 0)), - (ConstantByte(l8), ConstantByte(r8)) => - Some(ConstantByte(f(l8 as u64, r8 as u64) as u8)), - (ConstantInt(l, lty), ConstantInt(r, rty)) => - unify_int_type(lty, rty, Plus).map(|ty| ConstantInt(f(l, r), ty)), - _ => None + fn bitop<F>(&mut self, left: &Expr, right: &Expr, f: F) -> Option<Constant> + where F: Fn(u64, u64) -> u64 + { + self.binop_apply(left, right, |l, r| { + match (l, r) { + (ConstantBool(l), ConstantBool(r)) => Some(ConstantBool(f(l as u64, r as u64) != 0)), + (ConstantByte(l8), ConstantByte(r8)) => Some(ConstantByte(f(l8 as u64, r8 as u64) as u8)), + (ConstantInt(l, lty), ConstantInt(r, rty)) => { + unify_int_type(lty, rty, Plus).map(|ty| ConstantInt(f(l, r), ty)) + } + _ => None, + } }) } fn cmp(&mut self, left: &Expr, right: &Expr, ordering: Ordering, b: bool) -> Option<Constant> { - self.binop_apply(left, right, |l, r| l.partial_cmp(&r).map(|o| - ConstantBool(b == (o == ordering)))) + self.binop_apply(left, + right, + |l, r| l.partial_cmp(&r).map(|o| ConstantBool(b == (o == ordering)))) } fn binop_apply<F>(&mut self, left: &Expr, right: &Expr, op: F) -> Option<Constant> - where F: Fn(Constant, Constant) -> Option<Constant> { + where F: Fn(Constant, Constant) -> Option<Constant> + { if let (Some(lc), Some(rc)) = (self.expr(left), self.expr(right)) { op(lc, rc) - } else { None } + } else { + None + } } fn short_circuit(&mut self, left: &Expr, right: &Expr, b: bool) -> Option<Constant> { - self.expr(left).and_then(|left| + self.expr(left).and_then(|left| { if let ConstantBool(lbool) = left { if lbool == b { Some(left) } else { - self.expr(right).and_then(|right| + self.expr(right).and_then(|right| { if let ConstantBool(_) = right { Some(right) - } else { None } - ) + } else { + None + } + }) } - } else { None } - ) + } else { + None + } + }) } } diff --git a/src/cyclomatic_complexity.rs b/src/cyclomatic_complexity.rs index 7beb3296aac..d4375abcd05 100644 --- a/src/cyclomatic_complexity.rs +++ b/src/cyclomatic_complexity.rs @@ -27,9 +27,7 @@ pub struct CyclomaticComplexity { impl CyclomaticComplexity { pub fn new(limit: u64) -> Self { - CyclomaticComplexity { - limit: LimitStack::new(limit), - } + CyclomaticComplexity { limit: LimitStack::new(limit) } } } @@ -41,7 +39,9 @@ impl LintPass for CyclomaticComplexity { impl CyclomaticComplexity { fn check<'a, 'tcx>(&mut self, cx: &'a LateContext<'a, 'tcx>, block: &Block, span: Span) { - if in_macro(cx, span) { return; } + if in_macro(cx, span) { + return; + } let cfg = CFG::new(cx.tcx, block); let n = cfg.graph.len_nodes() as u64; let e = cfg.graph.len_edges() as u64; @@ -59,9 +59,11 @@ impl CyclomaticComplexity { } else { let rust_cc = cc + divergence - narms; if rust_cc > self.limit.limit() { - span_help_and_lint(cx, CYCLOMATIC_COMPLEXITY, span, - &format!("The function has a cyclomatic complexity of {}", rust_cc), - "You could split it up into multiple smaller functions"); + span_help_and_lint(cx, + CYCLOMATIC_COMPLEXITY, + span, + &format!("The function has a cyclomatic complexity of {}", rust_cc), + "You could split it up into multiple smaller functions"); } } } @@ -105,8 +107,8 @@ impl<'a> Visitor<'a> for MatchArmCounter { if arms_n > 1 { self.0 += arms_n - 2; } - }, - ExprClosure(..) => {}, + } + ExprClosure(..) => {} _ => walk_expr(self, e), } } @@ -125,8 +127,8 @@ impl<'a, 'b, 'tcx> Visitor<'a> for DivergenceCounter<'b, 'tcx> { self.0 += 1; } } - }, - ExprClosure(..) => {}, + } + ExprClosure(..) => {} _ => walk_expr(self, e), } } @@ -134,15 +136,22 @@ impl<'a, 'b, 'tcx> Visitor<'a> for DivergenceCounter<'b, 'tcx> { #[cfg(feature="debugging")] fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, div: u64, span: Span) { - cx.sess().span_bug(span, &format!("Clippy encountered a bug calculating cyclomatic complexity: \ - cc = {}, arms = {}, div = {}. Please file a bug report.", cc, narms, div));; + cx.sess().span_bug(span, + &format!("Clippy encountered a bug calculating cyclomatic complexity: cc = {}, arms = {}, \ + div = {}. Please file a bug report.", + cc, + narms, + div));; } #[cfg(not(feature="debugging"))] fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, div: u64, span: Span) { if cx.current_level(CYCLOMATIC_COMPLEXITY) != Level::Allow { cx.sess().span_note_without_error(span, &format!("Clippy encountered a bug calculating cyclomatic complexity \ - (hide this message with `#[allow(cyclomatic_complexity)]`): \ - cc = {}, arms = {}, div = {}. Please file a bug report.", cc, narms, div)); + (hide this message with `#[allow(cyclomatic_complexity)]`): cc \ + = {}, arms = {}, div = {}. Please file a bug report.", + cc, + narms, + div)); } } diff --git a/src/eq_op.rs b/src/eq_op.rs index 3d2f32e6c7f..31dc094385f 100644 --- a/src/eq_op.rs +++ b/src/eq_op.rs @@ -30,19 +30,29 @@ impl LateLintPass for EqOp { fn check_expr(&mut self, cx: &LateContext, e: &Expr) { if let ExprBinary(ref op, ref left, ref right) = e.node { if is_cmp_or_bit(op) && is_exp_equal(cx, left, right) { - span_lint(cx, EQ_OP, e.span, &format!( - "equal expressions as operands to {}", - ast_util::binop_to_string(op.node))); + span_lint(cx, + EQ_OP, + e.span, + &format!("equal expressions as operands to {}", ast_util::binop_to_string(op.node))); } } } } -fn is_cmp_or_bit(op : &BinOp) -> bool { +fn is_cmp_or_bit(op: &BinOp) -> bool { match op.node { - BiEq | BiLt | BiLe | BiGt | BiGe | BiNe | BiAnd | BiOr | - BiBitXor | BiBitAnd | BiBitOr => true, - _ => false + BiEq | + BiLt | + BiLe | + BiGt | + BiGe | + BiNe | + BiAnd | + BiOr | + BiBitXor | + BiBitAnd | + BiBitOr => true, + _ => false, } } diff --git a/src/escape.rs b/src/escape.rs index 4b6a8258dbe..079fdcf09a0 100644 --- a/src/escape.rs +++ b/src/escape.rs @@ -43,13 +43,7 @@ impl LintPass for EscapePass { } impl LateLintPass for EscapePass { - fn check_fn(&mut self, - cx: &LateContext, - _: visit::FnKind, - decl: &FnDecl, - body: &Block, - _: Span, - id: NodeId) { + fn check_fn(&mut self, cx: &LateContext, _: visit::FnKind, decl: &FnDecl, body: &Block, _: Span, id: NodeId) { let param_env = ty::ParameterEnvironment::for_item(cx.tcx, id); let infcx = infer::new_infer_ctxt(cx.tcx, &cx.tcx.tables, Some(param_env), false); let mut v = EscapeDelegate { @@ -70,11 +64,7 @@ impl LateLintPass for EscapePass { } impl<'a, 'tcx: 'a> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { - fn consume(&mut self, - _: NodeId, - _: Span, - cmt: cmt<'tcx>, - mode: ConsumeMode) { + fn consume(&mut self, _: NodeId, _: Span, cmt: cmt<'tcx>, mode: ConsumeMode) { if let Categorization::Local(lid) = cmt.cat { if self.set.contains(&lid) { @@ -119,12 +109,7 @@ impl<'a, 'tcx: 'a> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { } } - fn borrow(&mut self, - borrow_id: NodeId, - _: Span, - cmt: cmt<'tcx>, - _: ty::Region, - _: ty::BorrowKind, + fn borrow(&mut self, borrow_id: NodeId, _: Span, cmt: cmt<'tcx>, _: ty::Region, _: ty::BorrowKind, loan_cause: LoanCause) { if let Categorization::Local(lid) = cmt.cat { @@ -145,9 +130,15 @@ impl<'a, 'tcx: 'a> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { } } else if LoanCause::AddrOf == loan_cause { // &x - if let Some(&AutoAdjustment::AdjustDerefRef(adj)) = - self.cx.tcx.tables.borrow().adjustments - .get(&self.cx.tcx.map.get_parent_node(borrow_id)) { + if let Some(&AutoAdjustment::AdjustDerefRef(adj)) = self.cx + .tcx + .tables + .borrow() + .adjustments + .get(&self.cx + .tcx + .map + .get_parent_node(borrow_id)) { if adj.autoderefs <= 1 { // foo(&x) where no extra autoreffing is happening self.set.remove(&lid); @@ -162,10 +153,5 @@ impl<'a, 'tcx: 'a> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { } } fn decl_without_init(&mut self, _: NodeId, _: Span) {} - fn mutate(&mut self, - _: NodeId, - _: Span, - _: cmt<'tcx>, - _: MutateMode) { - } + fn mutate(&mut self, _: NodeId, _: Span, _: cmt<'tcx>, _: MutateMode) {} } diff --git a/src/eta_reduction.rs b/src/eta_reduction.rs index 6b561ff7a05..46c458c6bcb 100644 --- a/src/eta_reduction.rs +++ b/src/eta_reduction.rs @@ -73,22 +73,18 @@ fn check_closure(cx: &LateContext, expr: &Expr) { } if p.segments[0].identifier != ident.node { // The two idents should be the same - return + return; } } else { - return + return; } } else { - return + return; } } - span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, - "redundant closure found", - |db| { + span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure found", |db| { if let Some(snippet) = snippet_opt(cx, caller.span) { - db.span_suggestion(expr.span, - "remove closure as shown:", - snippet); + db.span_suggestion(expr.span, "remove closure as shown:", snippet); } }); } diff --git a/src/identity_op.rs b/src/identity_op.rs index 88a03e050be..fd5071d4013 100644 --- a/src/identity_op.rs +++ b/src/identity_op.rs @@ -27,26 +27,26 @@ impl LintPass for IdentityOp { impl LateLintPass for IdentityOp { fn check_expr(&mut self, cx: &LateContext, e: &Expr) { - if in_macro(cx, e.span) { return; } + if in_macro(cx, e.span) { + return; + } if let ExprBinary(ref cmp, ref left, ref right) = e.node { match cmp.node { BiAdd | BiBitOr | BiBitXor => { check(cx, left, 0, e.span, right.span); check(cx, right, 0, e.span, left.span); } - BiShl | BiShr | BiSub => - check(cx, right, 0, e.span, left.span), + BiShl | BiShr | BiSub => check(cx, right, 0, e.span, left.span), BiMul => { check(cx, left, 1, e.span, right.span); check(cx, right, 1, e.span, left.span); } - BiDiv => - check(cx, right, 1, e.span, left.span), + BiDiv => check(cx, right, 1, e.span, left.span), BiBitAnd => { check(cx, left, -1, e.span, right.span); check(cx, right, -1, e.span, left.span); } - _ => () + _ => (), } } } @@ -61,9 +61,11 @@ fn check(cx: &LateContext, e: &Expr, m: i8, span: Span, arg: Span) { 1 => !is_negative(ty) && v == 1, _ => unreachable!(), } { - span_lint(cx, IDENTITY_OP, span, &format!( - "the operation is ineffective. Consider reducing it to `{}`", - snippet(cx, arg, ".."))); + span_lint(cx, + IDENTITY_OP, + span, + &format!("the operation is ineffective. Consider reducing it to `{}`", + snippet(cx, arg, ".."))); } } } diff --git a/src/len_zero.rs b/src/len_zero.rs index d574fa2c336..6bc5bc4bf01 100644 --- a/src/len_zero.rs +++ b/src/len_zero.rs @@ -49,21 +49,18 @@ impl LintPass for LenZero { impl LateLintPass for LenZero { fn check_item(&mut self, cx: &LateContext, item: &Item) { match item.node { - ItemTrait(_, _, _, ref trait_items) => - check_trait_items(cx, item, trait_items), - ItemImpl(_, _, _, None, _, ref impl_items) => // only non-trait - check_impl_items(cx, item, impl_items), - _ => () + ItemTrait(_, _, _, ref trait_items) => check_trait_items(cx, item, trait_items), + ItemImpl(_, _, _, None, _, ref impl_items) => check_impl_items(cx, item, impl_items), + _ => (), } } fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { - if let ExprBinary(Spanned{node: cmp, ..}, ref left, ref right) = - expr.node { + if let ExprBinary(Spanned{node: cmp, ..}, ref left, ref right) = expr.node { match cmp { BiEq => check_cmp(cx, expr.span, left, right, ""), BiGt | BiNe => check_cmp(cx, expr.span, left, right, "!"), - _ => () + _ => (), } } } @@ -71,37 +68,52 @@ impl LateLintPass for LenZero { fn check_trait_items(cx: &LateContext, item: &Item, trait_items: &[TraitItem]) { fn is_named_self(item: &TraitItem, name: &str) -> bool { - item.name.as_str() == name && if let MethodTraitItem(ref sig, _) = - item.node { is_self_sig(sig) } else { false } + item.name.as_str() == name && + if let MethodTraitItem(ref sig, _) = item.node { + is_self_sig(sig) + } else { + false + } } if !trait_items.iter().any(|i| is_named_self(i, "is_empty")) { - //span_lint(cx, LEN_WITHOUT_IS_EMPTY, item.span, &format!("trait {}", item.ident)); + // span_lint(cx, LEN_WITHOUT_IS_EMPTY, item.span, &format!("trait {}", item.ident)); for i in trait_items { if is_named_self(i, "len") { - span_lint(cx, LEN_WITHOUT_IS_EMPTY, i.span, - &format!("trait `{}` has a `.len(_: &Self)` method, but no \ - `.is_empty(_: &Self)` method. Consider adding one", + span_lint(cx, + LEN_WITHOUT_IS_EMPTY, + i.span, + &format!("trait `{}` has a `.len(_: &Self)` method, but no `.is_empty(_: &Self)` method. \ + Consider adding one", item.name)); } - }; + } } } fn check_impl_items(cx: &LateContext, item: &Item, impl_items: &[ImplItem]) { fn is_named_self(item: &ImplItem, name: &str) -> bool { - item.name.as_str() == name && if let ImplItemKind::Method(ref sig, _) = - item.node { is_self_sig(sig) } else { false } + item.name.as_str() == name && + if let ImplItemKind::Method(ref sig, _) = item.node { + is_self_sig(sig) + } else { + false + } } if !impl_items.iter().any(|i| is_named_self(i, "is_empty")) { for i in impl_items { if is_named_self(i, "len") { let s = i.span; - span_lint(cx, LEN_WITHOUT_IS_EMPTY, - Span{ lo: s.lo, hi: s.lo, expn_id: s.expn_id }, - &format!("item `{}` has a `.len(_: &Self)` method, but no \ - `.is_empty(_: &Self)` method. Consider adding one", + span_lint(cx, + LEN_WITHOUT_IS_EMPTY, + Span { + lo: s.lo, + hi: s.lo, + expn_id: s.expn_id, + }, + &format!("item `{}` has a `.len(_: &Self)` method, but no `.is_empty(_: &Self)` method. \ + Consider adding one", item.name)); return; } @@ -111,32 +123,40 @@ fn check_impl_items(cx: &LateContext, item: &Item, impl_items: &[ImplItem]) { fn is_self_sig(sig: &MethodSig) -> bool { if let SelfStatic = sig.explicit_self.node { - false } else { sig.decl.inputs.len() == 1 } + false + } else { + sig.decl.inputs.len() == 1 + } } fn check_cmp(cx: &LateContext, span: Span, left: &Expr, right: &Expr, op: &str) { // check if we are in an is_empty() method if let Some(name) = get_item_name(cx, left) { - if name.as_str() == "is_empty" { return; } + if name.as_str() == "is_empty" { + return; + } } match (&left.node, &right.node) { - (&ExprLit(ref lit), &ExprMethodCall(ref method, _, ref args)) => - check_len_zero(cx, span, &method.node, args, lit, op), - (&ExprMethodCall(ref method, _, ref args), &ExprLit(ref lit)) => - check_len_zero(cx, span, &method.node, args, lit, op), - _ => () + (&ExprLit(ref lit), &ExprMethodCall(ref method, _, ref args)) => { + check_len_zero(cx, span, &method.node, args, lit, op) + } + (&ExprMethodCall(ref method, _, ref args), &ExprLit(ref lit)) => { + check_len_zero(cx, span, &method.node, args, lit, op) + } + _ => (), } } -fn check_len_zero(cx: &LateContext, span: Span, name: &Name, - args: &[P<Expr>], lit: &Lit, op: &str) { +fn check_len_zero(cx: &LateContext, span: Span, name: &Name, args: &[P<Expr>], lit: &Lit, op: &str) { if let Spanned{node: LitInt(0, _), ..} = *lit { - if name.as_str() == "len" && args.len() == 1 && - has_is_empty(cx, &args[0]) { - span_lint(cx, LEN_ZERO, span, &format!( - "consider replacing the len comparison with `{}{}.is_empty()`", - op, snippet(cx, args[0].span, "_"))); - } + if name.as_str() == "len" && args.len() == 1 && has_is_empty(cx, &args[0]) { + span_lint(cx, + LEN_ZERO, + span, + &format!("consider replacing the len comparison with `{}{}.is_empty()`", + op, + snippet(cx, args[0].span, "_"))); + } } } @@ -145,31 +165,35 @@ fn has_is_empty(cx: &LateContext, expr: &Expr) -> bool { /// get a ImplOrTraitItem and return true if it matches is_empty(self) fn is_is_empty(cx: &LateContext, id: &ImplOrTraitItemId) -> bool { if let MethodTraitItemId(def_id) = *id { - if let ty::MethodTraitItem(ref method) = - cx.tcx.impl_or_trait_item(def_id) { - method.name.as_str() == "is_empty" - && method.fty.sig.skip_binder().inputs.len() == 1 - } else { false } - } else { false } + if let ty::MethodTraitItem(ref method) = cx.tcx.impl_or_trait_item(def_id) { + method.name.as_str() == "is_empty" && method.fty.sig.skip_binder().inputs.len() == 1 + } else { + false + } + } else { + false + } } /// check the inherent impl's items for an is_empty(self) method fn has_is_empty_impl(cx: &LateContext, id: &DefId) -> bool { let impl_items = cx.tcx.impl_items.borrow(); - cx.tcx.inherent_impls.borrow().get(id).map_or(false, - |ids| ids.iter().any(|iid| impl_items.get(iid).map_or(false, - |iids| iids.iter().any(|i| is_is_empty(cx, i))))) + cx.tcx.inherent_impls.borrow().get(id).map_or(false, |ids| { + ids.iter().any(|iid| impl_items.get(iid).map_or(false, |iids| iids.iter().any(|i| is_is_empty(cx, i)))) + }) } let ty = &walk_ptrs_ty(&cx.tcx.expr_ty(expr)); match ty.sty { - ty::TyTrait(_) => cx.tcx.trait_item_def_ids.borrow().get( - &ty.ty_to_def_id().expect("trait impl not found")).map_or(false, - |ids| ids.iter().any(|i| is_is_empty(cx, i))), - ty::TyProjection(_) => ty.ty_to_def_id().map_or(false, - |id| has_is_empty_impl(cx, &id)), - ty::TyEnum(ref id, _) | ty::TyStruct(ref id, _) => - has_is_empty_impl(cx, &id.did), + ty::TyTrait(_) => { + cx.tcx + .trait_item_def_ids + .borrow() + .get(&ty.ty_to_def_id().expect("trait impl not found")) + .map_or(false, |ids| ids.iter().any(|i| is_is_empty(cx, i))) + } + ty::TyProjection(_) => ty.ty_to_def_id().map_or(false, |id| has_is_empty_impl(cx, &id)), + ty::TyEnum(ref id, _) | ty::TyStruct(ref id, _) => has_is_empty_impl(cx, &id.did), ty::TyArray(..) => true, _ => false, } diff --git a/src/lib.rs b/src/lib.rs index 76c04d53a0b..2ac8e9044db 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,7 +5,9 @@ // this only exists to allow the "dogfood" integration test to work #[allow(dead_code)] -fn main() { println!("What are you doing? Don't run clippy as an executable"); } +fn main() { + println!("What are you doing? Don't run clippy as an executable"); +} #[macro_use] extern crate syntax; @@ -128,7 +130,8 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box array_indexing::ArrayIndexing); reg.register_late_lint_pass(box panic::PanicPass); - reg.register_lint_group("clippy_pedantic", vec![ + reg.register_lint_group("clippy_pedantic", + vec![ methods::OPTION_UNWRAP_USED, methods::RESULT_UNWRAP_USED, methods::WRONG_PUB_SELF_CONVENTION, @@ -147,7 +150,8 @@ pub fn plugin_registrar(reg: &mut Registry) { unicode::UNICODE_NOT_NFC, ]); - reg.register_lint_group("clippy", vec![ + reg.register_lint_group("clippy", + vec![ approx_const::APPROX_CONSTANT, array_indexing::OUT_OF_BOUNDS_INDEXING, attrs::INLINE_ALWAYS, diff --git a/src/lifetimes.rs b/src/lifetimes.rs index 6ae10c09455..2de916cfebc 100644 --- a/src/lifetimes.rs +++ b/src/lifetimes.rs @@ -47,15 +47,13 @@ impl LateLintPass for LifetimePass { fn check_impl_item(&mut self, cx: &LateContext, item: &ImplItem) { if let ImplItemKind::Method(ref sig, _) = item.node { - check_fn_inner(cx, &sig.decl, Some(&sig.explicit_self), - &sig.generics, item.span); + check_fn_inner(cx, &sig.decl, Some(&sig.explicit_self), &sig.generics, item.span); } } fn check_trait_item(&mut self, cx: &LateContext, item: &TraitItem) { if let MethodTraitItem(ref sig, _) = item.node { - check_fn_inner(cx, &sig.decl, Some(&sig.explicit_self), - &sig.generics, item.span); + check_fn_inner(cx, &sig.decl, Some(&sig.explicit_self), &sig.generics, item.span); } } } @@ -69,20 +67,20 @@ enum RefLt { } use self::RefLt::*; -fn check_fn_inner(cx: &LateContext, decl: &FnDecl, slf: Option<&ExplicitSelf>, - generics: &Generics, span: Span) { +fn check_fn_inner(cx: &LateContext, decl: &FnDecl, slf: Option<&ExplicitSelf>, generics: &Generics, span: Span) { if in_external_macro(cx, span) || has_where_lifetimes(cx, &generics.where_clause) { return; } if could_use_elision(cx, decl, slf, &generics.lifetimes) { - span_lint(cx, NEEDLESS_LIFETIMES, span, + span_lint(cx, + NEEDLESS_LIFETIMES, + span, "explicit lifetimes given in parameter types where they could be elided"); } report_extra_lifetimes(cx, decl, &generics); } -fn could_use_elision(cx: &LateContext, func: &FnDecl, slf: Option<&ExplicitSelf>, - named_lts: &[LifetimeDef]) -> bool { +fn could_use_elision(cx: &LateContext, func: &FnDecl, slf: Option<&ExplicitSelf>, named_lts: &[LifetimeDef]) -> bool { // There are two scenarios where elision works: // * no output references, all input references have different LT // * output references, exactly one input reference with same LT @@ -102,7 +100,7 @@ fn could_use_elision(cx: &LateContext, func: &FnDecl, slf: Option<&ExplicitSelf> match slf.node { SelfRegion(ref opt_lt, _, _) => input_visitor.record(opt_lt), SelfExplicit(ref ty, _) => walk_ty(&mut input_visitor, ty), - _ => { } + _ => {} } } // extract lifetimes in input argument types @@ -147,8 +145,8 @@ fn could_use_elision(cx: &LateContext, func: &FnDecl, slf: Option<&ExplicitSelf> (&Named(n1), &Named(n2)) if n1 == n2 => true, (&Named(_), &Unnamed) => true, (&Unnamed, &Named(_)) => true, - _ => false // already elided, different named lifetimes - // or something static going on + _ => false, // already elided, different named lifetimes + // or something static going on } } else { false @@ -176,12 +174,15 @@ fn unique_lifetimes(lts: &[RefLt]) -> usize { /// A visitor usable for rustc_front::visit::walk_ty(). struct RefVisitor<'v, 't: 'v> { cx: &'v LateContext<'v, 't>, // context reference - lts: Vec<RefLt> + lts: Vec<RefLt>, } -impl <'v, 't> RefVisitor<'v, 't> { +impl<'v, 't> RefVisitor<'v, 't> { fn new(cx: &'v LateContext<'v, 't>) -> RefVisitor<'v, 't> { - RefVisitor { cx: cx, lts: Vec::new() } + RefVisitor { + cx: cx, + lts: Vec::new(), + } } fn record(&mut self, lifetime: &Option<Lifetime>) { @@ -211,13 +212,13 @@ impl <'v, 't> RefVisitor<'v, 't> { for _ in type_scheme.generics.regions.as_slice() { self.record(&None); } - }, + } DefTrait(def_id) => { let trait_def = self.cx.tcx.trait_defs.borrow()[&def_id]; for _ in &trait_def.generics.regions { self.record(&None); } - }, + } _ => {} } } @@ -227,7 +228,6 @@ impl <'v, 't> RefVisitor<'v, 't> { } impl<'v, 't> Visitor<'v> for RefVisitor<'v, 't> { - // for lifetimes as parameters of generics fn visit_lifetime(&mut self, lifetime: &'v Lifetime) { self.record(&Some(*lifetime)); @@ -258,7 +258,9 @@ fn has_where_lifetimes(cx: &LateContext, where_clause: &WhereClause) -> bool { let mut visitor = RefVisitor::new(cx); // walk the type F, it may not contain LT refs walk_ty(&mut visitor, &pred.bounded_ty); - if !visitor.lts.is_empty() { return true; } + if !visitor.lts.is_empty() { + return true; + } // if the bounds define new lifetimes, they are fine to occur let allowed_lts = allowed_lts_from(&pred.bound_lifetimes); // now walk the bounds @@ -275,7 +277,9 @@ fn has_where_lifetimes(cx: &LateContext, where_clause: &WhereClause) -> bool { WherePredicate::EqPredicate(ref pred) => { let mut visitor = RefVisitor::new(cx); walk_ty(&mut visitor, &pred.ty); - if !visitor.lts.is_empty() { return true; } + if !visitor.lts.is_empty() { + return true; + } } } } @@ -285,7 +289,6 @@ fn has_where_lifetimes(cx: &LateContext, where_clause: &WhereClause) -> bool { struct LifetimeChecker(HashMap<Name, Span>); impl<'v> Visitor<'v> for LifetimeChecker { - // for lifetimes as parameters of generics fn visit_lifetime(&mut self, lifetime: &'v Lifetime) { self.0.remove(&lifetime.name); @@ -300,16 +303,15 @@ impl<'v> Visitor<'v> for LifetimeChecker { } } -fn report_extra_lifetimes(cx: &LateContext, func: &FnDecl, - generics: &Generics) { - let hs = generics.lifetimes.iter() +fn report_extra_lifetimes(cx: &LateContext, func: &FnDecl, generics: &Generics) { + let hs = generics.lifetimes + .iter() .map(|lt| (lt.lifetime.name, lt.lifetime.span)) .collect(); let mut checker = LifetimeChecker(hs); walk_generics(&mut checker, generics); walk_fn_decl(&mut checker, func); for (_, v) in checker.0 { - span_lint(cx, UNUSED_LIFETIMES, v, - "this lifetime isn't used in the function definition"); + span_lint(cx, UNUSED_LIFETIMES, v, "this lifetime isn't used in the function definition"); } } diff --git a/src/loops.rs b/src/loops.rs index a0454a325e4..91bb898629f 100644 --- a/src/loops.rs +++ b/src/loops.rs @@ -5,14 +5,13 @@ use rustc_front::intravisit::{Visitor, walk_expr, walk_block, walk_decl}; use rustc::middle::ty; use rustc::middle::def::DefLocal; use consts::{constant_simple, Constant}; -use rustc::front::map::Node::{NodeBlock}; +use rustc::front::map::Node::NodeBlock; use std::borrow::Cow; -use std::collections::{HashSet,HashMap}; +use std::collections::{HashSet, HashMap}; use syntax::ast::Lit_::*; -use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, - in_external_macro, expr_block, span_help_and_lint, is_integer_literal, - get_enclosing_block}; +use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, in_external_macro, expr_block, + span_help_and_lint, is_integer_literal, get_enclosing_block}; use utils::{HASHMAP_PATH, VEC_PATH, LL_PATH}; /// **What it does:** This lint checks for looping over the range of `0..len` of some collection just to get the values by index. It is `Warn` by default. @@ -128,9 +127,14 @@ pub struct LoopsPass; impl LintPass for LoopsPass { fn get_lints(&self) -> LintArray { - lint_array!(NEEDLESS_RANGE_LOOP, EXPLICIT_ITER_LOOP, ITER_NEXT_LOOP, - WHILE_LET_LOOP, UNUSED_COLLECT, REVERSE_RANGE_LOOP, - EXPLICIT_COUNTER_LOOP, EMPTY_LOOP, + lint_array!(NEEDLESS_RANGE_LOOP, + EXPLICIT_ITER_LOOP, + ITER_NEXT_LOOP, + WHILE_LET_LOOP, + UNUSED_COLLECT, + REVERSE_RANGE_LOOP, + EXPLICIT_COUNTER_LOOP, + EMPTY_LOOP, WHILE_LET_ON_ITERATOR) } } @@ -146,10 +150,11 @@ impl LateLintPass for LoopsPass { if let ExprLoop(ref block, _) = expr.node { // also check for empty `loop {}` statements if block.stmts.is_empty() && block.expr.is_none() { - span_lint(cx, EMPTY_LOOP, expr.span, - "empty `loop {}` detected. You may want to either \ - use `panic!()` or add `std::thread::sleep(..);` to \ - the loop body."); + span_lint(cx, + EMPTY_LOOP, + expr.span, + "empty `loop {}` detected. You may want to either use `panic!()` or add \ + `std::thread::sleep(..);` to the loop body."); } // extract the expression from the first statement (if any) in a block @@ -159,11 +164,10 @@ impl LateLintPass for LoopsPass { if let ExprMatch(ref matchexpr, ref arms, ref source) = inner.node { // collect the remaining statements below the match let mut other_stuff = block.stmts - .iter() - .skip(1) - .map(|stmt| { - format!("{}", snippet(cx, stmt.span, "..")) - }).collect::<Vec<String>>(); + .iter() + .skip(1) + .map(|stmt| format!("{}", snippet(cx, stmt.span, ".."))) + .collect::<Vec<String>>(); if inner_stmt_expr.is_some() { // if we have a statement which has a match, if let Some(ref expr) = block.expr { @@ -174,29 +178,31 @@ impl LateLintPass for LoopsPass { // ensure "if let" compatible match structure match *source { - MatchSource::Normal | MatchSource::IfLetDesugar{..} => if - arms.len() == 2 && - arms[0].pats.len() == 1 && arms[0].guard.is_none() && - arms[1].pats.len() == 1 && arms[1].guard.is_none() && - // finally, check for "break" in the second clause - is_break_expr(&arms[1].body) - { - if in_external_macro(cx, expr.span) { return; } - let loop_body = if inner_stmt_expr.is_some() { - // FIXME: should probably be an ellipsis - // tabbing and newline is probably a bad idea, especially for large blocks - Cow::Owned(format!("{{\n {}\n}}", other_stuff.join("\n "))) - } else { - expr_block(cx, &arms[0].body, Some(other_stuff.join("\n ")), "..") - }; - span_help_and_lint(cx, WHILE_LET_LOOP, expr.span, - "this loop could be written as a `while let` loop", - &format!("try\nwhile let {} = {} {}", - snippet(cx, arms[0].pats[0].span, ".."), - snippet(cx, matchexpr.span, ".."), - loop_body)); - }, - _ => () + MatchSource::Normal | MatchSource::IfLetDesugar{..} => { + if arms.len() == 2 && arms[0].pats.len() == 1 && arms[0].guard.is_none() && + arms[1].pats.len() == 1 && arms[1].guard.is_none() && + is_break_expr(&arms[1].body) { + if in_external_macro(cx, expr.span) { + return; + } + let loop_body = if inner_stmt_expr.is_some() { + // FIXME: should probably be an ellipsis + // tabbing and newline is probably a bad idea, especially for large blocks + Cow::Owned(format!("{{\n {}\n}}", other_stuff.join("\n "))) + } else { + expr_block(cx, &arms[0].body, Some(other_stuff.join("\n ")), "..") + }; + span_help_and_lint(cx, + WHILE_LET_LOOP, + expr.span, + "this loop could be written as a `while let` loop", + &format!("try\nwhile let {} = {} {}", + snippet(cx, arms[0].pats[0].span, ".."), + snippet(cx, matchexpr.span, ".."), + loop_body)); + } + } + _ => (), } } } @@ -204,21 +210,20 @@ impl LateLintPass for LoopsPass { if let ExprMatch(ref match_expr, ref arms, MatchSource::WhileLetDesugar) = expr.node { let pat = &arms[0].pats[0].node; if let (&PatEnum(ref path, Some(ref pat_args)), - &ExprMethodCall(method_name, _, ref method_args)) = - (pat, &match_expr.node) { + &ExprMethodCall(method_name, _, ref method_args)) = (pat, &match_expr.node) { let iter_expr = &method_args[0]; if let Some(lhs_constructor) = path.segments.last() { if method_name.node.as_str() == "next" && - match_trait_method(cx, match_expr, &["core", "iter", "Iterator"]) && - lhs_constructor.identifier.name.as_str() == "Some" && - !is_iterator_used_after_while_let(cx, iter_expr) { + match_trait_method(cx, match_expr, &["core", "iter", "Iterator"]) && + lhs_constructor.identifier.name.as_str() == "Some" && + !is_iterator_used_after_while_let(cx, iter_expr) { let iterator = snippet(cx, method_args[0].span, "_"); let loop_var = snippet(cx, pat_args[0].span, "_"); - span_help_and_lint(cx, WHILE_LET_ON_ITERATOR, expr.span, + span_help_and_lint(cx, + WHILE_LET_ON_ITERATOR, + expr.span, "this loop could be written as a `for` loop", - &format!("try\nfor {} in {} {{...}}", - loop_var, - iterator)); + &format!("try\nfor {} in {} {{...}}", loop_var, iterator)); } } } @@ -229,10 +234,12 @@ impl LateLintPass for LoopsPass { if let StmtSemi(ref expr, _) = stmt.node { if let ExprMethodCall(ref method, _, ref args) = expr.node { if args.len() == 1 && method.node.as_str() == "collect" && - match_trait_method(cx, expr, &["core", "iter", "Iterator"]) { - span_lint(cx, UNUSED_COLLECT, expr.span, &format!( - "you are collect()ing an iterator and throwing away the result. \ - Consider using an explicit for loop to exhaust the iterator")); + match_trait_method(cx, expr, &["core", "iter", "Iterator"]) { + span_lint(cx, + UNUSED_COLLECT, + expr.span, + &format!("you are collect()ing an iterator and throwing away the result. Consider \ + using an explicit for loop to exhaust the iterator")); } } } @@ -249,23 +256,38 @@ fn check_for_loop(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &E // the var must be a single name if let PatIdent(_, ref ident, _) = pat.node { - let mut visitor = VarVisitor { cx: cx, var: ident.node.name, - indexed: HashSet::new(), nonindex: false }; + let mut visitor = VarVisitor { + cx: cx, + var: ident.node.name, + indexed: HashSet::new(), + nonindex: false, + }; walk_expr(&mut visitor, body); // linting condition: we only indexed one variable if visitor.indexed.len() == 1 { - let indexed = visitor.indexed.into_iter().next().expect( - "Len was nonzero, but no contents found"); + let indexed = visitor.indexed + .into_iter() + .next() + .expect("Len was nonzero, but no contents found"); if visitor.nonindex { - span_lint(cx, NEEDLESS_RANGE_LOOP, expr.span, &format!( - "the loop variable `{}` is used to index `{}`. Consider using \ - `for ({}, item) in {}.iter().enumerate()` or similar iterators", - ident.node.name, indexed, ident.node.name, indexed)); + span_lint(cx, + NEEDLESS_RANGE_LOOP, + expr.span, + &format!("the loop variable `{}` is used to index `{}`. Consider using `for \ + ({}, item) in {}.iter().enumerate()` or similar iterators", + ident.node.name, + indexed, + ident.node.name, + indexed)); } else { - span_lint(cx, NEEDLESS_RANGE_LOOP, expr.span, &format!( - "the loop variable `{}` is only used to index `{}`. \ - Consider using `for item in &{}` or similar iterators", - ident.node.name, indexed, indexed)); + span_lint(cx, + NEEDLESS_RANGE_LOOP, + expr.span, + &format!("the loop variable `{}` is only used to index `{}`. Consider using \ + `for item in &{}` or similar iterators", + ident.node.name, + indexed, + indexed)); } } } @@ -283,15 +305,21 @@ fn check_for_loop(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &E // who think that this will iterate from the larger value to the // smaller value. if start_idx > stop_idx { - span_help_and_lint(cx, REVERSE_RANGE_LOOP, expr.span, - "this range is empty so this for loop will never run", - &format!("Consider using `({}..{}).rev()` if you are attempting to \ - iterate over this range in reverse", stop_idx, start_idx)); + span_help_and_lint(cx, + REVERSE_RANGE_LOOP, + expr.span, + "this range is empty so this for loop will never run", + &format!("Consider using `({}..{}).rev()` if you are attempting to iterate \ + over this range in reverse", + stop_idx, + start_idx)); } else if start_idx == stop_idx { // if they are equal, it's also problematic - this loop // will never run. - span_lint(cx, REVERSE_RANGE_LOOP, expr.span, - "this range is empty so this for loop will never run"); + span_lint(cx, + REVERSE_RANGE_LOOP, + expr.span, + "this range is empty so this for loop will never run"); } } } @@ -305,46 +333,65 @@ fn check_for_loop(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &E if method_name.as_str() == "iter" || method_name.as_str() == "iter_mut" { if is_ref_iterable_type(cx, &args[0]) { let object = snippet(cx, args[0].span, "_"); - span_lint(cx, EXPLICIT_ITER_LOOP, expr.span, &format!( - "it is more idiomatic to loop over `&{}{}` instead of `{}.{}()`", - if method_name.as_str() == "iter_mut" { "mut " } else { "" }, - object, object, method_name)); + span_lint(cx, + EXPLICIT_ITER_LOOP, + expr.span, + &format!("it is more idiomatic to loop over `&{}{}` instead of `{}.{}()`", + if method_name.as_str() == "iter_mut" { + "mut " + } else { + "" + }, + object, + object, + method_name)); } - } - // check for looping over Iterator::next() which is not what you want - else if method_name.as_str() == "next" && - match_trait_method(cx, arg, &["core", "iter", "Iterator"]) { - span_lint(cx, ITER_NEXT_LOOP, expr.span, - "you are iterating over `Iterator::next()` which is an Option; \ - this will compile but is probably not what you want"); + } else if method_name.as_str() == "next" && match_trait_method(cx, arg, &["core", "iter", "Iterator"]) { + span_lint(cx, + ITER_NEXT_LOOP, + expr.span, + "you are iterating over `Iterator::next()` which is an Option; this will compile but is \ + probably not what you want"); } } } // Look for variables that are incremented once per loop iteration. - let mut visitor = IncrementVisitor { cx: cx, states: HashMap::new(), depth: 0, done: false }; + let mut visitor = IncrementVisitor { + cx: cx, + states: HashMap::new(), + depth: 0, + done: false, + }; walk_expr(&mut visitor, body); // For each candidate, check the parent block to see if // it's initialized to zero at the start of the loop. let map = &cx.tcx.map; - let parent_scope = map.get_enclosing_scope(expr.id).and_then(|id| map.get_enclosing_scope(id) ); + let parent_scope = map.get_enclosing_scope(expr.id).and_then(|id| map.get_enclosing_scope(id)); if let Some(parent_id) = parent_scope { if let NodeBlock(block) = map.get(parent_id) { - for (id, _) in visitor.states.iter().filter( |&(_,v)| *v == VarState::IncrOnce) { - let mut visitor2 = InitializeVisitor { cx: cx, end_expr: expr, var_id: id.clone(), - state: VarState::IncrOnce, name: None, - depth: 0, - past_loop: false }; + for (id, _) in visitor.states.iter().filter(|&(_, v)| *v == VarState::IncrOnce) { + let mut visitor2 = InitializeVisitor { + cx: cx, + end_expr: expr, + var_id: id.clone(), + state: VarState::IncrOnce, + name: None, + depth: 0, + past_loop: false, + }; walk_block(&mut visitor2, block); if visitor2.state == VarState::Warn { if let Some(name) = visitor2.name { - span_lint(cx, EXPLICIT_COUNTER_LOOP, expr.span, - &format!("the variable `{0}` is used as a loop counter. Consider \ - using `for ({0}, item) in {1}.enumerate()` \ - or similar iterators", - name, snippet(cx, arg.span, "_"))); + span_lint(cx, + EXPLICIT_COUNTER_LOOP, + expr.span, + &format!("the variable `{0}` is used as a loop counter. Consider using `for ({0}, \ + item) in {1}.enumerate()` or similar iterators", + name, + snippet(cx, arg.span, "_"))); } } } @@ -378,9 +425,9 @@ fn recover_for_loop(expr: &Expr) -> Option<(&Pat, &Expr, &Expr)> { struct VarVisitor<'v, 't: 'v> { cx: &'v LateContext<'v, 't>, // context reference - var: Name, // var name to look for as index - indexed: HashSet<Name>, // indexed variables - nonindex: bool, // has the var been used otherwise? + var: Name, // var name to look for as index + indexed: HashSet<Name>, // indexed variables + nonindex: bool, // has the var been used otherwise? } impl<'v, 't> Visitor<'v> for VarVisitor<'v, 't> { @@ -411,14 +458,14 @@ impl<'v, 't> Visitor<'v> for VarVisitor<'v, 't> { fn is_iterator_used_after_while_let(cx: &LateContext, iter_expr: &Expr) -> bool { let def_id = match var_def_id(cx, iter_expr) { Some(id) => id, - None => return false + None => return false, }; let mut visitor = VarUsedAfterLoopVisitor { cx: cx, def_id: def_id, iter_expr_id: iter_expr.id, past_while_let: false, - var_used_after_while_let: false + var_used_after_while_let: false, }; if let Some(enclosing_block) = get_enclosing_block(cx, def_id) { walk_block(&mut visitor, enclosing_block); @@ -431,10 +478,10 @@ struct VarUsedAfterLoopVisitor<'v, 't: 'v> { def_id: NodeId, iter_expr_id: NodeId, past_while_let: bool, - var_used_after_while_let: bool + var_used_after_while_let: bool, } -impl <'v, 't> Visitor<'v> for VarUsedAfterLoopVisitor<'v, 't> { +impl<'v, 't> Visitor<'v> for VarUsedAfterLoopVisitor<'v, 't> { fn visit_expr(&mut self, expr: &'v Expr) { if self.past_while_let { if Some(self.def_id) == var_def_id(self.cx, expr) { @@ -454,43 +501,52 @@ fn is_ref_iterable_type(cx: &LateContext, e: &Expr) -> bool { // no walk_ptrs_ty: calling iter() on a reference can make sense because it // will allow further borrows afterwards let ty = cx.tcx.expr_ty(e); - is_iterable_array(ty) || - match_type(cx, ty, &VEC_PATH) || - match_type(cx, ty, &LL_PATH) || - match_type(cx, ty, &HASHMAP_PATH) || - match_type(cx, ty, &["std", "collections", "hash", "set", "HashSet"]) || - match_type(cx, ty, &["collections", "vec_deque", "VecDeque"]) || - match_type(cx, ty, &["collections", "binary_heap", "BinaryHeap"]) || - match_type(cx, ty, &["collections", "btree", "map", "BTreeMap"]) || - match_type(cx, ty, &["collections", "btree", "set", "BTreeSet"]) + is_iterable_array(ty) || match_type(cx, ty, &VEC_PATH) || match_type(cx, ty, &LL_PATH) || + match_type(cx, ty, &HASHMAP_PATH) || match_type(cx, ty, &["std", "collections", "hash", "set", "HashSet"]) || + match_type(cx, ty, &["collections", "vec_deque", "VecDeque"]) || + match_type(cx, ty, &["collections", "binary_heap", "BinaryHeap"]) || + match_type(cx, ty, &["collections", "btree", "map", "BTreeMap"]) || + match_type(cx, ty, &["collections", "btree", "set", "BTreeSet"]) } fn is_iterable_array(ty: ty::Ty) -> bool { // IntoIterator is currently only implemented for array sizes <= 32 in rustc match ty.sty { ty::TyArray(_, 0...32) => true, - _ => false + _ => false, } } /// If a block begins with a statement (possibly a `let` binding) and has an expression, return it. fn extract_expr_from_first_stmt(block: &Block) -> Option<&Expr> { - if block.stmts.is_empty() { return None; } + if block.stmts.is_empty() { + return None; + } if let StmtDecl(ref decl, _) = block.stmts[0].node { if let DeclLocal(ref local) = decl.node { - if let Some(ref expr) = local.init { Some(expr) } else { None } - } else { None } - } else { None } + if let Some(ref expr) = local.init { + Some(expr) + } else { + None + } + } else { + None + } + } else { + None + } } /// If a block begins with an expression (with or without semicolon), return it. fn extract_first_expr(block: &Block) -> Option<&Expr> { match block.expr { Some(ref expr) => Some(expr), - None if !block.stmts.is_empty() => match block.stmts[0].node { - StmtExpr(ref expr, _) | StmtSemi(ref expr, _) => Some(expr), - _ => None, - }, + None if !block.stmts.is_empty() => { + match block.stmts[0].node { + StmtExpr(ref expr, _) | StmtSemi(ref expr, _) => Some(expr), + _ => None, + } + } _ => None, } } @@ -500,10 +556,12 @@ fn is_break_expr(expr: &Expr) -> bool { match expr.node { ExprBreak(None) => true, // there won't be a `let <pat> = break` and so we can safely ignore the StmtDecl case - ExprBlock(ref b) => match extract_first_expr(b) { - Some(ref subexpr) => is_break_expr(subexpr), - None => false, - }, + ExprBlock(ref b) => { + match extract_first_expr(b) { + Some(ref subexpr) => is_break_expr(subexpr), + None => false, + } + } _ => false, } } @@ -513,19 +571,19 @@ fn is_break_expr(expr: &Expr) -> bool { // at the start of the loop. #[derive(PartialEq)] enum VarState { - Initial, // Not examined yet - IncrOnce, // Incremented exactly once, may be a loop counter - Declared, // Declared but not (yet) initialized to zero + Initial, // Not examined yet + IncrOnce, // Incremented exactly once, may be a loop counter + Declared, // Declared but not (yet) initialized to zero Warn, - DontWarn + DontWarn, } // Scan a for loop for variables that are incremented exactly once. struct IncrementVisitor<'v, 't: 'v> { - cx: &'v LateContext<'v, 't>, // context reference - states: HashMap<NodeId, VarState>, // incremented variables - depth: u32, // depth of conditional expressions - done: bool + cx: &'v LateContext<'v, 't>, // context reference + states: HashMap<NodeId, VarState>, // incremented variables + depth: u32, // depth of conditional expressions + done: bool, } impl<'v, 't> Visitor<'v> for IncrementVisitor<'v, 't> { @@ -540,33 +598,29 @@ impl<'v, 't> Visitor<'v> for IncrementVisitor<'v, 't> { let state = self.states.entry(def_id).or_insert(VarState::Initial); match parent.node { - ExprAssignOp(op, ref lhs, ref rhs) => + ExprAssignOp(op, ref lhs, ref rhs) => { if lhs.id == expr.id { if op.node == BiAdd && is_integer_literal(rhs, 1) { *state = match *state { VarState::Initial if self.depth == 0 => VarState::IncrOnce, - _ => VarState::DontWarn + _ => VarState::DontWarn, }; - } - else { + } else { // Assigned some other value *state = VarState::DontWarn; } - }, + } + } ExprAssign(ref lhs, _) if lhs.id == expr.id => *state = VarState::DontWarn, - ExprAddrOf(mutability,_) if mutability == MutMutable => *state = VarState::DontWarn, - _ => () + ExprAddrOf(mutability, _) if mutability == MutMutable => *state = VarState::DontWarn, + _ => (), } } - } - // Give up if there are nested loops - else if is_loop(expr) { + } else if is_loop(expr) { self.states.clear(); self.done = true; return; - } - // Keep track of whether we're inside a conditional expression - else if is_conditional(expr) { + } else if is_conditional(expr) { self.depth += 1; walk_expr(self, expr); self.depth -= 1; @@ -579,12 +633,12 @@ impl<'v, 't> Visitor<'v> for IncrementVisitor<'v, 't> { // Check whether a variable is initialized to zero at the start of a loop. struct InitializeVisitor<'v, 't: 'v> { cx: &'v LateContext<'v, 't>, // context reference - end_expr: &'v Expr, // the for loop. Stop scanning here. + end_expr: &'v Expr, // the for loop. Stop scanning here. var_id: NodeId, state: VarState, name: Option<Name>, - depth: u32, // depth of conditional expressions - past_loop: bool + depth: u32, // depth of conditional expressions + past_loop: bool, } impl<'v, 't> Visitor<'v> for InitializeVisitor<'v, 't> { @@ -601,8 +655,7 @@ impl<'v, 't> Visitor<'v> for InitializeVisitor<'v, 't> { } else { VarState::Declared } - } - else { + } else { VarState::Declared } } @@ -637,9 +690,10 @@ impl<'v, 't> Visitor<'v> for InitializeVisitor<'v, 't> { VarState::Warn } else { VarState::DontWarn - }} - ExprAddrOf(mutability,_) if mutability == MutMutable => self.state = VarState::DontWarn, - _ => () + } + } + ExprAddrOf(mutability, _) if mutability == MutMutable => self.state = VarState::DontWarn, + _ => (), } } @@ -647,14 +701,10 @@ impl<'v, 't> Visitor<'v> for InitializeVisitor<'v, 't> { self.state = VarState::DontWarn; return; } - } - // If there are other loops between the declaration and the target loop, give up - else if !self.past_loop && is_loop(expr) { + } else if !self.past_loop && is_loop(expr) { self.state = VarState::DontWarn; return; - } - // Keep track of whether we're inside a conditional expression - else if is_conditional(expr) { + } else if is_conditional(expr) { self.depth += 1; walk_expr(self, expr); self.depth -= 1; @@ -667,7 +717,7 @@ impl<'v, 't> Visitor<'v> for InitializeVisitor<'v, 't> { fn var_def_id(cx: &LateContext, expr: &Expr) -> Option<NodeId> { if let Some(path_res) = cx.tcx.def_map.borrow().get(&expr.id) { if let DefLocal(_, node_id) = path_res.base_def { - return Some(node_id) + return Some(node_id); } } None @@ -675,14 +725,14 @@ fn var_def_id(cx: &LateContext, expr: &Expr) -> Option<NodeId> { fn is_loop(expr: &Expr) -> bool { match expr.node { - ExprLoop(..) | ExprWhile(..) => true, - _ => false + ExprLoop(..) | ExprWhile(..) => true, + _ => false, } } fn is_conditional(expr: &Expr) -> bool { match expr.node { ExprIf(..) | ExprMatch(..) => true, - _ => false + _ => false, } } diff --git a/src/map_clone.rs b/src/map_clone.rs index ef992ad086c..9db97a1b9f2 100644 --- a/src/map_clone.rs +++ b/src/map_clone.rs @@ -65,10 +65,13 @@ impl LateLintPass for MapClonePass { ExprPath(_, ref path) => { if match_path(path, &CLONE_PATH) { let type_name = get_type_name(cx, expr, &args[0]).unwrap_or("_"); - 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, ".."))); + 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, ".."))); } } _ => (), @@ -81,7 +84,10 @@ impl LateLintPass for MapClonePass { fn expr_eq_ident(expr: &Expr, id: Ident) -> bool { match expr.node { ExprPath(None, ref path) => { - let arg_segment = [PathSegment { identifier: id, parameters: PathParameters::none() }]; + let arg_segment = [PathSegment { + identifier: id, + parameters: PathParameters::none(), + }]; !path.global && path.segments[..] == arg_segment } _ => false, @@ -108,9 +114,7 @@ fn get_arg_name(pat: &Pat) -> Option<Ident> { fn only_derefs(cx: &LateContext, expr: &Expr, id: Ident) -> bool { match expr.node { - ExprUnary(UnDeref, ref subexpr) if !is_adjusted(cx, subexpr) => { - only_derefs(cx, subexpr, id) - } + ExprUnary(UnDeref, ref subexpr) if !is_adjusted(cx, subexpr) => only_derefs(cx, subexpr, id), _ => expr_eq_ident(expr, id), } } diff --git a/src/matches.rs b/src/matches.rs index 8564c36e617..5e207093c15 100644 --- a/src/matches.rs +++ b/src/matches.rs @@ -94,7 +94,9 @@ impl LintPass for MatchPass { impl LateLintPass for MatchPass { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { - if in_external_macro(cx, expr.span) { return; } + if in_external_macro(cx, expr.span) { + return; + } if let ExprMatch(ref ex, ref arms, MatchSource::Normal) = expr.node { check_single_match(cx, ex, arms, expr); check_match_bool(cx, ex, arms, expr); @@ -107,23 +109,14 @@ impl LateLintPass for MatchPass { } fn check_single_match(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) { - if arms.len() == 2 && - // both of the arms have a single pattern and no guard - arms[0].pats.len() == 1 && arms[0].guard.is_none() && - arms[1].pats.len() == 1 && arms[1].guard.is_none() && - // and the second pattern is a `_` wildcard: this is not strictly necessary, - // since the exhaustiveness check will ensure the last one is a catch-all, - // but in some cases, an explicit match is preferred to catch situations - // when an enum is extended, so we don't consider these cases - arms[1].pats[0].node == PatWild && - // we don't want any content in the second arm (unit or empty block) - is_unit_expr(&arms[1].body) && - // finally, MATCH_BOOL doesn't apply here - (cx.tcx.expr_ty(ex).sty != ty::TyBool || cx.current_level(MATCH_BOOL) == Allow) - { - span_help_and_lint(cx, SINGLE_MATCH, expr.span, - "you seem to be trying to use match for destructuring a \ - single pattern. Consider using `if let`", + if arms.len() == 2 && arms[0].pats.len() == 1 && arms[0].guard.is_none() && arms[1].pats.len() == 1 && + arms[1].guard.is_none() && arms[1].pats[0].node == PatWild && is_unit_expr(&arms[1].body) && + (cx.tcx.expr_ty(ex).sty != ty::TyBool || cx.current_level(MATCH_BOOL) == Allow) { + span_help_and_lint(cx, + SINGLE_MATCH, + expr.span, + "you seem to be trying to use match for destructuring a single pattern. Consider using \ + `if let`", &format!("try\nif let {} = {} {}", snippet(cx, arms[0].pats[0].span, ".."), snippet(cx, ex.span, ".."), @@ -134,7 +127,8 @@ fn check_single_match(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) { fn check_match_bool(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) { // type of expression == bool if cx.tcx.expr_ty(ex).sty == ty::TyBool { - if arms.len() == 2 && arms[0].pats.len() == 1 { // no guards + if arms.len() == 2 && arms[0].pats.len() == 1 { + // no guards let exprs = if let PatLit(ref arm_bool) = arms[0].pats[0].node { if let ExprLit(ref lit) = arm_bool.node { match lit.node { @@ -142,54 +136,67 @@ fn check_match_bool(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) { LitBool(false) => Some((&*arms[1].body, &*arms[0].body)), _ => None, } - } else { None } - } else { None }; + } else { + None + } + } else { + None + }; if let Some((ref true_expr, ref false_expr)) = exprs { if !is_unit_expr(true_expr) { if !is_unit_expr(false_expr) { - span_help_and_lint(cx, MATCH_BOOL, expr.span, - "you seem to be trying to match on a boolean expression. \ - Consider using an if..else block:", - &format!("try\nif {} {} else {}", - snippet(cx, ex.span, "b"), - expr_block(cx, true_expr, None, ".."), - expr_block(cx, false_expr, None, ".."))); + span_help_and_lint(cx, + MATCH_BOOL, + expr.span, + "you seem to be trying to match on a boolean expression. Consider using \ + an if..else block:", + &format!("try\nif {} {} else {}", + snippet(cx, ex.span, "b"), + expr_block(cx, true_expr, None, ".."), + expr_block(cx, false_expr, None, ".."))); } else { - span_help_and_lint(cx, MATCH_BOOL, expr.span, - "you seem to be trying to match on a boolean expression. \ - Consider using an if..else block:", - &format!("try\nif {} {}", - snippet(cx, ex.span, "b"), - expr_block(cx, true_expr, None, ".."))); + span_help_and_lint(cx, + MATCH_BOOL, + expr.span, + "you seem to be trying to match on a boolean expression. Consider using \ + an if..else block:", + &format!("try\nif {} {}", + snippet(cx, ex.span, "b"), + expr_block(cx, true_expr, None, ".."))); } } else if !is_unit_expr(false_expr) { - span_help_and_lint(cx, MATCH_BOOL, expr.span, - "you seem to be trying to match on a boolean expression. \ - Consider using an if..else block:", - &format!("try\nif !{} {}", - snippet(cx, ex.span, "b"), - expr_block(cx, false_expr, None, ".."))); + span_help_and_lint(cx, + MATCH_BOOL, + expr.span, + "you seem to be trying to match on a boolean expression. Consider using an \ + if..else block:", + &format!("try\nif !{} {}", + snippet(cx, ex.span, "b"), + expr_block(cx, false_expr, None, ".."))); } else { - span_lint(cx, MATCH_BOOL, expr.span, - "you seem to be trying to match on a boolean expression. \ - Consider using an if..else block"); + span_lint(cx, + MATCH_BOOL, + expr.span, + "you seem to be trying to match on a boolean expression. Consider using an if..else \ + block"); } } else { - span_lint(cx, MATCH_BOOL, expr.span, - "you seem to be trying to match on a boolean expression. \ - Consider using an if..else block"); + span_lint(cx, + MATCH_BOOL, + expr.span, + "you seem to be trying to match on a boolean expression. Consider using an if..else block"); } } else { - span_lint(cx, MATCH_BOOL, expr.span, - "you seem to be trying to match on a boolean expression. \ - Consider using an if..else block"); + span_lint(cx, + MATCH_BOOL, + expr.span, + "you seem to be trying to match on a boolean expression. Consider using an if..else block"); } } } fn check_overlapping_arms(cx: &LateContext, ex: &Expr, arms: &[Arm]) { - if arms.len() >= 2 && - cx.tcx.expr_ty(ex).is_integral() { + if arms.len() >= 2 && cx.tcx.expr_ty(ex).is_integral() { let ranges = all_ranges(cx, arms); let overlap = match type_ranges(&ranges) { TypedRanges::IntRanges(ranges) => overlapping(&ranges).map(|(start, end)| (start.span, end.span)), @@ -198,9 +205,12 @@ fn check_overlapping_arms(cx: &LateContext, ex: &Expr, arms: &[Arm]) { }; if let Some((start, end)) = overlap { - span_note_and_lint(cx, MATCH_OVERLAPPING_ARM, start, + span_note_and_lint(cx, + MATCH_OVERLAPPING_ARM, + start, "some ranges overlap", - end, "overlaps with this"); + end, + "overlaps with this"); } } } @@ -209,14 +219,18 @@ fn check_match_ref_pats(cx: &LateContext, ex: &Expr, arms: &[Arm], source: Match if has_only_ref_pats(arms) { if let ExprAddrOf(Mutability::MutImmutable, ref inner) = ex.node { let template = match_template(cx, expr.span, source, "", inner); - span_lint(cx, MATCH_REF_PATS, expr.span, &format!( - "you don't need to add `&` to both the expression \ - and the patterns: use `{}`", template)); + span_lint(cx, + MATCH_REF_PATS, + expr.span, + &format!("you don't need to add `&` to both the expression and the patterns: use `{}`", + template)); } else { let template = match_template(cx, expr.span, source, "*", ex); - span_lint(cx, MATCH_REF_PATS, expr.span, &format!( - "instead of prefixing all patterns with `&`, you can dereference the \ - expression: `{}`", template)); + span_lint(cx, + MATCH_REF_PATS, + expr.span, + &format!("instead of prefixing all patterns with `&`, you can dereference the expression: `{}`", + template)); } } } @@ -244,8 +258,7 @@ fn all_ranges(cx: &LateContext, arms: &[Arm]) -> Vec<SpannedRange<ConstVal>> { None })) - } - else { + } else { None } }) @@ -271,29 +284,36 @@ enum TypedRanges { fn type_ranges(ranges: &[SpannedRange<ConstVal>]) -> TypedRanges { if ranges.is_empty() { TypedRanges::None - } - else { + } else { match ranges[0].node { (Int(_), Int(_)) => { - TypedRanges::IntRanges(ranges.iter().filter_map(|range| { - if let (Int(start), Int(end)) = range.node { - Some(SpannedRange { span: range.span, node: (start, end) }) - } - else { - None - } - }).collect()) - }, + TypedRanges::IntRanges(ranges.iter() + .filter_map(|range| { + if let (Int(start), Int(end)) = range.node { + Some(SpannedRange { + span: range.span, + node: (start, end), + }) + } else { + None + } + }) + .collect()) + } (Uint(_), Uint(_)) => { - TypedRanges::UintRanges(ranges.iter().filter_map(|range| { - if let (Uint(start), Uint(end)) = range.node { - Some(SpannedRange { span: range.span, node: (start, end) }) - } - else { - None - } - }).collect()) - }, + TypedRanges::UintRanges(ranges.iter() + .filter_map(|range| { + if let (Uint(start), Uint(end)) = range.node { + Some(SpannedRange { + span: range.span, + node: (start, end), + }) + } else { + None + } + }) + .collect()) + } _ => TypedRanges::None, } } @@ -308,39 +328,33 @@ fn is_unit_expr(expr: &Expr) -> bool { } fn has_only_ref_pats(arms: &[Arm]) -> bool { - let mapped = arms.iter().flat_map(|a| &a.pats).map(|p| match p.node { - PatRegion(..) => Some(true), // &-patterns - PatWild => Some(false), // an "anything" wildcard is also fine - _ => None, // any other pattern is not fine - }).collect::<Option<Vec<bool>>>(); + let mapped = arms.iter() + .flat_map(|a| &a.pats) + .map(|p| { + match p.node { + PatRegion(..) => Some(true), // &-patterns + PatWild => Some(false), // an "anything" wildcard is also fine + _ => None, // any other pattern is not fine + } + }) + .collect::<Option<Vec<bool>>>(); // look for Some(v) where there's at least one true element mapped.map_or(false, |v| v.iter().any(|el| *el)) } -fn match_template(cx: &LateContext, - span: Span, - source: MatchSource, - op: &str, - expr: &Expr) -> String { +fn match_template(cx: &LateContext, span: Span, source: MatchSource, op: &str, expr: &Expr) -> String { let expr_snippet = snippet(cx, expr.span, ".."); match source { - MatchSource::Normal => { - format!("match {}{} {{ ...", op, expr_snippet) - } - MatchSource::IfLetDesugar { .. } => { - format!("if let ... = {}{} {{", op, expr_snippet) - } - MatchSource::WhileLetDesugar => { - format!("while let ... = {}{} {{", op, expr_snippet) - } - MatchSource::ForLoopDesugar => { - cx.sess().span_bug(span, "for loop desugared to match with &-patterns!") - } + MatchSource::Normal => format!("match {}{} {{ ...", op, expr_snippet), + MatchSource::IfLetDesugar { .. } => format!("if let ... = {}{} {{", op, expr_snippet), + MatchSource::WhileLetDesugar => format!("while let ... = {}{} {{", op, expr_snippet), + MatchSource::ForLoopDesugar => cx.sess().span_bug(span, "for loop desugared to match with &-patterns!"), } } pub fn overlapping<T>(ranges: &[SpannedRange<T>]) -> Option<(&SpannedRange<T>, &SpannedRange<T>)> - where T: Copy + Ord { + where T: Copy + Ord +{ #[derive(Copy, Clone, Debug, Eq, PartialEq)] enum Kind<'a, T: 'a> { Start(T, &'a SpannedRange<T>), @@ -350,13 +364,13 @@ pub fn overlapping<T>(ranges: &[SpannedRange<T>]) -> Option<(&SpannedRange<T>, & impl<'a, T: Copy> Kind<'a, T> { fn range(&self) -> &'a SpannedRange<T> { match *self { - Kind::Start(_, r) | Kind::End(_, r) => r + Kind::Start(_, r) | Kind::End(_, r) => r, } } fn value(self) -> T { match self { - Kind::Start(t, _) | Kind::End(t, _) => t + Kind::Start(t, _) | Kind::End(t, _) => t, } } } @@ -373,7 +387,7 @@ pub fn overlapping<T>(ranges: &[SpannedRange<T>]) -> Option<(&SpannedRange<T>, & } } - let mut values = Vec::with_capacity(2*ranges.len()); + let mut values = Vec::with_capacity(2 * ranges.len()); for r in ranges { values.push(Kind::Start(r.node.0, &r)); @@ -384,7 +398,11 @@ pub fn overlapping<T>(ranges: &[SpannedRange<T>]) -> Option<(&SpannedRange<T>, & for (a, b) in values.iter().zip(values.iter().skip(1)) { match (a, b) { - (&Kind::Start(_, ra), &Kind::End(_, rb)) => if ra.node != rb.node { return Some((ra, rb)) }, + (&Kind::Start(_, ra), &Kind::End(_, rb)) => { + if ra.node != rb.node { + return Some((ra, rb)); + } + } (&Kind::End(a, _), &Kind::Start(b, _)) if a != b => (), _ => return Some((&a.range(), &b.range())), } diff --git a/src/methods.rs b/src/methods.rs index 2aa3b040e55..83b9e60f4be 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -5,8 +5,8 @@ use rustc::middle::subst::{Subst, TypeSpace}; use std::iter; use std::borrow::Cow; -use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, method_chain_args, - match_trait_method, walk_ptrs_ty_depth, walk_ptrs_ty}; +use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, method_chain_args, match_trait_method, + walk_ptrs_ty_depth, walk_ptrs_ty}; use utils::{OPTION_PATH, RESULT_PATH, STRING_PATH}; use utils::MethodArgs; @@ -172,9 +172,16 @@ declare_lint!(pub SEARCH_IS_SOME, Warn, impl LintPass for MethodsPass { fn get_lints(&self) -> LintArray { - lint_array!(OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, STR_TO_STRING, STRING_TO_STRING, - SHOULD_IMPLEMENT_TRAIT, WRONG_SELF_CONVENTION, WRONG_PUB_SELF_CONVENTION, - OK_EXPECT, OPTION_MAP_UNWRAP_OR, OPTION_MAP_UNWRAP_OR_ELSE) + lint_array!(OPTION_UNWRAP_USED, + RESULT_UNWRAP_USED, + STR_TO_STRING, + STRING_TO_STRING, + SHOULD_IMPLEMENT_TRAIT, + WRONG_SELF_CONVENTION, + WRONG_PUB_SELF_CONVENTION, + OK_EXPECT, + OPTION_MAP_UNWRAP_OR, + OPTION_MAP_UNWRAP_OR_ELSE) } } @@ -183,29 +190,21 @@ impl LateLintPass for MethodsPass { if let ExprMethodCall(_, _, _) = expr.node { if let Some(arglists) = method_chain_args(expr, &["unwrap"]) { lint_unwrap(cx, expr, arglists[0]); - } - else if let Some(arglists) = method_chain_args(expr, &["to_string"]) { + } else if let Some(arglists) = method_chain_args(expr, &["to_string"]) { lint_to_string(cx, expr, arglists[0]); - } - else if let Some(arglists) = method_chain_args(expr, &["ok", "expect"]) { + } else if let Some(arglists) = method_chain_args(expr, &["ok", "expect"]) { lint_ok_expect(cx, expr, arglists[0]); - } - else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or"]) { + } else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or"]) { lint_map_unwrap_or(cx, expr, arglists[0], arglists[1]); - } - else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or_else"]) { + } else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or_else"]) { lint_map_unwrap_or_else(cx, expr, arglists[0], arglists[1]); - } - else if let Some(arglists) = method_chain_args(expr, &["filter", "next"]) { + } else if let Some(arglists) = method_chain_args(expr, &["filter", "next"]) { lint_filter_next(cx, expr, arglists[0]); - } - else if let Some(arglists) = method_chain_args(expr, &["find", "is_some"]) { + } else if let Some(arglists) = method_chain_args(expr, &["find", "is_some"]) { lint_search_is_some(cx, expr, "find", arglists[0], arglists[1]); - } - else if let Some(arglists) = method_chain_args(expr, &["position", "is_some"]) { + } else if let Some(arglists) = method_chain_args(expr, &["position", "is_some"]) { lint_search_is_some(cx, expr, "position", arglists[0], arglists[1]); - } - else if let Some(arglists) = method_chain_args(expr, &["rposition", "is_some"]) { + } else if let Some(arglists) = method_chain_args(expr, &["rposition", "is_some"]) { lint_search_is_some(cx, expr, "rposition", arglists[0], arglists[1]); } } @@ -235,16 +234,22 @@ impl LateLintPass for MethodsPass { let is_copy = is_copy(cx, &ty, &item); for &(prefix, self_kinds) in &CONVENTIONS { if name.as_str().starts_with(prefix) && - !self_kinds.iter().any(|k| k.matches(&sig.explicit_self.node, is_copy)) { + !self_kinds.iter().any(|k| k.matches(&sig.explicit_self.node, is_copy)) { let lint = if item.vis == Visibility::Public { WRONG_PUB_SELF_CONVENTION } else { WRONG_SELF_CONVENTION }; - span_lint(cx, lint, sig.explicit_self.span, &format!( - "methods called `{}*` usually take {}; consider choosing a less \ - ambiguous name", prefix, - &self_kinds.iter().map(|k| k.description()).collect::<Vec<_>>().join(" or "))); + span_lint(cx, + lint, + sig.explicit_self.span, + &format!("methods called `{}*` usually take {}; consider choosing a less \ + ambiguous name", + prefix, + &self_kinds.iter() + .map(|k| k.description()) + .collect::<Vec<_>>() + .join(" or "))); } } } @@ -253,30 +258,34 @@ impl LateLintPass for MethodsPass { } } -#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec +#[allow(ptr_arg)] +// Type of MethodArgs is potentially a Vec /// lint use of `unwrap()` for `Option`s and `Result`s fn lint_unwrap(cx: &LateContext, expr: &Expr, unwrap_args: &MethodArgs) { let (obj_ty, _) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&unwrap_args[0])); let mess = if match_type(cx, obj_ty, &OPTION_PATH) { Some((OPTION_UNWRAP_USED, "an Option", "None")) - } - else if match_type(cx, obj_ty, &RESULT_PATH) { + } else if match_type(cx, obj_ty, &RESULT_PATH) { Some((RESULT_UNWRAP_USED, "a Result", "Err")) - } - else { + } else { None }; if let Some((lint, kind, none_value)) = mess { - span_lint(cx, lint, expr.span, - &format!("used unwrap() on {} value. If you don't want to handle the {} \ - case gracefully, consider using expect() to provide a better panic - message", kind, none_value)); + span_lint(cx, + lint, + expr.span, + &format!("used unwrap() on {} value. If you don't want to handle the {} case gracefully, consider \ + using expect() to provide a better panic + message", + kind, + none_value)); } } -#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec +#[allow(ptr_arg)] +// Type of MethodArgs is potentially a Vec /// lint use of `to_string()` for `&str`s and `String`s fn lint_to_string(cx: &LateContext, expr: &Expr, to_string_args: &MethodArgs) { let (obj_ty, ptr_depth) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&to_string_args[0])); @@ -284,21 +293,19 @@ fn lint_to_string(cx: &LateContext, expr: &Expr, to_string_args: &MethodArgs) { if obj_ty.sty == ty::TyStr { let mut arg_str = snippet(cx, to_string_args[0].span, "_"); if ptr_depth > 1 { - arg_str = Cow::Owned(format!( - "({}{})", - iter::repeat('*').take(ptr_depth - 1).collect::<String>(), - arg_str)); + arg_str = Cow::Owned(format!("({}{})", iter::repeat('*').take(ptr_depth - 1).collect::<String>(), arg_str)); } - span_lint(cx, STR_TO_STRING, expr.span, - &format!("`{}.to_owned()` is faster", arg_str)); - } - else if match_type(cx, obj_ty, &STRING_PATH) { - span_lint(cx, STRING_TO_STRING, expr.span, + span_lint(cx, STR_TO_STRING, expr.span, &format!("`{}.to_owned()` is faster", arg_str)); + } else if match_type(cx, obj_ty, &STRING_PATH) { + span_lint(cx, + STRING_TO_STRING, + expr.span, "`String.to_string()` is a no-op; use `clone()` to make a copy"); } } -#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec +#[allow(ptr_arg)] +// Type of MethodArgs is potentially a Vec /// lint use of `ok().expect()` for `Result`s fn lint_ok_expect(cx: &LateContext, expr: &Expr, ok_args: &MethodArgs) { // lint if the caller of `ok()` is a `Result` @@ -306,107 +313,120 @@ fn lint_ok_expect(cx: &LateContext, expr: &Expr, ok_args: &MethodArgs) { let result_type = cx.tcx.expr_ty(&ok_args[0]); if let Some(error_type) = get_error_type(cx, result_type) { if has_debug_impl(error_type, cx) { - span_lint(cx, OK_EXPECT, expr.span, - "called `ok().expect()` on a Result value. You can call `expect` \ - directly on the `Result`"); + span_lint(cx, + OK_EXPECT, + expr.span, + "called `ok().expect()` on a Result value. You can call `expect` directly on the `Result`"); } } } } -#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec +#[allow(ptr_arg)] +// Type of MethodArgs is potentially a Vec /// lint use of `map().unwrap_or()` for `Option`s -fn lint_map_unwrap_or(cx: &LateContext, expr: &Expr, map_args: &MethodArgs, - unwrap_args: &MethodArgs) { +fn lint_map_unwrap_or(cx: &LateContext, expr: &Expr, map_args: &MethodArgs, unwrap_args: &MethodArgs) { // lint if the caller of `map()` is an `Option` if match_type(cx, cx.tcx.expr_ty(&map_args[0]), &OPTION_PATH) { // lint message - let msg = "called `map(f).unwrap_or(a)` on an Option value. This can be done more \ - directly by calling `map_or(a, f)` instead"; + let msg = "called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling \ + `map_or(a, f)` instead"; // get snippets for args to map() and unwrap_or() let map_snippet = snippet(cx, map_args[1].span, ".."); let unwrap_snippet = snippet(cx, unwrap_args[1].span, ".."); // lint, with note if neither arg is > 1 line and both map() and // unwrap_or() have the same span - let multiline = map_snippet.lines().count() > 1 - || unwrap_snippet.lines().count() > 1; + let multiline = map_snippet.lines().count() > 1 || unwrap_snippet.lines().count() > 1; let same_span = map_args[1].span.expn_id == unwrap_args[1].span.expn_id; if same_span && !multiline { - span_note_and_lint( - cx, OPTION_MAP_UNWRAP_OR, expr.span, msg, expr.span, - &format!("replace `map({0}).unwrap_or({1})` with `map_or({1}, {0})`", map_snippet, - unwrap_snippet) - ); - } - else if same_span && multiline { + span_note_and_lint(cx, + OPTION_MAP_UNWRAP_OR, + expr.span, + msg, + expr.span, + &format!("replace `map({0}).unwrap_or({1})` with `map_or({1}, {0})`", + map_snippet, + unwrap_snippet)); + } else if same_span && multiline { span_lint(cx, OPTION_MAP_UNWRAP_OR, expr.span, msg); }; } } -#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec +#[allow(ptr_arg)] +// Type of MethodArgs is potentially a Vec /// lint use of `map().unwrap_or_else()` for `Option`s -fn lint_map_unwrap_or_else(cx: &LateContext, expr: &Expr, map_args: &MethodArgs, - unwrap_args: &MethodArgs) { +fn lint_map_unwrap_or_else(cx: &LateContext, expr: &Expr, map_args: &MethodArgs, unwrap_args: &MethodArgs) { // lint if the caller of `map()` is an `Option` if match_type(cx, cx.tcx.expr_ty(&map_args[0]), &OPTION_PATH) { // lint message - let msg = "called `map(f).unwrap_or_else(g)` on an Option value. This can be done more \ - directly by calling `map_or_else(g, f)` instead"; + let msg = "called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling \ + `map_or_else(g, f)` instead"; // get snippets for args to map() and unwrap_or_else() let map_snippet = snippet(cx, map_args[1].span, ".."); let unwrap_snippet = snippet(cx, unwrap_args[1].span, ".."); // lint, with note if neither arg is > 1 line and both map() and // unwrap_or_else() have the same span - let multiline = map_snippet.lines().count() > 1 - || unwrap_snippet.lines().count() > 1; + let multiline = map_snippet.lines().count() > 1 || unwrap_snippet.lines().count() > 1; let same_span = map_args[1].span.expn_id == unwrap_args[1].span.expn_id; if same_span && !multiline { - span_note_and_lint( - cx, OPTION_MAP_UNWRAP_OR_ELSE, expr.span, msg, expr.span, - &format!("replace `map({0}).unwrap_or_else({1})` with `with map_or_else({1}, {0})`", - map_snippet, unwrap_snippet) - ); - } - else if same_span && multiline { + span_note_and_lint(cx, + OPTION_MAP_UNWRAP_OR_ELSE, + expr.span, + msg, + expr.span, + &format!("replace `map({0}).unwrap_or_else({1})` with `with map_or_else({1}, {0})`", + map_snippet, + unwrap_snippet)); + } else if same_span && multiline { span_lint(cx, OPTION_MAP_UNWRAP_OR_ELSE, expr.span, msg); }; } } -#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec +#[allow(ptr_arg)] +// Type of MethodArgs is potentially a Vec /// lint use of `filter().next() for Iterators` fn lint_filter_next(cx: &LateContext, expr: &Expr, filter_args: &MethodArgs) { // lint if caller of `.filter().next()` is an Iterator if match_trait_method(cx, expr, &["core", "iter", "Iterator"]) { - let msg = "called `filter(p).next()` on an Iterator. This is more succinctly expressed by \ - calling `.find(p)` instead."; + let msg = "called `filter(p).next()` on an Iterator. This is more succinctly expressed by calling `.find(p)` \ + instead."; let filter_snippet = snippet(cx, filter_args[1].span, ".."); - if filter_snippet.lines().count() <= 1 { // add note if not multi-line - span_note_and_lint(cx, FILTER_NEXT, expr.span, msg, expr.span, - &format!("replace `filter({0}).next()` with `find({0})`", filter_snippet)); - } - else { + if filter_snippet.lines().count() <= 1 { + // add note if not multi-line + span_note_and_lint(cx, + FILTER_NEXT, + expr.span, + msg, + expr.span, + &format!("replace `filter({0}).next()` with `find({0})`", filter_snippet)); + } else { span_lint(cx, FILTER_NEXT, expr.span, msg); } } } -#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec +#[allow(ptr_arg)] +// Type of MethodArgs is potentially a Vec /// lint searching an Iterator followed by `is_some()` fn lint_search_is_some(cx: &LateContext, expr: &Expr, search_method: &str, search_args: &MethodArgs, is_some_args: &MethodArgs) { // lint if caller of search is an Iterator if match_trait_method(cx, &*is_some_args[0], &["core", "iter", "Iterator"]) { - let msg = format!("called `is_some()` after searching an iterator with {}. This is more \ - succinctly expressed by calling `any()`.", search_method); + let msg = format!("called `is_some()` after searching an iterator with {}. This is more succinctly expressed \ + by calling `any()`.", + search_method); let search_snippet = snippet(cx, search_args[1].span, ".."); - if search_snippet.lines().count() <= 1 { // add note if not multi-line - span_note_and_lint(cx, SEARCH_IS_SOME, expr.span, &msg, expr.span, - &format!("replace `{0}({1}).is_some()` with `any({1})`", search_method, - search_snippet)); - } - else { + if search_snippet.lines().count() <= 1 { + // add note if not multi-line + span_note_and_lint(cx, + SEARCH_IS_SOME, + expr.span, + &msg, + expr.span, + &format!("replace `{0}({1}).is_some()` with `any({1})`", search_method, search_snippet)); + } else { span_lint(cx, SEARCH_IS_SOME, expr.span, &msg); } } @@ -432,7 +452,7 @@ fn has_debug_impl<'a, 'b>(ty: ty::Ty<'a>, cx: &LateContext<'b, 'a>) -> bool { let no_ref_ty = walk_ptrs_ty(ty); let debug = match cx.tcx.lang_items.debug_trait() { Some(debug) => debug, - None => return false + None => return false, }; let debug_def = cx.tcx.lookup_trait_def(debug); let mut debug_impl_exists = false; @@ -447,46 +467,162 @@ fn has_debug_impl<'a, 'b>(ty: ty::Ty<'a>, cx: &LateContext<'b, 'a>) -> bool { debug_impl_exists } -const CONVENTIONS: [(&'static str, &'static [SelfKind]); 5] = [ - ("into_", &[ValueSelf]), - ("to_", &[RefSelf]), - ("as_", &[RefSelf, RefMutSelf]), - ("is_", &[RefSelf, NoSelf]), - ("from_", &[NoSelf]), -]; - -const TRAIT_METHODS: [(&'static str, usize, SelfKind, OutType, &'static str); 30] = [ - ("add", 2, ValueSelf, AnyType, "std::ops::Add"), - ("sub", 2, ValueSelf, AnyType, "std::ops::Sub"), - ("mul", 2, ValueSelf, AnyType, "std::ops::Mul"), - ("div", 2, ValueSelf, AnyType, "std::ops::Div"), - ("rem", 2, ValueSelf, AnyType, "std::ops::Rem"), - ("shl", 2, ValueSelf, AnyType, "std::ops::Shl"), - ("shr", 2, ValueSelf, AnyType, "std::ops::Shr"), - ("bitand", 2, ValueSelf, AnyType, "std::ops::BitAnd"), - ("bitor", 2, ValueSelf, AnyType, "std::ops::BitOr"), - ("bitxor", 2, ValueSelf, AnyType, "std::ops::BitXor"), - ("neg", 1, ValueSelf, AnyType, "std::ops::Neg"), - ("not", 1, ValueSelf, AnyType, "std::ops::Not"), - ("drop", 1, RefMutSelf, UnitType, "std::ops::Drop"), - ("index", 2, RefSelf, RefType, "std::ops::Index"), - ("index_mut", 2, RefMutSelf, RefType, "std::ops::IndexMut"), - ("deref", 1, RefSelf, RefType, "std::ops::Deref"), - ("deref_mut", 1, RefMutSelf, RefType, "std::ops::DerefMut"), - ("clone", 1, RefSelf, AnyType, "std::clone::Clone"), - ("borrow", 1, RefSelf, RefType, "std::borrow::Borrow"), - ("borrow_mut", 1, RefMutSelf, RefType, "std::borrow::BorrowMut"), - ("as_ref", 1, RefSelf, RefType, "std::convert::AsRef"), - ("as_mut", 1, RefMutSelf, RefType, "std::convert::AsMut"), - ("eq", 2, RefSelf, BoolType, "std::cmp::PartialEq"), - ("cmp", 2, RefSelf, AnyType, "std::cmp::Ord"), - ("default", 0, NoSelf, AnyType, "std::default::Default"), - ("hash", 2, RefSelf, UnitType, "std::hash::Hash"), - ("next", 1, RefMutSelf, AnyType, "std::iter::Iterator"), - ("into_iter", 1, ValueSelf, AnyType, "std::iter::IntoIterator"), - ("from_iter", 1, NoSelf, AnyType, "std::iter::FromIterator"), - ("from_str", 1, NoSelf, AnyType, "std::str::FromStr"), -]; +const CONVENTIONS: [(&'static str, &'static [SelfKind]); 5] = [("into_", &[ValueSelf]), + ("to_", &[RefSelf]), + ("as_", &[RefSelf, RefMutSelf]), + ("is_", &[RefSelf, NoSelf]), + ("from_", &[NoSelf])]; + +const TRAIT_METHODS: [(&'static str, usize, SelfKind, OutType, &'static str); 30] = [("add", + 2, + ValueSelf, + AnyType, + "std::ops::Add"), + ("sub", + 2, + ValueSelf, + AnyType, + "std::ops::Sub"), + ("mul", + 2, + ValueSelf, + AnyType, + "std::ops::Mul"), + ("div", + 2, + ValueSelf, + AnyType, + "std::ops::Div"), + ("rem", + 2, + ValueSelf, + AnyType, + "std::ops::Rem"), + ("shl", + 2, + ValueSelf, + AnyType, + "std::ops::Shl"), + ("shr", + 2, + ValueSelf, + AnyType, + "std::ops::Shr"), + ("bitand", + 2, + ValueSelf, + AnyType, + "std::ops::BitAnd"), + ("bitor", + 2, + ValueSelf, + AnyType, + "std::ops::BitOr"), + ("bitxor", + 2, + ValueSelf, + AnyType, + "std::ops::BitXor"), + ("neg", + 1, + ValueSelf, + AnyType, + "std::ops::Neg"), + ("not", + 1, + ValueSelf, + AnyType, + "std::ops::Not"), + ("drop", + 1, + RefMutSelf, + UnitType, + "std::ops::Drop"), + ("index", + 2, + RefSelf, + RefType, + "std::ops::Index"), + ("index_mut", + 2, + RefMutSelf, + RefType, + "std::ops::IndexMut"), + ("deref", + 1, + RefSelf, + RefType, + "std::ops::Deref"), + ("deref_mut", + 1, + RefMutSelf, + RefType, + "std::ops::DerefMut"), + ("clone", + 1, + RefSelf, + AnyType, + "std::clone::Clone"), + ("borrow", + 1, + RefSelf, + RefType, + "std::borrow::Borrow"), + ("borrow_mut", + 1, + RefMutSelf, + RefType, + "std::borrow::BorrowMut"), + ("as_ref", + 1, + RefSelf, + RefType, + "std::convert::AsRef"), + ("as_mut", + 1, + RefMutSelf, + RefType, + "std::convert::AsMut"), + ("eq", + 2, + RefSelf, + BoolType, + "std::cmp::PartialEq"), + ("cmp", + 2, + RefSelf, + AnyType, + "std::cmp::Ord"), + ("default", + 0, + NoSelf, + AnyType, + "std::default::Default"), + ("hash", + 2, + RefSelf, + UnitType, + "std::hash::Hash"), + ("next", + 1, + RefMutSelf, + AnyType, + "std::iter::Iterator"), + ("into_iter", + 1, + ValueSelf, + AnyType, + "std::iter::IntoIterator"), + ("from_iter", + 1, + NoSelf, + AnyType, + "std::iter::FromIterator"), + ("from_str", + 1, + NoSelf, + AnyType, + "std::str::FromStr")]; #[derive(Clone, Copy)] enum SelfKind { @@ -506,7 +642,7 @@ impl SelfKind { (&RefMutSelf, &SelfValue(_)) => allow_value_for_ref, (&NoSelf, &SelfStatic) => true, (_, &SelfExplicit(ref ty, _)) => self.matches_explicit_type(ty, allow_value_for_ref), - _ => false + _ => false, } } @@ -517,7 +653,7 @@ impl SelfKind { (&RefMutSelf, &TyRptr(_, MutTy { mutbl: Mutability::MutMutable, .. })) => true, (&RefSelf, &TyPath(..)) => allow_value_for_ref, (&RefMutSelf, &TyPath(..)) => allow_value_for_ref, - _ => false + _ => false, } } @@ -545,11 +681,15 @@ impl OutType { (&UnitType, &DefaultReturn(_)) => true, (&UnitType, &Return(ref ty)) if ty.node == TyTup(vec![].into()) => true, (&BoolType, &Return(ref ty)) if is_bool(ty) => true, - (&AnyType, &Return(ref ty)) if ty.node != TyTup(vec![].into()) => true, + (&AnyType, &Return(ref ty)) if ty.node != TyTup(vec![].into()) => true, (&RefType, &Return(ref ty)) => { - if let TyRptr(_, _) = ty.node { true } else { false } + if let TyRptr(_, _) = ty.node { + true + } else { + false + } } - _ => false + _ => false, } } } diff --git a/src/minmax.rs b/src/minmax.rs index 2a8e064f9f4..2cce36f2a9c 100644 --- a/src/minmax.rs +++ b/src/minmax.rs @@ -32,12 +32,13 @@ impl LateLintPass for MinMaxPass { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { if let Some((outer_max, outer_c, oe)) = min_max(cx, expr) { if let Some((inner_max, inner_c, _)) = min_max(cx, oe) { - if outer_max == inner_max { return; } + if outer_max == inner_max { + return; + } match (outer_max, outer_c.partial_cmp(&inner_c)) { (_, None) | (Max, Some(Less)) | (Min, Some(Greater)) => (), _ => { - span_lint(cx, MIN_MAX, expr.span, - "this min/max combination leads to constant result"); + span_lint(cx, MIN_MAX, expr.span, "this min/max combination leads to constant result"); } } } @@ -65,20 +66,30 @@ fn min_max<'a>(cx: &LateContext, expr: &'a Expr) -> Option<(MinMax, Constant, &' None } } - } else { None } - } else { None } - } + } else { + None + } + } else { + None + } +} -fn fetch_const(args: &[P<Expr>], m: MinMax) -> - Option<(MinMax, Constant, &Expr)> { - if args.len() != 2 { return None } +fn fetch_const(args: &[P<Expr>], m: MinMax) -> Option<(MinMax, Constant, &Expr)> { + if args.len() != 2 { + return None; + } if let Some(c) = constant_simple(&args[0]) { - if let None = constant_simple(&args[1]) { // otherwise ignore + if let None = constant_simple(&args[1]) { + // otherwise ignore Some((m, c, &args[1])) - } else { None } + } else { + None + } } else { if let Some(c) = constant_simple(&args[1]) { Some((m, c, &args[0])) - } else { None } + } else { + None + } } } diff --git a/src/misc.rs b/src/misc.rs index 92276961d11..1b7f2a921b9 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -40,15 +40,14 @@ impl LateLintPass for TopLevelRefPass { fn check_fn(&mut self, cx: &LateContext, k: FnKind, decl: &FnDecl, _: &Block, _: Span, _: NodeId) { if let FnKind::Closure = k { // Does not apply to closures - return + return; } for ref arg in &decl.inputs { if let PatIdent(BindByRef(_), _, _) = arg.pat.node { span_lint(cx, - TOPLEVEL_REF_ARG, - arg.pat.span, - "`ref` directly on a function argument is ignored. Consider using a reference type instead." - ); + TOPLEVEL_REF_ARG, + arg.pat.span, + "`ref` directly on a function argument is ignored. Consider using a reference type instead."); } } } @@ -112,9 +111,13 @@ impl LateLintPass for CmpNan { } fn check_nan(cx: &LateContext, path: &Path, span: Span) { - path.segments.last().map(|seg| if seg.identifier.name.as_str() == "NAN" { - span_lint(cx, CMP_NAN, span, - "doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead"); + path.segments.last().map(|seg| { + if seg.identifier.name.as_str() == "NAN" { + span_lint(cx, + CMP_NAN, + span, + "doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead"); + } }); } @@ -144,20 +147,24 @@ impl LateLintPass for FloatCmp { if let ExprBinary(ref cmp, ref left, ref right) = expr.node { let op = cmp.node; if (op == BiEq || op == BiNe) && (is_float(cx, left) || is_float(cx, right)) { - if is_allowed(cx, left) || is_allowed(cx, right) { return; } + if is_allowed(cx, left) || is_allowed(cx, right) { + return; + } if let Some(name) = get_item_name(cx, expr) { let name = name.as_str(); - if name == "eq" || name == "ne" || name == "is_nan" || - name.starts_with("eq_") || - name.ends_with("_eq") { + if name == "eq" || name == "ne" || name == "is_nan" || name.starts_with("eq_") || + name.ends_with("_eq") { return; } } - span_lint(cx, FLOAT_CMP, expr.span, &format!( - "{}-comparison of f32 or f64 detected. Consider changing this to \ - `abs({} - {}) < epsilon` for some suitable value of epsilon", - binop_to_string(op), snippet(cx, left.span, ".."), - snippet(cx, right.span, ".."))); + span_lint(cx, + FLOAT_CMP, + expr.span, + &format!("{}-comparison of f32 or f64 detected. Consider changing this to `abs({} - {}) < \ + epsilon` for some suitable value of epsilon", + binop_to_string(op), + snippet(cx, left.span, ".."), + snippet(cx, right.span, ".."))); } } } @@ -167,7 +174,9 @@ fn is_allowed(cx: &LateContext, expr: &Expr) -> bool { let res = eval_const_expr_partial(cx.tcx, expr, ExprTypeChecked, None); if let Ok(Float(val)) = res { val == 0.0 || val == ::std::f64::INFINITY || val == ::std::f64::NEG_INFINITY - } else { false } + } else { + false + } } fn is_float(cx: &LateContext, expr: &Expr) -> bool { @@ -211,44 +220,54 @@ impl LateLintPass for CmpOwned { fn check_to_owned(cx: &LateContext, expr: &Expr, other_span: Span, left: bool, op: Span) { let snip = match expr.node { ExprMethodCall(Spanned{node: ref name, ..}, _, ref args) if args.len() == 1 => { - if name.as_str() == "to_string" || - name.as_str() == "to_owned" && is_str_arg(cx, args) { - snippet(cx, args[0].span, "..") - } else { - return - } + if name.as_str() == "to_string" || name.as_str() == "to_owned" && is_str_arg(cx, args) { + snippet(cx, args[0].span, "..") + } else { + return; + } } ExprCall(ref path, ref v) if v.len() == 1 => { if let ExprPath(None, ref path) = path.node { - if match_path(path, &["String", "from_str"]) || - match_path(path, &["String", "from"]) { - snippet(cx, v[0].span, "..") - } else { - return - } + if match_path(path, &["String", "from_str"]) || match_path(path, &["String", "from"]) { + snippet(cx, v[0].span, "..") + } else { + return; + } } else { - return + return; } } - _ => return + _ => return, }; if left { - span_lint(cx, CMP_OWNED, expr.span, &format!( - "this creates an owned instance just for comparison. Consider using \ - `{} {} {}` to compare without allocation", snip, - snippet(cx, op, "=="), snippet(cx, other_span, ".."))); + span_lint(cx, + CMP_OWNED, + expr.span, + &format!("this creates an owned instance just for comparison. Consider using `{} {} {}` to \ + compare without allocation", + snip, + snippet(cx, op, "=="), + snippet(cx, other_span, ".."))); } else { - span_lint(cx, CMP_OWNED, expr.span, &format!( - "this creates an owned instance just for comparison. Consider using \ - `{} {} {}` to compare without allocation", - snippet(cx, other_span, ".."), snippet(cx, op, "=="), snip)); + span_lint(cx, + CMP_OWNED, + expr.span, + &format!("this creates an owned instance just for comparison. Consider using `{} {} {}` to \ + compare without allocation", + snippet(cx, other_span, ".."), + snippet(cx, op, "=="), + snip)); } } fn is_str_arg(cx: &LateContext, args: &[P<Expr>]) -> bool { - args.len() == 1 && if let ty::TyStr = - walk_ptrs_ty(cx.tcx.expr_ty(&args[0])).sty { true } else { false } + args.len() == 1 && + if let ty::TyStr = walk_ptrs_ty(cx.tcx.expr_ty(&args[0])).sty { + true + } else { + false + } } /// **What it does:** This lint checks for getting the remainder of a division by one. It is `Warn` by default. @@ -309,9 +328,11 @@ impl LateLintPass for PatternPass { fn check_pat(&mut self, cx: &LateContext, pat: &Pat) { if let PatIdent(_, ref ident, Some(ref right)) = pat.node { if right.node == PatWild { - cx.span_lint(REDUNDANT_PATTERN, pat.span, &format!( - "the `{} @ _` pattern can be written as just `{}`", - ident.node.name, ident.node.name)); + cx.span_lint(REDUNDANT_PATTERN, + pat.span, + &format!("the `{} @ _` pattern can be written as just `{}`", + ident.node.name, + ident.node.name)); } } } @@ -345,30 +366,31 @@ impl LintPass for UsedUnderscoreBinding { impl LateLintPass for UsedUnderscoreBinding { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { - if in_attributes_expansion(cx, expr) { // Don't lint things expanded by #[derive(...)], etc + if in_attributes_expansion(cx, expr) { + // Don't lint things expanded by #[derive(...)], etc return; } let needs_lint = match expr.node { ExprPath(_, ref path) => { - let ident = path.segments.last() + let ident = path.segments + .last() .expect("path should always have at least one segment") .identifier; - ident.name.as_str().chars().next() == Some('_') //starts with '_' - && ident.name.as_str().chars().skip(1).next() != Some('_') //doesn't start with "__" - && ident.name != ident.unhygienic_name //not in bang macro - && is_used(cx, expr) - }, + ident.name.as_str().chars().next() == Some('_') && + ident.name.as_str().chars().skip(1).next() != Some('_') && + ident.name != ident.unhygienic_name && is_used(cx, expr) + } ExprField(_, spanned) => { let name = spanned.node.as_str(); - name.chars().next() == Some('_') - && name.chars().skip(1).next() != Some('_') - }, - _ => false + name.chars().next() == Some('_') && name.chars().skip(1).next() != Some('_') + } + _ => false, }; if needs_lint { - cx.span_lint(USED_UNDERSCORE_BINDING, expr.span, - "used binding which is prefixed with an underscore. A leading underscore \ - signals that a binding will not be used."); + cx.span_lint(USED_UNDERSCORE_BINDING, + expr.span, + "used binding which is prefixed with an underscore. A leading underscore signals that a \ + binding will not be used."); } } } @@ -380,10 +402,9 @@ fn is_used(cx: &LateContext, expr: &Expr) -> bool { match parent.node { ExprAssign(_, ref rhs) => **rhs == *expr, ExprAssignOp(_, _, ref rhs) => **rhs == *expr, - _ => is_used(cx, &parent) + _ => is_used(cx, &parent), } - } - else { + } else { true } } diff --git a/src/misc_early.rs b/src/misc_early.rs index 7a59c15275f..a90c901df8f 100644 --- a/src/misc_early.rs +++ b/src/misc_early.rs @@ -52,15 +52,15 @@ impl EarlyLintPass for MiscEarly { } } if !pfields.is_empty() && wilds == pfields.len() { - span_help_and_lint(cx, UNNEEDED_FIELD_PATTERN, pat.span, - "All the struct fields are matched to a wildcard pattern, \ - consider using `..`.", - &format!("Try with `{} {{ .. }}` instead", - type_name)); + span_help_and_lint(cx, + UNNEEDED_FIELD_PATTERN, + pat.span, + "All the struct fields are matched to a wildcard pattern, consider using `..`.", + &format!("Try with `{} {{ .. }}` instead", type_name)); return; } if wilds > 0 { - let mut normal = vec!(); + let mut normal = vec![]; for field in pfields { if field.node.pat.node != PatWild { @@ -73,13 +73,16 @@ impl EarlyLintPass for MiscEarly { if field.node.pat.node == PatWild { wilds -= 1; if wilds > 0 { - span_lint(cx, UNNEEDED_FIELD_PATTERN, field.span, - "You matched a field with a wildcard pattern. \ - Consider using `..` instead"); + span_lint(cx, + UNNEEDED_FIELD_PATTERN, + field.span, + "You matched a field with a wildcard pattern. Consider using `..` instead"); } else { - span_help_and_lint(cx, UNNEEDED_FIELD_PATTERN, field.span, - "You matched a field with a wildcard pattern. \ - Consider using `..` instead", + span_help_and_lint(cx, + UNNEEDED_FIELD_PATTERN, + field.span, + "You matched a field with a wildcard pattern. Consider using `..` \ + instead", &format!("Try with `{} {{ {}, .. }}`", type_name, normal[..].join(", "))); @@ -91,7 +94,7 @@ impl EarlyLintPass for MiscEarly { } fn check_fn(&mut self, cx: &EarlyContext, _: FnKind, decl: &FnDecl, _: &Block, _: Span, _: NodeId) { - let mut registered_names : HashMap<String, Span> = HashMap::new(); + let mut registered_names: HashMap<String, Span> = HashMap::new(); for ref arg in &decl.inputs { if let PatIdent(_, sp_ident, None) = arg.pat.node { @@ -99,10 +102,12 @@ impl EarlyLintPass for MiscEarly { if arg_name.starts_with("_") { if let Some(correspondance) = registered_names.get(&arg_name[1..]) { - span_lint(cx, DUPLICATE_UNDERSCORE_ARGUMENT, *correspondance, - &format!("`{}` already exists, having another argument having almost \ - the same name makes code comprehension and documentation \ - more difficult", arg_name[1..].to_owned())); + span_lint(cx, + DUPLICATE_UNDERSCORE_ARGUMENT, + *correspondance, + &format!("`{}` already exists, having another argument having almost the same \ + name makes code comprehension and documentation more difficult", + arg_name[1..].to_owned())); } } else { registered_names.insert(arg_name, arg.pat.span.clone()); diff --git a/src/mut_mut.rs b/src/mut_mut.rs index db6f0e0320b..1bdb4e9a3d6 100644 --- a/src/mut_mut.rs +++ b/src/mut_mut.rs @@ -30,42 +30,46 @@ impl LateLintPass for MutMut { } fn check_ty(&mut self, cx: &LateContext, ty: &Ty) { - unwrap_mut(ty).and_then(unwrap_mut).map_or((), |_| { span_lint(cx, MUT_MUT, - ty.span, "generally you want to avoid `&mut &mut _` if possible"); }); + unwrap_mut(ty).and_then(unwrap_mut).map_or((), |_| { + span_lint(cx, MUT_MUT, ty.span, "generally you want to avoid `&mut &mut _` if possible"); + }); } } fn check_expr_mut(cx: &LateContext, expr: &Expr) { - if in_external_macro(cx, expr.span) { return; } + if in_external_macro(cx, expr.span) { + return; + } fn unwrap_addr(expr: &Expr) -> Option<&Expr> { match expr.node { ExprAddrOf(MutMutable, ref e) => Some(e), - _ => None + _ => None, } } unwrap_addr(expr).map_or((), |e| { - unwrap_addr(e).map_or_else( - || { - if let TyRef(_, TypeAndMut{mutbl: MutMutable, ..}) = - cx.tcx.expr_ty(e).sty { - span_lint(cx, MUT_MUT, expr.span, - "this expression mutably borrows a mutable reference. \ - Consider reborrowing"); - } - }, - |_| { - span_lint(cx, MUT_MUT, expr.span, - "generally you want to avoid `&mut &mut _` if possible"); - } - ) + unwrap_addr(e).map_or_else(|| { + if let TyRef(_, TypeAndMut{mutbl: MutMutable, ..}) = cx.tcx.expr_ty(e).sty { + span_lint(cx, + MUT_MUT, + expr.span, + "this expression mutably borrows a mutable reference. Consider \ + reborrowing"); + } + }, + |_| { + span_lint(cx, + MUT_MUT, + expr.span, + "generally you want to avoid `&mut &mut _` if possible"); + }) }) } fn unwrap_mut(ty: &Ty) -> Option<&Ty> { match ty.node { TyRptr(_, MutTy{ ty: ref pty, mutbl: MutMutable }) => Some(pty), - _ => None + _ => None, } } diff --git a/src/mut_reference.rs b/src/mut_reference.rs index ddfd9ddcc13..d92e449e7f2 100644 --- a/src/mut_reference.rs +++ b/src/mut_reference.rs @@ -36,13 +36,12 @@ impl LateLintPass for UnnecessaryMutPassed { match borrowed_table.node_types.get(&fn_expr.id) { Some(function_type) => { if let ExprPath(_, ref path) = fn_expr.node { - check_arguments(cx, &arguments, function_type, - &format!("{}", path)); + check_arguments(cx, &arguments, function_type, &format!("{}", path)); } } None => unreachable!(), // A function with unknown type is called. - // If this happened the compiler would have aborted the - // compilation long ago. + // If this happened the compiler would have aborted the + // compilation long ago. }; @@ -50,8 +49,9 @@ impl LateLintPass for UnnecessaryMutPassed { ExprMethodCall(ref name, _, ref arguments) => { let method_call = MethodCall::expr(e.id); match borrowed_table.method_map.get(&method_call) { - Some(method_type) => check_arguments(cx, &arguments, method_type.ty, - &format!("{}", name.node.as_str())), + Some(method_type) => { + check_arguments(cx, &arguments, method_type.ty, &format!("{}", name.node.as_str())) + } None => unreachable!(), // Just like above, this should never happen. }; } @@ -68,10 +68,10 @@ fn check_arguments(cx: &LateContext, arguments: &[P<Expr>], type_definition: &Ty TypeVariants::TyRef(_, TypeAndMut {mutbl: MutImmutable, ..}) | TypeVariants::TyRawPtr(TypeAndMut {mutbl: MutImmutable, ..}) => { if let ExprAddrOf(MutMutable, _) = argument.node { - span_lint(cx, UNNECESSARY_MUT_PASSED, - argument.span, &format!("The function/method \"{}\" \ - doesn't need a mutable reference", - name)); + span_lint(cx, + UNNECESSARY_MUT_PASSED, + argument.span, + &format!("The function/method \"{}\" doesn't need a mutable reference", name)); } } _ => {} diff --git a/src/mutex_atomic.rs b/src/mutex_atomic.rs index 8899eb56d42..7b75bd747bb 100644 --- a/src/mutex_atomic.rs +++ b/src/mutex_atomic.rs @@ -52,17 +52,13 @@ impl LateLintPass for MutexAtomic { if match_type(cx, ty, &MUTEX_PATH) { let mutex_param = &subst.types.get(ParamSpace::TypeSpace, 0).sty; if let Some(atomic_name) = get_atomic_name(mutex_param) { - let msg = format!("Consider using an {} instead of a \ - Mutex here. If you just want the \ - locking behaviour and not the internal \ - type, consider using Mutex<()>.", + let msg = format!("Consider using an {} instead of a Mutex here. If you just want the locking \ + behaviour and not the internal type, consider using Mutex<()>.", atomic_name); match *mutex_param { - ty::TyUint(t) if t != ast::TyUs => - span_lint(cx, MUTEX_INTEGER, expr.span, &msg), - ty::TyInt(t) if t != ast::TyIs => - span_lint(cx, MUTEX_INTEGER, expr.span, &msg), - _ => span_lint(cx, MUTEX_ATOMIC, expr.span, &msg) + ty::TyUint(t) if t != ast::TyUs => span_lint(cx, MUTEX_INTEGER, expr.span, &msg), + ty::TyInt(t) if t != ast::TyIs => span_lint(cx, MUTEX_INTEGER, expr.span, &msg), + _ => span_lint(cx, MUTEX_ATOMIC, expr.span, &msg), }; } } @@ -76,6 +72,6 @@ fn get_atomic_name(ty: &ty::TypeVariants) -> Option<(&'static str)> { ty::TyUint(_) => Some("AtomicUsize"), ty::TyInt(_) => Some("AtomicIsize"), ty::TyRawPtr(_) => Some("AtomicPtr"), - _ => None + _ => None, } } diff --git a/src/needless_bool.rs b/src/needless_bool.rs index bd2e4116fd0..22edbc272bd 100644 --- a/src/needless_bool.rs +++ b/src/needless_bool.rs @@ -37,30 +37,42 @@ impl LateLintPass for NeedlessBool { if let ExprIf(ref pred, ref then_block, Some(ref else_expr)) = e.node { match (fetch_bool_block(then_block), fetch_bool_expr(else_expr)) { (Some(true), Some(true)) => { - span_lint(cx, NEEDLESS_BOOL, e.span, + span_lint(cx, + NEEDLESS_BOOL, + e.span, "this if-then-else expression will always return true"); } (Some(false), Some(false)) => { - span_lint(cx, NEEDLESS_BOOL, e.span, + span_lint(cx, + NEEDLESS_BOOL, + e.span, "this if-then-else expression will always return false"); } (Some(true), Some(false)) => { let pred_snip = snippet(cx, pred.span, ".."); - let hint = if pred_snip == ".." { "its predicate".into() } else { + let hint = if pred_snip == ".." { + "its predicate".into() + } else { format!("`{}`", pred_snip) }; - span_lint(cx, NEEDLESS_BOOL, e.span, &format!( - "you can reduce this if-then-else expression to just {}", hint)); + span_lint(cx, + NEEDLESS_BOOL, + e.span, + &format!("you can reduce this if-then-else expression to just {}", hint)); } (Some(false), Some(true)) => { let pred_snip = snippet(cx, pred.span, ".."); - let hint = if pred_snip == ".." { "`!` and its predicate".into() } else { + let hint = if pred_snip == ".." { + "`!` and its predicate".into() + } else { format!("`!{}`", pred_snip) }; - span_lint(cx, NEEDLESS_BOOL, e.span, &format!( - "you can reduce this if-then-else expression to just {}", hint)); + span_lint(cx, + NEEDLESS_BOOL, + e.span, + &format!("you can reduce this if-then-else expression to just {}", hint)); } - _ => () + _ => (), } } } @@ -69,14 +81,21 @@ impl LateLintPass for NeedlessBool { fn fetch_bool_block(block: &Block) -> Option<bool> { if block.stmts.is_empty() { block.expr.as_ref().and_then(|e| fetch_bool_expr(e)) - } else { None } + } else { + None + } } fn fetch_bool_expr(expr: &Expr) -> Option<bool> { match expr.node { ExprBlock(ref block) => fetch_bool_block(block), - ExprLit(ref lit_ptr) => if let LitBool(value) = lit_ptr.node { - Some(value) } else { None }, - _ => None + ExprLit(ref lit_ptr) => { + if let LitBool(value) = lit_ptr.node { + Some(value) + } else { + None + } + } + _ => None, } } diff --git a/src/needless_features.rs b/src/needless_features.rs index 2dd53c2d783..2c293d04600 100644 --- a/src/needless_features.rs +++ b/src/needless_features.rs @@ -49,14 +49,16 @@ impl LateLintPass for NeedlessFeaturesPass { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { if let ExprMethodCall(ref name, _, _) = expr.node { if name.node.as_str() == "as_slice" && check_paths(cx, expr) { - span_lint(cx, UNSTABLE_AS_SLICE, expr.span, - "used as_slice() from the 'convert' nightly feature. Use &[..] \ - instead"); + span_lint(cx, + UNSTABLE_AS_SLICE, + expr.span, + "used as_slice() from the 'convert' nightly feature. Use &[..] instead"); } if name.node.as_str() == "as_mut_slice" && check_paths(cx, expr) { - span_lint(cx, UNSTABLE_AS_MUT_SLICE, expr.span, - "used as_mut_slice() from the 'convert' nightly feature. Use &mut [..] \ - instead"); + span_lint(cx, + UNSTABLE_AS_MUT_SLICE, + expr.span, + "used as_mut_slice() from the 'convert' nightly feature. Use &mut [..] instead"); } } } diff --git a/src/needless_update.rs b/src/needless_update.rs index 9a314616cdc..1b306df8eed 100644 --- a/src/needless_update.rs +++ b/src/needless_update.rs @@ -32,9 +32,10 @@ impl LateLintPass for NeedlessUpdatePass { let ty = cx.tcx.expr_ty(expr); if let TyStruct(def, _) = ty.sty { if fields.len() == def.struct_variant().fields.len() { - span_lint(cx, NEEDLESS_UPDATE, base.span, - "struct update has no effect, all the fields \ - in the struct have already been specified"); + span_lint(cx, + NEEDLESS_UPDATE, + base.span, + "struct update has no effect, all the fields in the struct have already been specified"); } } } diff --git a/src/no_effect.rs b/src/no_effect.rs index b51b4235d54..3b2d91fc78e 100644 --- a/src/no_effect.rs +++ b/src/no_effect.rs @@ -37,9 +37,7 @@ fn has_no_effect(cx: &LateContext, expr: &Expr) -> bool { let def = cx.tcx.def_map.borrow().get(&callee.id).map(|d| d.full_def()); match def { Some(DefStruct(..)) | - Some(DefVariant(..)) => { - args.iter().all(|arg| has_no_effect(cx, arg)) - } + Some(DefVariant(..)) => args.iter().all(|arg| has_no_effect(cx, arg)), _ => false, } } @@ -60,8 +58,7 @@ impl LateLintPass for NoEffectPass { fn check_stmt(&mut self, cx: &LateContext, stmt: &Stmt) { if let StmtSemi(ref expr, _) = stmt.node { if has_no_effect(cx, expr) { - span_lint(cx, NO_EFFECT, stmt.span, - "statement with no effect"); + span_lint(cx, NO_EFFECT, stmt.span, "statement with no effect"); } } } diff --git a/src/open_options.rs b/src/open_options.rs index 31ca0d72939..26784cb6008 100644 --- a/src/open_options.rs +++ b/src/open_options.rs @@ -31,7 +31,7 @@ impl LateLintPass for NonSensicalOpenOptions { fn check_expr(&mut self, cx: &LateContext, e: &Expr) { if let ExprMethodCall(ref name, _, ref arguments) = e.node { let (obj_ty, _) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&arguments[0])); - if name.node.as_str() == "open" && match_type(cx, obj_ty, &OPEN_OPTIONS_PATH){ + if name.node.as_str() == "open" && match_type(cx, obj_ty, &OPEN_OPTIONS_PATH) { let mut options = Vec::new(); get_open_options(cx, &arguments[0], &mut options); check_open_options(cx, &options, e.span); @@ -44,7 +44,7 @@ impl LateLintPass for NonSensicalOpenOptions { enum Argument { True, False, - Unknown + Unknown, } #[derive(Debug)] @@ -53,31 +53,33 @@ enum OpenOption { Read, Truncate, Create, - Append + Append, } fn get_open_options(cx: &LateContext, argument: &Expr, options: &mut Vec<(OpenOption, Argument)>) { if let ExprMethodCall(ref name, _, ref arguments) = argument.node { let (obj_ty, _) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&arguments[0])); - + // Only proceed if this is a call on some object of type std::fs::OpenOptions if match_type(cx, obj_ty, &OPEN_OPTIONS_PATH) && arguments.len() >= 2 { - + let argument_option = match arguments[1].node { ExprLit(ref span) => { if let Spanned {node: LitBool(lit), ..} = **span { - if lit {Argument::True} else {Argument::False} + if lit { + Argument::True + } else { + Argument::False + } } else { return; // The function is called with a literal // which is not a boolean literal. This is theoretically // possible, but not very likely. } } - _ => { - Argument::Unknown - } + _ => Argument::Unknown, }; - + match &*name.node.as_str() { "create" => { options.push((OpenOption::Create, argument_option)); @@ -96,7 +98,7 @@ fn get_open_options(cx: &LateContext, argument: &Expr, options: &mut Vec<(OpenOp } _ => {} } - + get_open_options(cx, &arguments[0], options); } } @@ -104,39 +106,124 @@ fn get_open_options(cx: &LateContext, argument: &Expr, options: &mut Vec<(OpenOp fn check_for_duplicates(cx: &LateContext, options: &[(OpenOption, Argument)], span: Span) { // This code is almost duplicated (oh, the irony), but I haven't found a way to unify it. - if options.iter().filter(|o| if let (OpenOption::Create, _) = **o {true} else {false}).count() > 1 { - span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "The method \"create\" \ - is called more than once"); + if options.iter() + .filter(|o| { + if let (OpenOption::Create, _) = **o { + true + } else { + false + } + }) + .count() > 1 { + span_lint(cx, + NONSENSICAL_OPEN_OPTIONS, + span, + "The method \"create\" is called more than once"); } - if options.iter().filter(|o| if let (OpenOption::Append, _) = **o {true} else {false}).count() > 1 { - span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "The method \"append\" \ - is called more than once"); + if options.iter() + .filter(|o| { + if let (OpenOption::Append, _) = **o { + true + } else { + false + } + }) + .count() > 1 { + span_lint(cx, + NONSENSICAL_OPEN_OPTIONS, + span, + "The method \"append\" is called more than once"); } - if options.iter().filter(|o| if let (OpenOption::Truncate, _) = **o {true} else {false}).count() > 1 { - span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "The method \"truncate\" \ - is called more than once"); + if options.iter() + .filter(|o| { + if let (OpenOption::Truncate, _) = **o { + true + } else { + false + } + }) + .count() > 1 { + span_lint(cx, + NONSENSICAL_OPEN_OPTIONS, + span, + "The method \"truncate\" is called more than once"); } - if options.iter().filter(|o| if let (OpenOption::Read, _) = **o {true} else {false}).count() > 1 { - span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "The method \"read\" \ - is called more than once"); + if options.iter() + .filter(|o| { + if let (OpenOption::Read, _) = **o { + true + } else { + false + } + }) + .count() > 1 { + span_lint(cx, + NONSENSICAL_OPEN_OPTIONS, + span, + "The method \"read\" is called more than once"); } - if options.iter().filter(|o| if let (OpenOption::Write, _) = **o {true} else {false}).count() > 1 { - span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "The method \"write\" \ - is called more than once"); + if options.iter() + .filter(|o| { + if let (OpenOption::Write, _) = **o { + true + } else { + false + } + }) + .count() > 1 { + span_lint(cx, + NONSENSICAL_OPEN_OPTIONS, + span, + "The method \"write\" is called more than once"); } } fn check_for_inconsistencies(cx: &LateContext, options: &[(OpenOption, Argument)], span: Span) { // Truncate + read makes no sense. - if options.iter().filter(|o| if let (OpenOption::Read, Argument::True) = **o {true} else {false}).count() > 0 && - options.iter().filter(|o| if let (OpenOption::Truncate, Argument::True) = **o {true} else {false}).count() > 0 { + if options.iter() + .filter(|o| { + if let (OpenOption::Read, Argument::True) = **o { + true + } else { + false + } + }) + .count() > 0 && + options.iter() + .filter(|o| { + if let (OpenOption::Truncate, Argument::True) = **o { + true + } else { + false + } + }) + .count() > 0 { span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "File opened with \"truncate\" and \"read\""); } - + // Append + truncate makes no sense. - if options.iter().filter(|o| if let (OpenOption::Append, Argument::True) = **o {true} else {false}).count() > 0 && - options.iter().filter(|o| if let (OpenOption::Truncate, Argument::True) = **o {true} else {false}).count() > 0 { - span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "File opened with \"append\" and \"truncate\""); + if options.iter() + .filter(|o| { + if let (OpenOption::Append, Argument::True) = **o { + true + } else { + false + } + }) + .count() > 0 && + options.iter() + .filter(|o| { + if let (OpenOption::Truncate, Argument::True) = **o { + true + } else { + false + } + }) + .count() > 0 { + span_lint(cx, + NONSENSICAL_OPEN_OPTIONS, + span, + "File opened with \"append\" and \"truncate\""); } } diff --git a/src/precedence.rs b/src/precedence.rs index 91ff0680b3e..8253471f8b0 100644 --- a/src/precedence.rs +++ b/src/precedence.rs @@ -31,29 +31,40 @@ impl LintPass for Precedence { impl EarlyLintPass for Precedence { fn check_expr(&mut self, cx: &EarlyContext, expr: &Expr) { if let ExprBinary(Spanned { node: op, ..}, ref left, ref right) = expr.node { - if !is_bit_op(op) { return; } + if !is_bit_op(op) { + return; + } match (is_arith_expr(left), is_arith_expr(right)) { (true, true) => { - span_lint(cx, PRECEDENCE, expr.span, - &format!("operator precedence can trip the unwary. \ - Consider parenthesizing your expression:\ - `({}) {} ({})`", snippet(cx, left.span, ".."), - op.to_string(), snippet(cx, right.span, ".."))); - }, + span_lint(cx, + PRECEDENCE, + expr.span, + &format!("operator precedence can trip the unwary. Consider parenthesizing your \ + expression:`({}) {} ({})`", + snippet(cx, left.span, ".."), + op.to_string(), + snippet(cx, right.span, ".."))); + } (true, false) => { - span_lint(cx, PRECEDENCE, expr.span, - &format!("operator precedence can trip the unwary. \ - Consider parenthesizing your expression:\ - `({}) {} {}`", snippet(cx, left.span, ".."), - op.to_string(), snippet(cx, right.span, ".."))); - }, + span_lint(cx, + PRECEDENCE, + expr.span, + &format!("operator precedence can trip the unwary. Consider parenthesizing your \ + expression:`({}) {} {}`", + snippet(cx, left.span, ".."), + op.to_string(), + snippet(cx, right.span, ".."))); + } (false, true) => { - span_lint(cx, PRECEDENCE, expr.span, - &format!("operator precedence can trip the unwary. \ - Consider parenthesizing your expression:\ - `{} {} ({})`", snippet(cx, left.span, ".."), - op.to_string(), snippet(cx, right.span, ".."))); - }, + span_lint(cx, + PRECEDENCE, + expr.span, + &format!("operator precedence can trip the unwary. Consider parenthesizing your \ + expression:`{} {} ({})`", + snippet(cx, left.span, ".."), + op.to_string(), + snippet(cx, right.span, ".."))); + } _ => (), } } @@ -64,13 +75,14 @@ impl EarlyLintPass for Precedence { if let ExprLit(ref lit) = slf.node { match lit.node { LitInt(..) | LitFloat(..) | LitFloatUnsuffixed(..) => { - span_lint(cx, PRECEDENCE, expr.span, &format!( - "unary minus has lower precedence than \ - method call. Consider adding parentheses \ - to clarify your intent: -({})", - snippet(cx, rhs.span, ".."))); + span_lint(cx, + PRECEDENCE, + expr.span, + &format!("unary minus has lower precedence than method call. Consider \ + adding parentheses to clarify your intent: -({})", + snippet(cx, rhs.span, ".."))); } - _ => () + _ => (), } } } @@ -82,20 +94,20 @@ impl EarlyLintPass for Precedence { fn is_arith_expr(expr: &Expr) -> bool { match expr.node { ExprBinary(Spanned { node: op, ..}, _, _) => is_arith_op(op), - _ => false + _ => false, } } fn is_bit_op(op: BinOp_) -> bool { match op { BiBitXor | BiBitAnd | BiBitOr | BiShl | BiShr => true, - _ => false + _ => false, } } fn is_arith_op(op: BinOp_) -> bool { match op { BiAdd | BiSub | BiMul | BiDiv | BiRem => true, - _ => false + _ => false, } } diff --git a/src/ptr_arg.rs b/src/ptr_arg.rs index f748dbd9cfa..2e1e16cf22b 100644 --- a/src/ptr_arg.rs +++ b/src/ptr_arg.rs @@ -63,14 +63,17 @@ fn check_fn(cx: &LateContext, decl: &FnDecl) { if let Some(ty) = cx.tcx.ast_ty_to_ty_cache.borrow().get(&arg.ty.id) { if let ty::TyRef(_, ty::TypeAndMut { ty, mutbl: MutImmutable }) = ty.sty { if match_type(cx, ty, &VEC_PATH) { - span_lint(cx, PTR_ARG, arg.ty.span, - "writing `&Vec<_>` instead of `&[_]` involves one more reference \ - and cannot be used with non-Vec-based slices. Consider changing \ - the type to `&[...]`"); + span_lint(cx, + PTR_ARG, + arg.ty.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_type(cx, ty, &STRING_PATH) { - span_lint(cx, PTR_ARG, arg.ty.span, - "writing `&String` instead of `&str` involves a new object \ - where a slice will do. Consider changing the type to `&str`"); + span_lint(cx, + PTR_ARG, + arg.ty.span, + "writing `&String` instead of `&str` involves a new object where a slice will do. \ + Consider changing the type to `&str`"); } } } diff --git a/src/ranges.rs b/src/ranges.rs index 48bbba734ff..692a5a2da1d 100644 --- a/src/ranges.rs +++ b/src/ranges.rs @@ -37,18 +37,15 @@ impl LintPass for StepByZero { impl LateLintPass for StepByZero { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { - if let ExprMethodCall(Spanned { node: ref name, .. }, _, - ref args) = expr.node { + if let ExprMethodCall(Spanned { node: ref name, .. }, _, ref args) = expr.node { // Range with step_by(0). - if name.as_str() == "step_by" && args.len() == 2 && - is_range(cx, &args[0]) && is_integer_literal(&args[1], 0) { - cx.span_lint(RANGE_STEP_BY_ZERO, expr.span, - "Range::step_by(0) produces an infinite iterator. \ - Consider using `std::iter::repeat()` instead") - } - - // x.iter().zip(0..x.len()) - else if name.as_str() == "zip" && args.len() == 2 { + if name.as_str() == "step_by" && args.len() == 2 && is_range(cx, &args[0]) && + is_integer_literal(&args[1], 0) { + cx.span_lint(RANGE_STEP_BY_ZERO, + expr.span, + "Range::step_by(0) produces an infinite iterator. Consider using `std::iter::repeat()` \ + instead") + } else if name.as_str() == "zip" && args.len() == 2 { let iter = &args[0].node; let zip_arg = &args[1].node; if_let_chain! { diff --git a/src/returns.rs b/src/returns.rs index 15f9bb80d95..e4745b8766f 100644 --- a/src/returns.rs +++ b/src/returns.rs @@ -1,6 +1,6 @@ use rustc::lint::*; use syntax::ast::*; -//use reexport::*; +// use reexport::*; use syntax::codemap::{Span, Spanned}; use syntax::visit::FnKind; @@ -67,19 +67,17 @@ impl ReturnPass { self.check_final_expr(cx, &arm.body); } } - _ => { } + _ => {} } } fn emit_return_lint(&mut self, cx: &EarlyContext, spans: (Span, Span)) { - if in_external_macro(cx, spans.1) {return;} - span_lint_and_then(cx, NEEDLESS_RETURN, spans.0, - "unneeded return statement", - |db| { + if in_external_macro(cx, spans.1) { + return; + } + span_lint_and_then(cx, NEEDLESS_RETURN, spans.0, "unneeded return statement", |db| { if let Some(snippet) = snippet_opt(cx, spans.1) { - db.span_suggestion(spans.0, - "remove `return` as shown:", - snippet); + db.span_suggestion(spans.0, "remove `return` as shown:", snippet); } }); } @@ -104,13 +102,16 @@ impl ReturnPass { } fn emit_let_lint(&mut self, cx: &EarlyContext, lint_span: Span, note_span: Span) { - if in_external_macro(cx, note_span) {return;} - let mut db = span_lint(cx, LET_AND_RETURN, lint_span, - "returning the result of a let binding from a block. \ - Consider returning the expression directly."); + if in_external_macro(cx, note_span) { + return; + } + let mut db = span_lint(cx, + LET_AND_RETURN, + lint_span, + "returning the result of a let binding from a block. Consider returning the \ + expression directly."); if cx.current_level(LET_AND_RETURN) != Level::Allow { - db.span_note(note_span, - "this expression can be directly returned"); + db.span_note(note_span, "this expression can be directly returned"); } } } @@ -122,8 +123,7 @@ impl LintPass for ReturnPass { } impl EarlyLintPass for ReturnPass { - fn check_fn(&mut self, cx: &EarlyContext, _: FnKind, _: &FnDecl, - block: &Block, _: Span, _: NodeId) { + fn check_fn(&mut self, cx: &EarlyContext, _: FnKind, _: &FnDecl, block: &Block, _: Span, _: NodeId) { self.check_block_return(cx, block); } diff --git a/src/shadow.rs b/src/shadow.rs index 1210590bbcb..2d3e423eacb 100644 --- a/src/shadow.rs +++ b/src/shadow.rs @@ -45,13 +45,13 @@ impl LintPass for ShadowPass { fn get_lints(&self) -> LintArray { lint_array!(SHADOW_SAME, SHADOW_REUSE, SHADOW_UNRELATED) } - } impl LateLintPass for ShadowPass { - fn check_fn(&mut self, cx: &LateContext, _: FnKind, decl: &FnDecl, - block: &Block, _: Span, _: NodeId) { - if in_external_macro(cx, block.span) { return; } + fn check_fn(&mut self, cx: &LateContext, _: FnKind, decl: &FnDecl, block: &Block, _: Span, _: NodeId) { + if in_external_macro(cx, block.span) { + return; + } check_fn(cx, decl, block); } } @@ -71,20 +71,27 @@ fn check_block(cx: &LateContext, block: &Block, bindings: &mut Vec<(Name, Span)> for stmt in &block.stmts { match stmt.node { StmtDecl(ref decl, _) => check_decl(cx, decl, bindings), - StmtExpr(ref e, _) | StmtSemi(ref e, _) => - check_expr(cx, e, bindings) + StmtExpr(ref e, _) | StmtSemi(ref e, _) => check_expr(cx, e, bindings), } } - if let Some(ref o) = block.expr { check_expr(cx, o, bindings); } + if let Some(ref o) = block.expr { + check_expr(cx, o, bindings); + } bindings.truncate(len); } fn check_decl(cx: &LateContext, decl: &Decl, bindings: &mut Vec<(Name, Span)>) { - if in_external_macro(cx, decl.span) { return; } - if is_from_for_desugar(decl) { return; } + if in_external_macro(cx, decl.span) { + return; + } + if is_from_for_desugar(decl) { + return; + } if let DeclLocal(ref local) = decl.node { let Local{ ref pat, ref ty, ref init, span, .. } = **local; - if let Some(ref t) = *ty { check_ty(cx, t, bindings) } + if let Some(ref t) = *ty { + check_ty(cx, t, bindings) + } if let Some(ref o) = *init { check_expr(cx, o, bindings); check_pat(cx, pat, &Some(o), span, bindings); @@ -97,13 +104,12 @@ fn check_decl(cx: &LateContext, decl: &Decl, bindings: &mut Vec<(Name, Span)>) { fn is_binding(cx: &LateContext, pat: &Pat) -> bool { match cx.tcx.def_map.borrow().get(&pat.id).map(|d| d.full_def()) { Some(DefVariant(..)) | Some(DefStruct(..)) => false, - _ => true + _ => true, } } -fn check_pat(cx: &LateContext, pat: &Pat, init: &Option<&Expr>, span: Span, - bindings: &mut Vec<(Name, Span)>) { - //TODO: match more stuff / destructuring +fn check_pat(cx: &LateContext, pat: &Pat, init: &Option<&Expr>, span: Span, bindings: &mut Vec<(Name, Span)>) { + // TODO: match more stuff / destructuring match pat.node { PatIdent(_, ref ident, ref inner) => { let name = ident.node.unhygienic_name; @@ -121,17 +127,19 @@ fn check_pat(cx: &LateContext, pat: &Pat, init: &Option<&Expr>, span: Span, bindings.push((name, ident.span)); } } - if let Some(ref p) = *inner { check_pat(cx, p, init, span, bindings); } + if let Some(ref p) = *inner { + check_pat(cx, p, init, span, bindings); + } } - //PatEnum(Path, Option<Vec<P<Pat>>>), - PatStruct(_, ref pfields, _) => + // PatEnum(Path, Option<Vec<P<Pat>>>), + PatStruct(_, ref pfields, _) => { if let Some(ref init_struct) = *init { if let ExprStruct(_, ref efields, _) = init_struct.node { for field in pfields { let name = field.node.name; let efield = efields.iter() - .find(|ref f| f.name.node == name) - .map(|f| &*f.expr); + .find(|ref f| f.name.node == name) + .map(|f| &*f.expr); check_pat(cx, &field.node.pat, &efield, span, bindings); } } else { @@ -143,8 +151,9 @@ fn check_pat(cx: &LateContext, pat: &Pat, init: &Option<&Expr>, span: Span, for field in pfields { check_pat(cx, &field.node.pat, &None, span, bindings); } - }, - PatTup(ref inner) => + } + } + PatTup(ref inner) => { if let Some(ref init_tup) = *init { if let ExprTup(ref tup) = init_tup.node { for (i, p) in inner.iter().enumerate() { @@ -159,7 +168,8 @@ fn check_pat(cx: &LateContext, pat: &Pat, init: &Option<&Expr>, span: Span, for p in inner { check_pat(cx, p, &None, span, bindings); } - }, + } + } PatBox(ref inner) => { if let Some(ref initp) = *init { if let ExprBox(ref inner_init) = initp.node { @@ -171,15 +181,15 @@ fn check_pat(cx: &LateContext, pat: &Pat, init: &Option<&Expr>, span: Span, check_pat(cx, inner, init, span, bindings); } } - PatRegion(ref inner, _) => - check_pat(cx, inner, init, span, bindings), - //PatVec(Vec<P<Pat>>, Option<P<Pat>>, Vec<P<Pat>>), + PatRegion(ref inner, _) => check_pat(cx, inner, init, span, bindings), + // PatVec(Vec<P<Pat>>, Option<P<Pat>>, Vec<P<Pat>>), _ => (), } } -fn lint_shadow<T>(cx: &LateContext, name: Name, span: Span, lspan: Span, init: - &Option<T>, prev_span: Span) where T: Deref<Target=Expr> { +fn lint_shadow<T>(cx: &LateContext, name: Name, span: Span, lspan: Span, init: &Option<T>, prev_span: Span) + where T: Deref<Target = Expr> +{ fn note_orig(cx: &LateContext, mut db: DiagnosticWrapper, lint: &'static Lint, span: Span) { if cx.current_level(lint) != Level::Allow { db.span_note(span, "previous binding is here"); @@ -187,51 +197,69 @@ fn lint_shadow<T>(cx: &LateContext, name: Name, span: Span, lspan: Span, init: } if let Some(ref expr) = *init { if is_self_shadow(name, expr) { - let db = span_lint(cx, SHADOW_SAME, span, &format!( - "{} is shadowed by itself in {}", - snippet(cx, lspan, "_"), - snippet(cx, expr.span, ".."))); - note_orig(cx, db, SHADOW_SAME, prev_span); + let db = span_lint(cx, + SHADOW_SAME, + span, + &format!("{} is shadowed by itself in {}", + snippet(cx, lspan, "_"), + snippet(cx, expr.span, ".."))); + note_orig(cx, db, SHADOW_SAME, prev_span); } else { if contains_self(name, expr) { - let db = span_note_and_lint(cx, SHADOW_REUSE, lspan, &format!( - "{} is shadowed by {} which reuses the original value", - snippet(cx, lspan, "_"), - snippet(cx, expr.span, "..")), - expr.span, "initialization happens here"); + let db = span_note_and_lint(cx, + SHADOW_REUSE, + lspan, + &format!("{} is shadowed by {} which reuses the original value", + snippet(cx, lspan, "_"), + snippet(cx, expr.span, "..")), + expr.span, + "initialization happens here"); note_orig(cx, db, SHADOW_REUSE, prev_span); } else { - let db = span_note_and_lint(cx, SHADOW_UNRELATED, lspan, &format!( - "{} is shadowed by {}", - snippet(cx, lspan, "_"), - snippet(cx, expr.span, "..")), - expr.span, "initialization happens here"); + let db = span_note_and_lint(cx, + SHADOW_UNRELATED, + lspan, + &format!("{} is shadowed by {}", + snippet(cx, lspan, "_"), + snippet(cx, expr.span, "..")), + expr.span, + "initialization happens here"); note_orig(cx, db, SHADOW_UNRELATED, prev_span); } } } else { - let db = span_lint(cx, SHADOW_UNRELATED, span, &format!( - "{} shadows a previous declaration", snippet(cx, lspan, "_"))); + let db = span_lint(cx, + SHADOW_UNRELATED, + span, + &format!("{} shadows a previous declaration", snippet(cx, lspan, "_"))); note_orig(cx, db, SHADOW_UNRELATED, prev_span); } } fn check_expr(cx: &LateContext, expr: &Expr, bindings: &mut Vec<(Name, Span)>) { - if in_external_macro(cx, expr.span) { return; } + if in_external_macro(cx, expr.span) { + return; + } match expr.node { - ExprUnary(_, ref e) | ExprField(ref e, _) | - ExprTupField(ref e, _) | ExprAddrOf(_, ref e) | ExprBox(ref e) - => { check_expr(cx, e, bindings) } - ExprBlock(ref block) | ExprLoop(ref block, _) => - { check_block(cx, block, bindings) } - //ExprCall - //ExprMethodCall - ExprVec(ref v) | ExprTup(ref v) => - for ref e in v { check_expr(cx, e, bindings) }, + ExprUnary(_, ref e) | + ExprField(ref e, _) | + ExprTupField(ref e, _) | + ExprAddrOf(_, ref e) | + ExprBox(ref e) => check_expr(cx, e, bindings), + ExprBlock(ref block) | ExprLoop(ref block, _) => check_block(cx, block, bindings), + // ExprCall + // ExprMethodCall + ExprVec(ref v) | ExprTup(ref v) => { + for ref e in v { + check_expr(cx, e, bindings) + } + } ExprIf(ref cond, ref then, ref otherwise) => { check_expr(cx, cond, bindings); check_block(cx, then, bindings); - if let Some(ref o) = *otherwise { check_expr(cx, o, bindings); } + if let Some(ref o) = *otherwise { + check_expr(cx, o, bindings); + } } ExprWhile(ref cond, ref block, _) => { check_expr(cx, cond, bindings); @@ -243,7 +271,7 @@ fn check_expr(cx: &LateContext, expr: &Expr, bindings: &mut Vec<(Name, Span)>) { for ref arm in arms { for ref pat in &arm.pats { check_pat(cx, &pat, &Some(&**init), pat.span, bindings); - //This is ugly, but needed to get the right type + // This is ugly, but needed to get the right type if let Some(ref guard) = arm.guard { check_expr(cx, guard, bindings); } @@ -252,7 +280,7 @@ fn check_expr(cx: &LateContext, expr: &Expr, bindings: &mut Vec<(Name, Span)>) { } } } - _ => () + _ => (), } } @@ -266,7 +294,11 @@ fn check_ty(cx: &LateContext, ty: &Ty, bindings: &mut Vec<(Name, Span)>) { } TyPtr(MutTy{ ty: ref mty, .. }) | TyRptr(_, MutTy{ ty: ref mty, .. }) => check_ty(cx, mty, bindings), - TyTup(ref tup) => { for ref t in tup { check_ty(cx, t, bindings) } } + TyTup(ref tup) => { + for ref t in tup { + check_ty(cx, t, bindings) + } + } TyTypeof(ref expr) => check_expr(cx, expr, bindings), _ => (), } @@ -276,23 +308,22 @@ fn is_self_shadow(name: Name, expr: &Expr) -> bool { match expr.node { ExprBox(ref inner) | ExprAddrOf(_, ref inner) => is_self_shadow(name, inner), - ExprBlock(ref block) => block.stmts.is_empty() && block.expr.as_ref(). - map_or(false, |ref e| is_self_shadow(name, e)), - ExprUnary(op, ref inner) => (UnDeref == op) && - is_self_shadow(name, inner), + ExprBlock(ref block) => { + block.stmts.is_empty() && block.expr.as_ref().map_or(false, |ref e| is_self_shadow(name, e)) + } + ExprUnary(op, ref inner) => (UnDeref == op) && is_self_shadow(name, inner), ExprPath(_, ref path) => path_eq_name(name, path), _ => false, } } fn path_eq_name(name: Name, path: &Path) -> bool { - !path.global && path.segments.len() == 1 && - path.segments[0].identifier.unhygienic_name == name + !path.global && path.segments.len() == 1 && path.segments[0].identifier.unhygienic_name == name } struct ContainsSelf { name: Name, - result: bool + result: bool, } impl<'v> Visitor<'v> for ContainsSelf { @@ -304,7 +335,10 @@ impl<'v> Visitor<'v> for ContainsSelf { } fn contains_self(name: Name, expr: &Expr) -> bool { - let mut cs = ContainsSelf { name: name, result: false }; + let mut cs = ContainsSelf { + name: name, + result: false, + }; cs.visit_expr(expr); cs.result } diff --git a/src/strings.rs b/src/strings.rs index 2baf26095b7..55d1a0acf68 100644 --- a/src/strings.rs +++ b/src/strings.rs @@ -68,19 +68,24 @@ impl LateLintPass for StringAdd { if let Some(ref p) = parent { if let ExprAssign(ref target, _) = p.node { // avoid duplicate matches - if is_exp_equal(cx, target, left) { return; } + if is_exp_equal(cx, target, left) { + return; + } } } } - span_lint(cx, STRING_ADD, e.span, - "you added something to a string. \ - Consider using `String::push_str()` instead"); + span_lint(cx, + STRING_ADD, + e.span, + "you added something to a string. Consider using `String::push_str()` instead"); } } else if let ExprAssign(ref target, ref src) = e.node { if is_string(cx, target) && is_add(cx, src, target) { - span_lint(cx, STRING_ADD_ASSIGN, e.span, - "you assigned the result of adding something to this string. \ - Consider using `String::push_str()` instead"); + span_lint(cx, + STRING_ADD_ASSIGN, + e.span, + "you assigned the result of adding something to this string. Consider using \ + `String::push_str()` instead"); } } } @@ -92,11 +97,10 @@ fn is_string(cx: &LateContext, e: &Expr) -> bool { fn is_add(cx: &LateContext, src: &Expr, target: &Expr) -> bool { match src.node { - ExprBinary(Spanned{ node: BiAdd, .. }, ref left, _) => - is_exp_equal(cx, target, left), - ExprBlock(ref block) => block.stmts.is_empty() && - block.expr.as_ref().map_or(false, - |expr| is_add(cx, expr, target)), - _ => false + ExprBinary(Spanned{ node: BiAdd, .. }, ref left, _) => is_exp_equal(cx, target, left), + ExprBlock(ref block) => { + block.stmts.is_empty() && block.expr.as_ref().map_or(false, |expr| is_add(cx, expr, target)) + } + _ => false, } } diff --git a/src/temporary_assignment.rs b/src/temporary_assignment.rs index 622bf5bc25e..7d5057d8377 100644 --- a/src/temporary_assignment.rs +++ b/src/temporary_assignment.rs @@ -40,11 +40,10 @@ impl LateLintPass for TemporaryAssignmentPass { match target.node { ExprField(ref base, _) | ExprTupField(ref base, _) => { if is_temporary(base) && !is_adjusted(cx, base) { - span_lint(cx, TEMPORARY_ASSIGNMENT, expr.span, - "assignment to temporary"); + span_lint(cx, TEMPORARY_ASSIGNMENT, expr.span, "assignment to temporary"); } } - _ => () + _ => (), } } } diff --git a/src/transmute.rs b/src/transmute.rs index c71468bf5a0..24af45bc68d 100644 --- a/src/transmute.rs +++ b/src/transmute.rs @@ -34,7 +34,8 @@ impl LateLintPass for UselessTransmute { let to_ty = cx.tcx.expr_ty(e); if from_ty == to_ty { - cx.span_lint(USELESS_TRANSMUTE, e.span, + cx.span_lint(USELESS_TRANSMUTE, + e.span, &format!("transmute from a type (`{}`) to itself", from_ty)); } } diff --git a/src/types.rs b/src/types.rs index e119ef63436..6289e13c85a 100644 --- a/src/types.rs +++ b/src/types.rs @@ -49,22 +49,23 @@ impl LintPass for TypePass { impl LateLintPass for TypePass { fn check_ty(&mut self, cx: &LateContext, ast_ty: &Ty) { if in_macro(cx, ast_ty.span) { - return + return; } if let Some(ty) = cx.tcx.ast_ty_to_ty_cache.borrow().get(&ast_ty.id) { if let ty::TyBox(ref inner) = ty.sty { if match_type(cx, inner, &VEC_PATH) { - span_help_and_lint( - cx, BOX_VEC, ast_ty.span, - "you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`", - "`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation."); + span_help_and_lint(cx, + BOX_VEC, + ast_ty.span, + "you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`", + "`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation."); } - } - else if match_type(cx, ty, &LL_PATH) { - span_help_and_lint( - cx, LINKEDLIST, ast_ty.span, - "I see you're using a LinkedList! Perhaps you meant some other data structure?", - "a VecDeque might work"); + } else if match_type(cx, ty, &LL_PATH) { + span_help_and_lint(cx, + LINKEDLIST, + ast_ty.span, + "I see you're using a LinkedList! Perhaps you meant some other data structure?", + "a VecDeque might work"); } } } @@ -87,12 +88,17 @@ fn check_let_unit(cx: &LateContext, decl: &Decl) { if let DeclLocal(ref local) = decl.node { let bindtype = &cx.tcx.pat_ty(&local.pat).sty; if *bindtype == ty::TyTuple(vec![]) { - if in_external_macro(cx, decl.span) || - in_macro(cx, local.pat.span) { return; } - if is_from_for_desugar(decl) { return; } - span_lint(cx, LET_UNIT_VALUE, decl.span, &format!( - "this let-binding has unit value. Consider omitting `let {} =`", - snippet(cx, local.pat.span, ".."))); + if in_external_macro(cx, decl.span) || in_macro(cx, local.pat.span) { + return; + } + if is_from_for_desugar(decl) { + return; + } + span_lint(cx, + LET_UNIT_VALUE, + decl.span, + &format!("this let-binding has unit value. Consider omitting `let {} =`", + snippet(cx, local.pat.span, ".."))); } } } @@ -130,18 +136,23 @@ impl LintPass for UnitCmp { impl LateLintPass for UnitCmp { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { - if in_macro(cx, expr.span) { return; } + if in_macro(cx, expr.span) { + return; + } if let ExprBinary(ref cmp, ref left, _) = expr.node { let op = cmp.node; let sty = &cx.tcx.expr_ty(left).sty; if *sty == ty::TyTuple(vec![]) && is_comparison_binop(op) { let result = match op { BiEq | BiLe | BiGe => "true", - _ => "false" + _ => "false", }; - span_lint(cx, UNIT_CMP, expr.span, &format!( - "{}-comparison of unit values detected. This will always be {}", - binop_to_string(op), result)); + span_lint(cx, + UNIT_CMP, + expr.span, + &format!("{}-comparison of unit values detected. This will always be {}", + binop_to_string(op), + result)); } } } @@ -192,38 +203,64 @@ declare_lint!(pub CAST_POSSIBLE_WRAP, Allow, /// Will return 0 if the type is not an int or uint variant fn int_ty_to_nbits(typ: &ty::TyS) -> usize { let n = match typ.sty { - ty::TyInt(i) => 4 << (i as usize), + ty::TyInt(i) => 4 << (i as usize), ty::TyUint(u) => 4 << (u as usize), - _ => 0 + _ => 0, }; // n == 4 is the usize/isize case - if n == 4 { ::std::usize::BITS } else { n } + if n == 4 { + ::std::usize::BITS + } else { + n + } } fn is_isize_or_usize(typ: &ty::TyS) -> bool { match typ.sty { ty::TyInt(TyIs) | ty::TyUint(TyUs) => true, - _ => false + _ => false, } } fn span_precision_loss_lint(cx: &LateContext, expr: &Expr, cast_from: &ty::TyS, cast_to_f64: bool) { - let mantissa_nbits = if cast_to_f64 {52} else {23}; + let mantissa_nbits = if cast_to_f64 { + 52 + } else { + 23 + }; let arch_dependent = is_isize_or_usize(cast_from) && cast_to_f64; let arch_dependent_str = "on targets with 64-bit wide pointers "; - let from_nbits_str = if arch_dependent {"64".to_owned()} - else if is_isize_or_usize(cast_from) {"32 or 64".to_owned()} - else {int_ty_to_nbits(cast_from).to_string()}; - span_lint(cx, CAST_PRECISION_LOSS, expr.span, - &format!("casting {0} to {1} causes a loss of precision {2}\ - ({0} is {3} bits wide, but {1}'s mantissa is only {4} bits wide)", - cast_from, if cast_to_f64 {"f64"} else {"f32"}, - if arch_dependent {arch_dependent_str} else {""}, - from_nbits_str, mantissa_nbits)); + let from_nbits_str = if arch_dependent { + "64".to_owned() + } else if is_isize_or_usize(cast_from) { + "32 or 64".to_owned() + } else { + int_ty_to_nbits(cast_from).to_string() + }; + span_lint(cx, + CAST_PRECISION_LOSS, + expr.span, + &format!("casting {0} to {1} causes a loss of precision {2}({0} is {3} bits wide, but {1}'s mantissa \ + is only {4} bits wide)", + cast_from, + if cast_to_f64 { + "f64" + } else { + "f32" + }, + if arch_dependent { + arch_dependent_str + } else { + "" + }, + from_nbits_str, + mantissa_nbits)); } enum ArchSuffix { - _32, _64, None + _32, + _64, + None, } fn check_truncation_and_wrapping(cx: &LateContext, expr: &Expr, cast_from: &ty::TyS, cast_to: &ty::TyS) { @@ -231,44 +268,60 @@ fn check_truncation_and_wrapping(cx: &LateContext, expr: &Expr, cast_from: &ty:: let arch_32_suffix = " on targets with 32-bit wide pointers"; let cast_unsigned_to_signed = !cast_from.is_signed() && cast_to.is_signed(); let (from_nbits, to_nbits) = (int_ty_to_nbits(cast_from), int_ty_to_nbits(cast_to)); - let (span_truncation, suffix_truncation, span_wrap, suffix_wrap) = - match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) { - (true, true) | (false, false) => ( - to_nbits < from_nbits, - ArchSuffix::None, - to_nbits == from_nbits && cast_unsigned_to_signed, + let (span_truncation, suffix_truncation, span_wrap, suffix_wrap) = match (is_isize_or_usize(cast_from), + is_isize_or_usize(cast_to)) { + (true, true) | (false, false) => { + (to_nbits < from_nbits, + ArchSuffix::None, + to_nbits == from_nbits && cast_unsigned_to_signed, + ArchSuffix::None) + } + (true, false) => { + (to_nbits <= 32, + if to_nbits == 32 { + ArchSuffix::_64 + } else { ArchSuffix::None - ), - (true, false) => ( - to_nbits <= 32, - if to_nbits == 32 {ArchSuffix::_64} else {ArchSuffix::None}, - to_nbits <= 32 && cast_unsigned_to_signed, + }, + to_nbits <= 32 && cast_unsigned_to_signed, + ArchSuffix::_32) + } + (false, true) => { + (from_nbits == 64, + ArchSuffix::_32, + cast_unsigned_to_signed, + if from_nbits == 64 { + ArchSuffix::_64 + } else { ArchSuffix::_32 - ), - (false, true) => ( - from_nbits == 64, - ArchSuffix::_32, - cast_unsigned_to_signed, - if from_nbits == 64 {ArchSuffix::_64} else {ArchSuffix::_32} - ), - }; + }) + } + }; if span_truncation { - span_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span, - &format!("casting {} to {} may truncate the value{}", - cast_from, cast_to, - match suffix_truncation { - ArchSuffix::_32 => arch_32_suffix, - ArchSuffix::_64 => arch_64_suffix, - ArchSuffix::None => "" })); + span_lint(cx, + CAST_POSSIBLE_TRUNCATION, + expr.span, + &format!("casting {} to {} may truncate the value{}", + cast_from, + cast_to, + match suffix_truncation { + ArchSuffix::_32 => arch_32_suffix, + ArchSuffix::_64 => arch_64_suffix, + ArchSuffix::None => "", + })); } if span_wrap { - span_lint(cx, CAST_POSSIBLE_WRAP, expr.span, - &format!("casting {} to {} may wrap around the value{}", - cast_from, cast_to, - match suffix_wrap { - ArchSuffix::_32 => arch_32_suffix, - ArchSuffix::_64 => arch_64_suffix, - ArchSuffix::None => "" })); + span_lint(cx, + CAST_POSSIBLE_WRAP, + expr.span, + &format!("casting {} to {} may wrap around the value{}", + cast_from, + cast_to, + match suffix_wrap { + ArchSuffix::_32 => arch_32_suffix, + ArchSuffix::_64 => arch_64_suffix, + ArchSuffix::None => "", + })); } } @@ -289,35 +342,42 @@ impl LateLintPass for CastPass { match (cast_from.is_integral(), cast_to.is_integral()) { (true, false) => { let from_nbits = int_ty_to_nbits(cast_from); - let to_nbits = if let ty::TyFloat(TyF32) = cast_to.sty {32} else {64}; + let to_nbits = if let ty::TyFloat(TyF32) = cast_to.sty { + 32 + } else { + 64 + }; if is_isize_or_usize(cast_from) || from_nbits >= to_nbits { span_precision_loss_lint(cx, expr, cast_from, to_nbits == 64); } } (false, true) => { - span_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span, - &format!("casting {} to {} may truncate the value", - cast_from, cast_to)); + span_lint(cx, + CAST_POSSIBLE_TRUNCATION, + expr.span, + &format!("casting {} to {} may truncate the value", cast_from, cast_to)); if !cast_to.is_signed() { - span_lint(cx, CAST_SIGN_LOSS, expr.span, - &format!("casting {} to {} may lose the sign of the value", - cast_from, cast_to)); + span_lint(cx, + CAST_SIGN_LOSS, + expr.span, + &format!("casting {} to {} may lose the sign of the value", cast_from, cast_to)); } } (true, true) => { if cast_from.is_signed() && !cast_to.is_signed() { - span_lint(cx, CAST_SIGN_LOSS, expr.span, - &format!("casting {} to {} may lose the sign of the value", - cast_from, cast_to)); + span_lint(cx, + CAST_SIGN_LOSS, + expr.span, + &format!("casting {} to {} may lose the sign of the value", cast_from, cast_to)); } check_truncation_and_wrapping(cx, expr, cast_from, cast_to); } (false, false) => { - if let (&ty::TyFloat(TyF64), - &ty::TyFloat(TyF32)) = (&cast_from.sty, &cast_to.sty) { - span_lint(cx, CAST_POSSIBLE_TRUNCATION, - expr.span, - "casting f64 to f32 may truncate the value"); + if let (&ty::TyFloat(TyF64), &ty::TyFloat(TyF32)) = (&cast_from.sty, &cast_to.sty) { + span_lint(cx, + CAST_POSSIBLE_TRUNCATION, + expr.span, + "casting f64 to f32 may truncate the value"); } } } @@ -360,7 +420,7 @@ impl LateLintPass for TypeComplexityPass { ItemStatic(ref ty, _, _) | ItemConst(ref ty, _) => check_type(cx, ty), // functions, enums, structs, impls and traits are covered - _ => () + _ => (), } } @@ -370,7 +430,7 @@ impl LateLintPass for TypeComplexityPass { TypeTraitItem(_, Some(ref ty)) => check_type(cx, ty), MethodTraitItem(MethodSig { ref decl, .. }, None) => check_fndecl(cx, decl), // methods with default impl are covered by check_fn - _ => () + _ => (), } } @@ -379,7 +439,7 @@ impl LateLintPass for TypeComplexityPass { ImplItemKind::Const(ref ty, _) | ImplItemKind::Type(ref ty) => check_type(cx, ty), // methods are covered by check_fn - _ => () + _ => (), } } @@ -400,16 +460,23 @@ fn check_fndecl(cx: &LateContext, decl: &FnDecl) { } fn check_type(cx: &LateContext, ty: &Ty) { - if in_macro(cx, ty.span) { return; } + if in_macro(cx, ty.span) { + return; + } let score = { - let mut visitor = TypeComplexityVisitor { score: 0, nest: 1 }; + let mut visitor = TypeComplexityVisitor { + score: 0, + nest: 1, + }; visitor.visit_ty(ty); visitor.score }; // println!("{:?} --> {}", ty, score); if score > 250 { - span_lint(cx, TYPE_COMPLEXITY, ty.span, &format!( - "very complex type used. Consider factoring parts into `type` definitions")); + span_lint(cx, + TYPE_COMPLEXITY, + ty.span, + &format!("very complex type used. Consider factoring parts into `type` definitions")); } } @@ -442,7 +509,7 @@ impl<'v> Visitor<'v> for TypeComplexityVisitor { TyBareFn(..) | TyPolyTraitRef(..) => (50 * self.nest, 1), - _ => (0, 0) + _ => (0, 0), }; self.score += add_score; self.nest += sub_nest; diff --git a/src/unicode.rs b/src/unicode.rs index a5b03087604..d5ea7199e10 100644 --- a/src/unicode.rs +++ b/src/unicode.rs @@ -58,11 +58,13 @@ impl LateLintPass for Unicode { } } -fn escape<T: Iterator<Item=char>>(s: T) -> String { +fn escape<T: Iterator<Item = char>>(s: T) -> String { let mut result = String::new(); for c in s { if c as u32 > 0x7F { - for d in c.escape_unicode() { result.push(d) }; + for d in c.escape_unicode() { + result.push(d) + } } else { result.push(c); } @@ -73,26 +75,30 @@ fn escape<T: Iterator<Item=char>>(s: T) -> String { fn check_str(cx: &LateContext, span: Span) { let string = snippet(cx, span, ""); if string.contains('\u{200B}') { - span_help_and_lint(cx, ZERO_WIDTH_SPACE, span, - "zero-width space detected", - &format!("Consider replacing the string with:\n\"{}\"", - string.replace("\u{200B}", "\\u{200B}"))); + span_help_and_lint(cx, + ZERO_WIDTH_SPACE, + span, + "zero-width space detected", + &format!("Consider replacing the string with:\n\"{}\"", + string.replace("\u{200B}", "\\u{200B}"))); } if string.chars().any(|c| c as u32 > 0x7F) { - span_help_and_lint(cx, NON_ASCII_LITERAL, span, - "literal non-ASCII character detected", - &format!("Consider replacing the string with:\n\"{}\"", - if cx.current_level(UNICODE_NOT_NFC) == Level::Allow { - escape(string.chars()) - } else { - escape(string.nfc()) - })); + span_help_and_lint(cx, + NON_ASCII_LITERAL, + span, + "literal non-ASCII character detected", + &format!("Consider replacing the string with:\n\"{}\"", + if cx.current_level(UNICODE_NOT_NFC) == Level::Allow { + escape(string.chars()) + } else { + escape(string.nfc()) + })); } - if cx.current_level(NON_ASCII_LITERAL) == Level::Allow && - string.chars().zip(string.nfc()).any(|(a, b)| a != b) { - span_help_and_lint(cx, UNICODE_NOT_NFC, span, - "non-nfc unicode sequence detected", - &format!("Consider replacing the string with:\n\"{}\"", - string.nfc().collect::<String>())); + if cx.current_level(NON_ASCII_LITERAL) == Level::Allow && string.chars().zip(string.nfc()).any(|(a, b)| a != b) { + span_help_and_lint(cx, + UNICODE_NOT_NFC, + span, + "non-nfc unicode sequence detected", + &format!("Consider replacing the string with:\n\"{}\"", string.nfc().collect::<String>())); } } diff --git a/src/utils.rs b/src/utils.rs index 1b6c75a3b78..74a15927ad1 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -19,15 +19,15 @@ use std::ops::{Deref, DerefMut}; pub type MethodArgs = HirVec<P<Expr>>; // module DefPaths for certain structs/enums we check for -pub const OPTION_PATH: [&'static str; 3] = ["core", "option", "Option"]; -pub const RESULT_PATH: [&'static str; 3] = ["core", "result", "Result"]; -pub const STRING_PATH: [&'static str; 3] = ["collections", "string", "String"]; -pub const VEC_PATH: [&'static str; 3] = ["collections", "vec", "Vec"]; -pub const LL_PATH: [&'static str; 3] = ["collections", "linked_list", "LinkedList"]; +pub const OPTION_PATH: [&'static str; 3] = ["core", "option", "Option"]; +pub const RESULT_PATH: [&'static str; 3] = ["core", "result", "Result"]; +pub const STRING_PATH: [&'static str; 3] = ["collections", "string", "String"]; +pub const VEC_PATH: [&'static str; 3] = ["collections", "vec", "Vec"]; +pub const LL_PATH: [&'static str; 3] = ["collections", "linked_list", "LinkedList"]; pub const HASHMAP_PATH: [&'static str; 5] = ["std", "collections", "hash", "map", "HashMap"]; pub const OPEN_OPTIONS_PATH: [&'static str; 3] = ["std", "fs", "OpenOptions"]; -pub const MUTEX_PATH: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"]; -pub const CLONE_PATH: [&'static str; 2] = ["Clone", "clone"]; +pub const MUTEX_PATH: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"]; +pub const CLONE_PATH: [&'static str; 2] = ["Clone", "clone"]; pub const BEGIN_UNWIND: [&'static str; 3] = ["std", "rt", "begin_unwind"]; /// Produce a nested chain of if-lets and ifs from the patterns: @@ -82,8 +82,7 @@ pub fn differing_macro_contexts(sp1: Span, sp2: Span) -> bool { } /// returns true if this expn_info was expanded by any macro pub fn in_macro<T: LintContext>(cx: &T, span: Span) -> bool { - cx.sess().codemap().with_expn_info(span.expn_id, - |info| info.is_some()) + cx.sess().codemap().with_expn_info(span.expn_id, |info| info.is_some()) } /// returns true if the macro that expanded the crate was outside of @@ -95,42 +94,35 @@ pub fn in_external_macro<T: LintContext>(cx: &T, span: Span) -> bool { // no ExpnInfo = no macro opt_info.map_or(false, |info| { if let ExpnFormat::MacroAttribute(..) = info.callee.format { - // these are all plugins - return true; + // these are all plugins + return true; } // no span for the callee = external macro info.callee.span.map_or(true, |span| { // no snippet = external macro or compiler-builtin expansion - cx.sess().codemap().span_to_snippet(span).ok().map_or(true, |code| - // macro doesn't start with "macro_rules" - // = compiler plugin - !code.starts_with("macro_rules") - ) + cx.sess().codemap().span_to_snippet(span).ok().map_or(true, |code| !code.starts_with("macro_rules")) }) }) } - cx.sess().codemap().with_expn_info(span.expn_id, - |info| in_macro_ext(cx, info)) + cx.sess().codemap().with_expn_info(span.expn_id, |info| in_macro_ext(cx, info)) } /// check if a DefId's path matches the given absolute type path /// usage e.g. with /// `match_def_path(cx, id, &["core", "option", "Option"])` pub fn match_def_path(cx: &LateContext, def_id: DefId, path: &[&str]) -> bool { - cx.tcx.with_path(def_id, |iter| iter.zip(path) - .all(|(nm, p)| nm.name().as_str() == *p)) + cx.tcx.with_path(def_id, |iter| { + iter.zip(path) + .all(|(nm, p)| nm.name().as_str() == *p) + }) } /// check if type is struct or enum type with given def path pub fn match_type(cx: &LateContext, ty: ty::Ty, path: &[&str]) -> bool { match ty.sty { - ty::TyEnum(ref adt, _) | ty::TyStruct(ref adt, _) => { - match_def_path(cx, adt.did, path) - } - _ => { - false - } + ty::TyEnum(ref adt, _) | ty::TyStruct(ref adt, _) => match_def_path(cx, adt.did, path), + _ => false, } } @@ -138,9 +130,12 @@ pub fn match_type(cx: &LateContext, ty: ty::Ty, path: &[&str]) -> bool { pub fn match_impl_method(cx: &LateContext, expr: &Expr, path: &[&str]) -> bool { let method_call = ty::MethodCall::expr(expr.id); - let trt_id = cx.tcx.tables - .borrow().method_map.get(&method_call) - .and_then(|callee| cx.tcx.impl_of_method(callee.def_id)); + let trt_id = cx.tcx + .tables + .borrow() + .method_map + .get(&method_call) + .and_then(|callee| cx.tcx.impl_of_method(callee.def_id)); if let Some(trt_id) = trt_id { match_def_path(cx, trt_id, path) } else { @@ -151,9 +146,12 @@ pub fn match_impl_method(cx: &LateContext, expr: &Expr, path: &[&str]) -> bool { /// check if method call given in "expr" belongs to given trait pub fn match_trait_method(cx: &LateContext, expr: &Expr, path: &[&str]) -> bool { let method_call = ty::MethodCall::expr(expr.id); - let trt_id = cx.tcx.tables - .borrow().method_map.get(&method_call) - .and_then(|callee| cx.tcx.trait_of_item(callee.def_id)); + let trt_id = cx.tcx + .tables + .borrow() + .method_map + .get(&method_call) + .and_then(|callee| cx.tcx.trait_of_item(callee.def_id)); if let Some(trt_id) = trt_id { match_def_path(cx, trt_id, path) } else { @@ -164,15 +162,13 @@ pub fn match_trait_method(cx: &LateContext, expr: &Expr, path: &[&str]) -> bool /// match a Path against a slice of segment string literals, e.g. /// `match_path(path, &["std", "rt", "begin_unwind"])` pub fn match_path(path: &Path, segments: &[&str]) -> bool { - path.segments.iter().rev().zip(segments.iter().rev()).all( - |(a, b)| a.identifier.name.as_str() == *b) + path.segments.iter().rev().zip(segments.iter().rev()).all(|(a, b)| a.identifier.name.as_str() == *b) } /// match a Path against a slice of segment string literals, e.g. /// `match_path(path, &["std", "rt", "begin_unwind"])` pub fn match_path_ast(path: &ast::Path, segments: &[&str]) -> bool { - path.segments.iter().rev().zip(segments.iter().rev()).all( - |(a, b)| a.identifier.name.as_str() == *b) + path.segments.iter().rev().zip(segments.iter().rev()).all(|(a, b)| a.identifier.name.as_str() == *b) } /// match an Expr against a chain of methods, and return the matched Exprs. For example, if `expr` @@ -181,17 +177,16 @@ pub fn match_path_ast(path: &ast::Path, segments: &[&str]) -> bool { pub fn method_chain_args<'a>(expr: &'a Expr, methods: &[&str]) -> Option<Vec<&'a MethodArgs>> { let mut current = expr; let mut matched = Vec::with_capacity(methods.len()); - for method_name in methods.iter().rev() { // method chains are stored last -> first + for method_name in methods.iter().rev() { + // method chains are stored last -> first if let ExprMethodCall(ref name, _, ref args) = current.node { if name.node.as_str() == *method_name { matched.push(args); // build up `matched` backwards current = &args[0] // go to parent expression - } - else { + } else { return None; } - } - else { + } else { return None; } } @@ -206,9 +201,7 @@ pub fn get_item_name(cx: &LateContext, expr: &Expr) -> Option<Name> { match cx.tcx.map.find(parent_id) { Some(NodeItem(&Item{ ref name, .. })) | Some(NodeTraitItem(&TraitItem{ ref name, .. })) | - Some(NodeImplItem(&ImplItem{ ref name, .. })) => { - Some(*name) - } + Some(NodeImplItem(&ImplItem{ ref name, .. })) => Some(*name), _ => None, } } @@ -249,9 +242,7 @@ pub fn snippet_block<'a, T: LintContext>(cx: &T, span: Span, default: &'a str) - /// Like snippet_block, but add braces if the expr is not an ExprBlock /// Also takes an Option<String> which can be put inside the braces -pub fn expr_block<'a, T: LintContext>(cx: &T, expr: &Expr, - option: Option<String>, - default: &'a str) -> Cow<'a, str> { +pub fn expr_block<'a, T: LintContext>(cx: &T, expr: &Expr, option: Option<String>, default: &'a str) -> Cow<'a, str> { let code = snippet_block(cx, expr.span, default); let string = option.map_or("".to_owned(), |s| s); if let ExprBlock(_) = expr.node { @@ -272,21 +263,33 @@ pub fn trim_multiline(s: Cow<str>, ignore_first: bool) -> Cow<str> { } fn trim_multiline_inner(s: Cow<str>, ignore_first: bool, ch: char) -> Cow<str> { - let x = s.lines().skip(ignore_first as usize) - .filter_map(|l| { if l.len() > 0 { // ignore empty lines - Some(l.char_indices() - .find(|&(_,x)| x != ch) - .unwrap_or((l.len(), ch)).0) - } else {None}}) - .min().unwrap_or(0); + let x = s.lines() + .skip(ignore_first as usize) + .filter_map(|l| { + if l.len() > 0 { + // ignore empty lines + Some(l.char_indices() + .find(|&(_, x)| x != ch) + .unwrap_or((l.len(), ch)) + .0) + } else { + None + } + }) + .min() + .unwrap_or(0); if x > 0 { - Cow::Owned(s.lines().enumerate().map(|(i,l)| if (ignore_first && i == 0) || - l.len() == 0 { - l - } else { - l.split_at(x).1 - }).collect::<Vec<_>>() - .join("\n")) + Cow::Owned(s.lines() + .enumerate() + .map(|(i, l)| { + if (ignore_first && i == 0) || l.len() == 0 { + l + } else { + l.split_at(x).1 + } + }) + .collect::<Vec<_>>() + .join("\n")) } else { s } @@ -295,11 +298,18 @@ fn trim_multiline_inner(s: Cow<str>, ignore_first: bool, ch: char) -> Cow<str> { /// get a parent expr if any – this is useful to constrain a lint pub fn get_parent_expr<'c>(cx: &'c LateContext, e: &Expr) -> Option<&'c Expr> { let map = &cx.tcx.map; - let node_id : NodeId = e.id; - let parent_id : NodeId = map.get_parent_node(node_id); - if node_id == parent_id { return None; } - map.find(parent_id).and_then(|node| - if let NodeExpr(parent) = node { Some(parent) } else { None } ) + let node_id: NodeId = e.id; + let parent_id: NodeId = map.get_parent_node(node_id); + if node_id == parent_id { + return None; + } + map.find(parent_id).and_then(|node| { + if let NodeExpr(parent) = node { + Some(parent) + } else { + None + } + }) } pub fn get_enclosing_block<'c>(cx: &'c LateContext, node: NodeId) -> Option<&'c Block> { @@ -310,9 +320,11 @@ pub fn get_enclosing_block<'c>(cx: &'c LateContext, node: NodeId) -> Option<&'c match node { NodeBlock(ref block) => Some(block), NodeItem(&Item{ node: ItemFn(_, _, _, _, _, ref block), .. }) => Some(block), - _ => None + _ => None, } - } else { None } + } else { + None + } } pub struct DiagnosticWrapper<'a>(pub DiagnosticBuilder<'a>); @@ -326,56 +338,57 @@ impl<'a> Drop for DiagnosticWrapper<'a> { impl<'a> DerefMut for DiagnosticWrapper<'a> { fn deref_mut(&mut self) -> &mut DiagnosticBuilder<'a> { &mut self.0 - } + } } impl<'a> Deref for DiagnosticWrapper<'a> { type Target = DiagnosticBuilder<'a>; fn deref(&self) -> &DiagnosticBuilder<'a> { &self.0 - } + } } #[cfg(not(feature="structured_logging"))] -pub fn span_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, - sp: Span, msg: &str) -> DiagnosticWrapper<'a> { +pub fn span_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, sp: Span, msg: &str) -> DiagnosticWrapper<'a> { let mut db = cx.struct_span_lint(lint, sp, msg); if cx.current_level(lint) != Level::Allow { - db.fileline_help(sp, &format!("for further information visit \ - https://github.com/Manishearth/rust-clippy/wiki#{}", - lint.name_lower())); + db.fileline_help(sp, + &format!("for further information visit https://github.com/Manishearth/rust-clippy/wiki#{}", + lint.name_lower())); } DiagnosticWrapper(db) } #[cfg(feature="structured_logging")] -pub fn span_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, - sp: Span, msg: &str) -> DiagnosticWrapper<'a> { +pub fn span_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, sp: Span, msg: &str) -> DiagnosticWrapper<'a> { // lint.name / lint.desc is can give details of the lint // cx.sess().codemap() has all these nice functions for line/column/snippet details // http://doc.rust-lang.org/syntax/codemap/struct.CodeMap.html#method.span_to_string let mut db = cx.struct_span_lint(lint, sp, msg); if cx.current_level(lint) != Level::Allow { - db.fileline_help(sp, &format!("for further information visit \ - https://github.com/Manishearth/rust-clippy/wiki#{}", - lint.name_lower())); + db.fileline_help(sp, + &format!("for further information visit https://github.com/Manishearth/rust-clippy/wiki#{}", + lint.name_lower())); } DiagnosticWrapper(db) } -pub fn span_help_and_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, span: Span, - msg: &str, help: &str) -> DiagnosticWrapper<'a> { +pub fn span_help_and_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, span: Span, msg: &str, help: &str) + -> DiagnosticWrapper<'a> { let mut db = cx.struct_span_lint(lint, span, msg); if cx.current_level(lint) != Level::Allow { - db.fileline_help(span, &format!("{}\nfor further information \ - visit https://github.com/Manishearth/rust-clippy/wiki#{}", - help, lint.name_lower())); + db.fileline_help(span, + &format!("{}\nfor further information visit \ + https://github.com/Manishearth/rust-clippy/wiki#{}", + help, + lint.name_lower())); } DiagnosticWrapper(db) } -pub fn span_note_and_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, span: Span, - msg: &str, note_span: Span, note: &str) -> DiagnosticWrapper<'a> { +pub fn span_note_and_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, span: Span, msg: &str, note_span: Span, + note: &str) + -> DiagnosticWrapper<'a> { let mut db = cx.struct_span_lint(lint, span, msg); if cx.current_level(lint) != Level::Allow { if note_span == span { @@ -383,21 +396,23 @@ pub fn span_note_and_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, sp } else { db.span_note(note_span, note); } - db.fileline_help(span, &format!("for further information visit \ - https://github.com/Manishearth/rust-clippy/wiki#{}", - lint.name_lower())); + db.fileline_help(span, + &format!("for further information visit https://github.com/Manishearth/rust-clippy/wiki#{}", + lint.name_lower())); } DiagnosticWrapper(db) } -pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint, sp: Span, - msg: &str, f: F) -> DiagnosticWrapper<'a> where F: Fn(&mut DiagnosticWrapper) { +pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint, sp: Span, msg: &str, f: F) + -> DiagnosticWrapper<'a> + where F: Fn(&mut DiagnosticWrapper) +{ let mut db = DiagnosticWrapper(cx.struct_span_lint(lint, sp, msg)); if cx.current_level(lint) != Level::Allow { f(&mut db); - db.fileline_help(sp, &format!("for further information visit \ - https://github.com/Manishearth/rust-clippy/wiki#{}", - lint.name_lower())); + db.fileline_help(sp, + &format!("for further information visit https://github.com/Manishearth/rust-clippy/wiki#{}", + lint.name_lower())); } db } @@ -406,7 +421,7 @@ pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint, pub fn walk_ptrs_ty(ty: ty::Ty) -> ty::Ty { match ty.sty { ty::TyRef(_, ref tm) | ty::TyRawPtr(ref tm) => walk_ptrs_ty(tm.ty), - _ => ty + _ => ty, } } @@ -415,14 +430,13 @@ pub fn walk_ptrs_ty_depth(ty: ty::Ty) -> (ty::Ty, usize) { fn inner(ty: ty::Ty, depth: usize) -> (ty::Ty, usize) { match ty.sty { ty::TyRef(_, ref tm) | ty::TyRawPtr(ref tm) => inner(tm.ty, depth + 1), - _ => (ty, depth) + _ => (ty, depth), } } inner(ty, 0) } -pub fn is_integer_literal(expr: &Expr, value: u64) -> bool -{ +pub fn is_integer_literal(expr: &Expr, value: u64) -> bool { // FIXME: use constant folding if let ExprLit(ref spanned) = expr.node { if let LitInt(v, _) = spanned.node { @@ -448,37 +462,27 @@ impl Drop for LimitStack { impl LimitStack { pub fn new(limit: u64) -> LimitStack { - LimitStack { - stack: vec![limit], - } + LimitStack { stack: vec![limit] } } pub fn limit(&self) -> u64 { *self.stack.last().expect("there should always be a value in the stack") } pub fn push_attrs(&mut self, sess: &Session, attrs: &[ast::Attribute], name: &'static str) { let stack = &mut self.stack; - parse_attrs( - sess, - attrs, - name, - |val| stack.push(val), - ); + parse_attrs(sess, attrs, name, |val| stack.push(val)); } pub fn pop_attrs(&mut self, sess: &Session, attrs: &[ast::Attribute], name: &'static str) { let stack = &mut self.stack; - parse_attrs( - sess, - attrs, - name, - |val| assert_eq!(stack.pop(), Some(val)), - ); + parse_attrs(sess, attrs, name, |val| assert_eq!(stack.pop(), Some(val))); } } fn parse_attrs<F: FnMut(u64)>(sess: &Session, attrs: &[ast::Attribute], name: &'static str, mut f: F) { for attr in attrs { let attr = &attr.node; - if attr.is_sugared_doc { continue; } + if attr.is_sugared_doc { + continue; + } if let ast::MetaNameValue(ref key, ref value) = attr.value.node { if *key == name { if let LitStr(ref s, _) = value.node { @@ -495,70 +499,67 @@ fn parse_attrs<F: FnMut(u64)>(sess: &Session, attrs: &[ast::Attribute], name: &' } } -pub fn is_exp_equal(cx: &LateContext, left : &Expr, right : &Expr) -> bool { +pub fn is_exp_equal(cx: &LateContext, left: &Expr, right: &Expr) -> bool { if let (Some(l), Some(r)) = (constant(cx, left), constant(cx, right)) { if l == r { return true; } } match (&left.node, &right.node) { - (&ExprField(ref lfexp, ref lfident), - &ExprField(ref rfexp, ref rfident)) => - lfident.node == rfident.node && is_exp_equal(cx, lfexp, rfexp), + (&ExprField(ref lfexp, ref lfident), &ExprField(ref rfexp, ref rfident)) => { + lfident.node == rfident.node && is_exp_equal(cx, lfexp, rfexp) + } (&ExprLit(ref l), &ExprLit(ref r)) => l.node == r.node, - (&ExprPath(ref lqself, ref lsubpath), - &ExprPath(ref rqself, ref rsubpath)) => - both(lqself, rqself, is_qself_equal) && - is_path_equal(lsubpath, rsubpath), - (&ExprTup(ref ltup), &ExprTup(ref rtup)) => - is_exps_equal(cx, ltup, rtup), + (&ExprPath(ref lqself, ref lsubpath), &ExprPath(ref rqself, ref rsubpath)) => { + both(lqself, rqself, is_qself_equal) && is_path_equal(lsubpath, rsubpath) + } + (&ExprTup(ref ltup), &ExprTup(ref rtup)) => is_exps_equal(cx, ltup, rtup), (&ExprVec(ref l), &ExprVec(ref r)) => is_exps_equal(cx, l, r), - (&ExprCast(ref lx, ref lt), &ExprCast(ref rx, ref rt)) => - is_exp_equal(cx, lx, rx) && is_cast_ty_equal(lt, rt), - _ => false + (&ExprCast(ref lx, ref lt), &ExprCast(ref rx, ref rt)) => is_exp_equal(cx, lx, rx) && is_cast_ty_equal(lt, rt), + _ => false, } } -fn is_exps_equal(cx: &LateContext, left : &[P<Expr>], right : &[P<Expr>]) -> bool { +fn is_exps_equal(cx: &LateContext, left: &[P<Expr>], right: &[P<Expr>]) -> bool { over(left, right, |l, r| is_exp_equal(cx, l, r)) } -fn is_path_equal(left : &Path, right : &Path) -> bool { +fn is_path_equal(left: &Path, right: &Path) -> bool { // The == of idents doesn't work with different contexts, // we have to be explicit about hygiene - left.global == right.global && over(&left.segments, &right.segments, - |l, r| l.identifier.name == r.identifier.name - && l.parameters == r.parameters) + left.global == right.global && + over(&left.segments, + &right.segments, + |l, r| l.identifier.name == r.identifier.name && l.parameters == r.parameters) } -fn is_qself_equal(left : &QSelf, right : &QSelf) -> bool { +fn is_qself_equal(left: &QSelf, right: &QSelf) -> bool { left.ty.node == right.ty.node && left.position == right.position } 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)) + 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 { - l.as_ref().map_or_else(|| r.is_none(), |x| r.as_ref().map_or(false, - |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 +{ + l.as_ref().map_or_else(|| r.is_none(), |x| r.as_ref().map_or(false, |y| eq_fn(x, y))) } fn is_cast_ty_equal(left: &Ty, right: &Ty) -> bool { match (&left.node, &right.node) { (&TyVec(ref lvec), &TyVec(ref rvec)) => is_cast_ty_equal(lvec, rvec), - (&TyPtr(ref lmut), &TyPtr(ref rmut)) => - lmut.mutbl == rmut.mutbl && - is_cast_ty_equal(&*lmut.ty, &*rmut.ty), - (&TyRptr(_, ref lrmut), &TyRptr(_, ref rrmut)) => - lrmut.mutbl == rrmut.mutbl && - is_cast_ty_equal(&*lrmut.ty, &*rrmut.ty), - (&TyPath(ref lq, ref lpath), &TyPath(ref rq, ref rpath)) => - both(lq, rq, is_qself_equal) && is_path_equal(lpath, rpath), + (&TyPtr(ref lmut), &TyPtr(ref rmut)) => lmut.mutbl == rmut.mutbl && is_cast_ty_equal(&*lmut.ty, &*rmut.ty), + (&TyRptr(_, ref lrmut), &TyRptr(_, ref rrmut)) => { + lrmut.mutbl == rrmut.mutbl && is_cast_ty_equal(&*lrmut.ty, &*rrmut.ty) + } + (&TyPath(ref lq, ref lpath), &TyPath(ref rq, ref rpath)) => { + both(lq, rq, is_qself_equal) && is_path_equal(lpath, rpath) + } (&TyInfer, &TyInfer) => true, - _ => false + _ => false, } } |