Merge branch 'pr-581'

Conflicts:
	README.md
This commit is contained in:
Manish Goregaokar 2016-02-07 17:40:48 +05:30
commit 9ba5d45509
13 changed files with 157 additions and 104 deletions

View File

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

View File

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

View File

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

View File

@ -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<FloatTy> 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<Constant> {
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<Constant> {
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<LitIntType> {
use syntax::ast::LitIntType::*;
match (l, r) {
(SignedIntLit(lty, _), SignedIntLit(rty, _)) => {
if lty == rty {

61
src/enum_glob_use.rs Normal file
View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@ -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<Vec<&'a
pub fn get_item_name(cx: &LateContext, expr: &Expr) -> Option<Name> {
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<F: FnMut(u64)>(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 {

View File

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