diff --git a/README.md b/README.md index 360370361..9b249636c 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to usage instructions](#usage) ##Lints -There are 105 lints included in this crate: +There are 106 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -30,6 +30,7 @@ name [derive_hash_not_eq](https://github.com/Manishearth/rust-clippy/wiki#derive_hash_not_eq) | warn | deriving `Hash` but implementing `PartialEq` explicitly [duplicate_underscore_argument](https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument) | warn | Function arguments having names which only differ by an underscore [empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}` detected +[enum_variant_names](https://github.com/Manishearth/rust-clippy/wiki#enum_variant_names) | warn | finds enums where all variants share a prefix/postfix [eq_op](https://github.com/Manishearth/rust-clippy/wiki#eq_op) | warn | equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`) [expl_impl_clone_on_copy](https://github.com/Manishearth/rust-clippy/wiki#expl_impl_clone_on_copy) | warn | implementing `Clone` explicitly on `Copy` types [explicit_counter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_counter_loop) | warn | for-looping with an explicit counter when `_.enumerate()` would do diff --git a/src/consts.rs b/src/consts.rs index 31094e5b6..f590095d9 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -12,7 +12,6 @@ use std::cmp::Ordering::{self, Greater, Less, Equal}; use std::rc::Rc; use std::ops::Deref; use std::fmt; -use self::Constant::*; use self::FloatWidth::*; use syntax::ast::Lit_::*; @@ -44,25 +43,25 @@ impl From for FloatWidth { #[derive(Eq, Debug, Clone)] pub enum Constant { /// a String "abc" - ConstantStr(String, StrStyle), + Str(String, StrStyle), /// a Binary String b"abc" - ConstantBinary(Rc>), + Binary(Rc>), /// a single byte b'a' - ConstantByte(u8), + Byte(u8), /// a single char 'a' - ConstantChar(char), + Char(char), /// an integer - ConstantInt(u64, LitIntType), + Int(u64, LitIntType), /// a float with given type - ConstantFloat(String, FloatWidth), + Float(String, FloatWidth), /// true or false - ConstantBool(bool), + Bool(bool), /// an array of constants - ConstantVec(Vec), + Vec(Vec), /// also an array, but with only one constant, repeated N times - ConstantRepeat(Box, usize), + Repeat(Box, usize), /// a tuple of constants - ConstantTuple(Vec), + Tuple(Vec), } impl Constant { @@ -72,7 +71,7 @@ impl Constant { /// /// if the constant could not be converted to u64 losslessly fn as_u64(&self) -> u64 { - if let ConstantInt(val, _) = *self { + if let Constant::Int(val, _) = *self { val // TODO we may want to check the sign if any } else { panic!("Could not convert a {:?} to u64", self); @@ -83,9 +82,9 @@ impl Constant { #[allow(cast_precision_loss)] pub fn as_float(&self) -> Option { match *self { - ConstantByte(b) => Some(b as f64), - ConstantFloat(ref s, _) => s.parse().ok(), - ConstantInt(i, ty) => { + Constant::Byte(b) => Some(b as f64), + Constant::Float(ref s, _) => s.parse().ok(), + Constant::Int(i, ty) => { Some(if is_negative(ty) { -(i as f64) } else { @@ -100,14 +99,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, - (&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)) => { + (&Constant::Str(ref ls, ref lsty), &Constant::Str(ref rs, ref rsty)) => ls == rs && lsty == rsty, + (&Constant::Binary(ref l), &Constant::Binary(ref r)) => l == r, + (&Constant::Byte(l), &Constant::Byte(r)) => l == r, + (&Constant::Char(l), &Constant::Char(r)) => l == r, + (&Constant::Int(lv, lty), &Constant::Int(rv, rty)) => { lv == rv && (is_negative(lty) & (lv != 0)) == (is_negative(rty) & (rv != 0)) } - (&ConstantFloat(ref ls, lw), &ConstantFloat(ref rs, rw)) => { + (&Constant::Float(ref ls, lw), &Constant::Float(ref rs, rw)) => { if match (lw, rw) { (FwAny, _) | (_, FwAny) | (Fw32, Fw32) | (Fw64, Fw64) => true, _ => false, @@ -120,10 +119,10 @@ impl PartialEq for Constant { 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, - (&ConstantTuple(ref l), &ConstantTuple(ref r)) => l == r, + (&Constant::Bool(l), &Constant::Bool(r)) => l == r, + (&Constant::Vec(ref l), &Constant::Vec(ref r)) => l == r, + (&Constant::Repeat(ref lv, ref ls), &Constant::Repeat(ref rv, ref rs)) => ls == rs && lv == rv, + (&Constant::Tuple(ref l), &Constant::Tuple(ref r)) => l == r, _ => false, //TODO: Are there inter-type equalities? } } @@ -132,16 +131,16 @@ impl PartialEq for Constant { impl PartialOrd for Constant { fn partial_cmp(&self, other: &Constant) -> Option { match (self, other) { - (&ConstantStr(ref ls, ref lsty), &ConstantStr(ref rs, ref rsty)) => { + (&Constant::Str(ref ls, ref lsty), &Constant::Str(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)) => { + (&Constant::Byte(ref l), &Constant::Byte(ref r)) => Some(l.cmp(r)), + (&Constant::Char(ref l), &Constant::Char(ref r)) => Some(l.cmp(r)), + (&Constant::Int(ref lv, lty), &Constant::Int(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), @@ -149,7 +148,7 @@ impl PartialOrd for Constant { (false, true) => Greater, }) } - (&ConstantFloat(ref ls, lw), &ConstantFloat(ref rs, rw)) => { + (&Constant::Float(ref ls, lw), &Constant::Float(ref rs, rw)) => { if match (lw, rw) { (FwAny, _) | (_, FwAny) | (Fw32, Fw32) | (Fw64, Fw64) => true, _ => false, @@ -162,15 +161,15 @@ impl PartialOrd for Constant { 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)) => { + (&Constant::Bool(ref l), &Constant::Bool(ref r)) => Some(l.cmp(r)), + (&Constant::Vec(ref l), &Constant::Vec(ref r)) => l.partial_cmp(&r), + (&Constant::Repeat(ref lv, ref ls), &Constant::Repeat(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), + (&Constant::Tuple(ref l), &Constant::Tuple(ref r)) => l.partial_cmp(r), _ => None, //TODO: Are there any useful inter-type orderings? } } @@ -193,21 +192,21 @@ fn format_byte(fmt: &mut fmt::Formatter, b: u8) -> fmt::Result { 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) => { + Constant::Str(ref s, _) => write!(fmt, "{:?}", s), + Constant::Byte(ref b) => { write!(fmt, "b'") .and_then(|_| format_byte(fmt, *b)) .and_then(|_| write!(fmt, "'")) } - ConstantBinary(ref bs) => { + Constant::Binary(ref bs) => { try!(write!(fmt, "b\"")); for b in bs.iter() { try!(format_byte(fmt, *b)); } write!(fmt, "\"") } - ConstantChar(ref c) => write!(fmt, "'{}'", c), - ConstantInt(ref i, ref ity) => { + Constant::Char(ref c) => write!(fmt, "'{}'", c), + Constant::Int(ref i, ref ity) => { let (sign, suffix) = match *ity { LitIntType::SignedIntLit(ref sity, ref sign) => { (if let Sign::Minus = *sign { @@ -229,7 +228,7 @@ impl fmt::Display for Constant { }; write!(fmt, "{}{}{}", sign, i, suffix) } - ConstantFloat(ref s, ref fw) => { + Constant::Float(ref s, ref fw) => { let suffix = match *fw { FloatWidth::Fw32 => "f32", FloatWidth::Fw64 => "f64", @@ -237,9 +236,9 @@ impl fmt::Display for Constant { }; write!(fmt, "{}{}", s, suffix) } - ConstantBool(ref b) => write!(fmt, "{}", b), - ConstantRepeat(ref c, ref n) => write!(fmt, "[{}; {}]", c, n), - ConstantVec(ref v) => { + Constant::Bool(ref b) => write!(fmt, "{}", b), + Constant::Repeat(ref c, ref n) => write!(fmt, "[{}; {}]", c, n), + Constant::Vec(ref v) => { write!(fmt, "[{}]", v.iter() @@ -247,7 +246,7 @@ impl fmt::Display for Constant { .collect::>() .join(", ")) } - ConstantTuple(ref t) => { + Constant::Tuple(ref t) => { write!(fmt, "({})", t.iter() @@ -262,21 +261,21 @@ impl fmt::Display for Constant { fn lit_to_constant(lit: &Lit_) -> Constant { match *lit { - LitStr(ref is, style) => ConstantStr(is.to_string(), style), - LitByte(b) => ConstantByte(b), - LitByteStr(ref s) => ConstantBinary(s.clone()), - LitChar(c) => ConstantChar(c), - LitInt(value, ty) => ConstantInt(value, ty), - LitFloat(ref is, ty) => ConstantFloat(is.to_string(), ty.into()), - LitFloatUnsuffixed(ref is) => ConstantFloat(is.to_string(), FwAny), - LitBool(b) => ConstantBool(b), + LitStr(ref is, style) => Constant::Str(is.to_string(), style), + LitByte(b) => Constant::Byte(b), + LitByteStr(ref s) => Constant::Binary(s.clone()), + LitChar(c) => Constant::Char(c), + LitInt(value, ty) => Constant::Int(value, ty), + LitFloat(ref is, ty) => Constant::Float(is.to_string(), ty.into()), + LitFloatUnsuffixed(ref is) => Constant::Float(is.to_string(), FwAny), + LitBool(b) => Constant::Bool(b), } } fn constant_not(o: Constant) -> Option { Some(match o { - ConstantBool(b) => ConstantBool(!b), - ConstantInt(value, ty) => { + Constant::Bool(b) => Constant::Bool(!b), + Constant::Int(value, ty) => { let (nvalue, nty) = match ty { SignedIntLit(ity, Plus) => { if value == ::std::u64::MAX { @@ -307,7 +306,7 @@ fn constant_not(o: Constant) -> Option { return None; } // refuse to guess }; - ConstantInt(nvalue, nty) + Constant::Int(nvalue, nty) } _ => { return None; @@ -317,8 +316,8 @@ fn constant_not(o: Constant) -> Option { fn constant_negate(o: Constant) -> Option { Some(match o { - ConstantInt(value, ty) => { - ConstantInt(value, + Constant::Int(value, ty) => { + Constant::Int(value, match ty { SignedIntLit(ity, sign) => SignedIntLit(ity, neg_sign(sign)), UnsuffixedIntLit(sign) => UnsuffixedIntLit(neg_sign(sign)), @@ -327,7 +326,7 @@ fn constant_negate(o: Constant) -> Option { } }) } - ConstantFloat(is, ty) => ConstantFloat(neg_float_str(is), ty), + Constant::Float(is, ty) => Constant::Float(neg_float_str(is), ty), _ => { return None; } @@ -402,9 +401,9 @@ fn unify_int_type(l: LitIntType, r: LitIntType, s: Sign) -> Option { fn add_neg_int(pos: u64, pty: LitIntType, neg: u64, nty: LitIntType) -> Option { if neg > pos { - unify_int_type(nty, pty, Minus).map(|ty| ConstantInt(neg - pos, ty)) + unify_int_type(nty, pty, Minus).map(|ty| Constant::Int(neg - pos, ty)) } else { - unify_int_type(nty, pty, Plus).map(|ty| ConstantInt(pos - neg, ty)) + unify_int_type(nty, pty, Plus).map(|ty| Constant::Int(pos - neg, ty)) } } @@ -416,7 +415,7 @@ fn sub_int(l: u64, lty: LitIntType, r: u64, rty: LitIntType, neg: bool) -> Optio } else { Plus }) - .and_then(|ty| l.checked_sub(r).map(|v| ConstantInt(v, ty))) + .and_then(|ty| l.checked_sub(r).map(|v| Constant::Int(v, ty))) } @@ -449,10 +448,10 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { ExprBlock(ref block) => self.block(block), 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), + ExprVec(ref vec) => self.multi(vec).map(Constant::Vec), + ExprTup(ref tup) => self.multi(tup).map(Constant::Tuple), ExprRepeat(ref value, ref number) => { - self.binop_apply(value, number, |v, n| Some(ConstantRepeat(Box::new(v), n.as_u64() as usize))) + self.binop_apply(value, number, |v, n| Some(Constant::Repeat(Box::new(v), n.as_u64() as usize))) } ExprUnary(op, ref operand) => { self.expr(operand).and_then(|o| { @@ -508,7 +507,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { } fn ifthenelse(&mut self, cond: &Expr, then: &Block, otherwise: &Option>) -> Option { - if let Some(ConstantBool(b)) = self.expr(cond) { + if let Some(Constant::Bool(b)) = self.expr(cond) { if b { self.block(then) } else { @@ -524,8 +523,8 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { 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)) => { + (Constant::Byte(l8), Constant::Byte(r8)) => l8.checked_add(r8).map(Constant::Byte), + (Constant::Int(l64, lty), Constant::Int(r64, rty)) => { let (ln, rn) = (is_negative(lty), is_negative(rty)); if ln == rn { unify_int_type(lty, @@ -535,7 +534,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { } else { Plus }) - .and_then(|ty| l64.checked_add(r64).map(|v| ConstantInt(v, ty))) + .and_then(|ty| l64.checked_add(r64).map(|v| Constant::Int(v, ty))) } else if ln { add_neg_int(r64, rty, l64, lty) } else { @@ -550,24 +549,24 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { BiSub => { self.binop_apply(left, right, |l, r| { match (l, r) { - (ConstantByte(l8), ConstantByte(r8)) => { + (Constant::Byte(l8), Constant::Byte(r8)) => { if r8 > l8 { None } else { - Some(ConstantByte(l8 - r8)) + Some(Constant::Byte(l8 - r8)) } } - (ConstantInt(l64, lty), ConstantInt(r64, rty)) => { + (Constant::Int(l64, lty), Constant::Int(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))) + .and_then(|ty| l64.checked_add(r64).map(|v| Constant::Int(v, ty))) } (false, true) => { unify_int_type(lty, rty, Plus) - .and_then(|ty| l64.checked_add(r64).map(|v| ConstantInt(v, ty))) + .and_then(|ty| l64.checked_add(r64).map(|v| Constant::Int(v, ty))) } } } @@ -585,8 +584,8 @@ 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(Constant::Bool(l == r))), + BiNe => self.binop_apply(left, right, |l, r| Some(Constant::Bool(l != r))), BiLt => self.cmp(left, right, Less, true), BiLe => self.cmp(left, right, Greater, false), BiGe => self.cmp(left, right, Less, false), @@ -600,7 +599,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { { self.binop_apply(left, right, |l, r| { match (l, r) { - (ConstantInt(l64, lty), ConstantInt(r64, rty)) => { + (Constant::Int(l64, lty), Constant::Int(r64, rty)) => { f(l64, r64).and_then(|value| { unify_int_type(lty, rty, @@ -609,7 +608,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { } else { Minus }) - .map(|ty| ConstantInt(value, ty)) + .map(|ty| Constant::Int(value, ty)) }) } _ => None, @@ -622,10 +621,10 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { { 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)) + (Constant::Bool(l), Constant::Bool(r)) => Some(Constant::Bool(f(l as u64, r as u64) != 0)), + (Constant::Byte(l8), Constant::Byte(r8)) => Some(Constant::Byte(f(l8 as u64, r8 as u64) as u8)), + (Constant::Int(l, lty), Constant::Int(r, rty)) => { + unify_int_type(lty, rty, Plus).map(|ty| Constant::Int(f(l, r), ty)) } _ => None, } @@ -635,7 +634,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { fn cmp(&mut self, left: &Expr, right: &Expr, ordering: Ordering, b: bool) -> Option { self.binop_apply(left, right, - |l, r| l.partial_cmp(&r).map(|o| ConstantBool(b == (o == ordering)))) + |l, r| l.partial_cmp(&r).map(|o| Constant::Bool(b == (o == ordering)))) } fn binop_apply(&mut self, left: &Expr, right: &Expr, op: F) -> Option @@ -650,12 +649,12 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { fn short_circuit(&mut self, left: &Expr, right: &Expr, b: bool) -> Option { self.expr(left).and_then(|left| { - if let ConstantBool(lbool) = left { + if let Constant::Bool(lbool) = left { if lbool == b { Some(left) } else { self.expr(right).and_then(|right| { - if let ConstantBool(_) = right { + if let Constant::Bool(_) = right { Some(right) } else { None diff --git a/src/enum_variants.rs b/src/enum_variants.rs new file mode 100644 index 000000000..bf0025e02 --- /dev/null +++ b/src/enum_variants.rs @@ -0,0 +1,90 @@ +//! lint on enum variants that are prefixed or suffixed by the same characters + +use rustc::lint::*; +use syntax::attr::*; +use syntax::ast::*; +use syntax::parse::token::InternedString; + +use utils::span_help_and_lint; + +/// **What it does:** Warns on enum variants that are prefixed or suffixed by the same characters +/// +/// **Why is this bad?** Enum variant names should specify their variant, not the enum, too. +/// +/// **Known problems:** None +/// +/// **Example:** enum Cake { BlackForestCake, HummingbirdCake } +declare_lint! { pub ENUM_VARIANT_NAMES, Warn, + "finds enums where all variants share a prefix/postfix" } + +pub struct EnumVariantNames; + +impl LintPass for EnumVariantNames { + fn get_lints(&self) -> LintArray { + lint_array!(ENUM_VARIANT_NAMES) + } +} + +fn var2str(var: &Variant) -> InternedString { + var.node.name.name.as_str() +} + +fn partial_match(left: &str, right: &str) -> usize { + left.chars().zip(right.chars()).take_while(|&(l, r)| l == r).count() +} + +fn partial_rmatch(left: &str, right: &str) -> usize { + left.chars().rev().zip(right.chars().rev()).take_while(|&(l, r)| l == r).count() +} + +impl EarlyLintPass for EnumVariantNames { + fn check_item(&mut self, cx: &EarlyContext, item: &Item) { + if let ItemEnum(ref def, _) = item.node { + if def.variants.len() < 2 { + return; + } + let first = var2str(&*def.variants[0]); + let mut pre = first.to_string(); + let mut post = pre.clone(); + for var in &def.variants[1..] { + let name = var2str(var); + let pre_match = partial_match(&pre, &name); + let post_match = partial_rmatch(&post, &name); + pre.truncate(pre_match); + let post_end = post.len() - post_match; + post.drain(..post_end); + } + if let Some(c) = first[pre.len()..].chars().next() { + if !c.is_uppercase() { + // non camel case prefix + pre.clear() + } + } + if let Some(c) = first[..(first.len() - post.len())].chars().rev().next() { + if let Some(c1) = post.chars().next() { + if !c.is_lowercase() || !c1.is_uppercase() { + // non camel case postfix + post.clear() + } + } + } + if pre == "_" { + // don't lint on underscores which are meant to allow dead code + pre.clear(); + } + let (what, value) = if !pre.is_empty() { + ("pre", pre) + } else if !post.is_empty() { + ("post", post) + } else { + return + }; + span_help_and_lint(cx, + ENUM_VARIANT_NAMES, + item.span, + &format!("All variants have the same {}fix: `{}`", what, value), + &format!("remove the {}fixes and use full paths to \ + the variants instead of glob imports", what)); + } + } +} diff --git a/src/identity_op.rs b/src/identity_op.rs index fd5071d40..5fa9c7588 100644 --- a/src/identity_op.rs +++ b/src/identity_op.rs @@ -2,8 +2,7 @@ use rustc::lint::*; use rustc_front::hir::*; use syntax::codemap::Span; -use consts::{constant_simple, is_negative}; -use consts::Constant::ConstantInt; +use consts::{constant_simple, is_negative, Constant}; use utils::{span_lint, snippet, in_macro}; /// **What it does:** This lint checks for identity operations, e.g. `x + 0`. It is `Warn` by default. @@ -54,7 +53,7 @@ impl LateLintPass for IdentityOp { fn check(cx: &LateContext, e: &Expr, m: i8, span: Span, arg: Span) { - if let Some(ConstantInt(v, ty)) = constant_simple(e) { + if let Some(Constant::Int(v, ty)) = constant_simple(e) { if match m { 0 => v == 0, -1 => is_negative(ty) && v == 1, diff --git a/src/lib.rs b/src/lib.rs index 6d8ea99eb..15d1db19d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,6 +43,7 @@ pub mod ptr_arg; pub mod needless_bool; pub mod approx_const; pub mod eta_reduction; +pub mod enum_variants; pub mod identity_op; pub mod items_after_statements; pub mod minmax; @@ -93,6 +94,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box misc::TopLevelRefPass); reg.register_late_lint_pass(box misc::CmpNan); reg.register_late_lint_pass(box eq_op::EqOp); + reg.register_early_lint_pass(box enum_variants::EnumVariantNames); reg.register_late_lint_pass(box bit_mask::BitMask); reg.register_late_lint_pass(box ptr_arg::PtrArg); reg.register_late_lint_pass(box needless_bool::NeedlessBool); @@ -183,6 +185,7 @@ pub fn plugin_registrar(reg: &mut Registry) { derive::DERIVE_HASH_NOT_EQ, derive::EXPL_IMPL_CLONE_ON_COPY, entry::MAP_ENTRY, + enum_variants::ENUM_VARIANT_NAMES, eq_op::EQ_OP, escape::BOXED_LOCAL, eta_reduction::REDUNDANT_CLOSURE, diff --git a/src/loops.rs b/src/loops.rs index 74ff9edcb..1baaab6ab 100644 --- a/src/loops.rs +++ b/src/loops.rs @@ -365,8 +365,8 @@ fn check_for_loop_reverse_range(cx: &LateContext, arg: &Expr, expr: &Expr) { // if this for loop is iterating over a two-sided range... if let ExprRange(Some(ref start_expr), Some(ref stop_expr)) = arg.node { // ...and both sides are compile-time constant integers... - if let Some(start_idx @ Constant::ConstantInt(..)) = constant_simple(start_expr) { - if let Some(stop_idx @ Constant::ConstantInt(..)) = constant_simple(stop_expr) { + if let Some(start_idx @ Constant::Int(..)) = constant_simple(start_expr) { + if let Some(stop_idx @ Constant::Int(..)) = constant_simple(stop_expr) { // ...and the start index is greater than the stop index, // this loop will never run. This is often confusing for developers // who think that this will iterate from the larger value to the diff --git a/src/methods.rs b/src/methods.rs index a5f27440f..111f4d564 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -19,9 +19,6 @@ use utils::{ use utils::MethodArgs; use rustc::middle::cstore::CrateStore; -use self::SelfKind::*; -use self::OutType::*; - #[derive(Clone)] pub struct MethodsPass; @@ -456,11 +453,11 @@ fn lint_extend(cx: &LateContext, expr: &Expr, args: &MethodArgs) { } } -fn derefs_to_slice(cx: &LateContext, expr: &Expr, ty: &ty::Ty) +fn derefs_to_slice(cx: &LateContext, expr: &Expr, ty: &ty::Ty) -> Option<(Span, &'static str)> { fn may_slice(cx: &LateContext, ty: &ty::Ty) -> bool { match ty.sty { - ty::TySlice(_) => true, + ty::TySlice(_) => true, ty::TyStruct(..) => match_type(cx, ty, &VEC_PATH), ty::TyArray(_, size) => size < 32, ty::TyRef(_, ty::TypeAndMut { ty: ref inner, .. }) | @@ -469,7 +466,7 @@ fn derefs_to_slice(cx: &LateContext, expr: &Expr, ty: &ty::Ty) } } if let ExprMethodCall(name, _, ref args) = expr.node { - if &name.node.as_str() == &"iter" && + if &name.node.as_str() == &"iter" && may_slice(cx, &cx.tcx.expr_ty(&args[0])) { Some((args[0].span, "&")) } else { @@ -479,7 +476,7 @@ fn derefs_to_slice(cx: &LateContext, expr: &Expr, ty: &ty::Ty) match ty.sty { ty::TySlice(_) => Some((expr.span, "")), ty::TyRef(_, ty::TypeAndMut { ty: ref inner, .. }) | - ty::TyBox(ref inner) => if may_slice(cx, inner) { + ty::TyBox(ref inner) => if may_slice(cx, inner) { Some((expr.span, "")) } else { None }, _ => None @@ -731,180 +728,180 @@ 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_", &[SelfKind::Value]), + ("to_", &[SelfKind::Ref]), + ("as_", &[SelfKind::Ref, SelfKind::RefMut]), + ("is_", &[SelfKind::Ref, SelfKind::No]), + ("from_", &[SelfKind::No])]; const TRAIT_METHODS: [(&'static str, usize, SelfKind, OutType, &'static str); 30] = [("add", 2, - ValueSelf, - AnyType, + SelfKind::Value, + OutType::Any, "std::ops::Add"), ("sub", 2, - ValueSelf, - AnyType, + SelfKind::Value, + OutType::Any, "std::ops::Sub"), ("mul", 2, - ValueSelf, - AnyType, + SelfKind::Value, + OutType::Any, "std::ops::Mul"), ("div", 2, - ValueSelf, - AnyType, + SelfKind::Value, + OutType::Any, "std::ops::Div"), ("rem", 2, - ValueSelf, - AnyType, + SelfKind::Value, + OutType::Any, "std::ops::Rem"), ("shl", 2, - ValueSelf, - AnyType, + SelfKind::Value, + OutType::Any, "std::ops::Shl"), ("shr", 2, - ValueSelf, - AnyType, + SelfKind::Value, + OutType::Any, "std::ops::Shr"), ("bitand", 2, - ValueSelf, - AnyType, + SelfKind::Value, + OutType::Any, "std::ops::BitAnd"), ("bitor", 2, - ValueSelf, - AnyType, + SelfKind::Value, + OutType::Any, "std::ops::BitOr"), ("bitxor", 2, - ValueSelf, - AnyType, + SelfKind::Value, + OutType::Any, "std::ops::BitXor"), ("neg", 1, - ValueSelf, - AnyType, + SelfKind::Value, + OutType::Any, "std::ops::Neg"), ("not", 1, - ValueSelf, - AnyType, + SelfKind::Value, + OutType::Any, "std::ops::Not"), ("drop", 1, - RefMutSelf, - UnitType, + SelfKind::RefMut, + OutType::Unit, "std::ops::Drop"), ("index", 2, - RefSelf, - RefType, + SelfKind::Ref, + OutType::Ref, "std::ops::Index"), ("index_mut", 2, - RefMutSelf, - RefType, + SelfKind::RefMut, + OutType::Ref, "std::ops::IndexMut"), ("deref", 1, - RefSelf, - RefType, + SelfKind::Ref, + OutType::Ref, "std::ops::Deref"), ("deref_mut", 1, - RefMutSelf, - RefType, + SelfKind::RefMut, + OutType::Ref, "std::ops::DerefMut"), ("clone", 1, - RefSelf, - AnyType, + SelfKind::Ref, + OutType::Any, "std::clone::Clone"), ("borrow", 1, - RefSelf, - RefType, + SelfKind::Ref, + OutType::Ref, "std::borrow::Borrow"), ("borrow_mut", 1, - RefMutSelf, - RefType, + SelfKind::RefMut, + OutType::Ref, "std::borrow::BorrowMut"), ("as_ref", 1, - RefSelf, - RefType, + SelfKind::Ref, + OutType::Ref, "std::convert::AsRef"), ("as_mut", 1, - RefMutSelf, - RefType, + SelfKind::RefMut, + OutType::Ref, "std::convert::AsMut"), ("eq", 2, - RefSelf, - BoolType, + SelfKind::Ref, + OutType::Bool, "std::cmp::PartialEq"), ("cmp", 2, - RefSelf, - AnyType, + SelfKind::Ref, + OutType::Any, "std::cmp::Ord"), ("default", 0, - NoSelf, - AnyType, + SelfKind::No, + OutType::Any, "std::default::Default"), ("hash", 2, - RefSelf, - UnitType, + SelfKind::Ref, + OutType::Unit, "std::hash::Hash"), ("next", 1, - RefMutSelf, - AnyType, + SelfKind::RefMut, + OutType::Any, "std::iter::Iterator"), ("into_iter", 1, - ValueSelf, - AnyType, + SelfKind::Value, + OutType::Any, "std::iter::IntoIterator"), ("from_iter", 1, - NoSelf, - AnyType, + SelfKind::No, + OutType::Any, "std::iter::FromIterator"), ("from_str", 1, - NoSelf, - AnyType, + SelfKind::No, + OutType::Any, "std::str::FromStr")]; #[derive(Clone, Copy)] enum SelfKind { - ValueSelf, - RefSelf, - RefMutSelf, - NoSelf, + Value, + Ref, + RefMut, + No, } impl SelfKind { fn matches(&self, slf: &ExplicitSelf_, allow_value_for_ref: bool) -> bool { match (self, slf) { - (&ValueSelf, &SelfValue(_)) => true, - (&RefSelf, &SelfRegion(_, Mutability::MutImmutable, _)) => true, - (&RefMutSelf, &SelfRegion(_, Mutability::MutMutable, _)) => true, - (&RefSelf, &SelfValue(_)) => allow_value_for_ref, - (&RefMutSelf, &SelfValue(_)) => allow_value_for_ref, - (&NoSelf, &SelfStatic) => true, + (&SelfKind::Value, &SelfValue(_)) => true, + (&SelfKind::Ref, &SelfRegion(_, Mutability::MutImmutable, _)) => true, + (&SelfKind::RefMut, &SelfRegion(_, Mutability::MutMutable, _)) => true, + (&SelfKind::Ref, &SelfValue(_)) => allow_value_for_ref, + (&SelfKind::RefMut, &SelfValue(_)) => allow_value_for_ref, + (&SelfKind::No, &SelfStatic) => true, (_, &SelfExplicit(ref ty, _)) => self.matches_explicit_type(ty, allow_value_for_ref), _ => false, } @@ -912,41 +909,41 @@ impl SelfKind { fn matches_explicit_type(&self, ty: &Ty, allow_value_for_ref: bool) -> bool { match (self, &ty.node) { - (&ValueSelf, &TyPath(..)) => true, - (&RefSelf, &TyRptr(_, MutTy { mutbl: Mutability::MutImmutable, .. })) => true, - (&RefMutSelf, &TyRptr(_, MutTy { mutbl: Mutability::MutMutable, .. })) => true, - (&RefSelf, &TyPath(..)) => allow_value_for_ref, - (&RefMutSelf, &TyPath(..)) => allow_value_for_ref, + (&SelfKind::Value, &TyPath(..)) => true, + (&SelfKind::Ref, &TyRptr(_, MutTy { mutbl: Mutability::MutImmutable, .. })) => true, + (&SelfKind::RefMut, &TyRptr(_, MutTy { mutbl: Mutability::MutMutable, .. })) => true, + (&SelfKind::Ref, &TyPath(..)) => allow_value_for_ref, + (&SelfKind::RefMut, &TyPath(..)) => allow_value_for_ref, _ => false, } } fn description(&self) -> &'static str { match *self { - ValueSelf => "self by value", - RefSelf => "self by reference", - RefMutSelf => "self by mutable reference", - NoSelf => "no self", + SelfKind::Value => "self by value", + SelfKind::Ref => "self by reference", + SelfKind::RefMut => "self by mutable reference", + SelfKind::No => "no self", } } } #[derive(Clone, Copy)] enum OutType { - UnitType, - BoolType, - AnyType, - RefType, + Unit, + Bool, + Any, + Ref, } impl OutType { fn matches(&self, ty: &FunctionRetTy) -> bool { match (self, ty) { - (&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, - (&RefType, &Return(ref ty)) => { + (&OutType::Unit, &DefaultReturn(_)) => true, + (&OutType::Unit, &Return(ref ty)) if ty.node == TyTup(vec![].into()) => true, + (&OutType::Bool, &Return(ref ty)) if is_bool(ty) => true, + (&OutType::Any, &Return(ref ty)) if ty.node != TyTup(vec![].into()) => true, + (&OutType::Ref, &Return(ref ty)) => { if let TyRptr(_, _) = ty.node { true } else { diff --git a/src/zero_div_zero.rs b/src/zero_div_zero.rs index 5a4d39316..2c3ec8693 100644 --- a/src/zero_div_zero.rs +++ b/src/zero_div_zero.rs @@ -35,8 +35,8 @@ impl LateLintPass for ZeroDivZeroPass { // TODO - constant_simple does not fold many operations involving floats. // That's probably fine for this lint - it's pretty unlikely that someone would // do something like 0.0/(2.0 - 2.0), but it would be nice to warn on that case too. - let Some(Constant::ConstantFloat(ref lhs_value, lhs_width)) = constant_simple(left), - let Some(Constant::ConstantFloat(ref rhs_value, rhs_width)) = constant_simple(right), + let Some(Constant::Float(ref lhs_value, lhs_width)) = constant_simple(left), + let Some(Constant::Float(ref rhs_value, rhs_width)) = constant_simple(right), let Some(0.0) = lhs_value.parse().ok(), let Some(0.0) = rhs_value.parse().ok() ], diff --git a/tests/cc_seme.rs b/tests/cc_seme.rs index a26731c39..cc02853c7 100644 --- a/tests/cc_seme.rs +++ b/tests/cc_seme.rs @@ -3,8 +3,8 @@ #[allow(dead_code)] enum Baz { - Baz1, - Baz2, + One, + Two, } struct Test { @@ -14,11 +14,11 @@ struct Test { fn main() { use Baz::*; - let x = Test { t: Some(0), b: Baz1 }; + let x = Test { t: Some(0), b: One }; match x { - Test { t: Some(_), b: Baz1 } => unreachable!(), - Test { t: Some(42), b: Baz2 } => unreachable!(), + Test { t: Some(_), b: One } => unreachable!(), + Test { t: Some(42), b: Two } => unreachable!(), Test { t: None, .. } => unreachable!(), Test { .. } => unreachable!(), } diff --git a/tests/compile-fail/complex_types.rs b/tests/compile-fail/complex_types.rs index 995132ba8..ad01e4fad 100644 --- a/tests/compile-fail/complex_types.rs +++ b/tests/compile-fail/complex_types.rs @@ -16,8 +16,8 @@ struct S { struct TS(Vec>>); //~ERROR very complex type enum E { - V1(Vec>>), //~ERROR very complex type - V2 { f: Vec>> }, //~ERROR very complex type + Tuple(Vec>>), //~ERROR very complex type + Struct { f: Vec>> }, //~ERROR very complex type } impl S { diff --git a/tests/compile-fail/no_effect.rs b/tests/compile-fail/no_effect.rs index 8da119eb1..21118b827 100644 --- a/tests/compile-fail/no_effect.rs +++ b/tests/compile-fail/no_effect.rs @@ -11,8 +11,8 @@ struct Struct { field: i32 } enum Enum { - TupleVariant(i32), - StructVariant { field: i32 }, + Tuple(i32), + Struct { field: i32 }, } fn get_number() -> i32 { 0 } @@ -26,14 +26,14 @@ fn main() { Tuple(0); //~ERROR statement with no effect Struct { field: 0 }; //~ERROR statement with no effect Struct { ..s }; //~ERROR statement with no effect - Enum::TupleVariant(0); //~ERROR statement with no effect - Enum::StructVariant { field: 0 }; //~ERROR statement with no effect + Enum::Tuple(0); //~ERROR statement with no effect + Enum::Struct { field: 0 }; //~ERROR statement with no effect // Do not warn get_number(); Tuple(get_number()); Struct { field: get_number() }; Struct { ..get_struct() }; - Enum::TupleVariant(get_number()); - Enum::StructVariant { field: get_number() }; + Enum::Tuple(get_number()); + Enum::Struct { field: get_number() }; } diff --git a/tests/compile-fail/used_underscore_binding.rs b/tests/compile-fail/used_underscore_binding.rs index 39a33c968..281d92c46 100644 --- a/tests/compile-fail/used_underscore_binding.rs +++ b/tests/compile-fail/used_underscore_binding.rs @@ -50,17 +50,17 @@ fn multiple_underscores(__foo: u32) -> u32 { fn _fn_test() {} struct _StructTest; enum _EnumTest { - _FieldA, - _FieldB(_StructTest) + _Empty, + _Value(_StructTest) } /// Test that we do not lint for non-variable bindings fn non_variables() { _fn_test(); let _s = _StructTest; - let _e = match _EnumTest::_FieldB(_StructTest) { - _EnumTest::_FieldA => 0, - _EnumTest::_FieldB(_st) => 1, + let _e = match _EnumTest::_Value(_StructTest) { + _EnumTest::_Empty => 0, + _EnumTest::_Value(_st) => 1, }; let f = _fn_test; f(); diff --git a/tests/consts.rs b/tests/consts.rs index 5ddcd6df7..75082e646 100644 --- a/tests/consts.rs +++ b/tests/consts.rs @@ -18,7 +18,6 @@ use syntax::ast::StrStyle::*; use syntax::ast::Sign::*; use clippy::consts::{constant_simple, Constant}; -use clippy::consts::Constant::*; fn spanned(t: T) -> Spanned { Spanned{ node: t, span: COMMAND_LINE_SP } @@ -45,18 +44,18 @@ fn check(expect: Constant, expr: &Expr) { assert_eq!(Some(expect), constant_simple(expr)) } -const TRUE : Constant = ConstantBool(true); -const FALSE : Constant = ConstantBool(false); -const ZERO : Constant = ConstantInt(0, UnsuffixedIntLit(Plus)); -const ONE : Constant = ConstantInt(1, UnsuffixedIntLit(Plus)); -const TWO : Constant = ConstantInt(2, UnsuffixedIntLit(Plus)); +const TRUE : Constant = Constant::Bool(true); +const FALSE : Constant = Constant::Bool(false); +const ZERO : Constant = Constant::Int(0, UnsuffixedIntLit(Plus)); +const ONE : Constant = Constant::Int(1, UnsuffixedIntLit(Plus)); +const TWO : Constant = Constant::Int(2, UnsuffixedIntLit(Plus)); #[test] fn test_lit() { check(TRUE, &lit(LitBool(true))); check(FALSE, &lit(LitBool(false))); check(ZERO, &lit(LitInt(0, UnsuffixedIntLit(Plus)))); - check(ConstantStr("cool!".into(), CookedStr), &lit(LitStr( + check(Constant::Str("cool!".into(), CookedStr), &lit(LitStr( InternedString::new("cool!"), CookedStr))); }