Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-24 05:33:27 +00:00
Code Issues Projects Releases Packages Wiki Activity

fmt clippy

Browse source
This commit is contained in:
Manish Goregaokar 2016-01-04 09:56:12 +05:30
parent 839ad09689
commit c9342d0121
41 changed files with 2187 additions and 1488 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

46
src/approx_const.rs
View file

@ -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;
}
}

3
src/array_indexing.rs
View file

@ -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");
}
}
}

38
src/attrs.rs
View file

@ -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));
}
}
}

229
src/bit_mask.rs
View file

@ -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 }
}
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
} else {
None
}
}
.and_then(|def_id| lookup_const_by_id(cx.tcx, def_id, None))
.and_then(|l| fetch_int_literal(cx, l)),
_ => 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,
}
}
.and_then(|def_id| lookup_const_by_id(cx.tcx, def_id, None))
.and_then(|l| fetch_int_literal(cx, l))
}
_ => None,
}
}

37
src/block_in_if_condition.rs
View file

@ -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, "");
}
}
}

41
src/collapsible_if.rs
View file

@ -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)
}
}

443
src/consts.rs
View file

@ -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
}
})
}
}

39
src/cyclomatic_complexity.rs
View file

@ -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));
}
}

24
src/eq_op.rs
View file

@ -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,
}
}

40
src/escape.rs
View file

@ -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) {}
}

14
src/eta_reduction.rs
View file

@ -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);
}
});
}

20
src/identity_op.rs
View file

@ -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, "..")));
}
}
}

128
src/len_zero.rs
View file

@ -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,
}

10
src/lib.rs
View file

@ -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,

54
src/lifetimes.rs
View file

@ -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");
}
}

374
src/loops.rs
View file

@ -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,
}
}

20
src/map_clone.rs
View file

@ -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),
}
}

234
src/matches.rs
View file

@ -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())),
}

418
src/methods.rs
View file

@ -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 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 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,
}
}
}

35
src/minmax.rs
View file

@ -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
}
}
}

145
src/misc.rs
View file

@ -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
}
}

39
src/misc_early.rs
View file

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

42
src/mut_mut.rs
View file

@ -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,
}
}

20
src/mut_reference.rs
View file

@ -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));
}
}
_ => {}

16
src/mutex_atomic.rs
View file

@ -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,
}
}

45
src/needless_bool.rs
View file

@ -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,
}
}

14
src/needless_features.rs
View file

@ -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");
}
}
}

7
src/needless_update.rs
View file

@ -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");
}
}
}

7
src/no_effect.rs
View file

@ -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");
}
}
}

151
src/open_options.rs
View file

@ -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\"");
}
}

68
src/precedence.rs
View file

@ -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,
}
}

17
src/ptr_arg.rs
View file

@ -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`");
}
}
}

19
src/ranges.rs
View file

@ -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! {

34
src/returns.rs
View file

@ -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);
}

166
src/shadow.rs
View file

@ -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
}

30
src/strings.rs
View file

@ -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,
}
}

5
src/temporary_assignment.rs
View file

@ -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");
}
}
_ => ()
_ => (),
}
}
}

3
src/transmute.rs
View file

@ -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));
}
}

253
src/types.rs
View file

@ -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;

46
src/unicode.rs
View file

@ -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>()));
}
}

301
src/utils.rs
View file

@ -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,
}
}