diff --git a/README.md b/README.md index 08ad214e690..20aaa03c51c 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 113 lints included in this crate: +There are 114 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -34,6 +34,7 @@ name [drop_ref](https://github.com/Manishearth/rust-clippy/wiki#drop_ref) | warn | call to `std::mem::drop` with a reference instead of an owned value, which will not call the `Drop::drop` method on the underlying value [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_glob_use](https://github.com/Manishearth/rust-clippy/wiki#enum_glob_use) | allow | finds use items that import all variants of an enum [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 diff --git a/src/approx_const.rs b/src/approx_const.rs index 98e2fa97eba..d18785ab818 100644 --- a/src/approx_const.rs +++ b/src/approx_const.rs @@ -2,9 +2,7 @@ use rustc::lint::*; use rustc_front::hir::*; use std::f64::consts as f64; use utils::span_lint; -use syntax::ast::Lit_::*; -use syntax::ast::Lit; -use syntax::ast::FloatTy::*; +use syntax::ast::{Lit, Lit_, FloatTy}; /// **What it does:** This lint checks for floating point literals that approximate constants which are defined in [`std::f32::consts`](https://doc.rust-lang.org/stable/std/f32/consts/#constants) or [`std::f64::consts`](https://doc.rust-lang.org/stable/std/f64/consts/#constants), respectively, suggesting to use the predefined constant. /// @@ -57,9 +55,9 @@ impl LateLintPass for ApproxConstant { 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}"), + Lit_::LitFloat(ref s, FloatTy::TyF32) => check_known_consts(cx, e, s, "f32"), + Lit_::LitFloat(ref s, FloatTy::TyF64) => check_known_consts(cx, e, s, "f64"), + Lit_::LitFloatUnsuffixed(ref s) => check_known_consts(cx, e, s, "f{32, 64}"), _ => (), } } diff --git a/src/bit_mask.rs b/src/bit_mask.rs index b9925f10fa2..6de00167571 100644 --- a/src/bit_mask.rs +++ b/src/bit_mask.rs @@ -4,7 +4,7 @@ use rustc::middle::def::{Def, PathResolution}; use rustc_front::hir::*; use rustc_front::util::is_comparison_binop; use syntax::codemap::Span; -use syntax::ast::Lit_::*; +use syntax::ast::Lit_; use utils::span_lint; @@ -254,7 +254,7 @@ fn check_ineffective_gt(cx: &LateContext, span: Span, m: u64, c: u64, op: &str) fn fetch_int_literal(cx: &LateContext, lit: &Expr) -> Option { match lit.node { ExprLit(ref lit_ptr) => { - if let LitInt(value, _) = lit_ptr.node { + if let Lit_::LitInt(value, _) = lit_ptr.node { Some(value) //TODO: Handle sign } else { None diff --git a/src/consts.rs b/src/consts.rs index f590095d9f2..5f40aff92cc 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -12,14 +12,10 @@ use std::cmp::Ordering::{self, Greater, Less, Equal}; use std::rc::Rc; use std::ops::Deref; use std::fmt; -use self::FloatWidth::*; -use syntax::ast::Lit_::*; use syntax::ast::Lit_; -use syntax::ast::LitIntType::*; use syntax::ast::LitIntType; use syntax::ast::{UintTy, FloatTy, StrStyle}; -use syntax::ast::FloatTy::*; use syntax::ast::Sign::{self, Plus, Minus}; @@ -33,8 +29,8 @@ pub enum FloatWidth { impl From for FloatWidth { fn from(ty: FloatTy) -> FloatWidth { match ty { - TyF32 => Fw32, - TyF64 => Fw64, + FloatTy::TyF32 => FloatWidth::Fw32, + FloatTy::TyF64 => FloatWidth::Fw64, } } } @@ -107,6 +103,7 @@ impl PartialEq for Constant { lv == rv && (is_negative(lty) & (lv != 0)) == (is_negative(rty) & (rv != 0)) } (&Constant::Float(ref ls, lw), &Constant::Float(ref rs, rw)) => { + use self::FloatWidth::*; if match (lw, rw) { (FwAny, _) | (_, FwAny) | (Fw32, Fw32) | (Fw64, Fw64) => true, _ => false, @@ -149,6 +146,7 @@ impl PartialOrd for Constant { }) } (&Constant::Float(ref ls, lw), &Constant::Float(ref rs, rw)) => { + use self::FloatWidth::*; if match (lw, rw) { (FwAny, _) | (_, FwAny) | (Fw32, Fw32) | (Fw64, Fw64) => true, _ => false, @@ -261,76 +259,51 @@ impl fmt::Display for Constant { fn lit_to_constant(lit: &Lit_) -> Constant { match *lit { - 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), + Lit_::LitStr(ref is, style) => Constant::Str(is.to_string(), style), + Lit_::LitByte(b) => Constant::Byte(b), + Lit_::LitByteStr(ref s) => Constant::Binary(s.clone()), + Lit_::LitChar(c) => Constant::Char(c), + Lit_::LitInt(value, ty) => Constant::Int(value, ty), + Lit_::LitFloat(ref is, ty) => Constant::Float(is.to_string(), ty.into()), + Lit_::LitFloatUnsuffixed(ref is) => Constant::Float(is.to_string(), FloatWidth::FwAny), + Lit_::LitBool(b) => Constant::Bool(b), } } fn constant_not(o: Constant) -> Option { - Some(match o { - Constant::Bool(b) => Constant::Bool(!b), - Constant::Int(value, ty) => { - let (nvalue, nty) = match ty { - SignedIntLit(ity, Plus) => { - if value == ::std::u64::MAX { - return None; - } - (value + 1, SignedIntLit(ity, Minus)) - } - SignedIntLit(ity, Minus) => { - if value == 0 { - (1, SignedIntLit(ity, Minus)) - } else { - (value - 1, SignedIntLit(ity, Plus)) - } - } - UnsignedIntLit(ity) => { - let mask = match ity { - UintTy::TyU8 => ::std::u8::MAX as u64, - 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 - }; - (!value & mask, UnsignedIntLit(ity)) - } - UnsuffixedIntLit(_) => { + use syntax::ast::LitIntType::*; + use self::Constant::*; + match o { + Bool(b) => Some(Bool(!b)), + Int(::std::u64::MAX, SignedIntLit(_, Plus)) => None, + Int(value, SignedIntLit(ity, Plus)) => Some(Int(value + 1, SignedIntLit(ity, Minus))), + Int(0, SignedIntLit(ity, Minus)) => Some(Int(1, SignedIntLit(ity, Minus))), + Int(value, SignedIntLit(ity, Minus)) => Some(Int(value - 1, SignedIntLit(ity, Plus))), + Int(value, UnsignedIntLit(ity)) => { + let mask = match ity { + UintTy::TyU8 => ::std::u8::MAX as u64, + 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 }; - Constant::Int(nvalue, nty) - } - _ => { - return None; - } - }) + Some(Int(!value & mask, UnsignedIntLit(ity))) + }, + _ => None, + } } fn constant_negate(o: Constant) -> Option { - Some(match o { - Constant::Int(value, ty) => { - Constant::Int(value, - match ty { - SignedIntLit(ity, sign) => SignedIntLit(ity, neg_sign(sign)), - UnsuffixedIntLit(sign) => UnsuffixedIntLit(neg_sign(sign)), - _ => { - return None; - } - }) - } - Constant::Float(is, ty) => Constant::Float(neg_float_str(is), ty), - _ => { - return None; - } - }) + use syntax::ast::LitIntType::*; + use self::Constant::*; + match o { + Int(value, SignedIntLit(ity, sign)) => Some(Int(value, SignedIntLit(ity, neg_sign(sign)))), + Int(value, UnsuffixedIntLit(sign)) => Some(Int(value, UnsuffixedIntLit(neg_sign(sign)))), + Float(is, ty) => Some(Float(neg_float_str(is), ty)), + _ => None, + } } fn neg_sign(s: Sign) -> Sign { @@ -357,12 +330,13 @@ fn neg_float_str(s: String) -> String { /// ``` pub fn is_negative(ty: LitIntType) -> bool { match ty { - SignedIntLit(_, sign) | UnsuffixedIntLit(sign) => sign == Minus, - UnsignedIntLit(_) => false, + LitIntType::SignedIntLit(_, sign) | LitIntType::UnsuffixedIntLit(sign) => sign == Minus, + LitIntType::UnsignedIntLit(_) => false, } } fn unify_int_type(l: LitIntType, r: LitIntType, s: Sign) -> Option { + use syntax::ast::LitIntType::*; match (l, r) { (SignedIntLit(lty, _), SignedIntLit(rty, _)) => { if lty == rty { diff --git a/src/enum_glob_use.rs b/src/enum_glob_use.rs new file mode 100644 index 00000000000..d6df307fec2 --- /dev/null +++ b/src/enum_glob_use.rs @@ -0,0 +1,61 @@ +//! lint on `use`ing all variants of an enum + +use rustc::lint::{LateLintPass, LintPass, LateContext, LintArray, LintContext}; +use rustc_front::hir::*; +use rustc::front::map::Node::NodeItem; +use rustc::front::map::PathElem::PathName; +use rustc::middle::ty::TyEnum; +use utils::span_lint; +use syntax::codemap::Span; +use syntax::ast::NodeId; + +/// **What it does:** Warns when `use`ing all variants of an enum +/// +/// **Why is this bad?** It is usually better style to use the prefixed name of an enum variant, rather than importing variants +/// +/// **Known problems:** Old-style enums that prefix the variants are still around +/// +/// **Example:** `use std::cmp::Ordering::*;` +declare_lint! { pub ENUM_GLOB_USE, Allow, + "finds use items that import all variants of an enum" } + +pub struct EnumGlobUse; + +impl LintPass for EnumGlobUse { + fn get_lints(&self) -> LintArray { + lint_array!(ENUM_GLOB_USE) + } +} + +impl LateLintPass for EnumGlobUse { + fn check_mod(&mut self, cx: &LateContext, m: &Mod, _: Span, _: NodeId) { + // only check top level `use` statements + for item in &m.item_ids { + self.lint_item(cx, cx.krate.item(item.id)); + } + } +} + +impl EnumGlobUse { + fn lint_item(&self, cx: &LateContext, item: &Item) { + if item.vis == Visibility::Public { + return; // re-exports are fine + } + if let ItemUse(ref item_use) = item.node { + if let ViewPath_::ViewPathGlob(_) = item_use.node { + let def = cx.tcx.def_map.borrow()[&item.id]; + if let Some(NodeItem(it)) = cx.tcx.map.get_if_local(def.def_id()) { + if let ItemEnum(..) = it.node { + span_lint(cx, ENUM_GLOB_USE, item.span, "don't use glob imports for enum variants"); + } + } else { + if let Some(&PathName(_)) = cx.sess().cstore.item_path(def.def_id()).last() { + if let TyEnum(..) = cx.sess().cstore.item_type(&cx.tcx, def.def_id()).ty.sty { + span_lint(cx, ENUM_GLOB_USE, item.span, "don't use glob imports for enum variants"); + } + } + } + } + } + } +} diff --git a/src/len_zero.rs b/src/len_zero.rs index 120e880cde6..ea0c873cb54 100644 --- a/src/len_zero.rs +++ b/src/len_zero.rs @@ -6,8 +6,7 @@ use syntax::codemap::{Span, Spanned}; use rustc::middle::def_id::DefId; use rustc::middle::ty::{self, MethodTraitItemId, ImplOrTraitItemId}; -use syntax::ast::Lit_::*; -use syntax::ast::Lit; +use syntax::ast::{Lit, Lit_}; use utils::{get_item_name, snippet, span_lint, walk_ptrs_ty}; @@ -152,7 +151,7 @@ fn check_cmp(cx: &LateContext, span: Span, left: &Expr, right: &Expr, op: &str) } fn check_len_zero(cx: &LateContext, span: Span, name: &Name, args: &[P], lit: &Lit, op: &str) { - if let Spanned{node: LitInt(0, _), ..} = *lit { + if let Spanned{node: Lit_::LitInt(0, _), ..} = *lit { if name.as_str() == "len" && args.len() == 1 && has_is_empty(cx, &args[0]) { span_lint(cx, LEN_ZERO, diff --git a/src/lib.rs b/src/lib.rs index 33de4d6fb79..fa98beb8d74 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -40,6 +40,7 @@ pub mod utils; pub mod consts; pub mod types; pub mod misc; +pub mod enum_glob_use; pub mod eq_op; pub mod bit_mask; pub mod ptr_arg; @@ -99,6 +100,7 @@ pub fn plugin_registrar(reg: &mut Registry) { 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 enum_glob_use::EnumGlobUse); 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); @@ -157,6 +159,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box regex::RegexPass); reg.register_lint_group("clippy_pedantic", vec![ + enum_glob_use::ENUM_GLOB_USE, matches::SINGLE_MATCH_ELSE, methods::OPTION_UNWRAP_USED, methods::RESULT_UNWRAP_USED, diff --git a/src/minmax.rs b/src/minmax.rs index 57ec9d91734..03e2d0a4ec3 100644 --- a/src/minmax.rs +++ b/src/minmax.rs @@ -1,8 +1,7 @@ use rustc::lint::*; use rustc_front::hir::*; use syntax::ptr::P; -use std::cmp::PartialOrd; -use std::cmp::Ordering::*; +use std::cmp::{PartialOrd, Ordering}; use consts::{Constant, constant_simple}; use utils::{match_def_path, span_lint}; @@ -37,7 +36,7 @@ impl LateLintPass for MinMaxPass { return; } match (outer_max, outer_c.partial_cmp(&inner_c)) { - (_, None) | (Max, Some(Less)) | (Min, Some(Greater)) => (), + (_, None) | (Max, Some(Ordering::Less)) | (Min, Some(Ordering::Greater)) => (), _ => { span_lint(cx, MIN_MAX, expr.span, "this min/max combination leads to constant result"); } diff --git a/src/needless_bool.rs b/src/needless_bool.rs index 52b3108f6fa..bfd819edcb6 100644 --- a/src/needless_bool.rs +++ b/src/needless_bool.rs @@ -5,7 +5,7 @@ use rustc::lint::*; use rustc_front::hir::*; -use syntax::ast::Lit_::*; +use syntax::ast::Lit_; use utils::{span_lint, snippet}; @@ -90,7 +90,7 @@ fn fetch_bool_expr(expr: &Expr) -> Option { match expr.node { ExprBlock(ref block) => fetch_bool_block(block), ExprLit(ref lit_ptr) => { - if let LitBool(value) = lit_ptr.node { + if let Lit_::LitBool(value) = lit_ptr.node { Some(value) } else { None diff --git a/src/types.rs b/src/types.rs index 498a084984f..7034b0d2ddb 100644 --- a/src/types.rs +++ b/src/types.rs @@ -5,9 +5,7 @@ use rustc_front::util::{is_comparison_binop, binop_to_string}; use syntax::codemap::Span; use rustc_front::intravisit::{FnKind, Visitor, walk_ty}; use rustc::middle::ty; -use syntax::ast::IntTy::*; -use syntax::ast::UintTy::*; -use syntax::ast::FloatTy::*; +use syntax::ast::{IntTy, UintTy, FloatTy}; use utils::*; @@ -236,7 +234,7 @@ fn int_ty_to_nbits(typ: &ty::TyS) -> usize { fn is_isize_or_usize(typ: &ty::TyS) -> bool { match typ.sty { - ty::TyInt(TyIs) | ty::TyUint(TyUs) => true, + ty::TyInt(IntTy::TyIs) | ty::TyUint(UintTy::TyUs) => true, _ => false, } } @@ -361,7 +359,7 @@ 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 { + let to_nbits = if let ty::TyFloat(FloatTy::TyF32) = cast_to.sty { 32 } else { 64 @@ -392,7 +390,7 @@ impl LateLintPass for CastPass { 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) { + if let (&ty::TyFloat(FloatTy::TyF64), &ty::TyFloat(FloatTy::TyF32)) = (&cast_from.sty, &cast_to.sty) { span_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span, diff --git a/src/unicode.rs b/src/unicode.rs index 63ffe219d9b..f363eace713 100644 --- a/src/unicode.rs +++ b/src/unicode.rs @@ -2,7 +2,7 @@ use rustc::lint::*; use rustc_front::hir::*; use syntax::codemap::Span; -use syntax::ast::Lit_::*; +use syntax::ast::Lit_; use unicode_normalization::UnicodeNormalization; @@ -59,7 +59,7 @@ impl LintPass for Unicode { impl LateLintPass for Unicode { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { if let ExprLit(ref lit) = expr.node { - if let LitStr(_, _) = lit.node { + if let Lit_::LitStr(_, _) = lit.node { check_str(cx, lit.span) } } diff --git a/src/utils.rs b/src/utils.rs index 8f542431d64..74c1de6976f 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,7 +1,7 @@ use consts::constant; use reexport::*; -use rustc::front::map::Node::*; -use rustc::lint::*; +use rustc::front::map::Node; +use rustc::lint::{LintContext, LateContext, Level, Lint}; use rustc::middle::def_id::DefId; use rustc::middle::{cstore, def, infer, ty, traits}; use rustc::session::Session; @@ -10,7 +10,7 @@ use std::borrow::Cow; use std::mem; use std::ops::{Deref, DerefMut}; use std::str::FromStr; -use syntax::ast::Lit_::*; +use syntax::ast::Lit_; use syntax::ast; use syntax::codemap::{ExpnInfo, Span, ExpnFormat}; use syntax::errors::DiagnosticBuilder; @@ -296,9 +296,9 @@ pub fn method_chain_args<'a>(expr: &'a Expr, methods: &[&str]) -> Option Option { let parent_id = cx.tcx.map.get_parent(expr.id); 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(Node::NodeItem(&Item{ ref name, .. })) | + Some(Node::NodeTraitItem(&TraitItem{ ref name, .. })) | + Some(Node::NodeImplItem(&ImplItem{ ref name, .. })) => Some(*name), _ => None, } } @@ -408,7 +408,7 @@ pub fn get_parent_expr<'c>(cx: &'c LateContext, e: &Expr) -> Option<&'c Expr> { return None; } map.find(parent_id).and_then(|node| { - if let NodeExpr(parent) = node { + if let Node::NodeExpr(parent) = node { Some(parent) } else { None @@ -422,8 +422,8 @@ pub fn get_enclosing_block<'c>(cx: &'c LateContext, node: NodeId) -> Option<&'c .and_then(|enclosing_id| map.find(enclosing_id)); if let Some(node) = enclosing_node { match node { - NodeBlock(ref block) => Some(block), - NodeItem(&Item{ node: ItemFn(_, _, _, _, _, ref block), .. }) => Some(block), + Node::NodeBlock(ref block) => Some(block), + Node::NodeItem(&Item{ node: ItemFn(_, _, _, _, _, ref block), .. }) => Some(block), _ => None, } } else { @@ -529,7 +529,7 @@ pub fn walk_ptrs_ty_depth(ty: ty::Ty) -> (ty::Ty, usize) { 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 { + if let Lit_::LitInt(v, _) = spanned.node { return v == value; } } @@ -575,7 +575,7 @@ fn parse_attrs(sess: &Session, attrs: &[ast::Attribute], name: &' } if let ast::MetaNameValue(ref key, ref value) = attr.value.node { if *key == name { - if let LitStr(ref s, _) = value.node { + if let Lit_::LitStr(ref s, _) = value.node { if let Ok(value) = FromStr::from_str(s) { f(value) } else { diff --git a/tests/compile-fail/enum_glob_use.rs b/tests/compile-fail/enum_glob_use.rs new file mode 100644 index 00000000000..fc5f531ba90 --- /dev/null +++ b/tests/compile-fail/enum_glob_use.rs @@ -0,0 +1,20 @@ +#![feature(plugin)] +#![plugin(clippy)] +#![deny(clippy, clippy_pedantic)] +#![allow(unused_imports, dead_code)] + +use std::cmp::Ordering::*; //~ ERROR: don't use glob imports for enum variants + +enum Enum {} + +use self::Enum::*; //~ ERROR: don't use glob imports for enum variants + +fn blarg() { + use self::Enum::*; // ok, just for a function +} + +mod blurg { + pub use std::cmp::Ordering::*; // ok, re-export +} + +fn main() {}