diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d31a597b66ac..38f8d007c72f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -996,7 +996,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box checked_conversions::CheckedConversions); store.register_late_pass(|| box integer_division::IntegerDivision); store.register_late_pass(|| box inherent_to_string::InherentToString); - store.register_late_pass(|| box trait_bounds::TraitBounds); + let max_trait_bounds = conf.max_trait_bounds; + store.register_late_pass(move || box trait_bounds::TraitBounds::new(max_trait_bounds)); store.register_late_pass(|| box comparison_chain::ComparisonChain); store.register_late_pass(|| box mut_key::MutableKeyType); store.register_late_pass(|| box modulo_arithmetic::ModuloArithmetic); @@ -1033,7 +1034,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: let array_size_threshold = conf.array_size_threshold; store.register_late_pass(move || box large_stack_arrays::LargeStackArrays::new(array_size_threshold)); store.register_late_pass(move || box large_const_arrays::LargeConstArrays::new(array_size_threshold)); - store.register_late_pass(move || box floating_point_arithmetic::FloatingPointArithmetic); + store.register_late_pass(|| box floating_point_arithmetic::FloatingPointArithmetic); store.register_early_pass(|| box as_conversions::AsConversions); store.register_early_pass(|| box utils::internal_lints::ProduceIce); store.register_late_pass(|| box let_underscore::LetUnderscore); diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 9eb2079c3ca2..0ef70311fb1c 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -1,19 +1,19 @@ use crate::utils::{in_macro, snippet, snippet_with_applicability, span_lint_and_help, SpanlessHash}; +use if_chain::if_chain; use rustc_data_structures::fx::FxHashMap; use rustc_errors::Applicability; use rustc_hir::{GenericBound, Generics, WherePredicate}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; -#[derive(Copy, Clone)] -pub struct TraitBounds; - declare_clippy_lint! { /// **What it does:** This lint warns about unnecessary type repetitions in trait bounds /// /// **Why is this bad?** Repeating the type for every bound makes the code /// less readable than combining the bounds /// + /// **Known problems:** None. + /// /// **Example:** /// ```rust /// pub fn foo(t: T) where T: Copy, T: Clone {} @@ -29,6 +29,18 @@ declare_clippy_lint! { "Types are repeated unnecessary in trait bounds use `+` instead of using `T: _, T: _`" } +#[derive(Copy, Clone)] +pub struct TraitBounds { + max_trait_bounds: u64, +} + +impl TraitBounds { + #[must_use] + pub fn new(max_trait_bounds: u64) -> Self { + Self { max_trait_bounds } + } +} + impl_lint_pass!(TraitBounds => [TYPE_REPETITION_IN_BOUNDS]); impl<'tcx> LateLintPass<'tcx> for TraitBounds { @@ -44,9 +56,14 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds { let mut map = FxHashMap::default(); let mut applicability = Applicability::MaybeIncorrect; for bound in gen.where_clause.predicates { - if let WherePredicate::BoundPredicate(ref p) = bound { + if_chain! { + if let WherePredicate::BoundPredicate(ref p) = bound; + if p.bounds.len() as u64 <= self.max_trait_bounds; + if !in_macro(p.span); let h = hash(&p.bounded_ty); - if let Some(ref v) = map.insert(h, p.bounds.iter().collect::>()) { + if let Some(ref v) = map.insert(h, p.bounds.iter().collect::>()); + + then { let mut hint_string = format!( "consider combining the bounds: `{}:", snippet(cx, p.bounded_ty.span, "_") diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index c41befbf147b..de425211e38e 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -156,6 +156,8 @@ define_Conf! { (array_size_threshold, "array_size_threshold": u64, 512_000), /// Lint: VEC_BOX. The size of the boxed type in bytes, where boxing in a `Vec` is allowed (vec_box_size_threshold, "vec_box_size_threshold": u64, 4096), + /// Lint: TYPE_REPETITION_IN_BOUNDS. The maximum number of bounds a trait can have to be linted + (max_trait_bounds, "max_trait_bounds": u64, 3), /// Lint: STRUCT_EXCESSIVE_BOOLS. The maximum number of bools a struct can have (max_struct_bools, "max_struct_bools": u64, 3), /// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bools function parameters can have diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index ae58f0a1521e..34341594c198 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -703,6 +703,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } for segment in path.segments { segment.ident.name.hash(&mut self.s); + self.hash_generic_args(segment.generic_args().args); } }, QPath::TypeRelative(ref ty, ref segment) => { @@ -711,13 +712,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { }, }, TyKind::OpaqueDef(_, 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_body(ca.value.body), - } - } + self.hash_generic_args(arg_list); }, TyKind::TraitObject(_, lifetime) => { self.hash_lifetime(lifetime); @@ -735,4 +730,14 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_expr(&self.cx.tcx.hir().body(body_id).value); self.maybe_typeck_tables = old_maybe_typeck_tables; } + + fn hash_generic_args(&mut self, arg_list: &[GenericArg<'_>]) { + 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_body(ca.value.body), + } + } + } } diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 53970af41079..6fbba01416a8 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -1,4 +1,4 @@ -error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `third-party` at line 5 column 1 +error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `third-party` at line 5 column 1 error: aborting due to previous error diff --git a/tests/ui/type_repetition_in_bounds.rs b/tests/ui/type_repetition_in_bounds.rs index 8b538be762b0..766190f20997 100644 --- a/tests/ui/type_repetition_in_bounds.rs +++ b/tests/ui/type_repetition_in_bounds.rs @@ -1,4 +1,6 @@ -#[deny(clippy::type_repetition_in_bounds)] +#![deny(clippy::type_repetition_in_bounds)] + +use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign}; pub fn foo(_t: T) where @@ -16,4 +18,55 @@ where unimplemented!(); } +// Threshold test (see #4380) +trait LintBounds +where + Self: Clone, + Self: Copy + Default + Ord, + Self: Add + AddAssign + Sub + SubAssign, + Self: Mul + MulAssign + Div + DivAssign, +{ +} + +trait LotsOfBounds +where + Self: Clone + Copy + Default + Ord, + Self: Add + AddAssign + Sub + SubAssign, + Self: Mul + MulAssign + Div + DivAssign, +{ +} + +// Generic distinction (see #4323) +mod issue4323 { + pub struct Foo(A); + pub struct Bar { + a: Foo, + b: Foo, + } + + impl Unpin for Bar + where + Foo: Unpin, + Foo: Unpin, + { + } +} + +// Extern macros shouldn't lint (see #4326) +extern crate serde; +mod issue4326 { + use serde::{Deserialize, Serialize}; + + trait Foo {} + impl Foo for String {} + + #[derive(Debug, Serialize, Deserialize)] + struct Bar + where + S: Foo, + { + foo: S, + } +} + fn main() {} diff --git a/tests/ui/type_repetition_in_bounds.stderr b/tests/ui/type_repetition_in_bounds.stderr index 4264e2e10bf1..148c19c7d070 100644 --- a/tests/ui/type_repetition_in_bounds.stderr +++ b/tests/ui/type_repetition_in_bounds.stderr @@ -1,15 +1,23 @@ error: this type has already been used as a bound predicate - --> $DIR/type_repetition_in_bounds.rs:6:5 + --> $DIR/type_repetition_in_bounds.rs:8:5 | LL | T: Clone, | ^^^^^^^^ | note: the lint level is defined here - --> $DIR/type_repetition_in_bounds.rs:1:8 + --> $DIR/type_repetition_in_bounds.rs:1:9 | -LL | #[deny(clippy::type_repetition_in_bounds)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(clippy::type_repetition_in_bounds)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = help: consider combining the bounds: `T: Copy + Clone` -error: aborting due to previous error +error: this type has already been used as a bound predicate + --> $DIR/type_repetition_in_bounds.rs:25:5 + | +LL | Self: Copy + Default + Ord, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider combining the bounds: `Self: Clone + Copy + Default + Ord` + +error: aborting due to 2 previous errors