Lint for type repetition in trait bounds.

This lint adds warning if types are redundantly repeated in trait bounds i.e. `T: Copy, T: Clone` instead of `T: Copy + Clone`. This is a late pass trait lint and has necessitated the addition of code to allow hashing of TyKinds without taking into account Span information.

Author: xd009642

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,

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

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 => {},
+        }
+    }
 }

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

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
+