diff options
| -rw-r--r-- | clippy_lints/src/lib.rs | 3 | ||||
| -rw-r--r-- | clippy_lints/src/trait_bounds.rs | 63 | ||||
| -rw-r--r-- | clippy_lints/src/utils/hir_utils.rs | 119 | ||||
| -rw-r--r-- | tests/ui/type_repetition_in_bounds.rs | 14 | ||||
| -rw-r--r-- | tests/ui/type_repetition_in_bounds.stderr | 15 |
5 files changed, 210 insertions, 4 deletions
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 6d90d315f00..faffb1a67f1 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -265,6 +265,7 @@ pub mod swap; pub mod temporary_assignment; pub mod transmute; pub mod transmuting_null; +pub mod trait_bounds; pub mod trivially_copy_pass_by_ref; pub mod try_err; pub mod types; @@ -588,6 +589,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box checked_conversions::CheckedConversions); reg.register_late_lint_pass(box integer_division::IntegerDivision); reg.register_late_lint_pass(box inherent_to_string::InherentToString); + reg.register_late_lint_pass(box trait_bounds::TraitBounds); reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![ arithmetic::FLOAT_ARITHMETIC, @@ -868,6 +870,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { transmute::USELESS_TRANSMUTE, transmute::WRONG_TRANSMUTE, transmuting_null::TRANSMUTING_NULL, + trait_bounds::TYPE_REPETITION_IN_BOUNDS, trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF, try_err::TRY_ERR, types::ABSURD_EXTREME_COMPARISONS, diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs new file mode 100644 index 00000000000..1f4c6850760 --- /dev/null +++ b/clippy_lints/src/trait_bounds.rs @@ -0,0 +1,63 @@ +use crate::utils::{in_macro, span_help_and_lint, SpanlessHash}; +use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; +use rustc::{declare_tool_lint, lint_array}; +use rustc_data_structures::fx::FxHashMap; +use rustc::hir::*; + +declare_clippy_lint! { + pub TYPE_REPETITION_IN_BOUNDS, + complexity, + "Types are repeated unnecessary in trait bounds use `+` instead of using `T: _, T: _`" +} + +#[derive(Copy, Clone)] +pub struct TraitBounds; + +impl LintPass for TraitBounds { + fn get_lints(&self) -> LintArray { + lint_array!(TYPE_REPETITION_IN_BOUNDS) + } + + fn name(&self) -> &'static str { + "TypeRepetitionInBounds" + } +} + + + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TraitBounds { + fn check_generics(&mut self, cx: &LateContext<'a, 'tcx>, gen: &'tcx Generics) { + if in_macro(gen.span) { + return; + } + let hash = | ty | -> u64 { + let mut hasher = SpanlessHash::new(cx, cx.tables); + hasher.hash_ty(ty); + hasher.finish() + }; + let mut map = FxHashMap::default(); + for bound in &gen.where_clause.predicates { + if let WherePredicate::BoundPredicate(ref p) = bound { + let h = hash(&p.bounded_ty); + if let Some(ref v) = map.insert(h, p.bounds) { + let mut hint_string = format!("consider combining the bounds: `{:?}: ", p.bounded_ty); + for &b in v.iter() { + hint_string.push_str(&format!("{:?}, ", b)); + } + for &b in p.bounds.iter() { + hint_string.push_str(&format!("{:?}, ", b)); + } + hint_string.truncate(hint_string.len() - 2); + hint_string.push('`'); + span_help_and_lint( + cx, + TYPE_REPETITION_IN_BOUNDS, + p.span, + "this type has already been used as a bound predicate", + &hint_string, + ); + } + } + } + } +} diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index e9b5cee430e..929c15a6ee5 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -3,7 +3,7 @@ use crate::utils::differing_macro_contexts; use rustc::hir::ptr::P; use rustc::hir::*; use rustc::lint::LateContext; -use rustc::ty::TypeckTables; +use rustc::ty::{Ty, TypeckTables}; use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; use syntax::ast::Name; @@ -438,9 +438,11 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_expr(fun); self.hash_exprs(args); }, - ExprKind::Cast(ref e, ref _ty) | ExprKind::Type(ref e, ref _ty) => { + ExprKind::Cast(ref e, ref ty) => { + let c: fn(_, _) -> _ = ExprKind::Cast; + c.hash(&mut self.s); self.hash_expr(e); - // TODO: _ty + self.hash_ty(ty); }, ExprKind::Closure(cap, _, eid, _, _) => { match cap { @@ -512,9 +514,22 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_expr(e); } }, - ExprKind::Tup(ref v) | ExprKind::Array(ref v) => { + ExprKind::Tup(ref tup) => { + let c: fn(_) -> _ = ExprKind::Tup; + c.hash(&mut self.s); + self.hash_exprs(tup); + }, + ExprKind::Array(ref v) => { + let c: fn(_) -> _ = ExprKind::Array; + c.hash(&mut self.s); self.hash_exprs(v); }, + ExprKind::Type(ref e, ref ty) => { + let c: fn(_, _) -> _ = ExprKind::Type; + c.hash(&mut self.s); + self.hash_expr(e); + self.hash_ty(ty); + }, ExprKind::Unary(lop, ref le) => { lop.hash(&mut self.s); self.hash_expr(le); @@ -574,4 +589,100 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { }, } } + + pub fn hash_lifetime(&mut self, lifetime: &Lifetime) { + if let LifetimeName::Param(ref name) = lifetime.name { + match name { + ParamName::Plain(ref ident) => { + ident.name.hash(&mut self.s); + }, + ParamName::Fresh(ref size) => { + size.hash(&mut self.s); + }, + _ => {}, + } + } + } + + pub fn hash_ty(&mut self, ty: &Ty) { + std::mem::discriminant(&ty.node).hash(&mut self.s); + match ty.node { + Ty::Slice(ty) => { + self.hash_ty(ty); + }, + Ty::Array(ty, anon_const) => { + self.hash_ty(ty); + self.hash_expr(&self.cx.tcx.hir().body(anon_const.body).value); + }, + Ty::Ptr(mut_ty) => { + self.hash_ty(&mut_ty.ty); + mut_ty.mutbl.hash(&mut self.s); + }, + Ty::Rptr(lifetime, mut_ty) => { + self.hash_lifetime(lifetime); + self.hash_ty(&mut_ty.ty); + mut_ty.mutbl.hash(&mut self.s); + }, + Ty::BareFn(bfn) => { + bfn.unsafety.hash(&mut self.s); + bfn.abi.hash(&mut self.s); + for arg in &bfn.decl.inputs { + self.hash_ty(&arg); + } + match bfn.decl.output { + FunctionRetTy::DefaultReturn(_) => { + ().hash(&mut self.s); + }, + FunctionRetTy::Return(ref ty) => { + self.hash_ty(ty); + }, + } + bfn.decl.c_variadic.hash(&mut self.s); + }, + Ty::Tup(ty_list) => { + for ty in ty_list { + self.hash_ty(ty); + } + + }, + Ty::Path(qpath) => { + match qpath { + QPath::Resolved(ref maybe_ty, ref path) => { + if let Some(ref ty) = maybe_ty { + self.hash_ty(ty); + } + for segment in &path.segments { + segment.ident.name.hash(&mut self.s); + } + }, + QPath::TypeRelative(ref ty, ref segment) => { + self.hash_ty(ty); + segment.ident.name.hash(&mut self.s); + }, + } + }, + Ty::Def(_, arg_list) => { + for arg in arg_list { + match arg { + GenericArg::Lifetime(ref l) => self.hash_lifetime(l), + GenericArg::Type(ref ty) => self.hash_ty(ty), + GenericArg::Const(ref ca) => { + self.hash_expr(&self.cx.tcx.hir().body(ca.value.body).value); + }, + } + } + }, + Ty::TraitObject(_, lifetime) => { + self.hash_lifetime(lifetime); + + }, + Ty::Typeof(anon_const) => { + self.hash_expr(&self.cx.tcx.hir().body(anon_const.body).value); + }, + Ty::CVarArgs(lifetime) => { + self.hash_lifetime(lifetime); + }, + Ty::Err | Ty::Infer | Ty::Never => {}, + } + } } diff --git a/tests/ui/type_repetition_in_bounds.rs b/tests/ui/type_repetition_in_bounds.rs new file mode 100644 index 00000000000..3aa0d0da561 --- /dev/null +++ b/tests/ui/type_repetition_in_bounds.rs @@ -0,0 +1,14 @@ +#[deny(clippy::type_repetition_in_bounds)] + +pub fn foo<T>(_t: T) where T: Copy, T: Clone { + unimplemented!(); +} + +pub fn bar<T, U>(_t: T, _u: U) where T: Copy, U: Clone { + unimplemented!(); +} + + +fn main() { + +} diff --git a/tests/ui/type_repetition_in_bounds.stderr b/tests/ui/type_repetition_in_bounds.stderr new file mode 100644 index 00000000000..1b49c69533c --- /dev/null +++ b/tests/ui/type_repetition_in_bounds.stderr @@ -0,0 +1,15 @@ +error: this type has already been used as a bound predicate + --> $DIR/type_repetition_in_bounds.rs:3:37 + | +LL | pub fn foo<T>(_t: T) where T: Copy, T: Clone { + | ^^^^^^^^ + | +note: lint level defined here + --> $DIR/type_repetition_in_bounds.rs:1:8 + | +LL | #[deny(clippy::type_repetition_in_bounds)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: consider combining the bounds: `T: Copy + Clone` + +error: aborting due to previous error + |