From c90f42a5cf857b7191092a3be4d77f3a558b4aeb Mon Sep 17 00:00:00 2001 From: ibraheemdev Date: Mon, 16 Aug 2021 17:15:28 -0400 Subject: [PATCH 1/9] uplift `trait_duplication_in_bounds` from clippy to rustc --- compiler/rustc_lint/src/builtin.rs | 98 +++++++++++++++++++ compiler/rustc_lint/src/lib.rs | 1 + .../ui/lint}/trait_duplication_in_bounds.rs | 4 +- .../lint}/trait_duplication_in_bounds.stderr | 4 +- src/tools/clippy/clippy_lints/src/lib.rs | 2 - .../clippy/clippy_lints/src/trait_bounds.rs | 88 +---------------- 6 files changed, 106 insertions(+), 91 deletions(-) rename src/{tools/clippy/tests/ui => test/ui/lint}/trait_duplication_in_bounds.rs (71%) rename src/{tools/clippy/tests/ui => test/ui/lint}/trait_duplication_in_bounds.stderr (86%) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 77c7040e6a76d..1c48ff66fb324 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -3140,3 +3140,101 @@ impl<'tcx> LateLintPass<'tcx> for DerefNullPtr { } } } + +declare_lint! { + /// ### what it does + /// checks for cases where generics are being used and multiple + /// syntax specifications for trait bounds are used simultaneously. + /// + /// ### why is this bad? + /// duplicate bounds makes the code + /// less readable than specifing them only once. + /// + /// ### example + /// ```rust + /// fn func(arg: t) where t: clone + default {} + /// ``` + /// + /// could be written as: + /// + /// ```rust + /// fn func(arg: T) {} + /// ``` + /// or + /// + /// ```rust + /// fn func(arg: T) where T: Clone + Default {} + /// ``` + pub TRAIT_DUPLICATION_IN_BOUNDS, + Warn, + "Check if the same trait bounds are specified twice during a function declaration" +} + +declare_lint_pass!(TraitDuplicationInBounds => [TRAIT_DUPLICATION_IN_BOUNDS]); + +impl<'tcx> LateLintPass<'tcx> for TraitDuplicationInBounds { + fn check_generics(&mut self, cx: &LateContext<'tcx>, gen: &'tcx hir::Generics<'_>) { + fn get_trait_res_span_from_bound(bound: &hir::GenericBound<'_>) -> Option<(Res, Span)> { + if let hir::GenericBound::Trait(t, _) = bound { + Some((t.trait_ref.path.res, t.span)) + } else { + None + } + } + if gen.span.from_expansion() + || gen.params.is_empty() + || gen.where_clause.predicates.is_empty() + { + return; + } + + let mut map = FxHashMap::default(); + for param in gen.params { + if let hir::ParamName::Plain(ref ident) = param.name { + let res = param + .bounds + .iter() + .filter_map(get_trait_res_span_from_bound) + .collect::>(); + map.insert(*ident, res); + } + } + + for predicate in gen.where_clause.predicates { + if let hir::WherePredicate::BoundPredicate(ref bound_predicate) = predicate { + if !bound_predicate.span.from_expansion() { + if let hir::TyKind::Path(hir::QPath::Resolved(_, hir::Path { segments, .. })) = + bound_predicate.bounded_ty.kind + { + if let Some(segment) = segments.first() { + if let Some(trait_resolutions_direct) = map.get(&segment.ident) { + for (res_where, _) in bound_predicate + .bounds + .iter() + .filter_map(get_trait_res_span_from_bound) + { + if let Some((_, span_direct)) = trait_resolutions_direct + .iter() + .find(|(res_direct, _)| *res_direct == res_where) + { + cx.struct_span_lint( + TRAIT_DUPLICATION_IN_BOUNDS, + *span_direct, + |lint| { + lint.build( + "this trait bound is already specified in the where clause" + ) + .help("consider removing this trait bound") + .emit() + }, + ); + } + } + } + } + } + } + } + } + } +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 79f850a781bd8..2c12c2dfe1d72 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -197,6 +197,7 @@ macro_rules! late_lint_mod_passes { // Depends on referenced function signatures in expressions MutableTransmutes: MutableTransmutes, TypeAliasBounds: TypeAliasBounds, + TraitDuplicationInBounds: TraitDuplicationInBounds, TrivialConstraints: TrivialConstraints, TypeLimits: TypeLimits::new(), NonSnakeCase: NonSnakeCase, diff --git a/src/tools/clippy/tests/ui/trait_duplication_in_bounds.rs b/src/test/ui/lint/trait_duplication_in_bounds.rs similarity index 71% rename from src/tools/clippy/tests/ui/trait_duplication_in_bounds.rs rename to src/test/ui/lint/trait_duplication_in_bounds.rs index cb2b0054e352b..457b2ef574271 100644 --- a/src/tools/clippy/tests/ui/trait_duplication_in_bounds.rs +++ b/src/test/ui/lint/trait_duplication_in_bounds.rs @@ -1,8 +1,10 @@ -#![deny(clippy::trait_duplication_in_bounds)] +#![deny(trait_duplication_in_bounds)] use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign}; fn bad_foo(arg0: T, arg1: Z) +//~^ ERROR this trait bound is already specified in the where clause +//~| ERROR this trait bound is already specified in the where clause where T: Clone, T: Default, diff --git a/src/tools/clippy/tests/ui/trait_duplication_in_bounds.stderr b/src/test/ui/lint/trait_duplication_in_bounds.stderr similarity index 86% rename from src/tools/clippy/tests/ui/trait_duplication_in_bounds.stderr rename to src/test/ui/lint/trait_duplication_in_bounds.stderr index 027e1c7520412..5a64fab21ebec 100644 --- a/src/tools/clippy/tests/ui/trait_duplication_in_bounds.stderr +++ b/src/test/ui/lint/trait_duplication_in_bounds.stderr @@ -7,8 +7,8 @@ LL | fn bad_foo(arg0: T, arg1: Z) note: the lint level is defined here --> $DIR/trait_duplication_in_bounds.rs:1:9 | -LL | #![deny(clippy::trait_duplication_in_bounds)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(trait_duplication_in_bounds)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ = help: consider removing this trait bound error: this trait bound is already specified in the where clause diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index 6f73a00d1f782..4cac9a6dd6e2b 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -925,7 +925,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: temporary_assignment::TEMPORARY_ASSIGNMENT, to_digit_is_some::TO_DIGIT_IS_SOME, to_string_in_display::TO_STRING_IN_DISPLAY, - trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS, trait_bounds::TYPE_REPETITION_IN_BOUNDS, transmute::CROSSPOINTER_TRANSMUTE, transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS, @@ -1133,7 +1132,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED), LintId::of(shadow::SHADOW_UNRELATED), LintId::of(strings::STRING_ADD_ASSIGN), - LintId::of(trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS), LintId::of(trait_bounds::TYPE_REPETITION_IN_BOUNDS), LintId::of(transmute::TRANSMUTE_PTR_TO_PTR), LintId::of(types::LINKEDLIST), diff --git a/src/tools/clippy/clippy_lints/src/trait_bounds.rs b/src/tools/clippy/clippy_lints/src/trait_bounds.rs index 79367c4230c2a..91fe47437f6e6 100644 --- a/src/tools/clippy/clippy_lints/src/trait_bounds.rs +++ b/src/tools/clippy/clippy_lints/src/trait_bounds.rs @@ -2,13 +2,11 @@ use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::{in_macro, SpanlessHash}; use if_chain::if_chain; -use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::unhash::UnhashMap; use rustc_errors::Applicability; -use rustc_hir::{def::Res, GenericBound, Generics, ParamName, Path, QPath, TyKind, WherePredicate}; +use rustc_hir::{GenericBound, Generics, WherePredicate}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::Span; declare_clippy_lint! { /// ### What it does @@ -33,35 +31,6 @@ declare_clippy_lint! { "Types are repeated unnecessary in trait bounds use `+` instead of using `T: _, T: _`" } -declare_clippy_lint! { - /// ### What it does - /// Checks for cases where generics are being used and multiple - /// syntax specifications for trait bounds are used simultaneously. - /// - /// ### Why is this bad? - /// Duplicate bounds makes the code - /// less readable than specifing them only once. - /// - /// ### Example - /// ```rust - /// fn func(arg: T) where T: Clone + Default {} - /// ``` - /// - /// Could be written as: - /// - /// ```rust - /// fn func(arg: T) {} - /// ``` - /// or - /// - /// ```rust - /// fn func(arg: T) where T: Clone + Default {} - /// ``` - pub TRAIT_DUPLICATION_IN_BOUNDS, - pedantic, - "Check if the same trait bounds are specified twice during a function declaration" -} - #[derive(Copy, Clone)] pub struct TraitBounds { max_trait_bounds: u64, @@ -74,20 +43,11 @@ impl TraitBounds { } } -impl_lint_pass!(TraitBounds => [TYPE_REPETITION_IN_BOUNDS, TRAIT_DUPLICATION_IN_BOUNDS]); +impl_lint_pass!(TraitBounds => [TYPE_REPETITION_IN_BOUNDS]); impl<'tcx> LateLintPass<'tcx> for TraitBounds { fn check_generics(&mut self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) { self.check_type_repetition(cx, gen); - check_trait_bound_duplication(cx, gen); - } -} - -fn get_trait_res_span_from_bound(bound: &GenericBound<'_>) -> Option<(Res, Span)> { - if let GenericBound::Trait(t, _) = bound { - Some((t.trait_ref.path.res, t.span)) - } else { - None } } @@ -149,47 +109,3 @@ impl TraitBounds { } } } - -fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) { - if in_macro(gen.span) || gen.params.is_empty() || gen.where_clause.predicates.is_empty() { - return; - } - - let mut map = FxHashMap::default(); - for param in gen.params { - if let ParamName::Plain(ref ident) = param.name { - let res = param - .bounds - .iter() - .filter_map(get_trait_res_span_from_bound) - .collect::>(); - map.insert(*ident, res); - } - } - - for predicate in gen.where_clause.predicates { - if_chain! { - if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate; - if !in_macro(bound_predicate.span); - if let TyKind::Path(QPath::Resolved(_, Path { segments, .. })) = bound_predicate.bounded_ty.kind; - if let Some(segment) = segments.first(); - if let Some(trait_resolutions_direct) = map.get(&segment.ident); - then { - for (res_where, _) in bound_predicate.bounds.iter().filter_map(get_trait_res_span_from_bound) { - if let Some((_, span_direct)) = trait_resolutions_direct - .iter() - .find(|(res_direct, _)| *res_direct == res_where) { - span_lint_and_help( - cx, - TRAIT_DUPLICATION_IN_BOUNDS, - *span_direct, - "this trait bound is already specified in the where clause", - None, - "consider removing this trait bound", - ); - } - } - } - } - } -} From 9bc25262f819bc9581a47a27ac0167a6dfe5ef9e Mon Sep 17 00:00:00 2001 From: ibraheemdev Date: Mon, 16 Aug 2021 17:21:00 -0400 Subject: [PATCH 2/9] `trait_duplication_in_bounds`: warn on duplicate trait bounds of the same syntax --- compiler/rustc_lint/src/builtin.rs | 108 +++++++++++------- library/core/src/iter/traits/iterator.rs | 1 + library/proc_macro/src/lib.rs | 1 + .../ui/lint/trait_duplication_in_bounds.rs | 40 +++++-- .../lint/trait_duplication_in_bounds.stderr | 82 +++++++++++-- 5 files changed, 168 insertions(+), 64 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 1c48ff66fb324..3cfb7a4d34d9c 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -57,6 +57,7 @@ use rustc_trait_selection::traits::misc::can_type_implement_copy; use crate::nonstandard_style::{method_context, MethodLateContext}; use std::fmt::Write; +use std::hash::{Hash, Hasher}; use tracing::{debug, trace}; // hardwired lints from librustc_middle @@ -3174,61 +3175,82 @@ declare_lint_pass!(TraitDuplicationInBounds => [TRAIT_DUPLICATION_IN_BOUNDS]); impl<'tcx> LateLintPass<'tcx> for TraitDuplicationInBounds { fn check_generics(&mut self, cx: &LateContext<'tcx>, gen: &'tcx hir::Generics<'_>) { - fn get_trait_res_span_from_bound(bound: &hir::GenericBound<'_>) -> Option<(Res, Span)> { - if let hir::GenericBound::Trait(t, _) = bound { - Some((t.trait_ref.path.res, t.span)) - } else { - None + struct TraitRes { + res: Res, + span: Span, + } + + impl TraitRes { + fn from_bound(bound: &hir::GenericBound<'_>) -> Option { + if let hir::GenericBound::Trait(t, _) = bound { + Some(Self { res: t.trait_ref.path.res, span: t.span }) + } else { + None + } } } - if gen.span.from_expansion() - || gen.params.is_empty() - || gen.where_clause.predicates.is_empty() - { + + impl PartialEq for TraitRes { + fn eq(&self, other: &Self) -> bool { + self.res == other.res + } + } + + impl Hash for TraitRes { + fn hash(&self, state: &mut H) { + self.res.hash(state) + } + } + + impl Eq for TraitRes {} + + if gen.span.from_expansion() { return; } - let mut map = FxHashMap::default(); + let mut trait_resolutions = FxHashMap::default(); for param in gen.params { if let hir::ParamName::Plain(ref ident) = param.name { - let res = param - .bounds - .iter() - .filter_map(get_trait_res_span_from_bound) - .collect::>(); - map.insert(*ident, res); + let mut uniq = FxHashSet::default(); + + for res in param.bounds.iter().filter_map(TraitRes::from_bound) { + let span = res.span.clone(); + if !uniq.insert(res) { + cx.struct_span_lint(TRAIT_DUPLICATION_IN_BOUNDS, span, |lint| { + lint.build("this trait bound has already been specified") + .help("consider removing this trait bound") + .emit() + }); + } + } + + trait_resolutions.insert(*ident, uniq); } } for predicate in gen.where_clause.predicates { if let hir::WherePredicate::BoundPredicate(ref bound_predicate) = predicate { - if !bound_predicate.span.from_expansion() { - if let hir::TyKind::Path(hir::QPath::Resolved(_, hir::Path { segments, .. })) = - bound_predicate.bounded_ty.kind - { - if let Some(segment) = segments.first() { - if let Some(trait_resolutions_direct) = map.get(&segment.ident) { - for (res_where, _) in bound_predicate - .bounds - .iter() - .filter_map(get_trait_res_span_from_bound) - { - if let Some((_, span_direct)) = trait_resolutions_direct - .iter() - .find(|(res_direct, _)| *res_direct == res_where) - { - cx.struct_span_lint( - TRAIT_DUPLICATION_IN_BOUNDS, - *span_direct, - |lint| { - lint.build( - "this trait bound is already specified in the where clause" - ) - .help("consider removing this trait bound") - .emit() - }, - ); - } + if let hir::TyKind::Path(hir::QPath::Resolved(_, hir::Path { segments, .. })) = + bound_predicate.bounded_ty.kind + { + if let Some(segment) = segments.first() { + if let Some(trait_resolutions) = trait_resolutions.get_mut(&segment.ident) { + for res in + bound_predicate.bounds.iter().filter_map(TraitRes::from_bound) + { + let span = res.span.clone(); + if !trait_resolutions.insert(res) { + cx.struct_span_lint( + TRAIT_DUPLICATION_IN_BOUNDS, + span, + |lint| { + lint.build( + "this trait bound has already been specified", + ) + .help("consider removing this trait bound") + .emit() + }, + ); } } } diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index 524d8f857e2a5..18ff8aa472e0e 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -2434,6 +2434,7 @@ pub trait Iterator { /// assert!(result.is_err()); /// ``` #[inline] + #[cfg_attr(not(bootstrap), allow(trait_duplication_in_bounds))] #[unstable(feature = "try_find", reason = "new API", issue = "63178")] fn try_find(&mut self, f: F) -> Result, E> where diff --git a/library/proc_macro/src/lib.rs b/library/proc_macro/src/lib.rs index f25e257bf3168..7c22c4e4dbe65 100644 --- a/library/proc_macro/src/lib.rs +++ b/library/proc_macro/src/lib.rs @@ -1233,6 +1233,7 @@ pub mod tracked_env { /// Besides the dependency tracking this function should be equivalent to `env::var` from the /// standard library, except that the argument must be UTF-8. #[unstable(feature = "proc_macro_tracked_env", issue = "74690")] + #[cfg_attr(not(bootstrap), allow(trait_duplication_in_bounds))] pub fn var + AsRef>(key: K) -> Result { let key: &str = key.as_ref(); let value = env::var(key); diff --git a/src/test/ui/lint/trait_duplication_in_bounds.rs b/src/test/ui/lint/trait_duplication_in_bounds.rs index 457b2ef574271..9c3fcb91625c2 100644 --- a/src/test/ui/lint/trait_duplication_in_bounds.rs +++ b/src/test/ui/lint/trait_duplication_in_bounds.rs @@ -1,31 +1,47 @@ #![deny(trait_duplication_in_bounds)] -use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign}; - -fn bad_foo(arg0: T, arg1: Z) -//~^ ERROR this trait bound is already specified in the where clause -//~| ERROR this trait bound is already specified in the where clause +trait DupDirectAndWhere {} +fn dup_direct_and_where(t: T) where - T: Clone, - T: Default, + T: DupDirectAndWhere, + //~^ ERROR this trait bound has already been specified + T: DupDirectAndWhere, + //~^ ERROR this trait bound has already been specified { unimplemented!(); } -fn good_bar(arg: T) { +trait DupDirect {} +fn dup_direct(t: T) { + //~^ ERROR this trait bound has already been specified unimplemented!(); } -fn good_foo(arg: T) +trait DupWhere {} +fn dup_where(t: T) where - T: Clone + Default, + T: DupWhere + DupWhere, + //~^ ERROR this trait bound has already been specified { unimplemented!(); } -fn good_foobar(arg: T) +trait NotDup {} +fn not_dup((t, u): (T, U)) { + unimplemented!(); +} + +trait Everything {} +fn everything((t, u): (T, U)) +//~^ ERROR this trait bound has already been specified +//~| ERROR this trait bound has already been specified where - T: Clone, + T: Everything + Everything + Everything, + //~^ ERROR this trait bound has already been specified + //~| ERROR this trait bound has already been specified + //~| ERROR this trait bound has already been specified + U: Everything, + //~^ ERROR this trait bound has already been specified { unimplemented!(); } diff --git a/src/test/ui/lint/trait_duplication_in_bounds.stderr b/src/test/ui/lint/trait_duplication_in_bounds.stderr index 5a64fab21ebec..945b3c74f91ee 100644 --- a/src/test/ui/lint/trait_duplication_in_bounds.stderr +++ b/src/test/ui/lint/trait_duplication_in_bounds.stderr @@ -1,8 +1,8 @@ -error: this trait bound is already specified in the where clause - --> $DIR/trait_duplication_in_bounds.rs:5:15 +error: this trait bound has already been specified + --> $DIR/trait_duplication_in_bounds.rs:6:8 | -LL | fn bad_foo(arg0: T, arg1: Z) - | ^^^^^ +LL | T: DupDirectAndWhere, + | ^^^^^^^^^^^^^^^^^ | note: the lint level is defined here --> $DIR/trait_duplication_in_bounds.rs:1:9 @@ -11,13 +11,77 @@ LL | #![deny(trait_duplication_in_bounds)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ = help: consider removing this trait bound -error: this trait bound is already specified in the where clause - --> $DIR/trait_duplication_in_bounds.rs:5:23 +error: this trait bound has already been specified + --> $DIR/trait_duplication_in_bounds.rs:8:8 | -LL | fn bad_foo(arg0: T, arg1: Z) - | ^^^^^^^ +LL | T: DupDirectAndWhere, + | ^^^^^^^^^^^^^^^^^ | = help: consider removing this trait bound -error: aborting due to 2 previous errors +error: this trait bound has already been specified + --> $DIR/trait_duplication_in_bounds.rs:15:30 + | +LL | fn dup_direct(t: T) { + | ^^^^^^^^^ + | + = help: consider removing this trait bound + +error: this trait bound has already been specified + --> $DIR/trait_duplication_in_bounds.rs:23:19 + | +LL | T: DupWhere + DupWhere, + | ^^^^^^^^ + | + = help: consider removing this trait bound + +error: this trait bound has already been specified + --> $DIR/trait_duplication_in_bounds.rs:35:31 + | +LL | fn everything((t, u): (T, U)) + | ^^^^^^^^^^ + | + = help: consider removing this trait bound + +error: this trait bound has already been specified + --> $DIR/trait_duplication_in_bounds.rs:35:59 + | +LL | fn everything((t, u): (T, U)) + | ^^^^^^^^^^ + | + = help: consider removing this trait bound + +error: this trait bound has already been specified + --> $DIR/trait_duplication_in_bounds.rs:39:8 + | +LL | T: Everything + Everything + Everything, + | ^^^^^^^^^^ + | + = help: consider removing this trait bound + +error: this trait bound has already been specified + --> $DIR/trait_duplication_in_bounds.rs:39:21 + | +LL | T: Everything + Everything + Everything, + | ^^^^^^^^^^ + | + = help: consider removing this trait bound + +error: this trait bound has already been specified + --> $DIR/trait_duplication_in_bounds.rs:39:34 + | +LL | T: Everything + Everything + Everything, + | ^^^^^^^^^^ + | + = help: consider removing this trait bound + +error: this trait bound has already been specified + --> $DIR/trait_duplication_in_bounds.rs:43:8 + | +LL | U: Everything, + | ^^^^^^^^^^ + | + = help: consider removing this trait bound + +error: aborting due to 10 previous errors From d56000881c97d432b701f13077f1aaee358f318e Mon Sep 17 00:00:00 2001 From: ibraheemdev Date: Mon, 16 Aug 2021 17:29:37 -0400 Subject: [PATCH 3/9] deprecate `clippy::trait_duplication_in_bounds` --- src/tools/clippy/clippy_lints/src/lib.rs | 1 + src/tools/clippy/tests/ui/deprecated.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index 4cac9a6dd6e2b..608383b214ac1 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -2180,6 +2180,7 @@ pub fn register_renamed(ls: &mut rustc_lint::LintStore) { ls.register_renamed("clippy::panic_params", "non_fmt_panics"); ls.register_renamed("clippy::unknown_clippy_lints", "unknown_lints"); ls.register_renamed("clippy::invalid_atomic_ordering", "invalid_atomic_ordering"); + ls.register_renamed("clippy::trait_duplication_in_bounds", "trait_duplication_in_bounds"); } // only exists to let the dogfood integration test works. diff --git a/src/tools/clippy/tests/ui/deprecated.rs b/src/tools/clippy/tests/ui/deprecated.rs index 1943d0092e624..b4c0b25026834 100644 --- a/src/tools/clippy/tests/ui/deprecated.rs +++ b/src/tools/clippy/tests/ui/deprecated.rs @@ -15,5 +15,6 @@ #[warn(clippy::pub_enum_variant_names)] #[warn(clippy::wrong_pub_self_convention)] #[warn(clippy::invalid_atomic_ordering)] +#[warn(clippy::trait_duplication_in_bounds)] fn main() {} From 46868a7861031afeff91b222eb0337b49eb23e3b Mon Sep 17 00:00:00 2001 From: ibraheemdev Date: Mon, 16 Aug 2021 17:39:46 -0400 Subject: [PATCH 4/9] `trait_duplication_in_bounds` => `duplicate_bounds` --- compiler/rustc_lint/src/builtin.rs | 36 ++++++++----------- compiler/rustc_lint/src/lib.rs | 2 +- library/core/src/iter/traits/iterator.rs | 2 +- library/proc_macro/src/lib.rs | 2 +- ...ation_in_bounds.rs => duplicate_bounds.rs} | 2 +- ..._bounds.stderr => duplicate_bounds.stderr} | 26 +++++++------- 6 files changed, 31 insertions(+), 39 deletions(-) rename src/test/ui/lint/{trait_duplication_in_bounds.rs => duplicate_bounds.rs} (97%) rename src/test/ui/lint/{trait_duplication_in_bounds.stderr => duplicate_bounds.stderr} (77%) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 3cfb7a4d34d9c..393bfa8263a14 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -3143,17 +3143,15 @@ impl<'tcx> LateLintPass<'tcx> for DerefNullPtr { } declare_lint! { - /// ### what it does - /// checks for cases where generics are being used and multiple - /// syntax specifications for trait bounds are used simultaneously. + /// ### What it does + /// Checks for cases where the same trait bound is specified more than once. /// - /// ### why is this bad? - /// duplicate bounds makes the code - /// less readable than specifing them only once. + /// ### Why is this bad? + /// Duplicate bounds makes the code less readable than specifing them only once. /// - /// ### example + /// ### Example /// ```rust - /// fn func(arg: t) where t: clone + default {} + /// fn func(arg: T) where T: Clone + Default {} /// ``` /// /// could be written as: @@ -3166,14 +3164,14 @@ declare_lint! { /// ```rust /// fn func(arg: T) where T: Clone + Default {} /// ``` - pub TRAIT_DUPLICATION_IN_BOUNDS, + pub DUPLICATE_BOUNDS, Warn, - "Check if the same trait bounds are specified twice during a function declaration" + "Check if the same bounds is specified more than once" } -declare_lint_pass!(TraitDuplicationInBounds => [TRAIT_DUPLICATION_IN_BOUNDS]); +declare_lint_pass!(DuplicateBounds => [DUPLICATE_BOUNDS]); -impl<'tcx> LateLintPass<'tcx> for TraitDuplicationInBounds { +impl<'tcx> LateLintPass<'tcx> for DuplicateBounds { fn check_generics(&mut self, cx: &LateContext<'tcx>, gen: &'tcx hir::Generics<'_>) { struct TraitRes { res: Res, @@ -3216,7 +3214,7 @@ impl<'tcx> LateLintPass<'tcx> for TraitDuplicationInBounds { for res in param.bounds.iter().filter_map(TraitRes::from_bound) { let span = res.span.clone(); if !uniq.insert(res) { - cx.struct_span_lint(TRAIT_DUPLICATION_IN_BOUNDS, span, |lint| { + cx.struct_span_lint(DUPLICATE_BOUNDS, span, |lint| { lint.build("this trait bound has already been specified") .help("consider removing this trait bound") .emit() @@ -3240,17 +3238,11 @@ impl<'tcx> LateLintPass<'tcx> for TraitDuplicationInBounds { { let span = res.span.clone(); if !trait_resolutions.insert(res) { - cx.struct_span_lint( - TRAIT_DUPLICATION_IN_BOUNDS, - span, - |lint| { - lint.build( - "this trait bound has already been specified", - ) + cx.struct_span_lint(DUPLICATE_BOUNDS, span, |lint| { + lint.build("this trait bound has already been specified") .help("consider removing this trait bound") .emit() - }, - ); + }); } } } diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 2c12c2dfe1d72..54a919d096bf2 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -197,7 +197,7 @@ macro_rules! late_lint_mod_passes { // Depends on referenced function signatures in expressions MutableTransmutes: MutableTransmutes, TypeAliasBounds: TypeAliasBounds, - TraitDuplicationInBounds: TraitDuplicationInBounds, + DuplicateBounds: DuplicateBounds, TrivialConstraints: TrivialConstraints, TypeLimits: TypeLimits::new(), NonSnakeCase: NonSnakeCase, diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index 18ff8aa472e0e..6269f79f6df3d 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -2434,7 +2434,7 @@ pub trait Iterator { /// assert!(result.is_err()); /// ``` #[inline] - #[cfg_attr(not(bootstrap), allow(trait_duplication_in_bounds))] + #[cfg_attr(not(bootstrap), allow(duplicate_bounds))] #[unstable(feature = "try_find", reason = "new API", issue = "63178")] fn try_find(&mut self, f: F) -> Result, E> where diff --git a/library/proc_macro/src/lib.rs b/library/proc_macro/src/lib.rs index 7c22c4e4dbe65..27d4ec539083b 100644 --- a/library/proc_macro/src/lib.rs +++ b/library/proc_macro/src/lib.rs @@ -1233,7 +1233,7 @@ pub mod tracked_env { /// Besides the dependency tracking this function should be equivalent to `env::var` from the /// standard library, except that the argument must be UTF-8. #[unstable(feature = "proc_macro_tracked_env", issue = "74690")] - #[cfg_attr(not(bootstrap), allow(trait_duplication_in_bounds))] + #[cfg_attr(not(bootstrap), allow(duplicate_bounds))] pub fn var + AsRef>(key: K) -> Result { let key: &str = key.as_ref(); let value = env::var(key); diff --git a/src/test/ui/lint/trait_duplication_in_bounds.rs b/src/test/ui/lint/duplicate_bounds.rs similarity index 97% rename from src/test/ui/lint/trait_duplication_in_bounds.rs rename to src/test/ui/lint/duplicate_bounds.rs index 9c3fcb91625c2..6b4f6a169c5e8 100644 --- a/src/test/ui/lint/trait_duplication_in_bounds.rs +++ b/src/test/ui/lint/duplicate_bounds.rs @@ -1,4 +1,4 @@ -#![deny(trait_duplication_in_bounds)] +#![deny(duplicate_bounds)] trait DupDirectAndWhere {} fn dup_direct_and_where(t: T) diff --git a/src/test/ui/lint/trait_duplication_in_bounds.stderr b/src/test/ui/lint/duplicate_bounds.stderr similarity index 77% rename from src/test/ui/lint/trait_duplication_in_bounds.stderr rename to src/test/ui/lint/duplicate_bounds.stderr index 945b3c74f91ee..ffd35403686f4 100644 --- a/src/test/ui/lint/trait_duplication_in_bounds.stderr +++ b/src/test/ui/lint/duplicate_bounds.stderr @@ -1,18 +1,18 @@ error: this trait bound has already been specified - --> $DIR/trait_duplication_in_bounds.rs:6:8 + --> $DIR/duplicate_bounds.rs:6:8 | LL | T: DupDirectAndWhere, | ^^^^^^^^^^^^^^^^^ | note: the lint level is defined here - --> $DIR/trait_duplication_in_bounds.rs:1:9 + --> $DIR/duplicate_bounds.rs:1:9 | -LL | #![deny(trait_duplication_in_bounds)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(duplicate_bounds)] + | ^^^^^^^^^^^^^^^^ = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/trait_duplication_in_bounds.rs:8:8 + --> $DIR/duplicate_bounds.rs:8:8 | LL | T: DupDirectAndWhere, | ^^^^^^^^^^^^^^^^^ @@ -20,7 +20,7 @@ LL | T: DupDirectAndWhere, = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/trait_duplication_in_bounds.rs:15:30 + --> $DIR/duplicate_bounds.rs:15:30 | LL | fn dup_direct(t: T) { | ^^^^^^^^^ @@ -28,7 +28,7 @@ LL | fn dup_direct(t: T) { = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/trait_duplication_in_bounds.rs:23:19 + --> $DIR/duplicate_bounds.rs:23:19 | LL | T: DupWhere + DupWhere, | ^^^^^^^^ @@ -36,7 +36,7 @@ LL | T: DupWhere + DupWhere, = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/trait_duplication_in_bounds.rs:35:31 + --> $DIR/duplicate_bounds.rs:35:31 | LL | fn everything((t, u): (T, U)) | ^^^^^^^^^^ @@ -44,7 +44,7 @@ LL | fn everything((t, u = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/trait_duplication_in_bounds.rs:35:59 + --> $DIR/duplicate_bounds.rs:35:59 | LL | fn everything((t, u): (T, U)) | ^^^^^^^^^^ @@ -52,7 +52,7 @@ LL | fn everything((t, u = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/trait_duplication_in_bounds.rs:39:8 + --> $DIR/duplicate_bounds.rs:39:8 | LL | T: Everything + Everything + Everything, | ^^^^^^^^^^ @@ -60,7 +60,7 @@ LL | T: Everything + Everything + Everything, = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/trait_duplication_in_bounds.rs:39:21 + --> $DIR/duplicate_bounds.rs:39:21 | LL | T: Everything + Everything + Everything, | ^^^^^^^^^^ @@ -68,7 +68,7 @@ LL | T: Everything + Everything + Everything, = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/trait_duplication_in_bounds.rs:39:34 + --> $DIR/duplicate_bounds.rs:39:34 | LL | T: Everything + Everything + Everything, | ^^^^^^^^^^ @@ -76,7 +76,7 @@ LL | T: Everything + Everything + Everything, = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/trait_duplication_in_bounds.rs:43:8 + --> $DIR/duplicate_bounds.rs:43:8 | LL | U: Everything, | ^^^^^^^^^^ From ab0a96211355a986ed8a57e313862104d2a2443d Mon Sep 17 00:00:00 2001 From: ibraheemdev Date: Mon, 16 Aug 2021 18:06:50 -0400 Subject: [PATCH 5/9] extend `duplicate_bounds` to track lifetimes --- compiler/rustc_lint/src/builtin.rs | 77 +++++++++++++++--------- src/test/ui/lint/duplicate_bounds.rs | 13 ++++ src/test/ui/lint/duplicate_bounds.stderr | 38 +++++++++--- 3 files changed, 94 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 393bfa8263a14..a6a4616702048 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -3173,56 +3173,76 @@ declare_lint_pass!(DuplicateBounds => [DUPLICATE_BOUNDS]); impl<'tcx> LateLintPass<'tcx> for DuplicateBounds { fn check_generics(&mut self, cx: &LateContext<'tcx>, gen: &'tcx hir::Generics<'_>) { - struct TraitRes { - res: Res, + struct Bound { + kind: BoundKind, span: Span, } - impl TraitRes { - fn from_bound(bound: &hir::GenericBound<'_>) -> Option { - if let hir::GenericBound::Trait(t, _) = bound { - Some(Self { res: t.trait_ref.path.res, span: t.span }) - } else { - None + #[derive(Hash, PartialEq, Eq)] + enum BoundKind { + Trait(Res), + Lifetime(hir::LifetimeName), + } + + impl BoundKind { + fn as_str(&self) -> &'static str { + match self { + BoundKind::Trait(_) => "trait", + BoundKind::Lifetime(_) => "lifetime", + } + } + } + + impl Bound { + fn from_generic(bound: &hir::GenericBound<'_>) -> Option { + match bound { + hir::GenericBound::Trait(t, _) => { + Some(Self { kind: BoundKind::Trait(t.trait_ref.path.res), span: t.span }) + } + hir::GenericBound::Outlives(lifetime) => { + Some(Self { kind: BoundKind::Lifetime(lifetime.name), span: lifetime.span }) + } + _ => None, } } } - impl PartialEq for TraitRes { + impl PartialEq for Bound { fn eq(&self, other: &Self) -> bool { - self.res == other.res + self.kind == other.kind } } - impl Hash for TraitRes { + impl Hash for Bound { fn hash(&self, state: &mut H) { - self.res.hash(state) + self.kind.hash(state) } } - impl Eq for TraitRes {} + impl Eq for Bound {} if gen.span.from_expansion() { return; } - let mut trait_resolutions = FxHashMap::default(); + let mut bounds = FxHashMap::default(); for param in gen.params { if let hir::ParamName::Plain(ref ident) = param.name { - let mut uniq = FxHashSet::default(); + let mut uniq_bounds = FxHashSet::default(); - for res in param.bounds.iter().filter_map(TraitRes::from_bound) { + for res in param.bounds.iter().filter_map(Bound::from_generic) { let span = res.span.clone(); - if !uniq.insert(res) { + let kind = res.kind.as_str(); + if !uniq_bounds.insert(res) { cx.struct_span_lint(DUPLICATE_BOUNDS, span, |lint| { - lint.build("this trait bound has already been specified") - .help("consider removing this trait bound") + lint.build(&format!("this {} bound has already been specified", kind)) + .help(&format!("consider removing this {} bound", kind)) .emit() }); } } - trait_resolutions.insert(*ident, uniq); + bounds.insert(*ident, uniq_bounds); } } @@ -3232,16 +3252,19 @@ impl<'tcx> LateLintPass<'tcx> for DuplicateBounds { bound_predicate.bounded_ty.kind { if let Some(segment) = segments.first() { - if let Some(trait_resolutions) = trait_resolutions.get_mut(&segment.ident) { - for res in - bound_predicate.bounds.iter().filter_map(TraitRes::from_bound) + if let Some(bounds) = bounds.get_mut(&segment.ident) { + for res in bound_predicate.bounds.iter().filter_map(Bound::from_generic) { let span = res.span.clone(); - if !trait_resolutions.insert(res) { + let kind = res.kind.as_str(); + if !bounds.insert(res) { cx.struct_span_lint(DUPLICATE_BOUNDS, span, |lint| { - lint.build("this trait bound has already been specified") - .help("consider removing this trait bound") - .emit() + lint.build(&format!( + "this {} bound has already been specified", + kind + )) + .help(&format!("consider removing this {} bound", kind)) + .emit() }); } } diff --git a/src/test/ui/lint/duplicate_bounds.rs b/src/test/ui/lint/duplicate_bounds.rs index 6b4f6a169c5e8..85b1c2d5803fc 100644 --- a/src/test/ui/lint/duplicate_bounds.rs +++ b/src/test/ui/lint/duplicate_bounds.rs @@ -31,6 +31,19 @@ fn not_dup((t, u): (T, U)) { unimplemented!(); } +fn dup_lifetimes<'a, 'b: 'a + 'a>() {} +//~^ ERROR this lifetime bound has already been specified + +fn dup_lifetimes_generic<'a, T: 'a + 'a>() {} +//~^ ERROR this lifetime bound has already been specified + +fn dup_lifetimes_where() +where + T: 'static, + //~^ ERROR this lifetime bound has already been specified +{ +} + trait Everything {} fn everything((t, u): (T, U)) //~^ ERROR this trait bound has already been specified diff --git a/src/test/ui/lint/duplicate_bounds.stderr b/src/test/ui/lint/duplicate_bounds.stderr index ffd35403686f4..6cf4441c5b0c2 100644 --- a/src/test/ui/lint/duplicate_bounds.stderr +++ b/src/test/ui/lint/duplicate_bounds.stderr @@ -35,8 +35,32 @@ LL | T: DupWhere + DupWhere, | = help: consider removing this trait bound +error: this lifetime bound has already been specified + --> $DIR/duplicate_bounds.rs:34:31 + | +LL | fn dup_lifetimes<'a, 'b: 'a + 'a>() {} + | ^^ + | + = help: consider removing this lifetime bound + +error: this lifetime bound has already been specified + --> $DIR/duplicate_bounds.rs:37:38 + | +LL | fn dup_lifetimes_generic<'a, T: 'a + 'a>() {} + | ^^ + | + = help: consider removing this lifetime bound + +error: this lifetime bound has already been specified + --> $DIR/duplicate_bounds.rs:42:8 + | +LL | T: 'static, + | ^^^^^^^ + | + = help: consider removing this lifetime bound + error: this trait bound has already been specified - --> $DIR/duplicate_bounds.rs:35:31 + --> $DIR/duplicate_bounds.rs:48:31 | LL | fn everything((t, u): (T, U)) | ^^^^^^^^^^ @@ -44,7 +68,7 @@ LL | fn everything((t, u = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/duplicate_bounds.rs:35:59 + --> $DIR/duplicate_bounds.rs:48:59 | LL | fn everything((t, u): (T, U)) | ^^^^^^^^^^ @@ -52,7 +76,7 @@ LL | fn everything((t, u = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/duplicate_bounds.rs:39:8 + --> $DIR/duplicate_bounds.rs:52:8 | LL | T: Everything + Everything + Everything, | ^^^^^^^^^^ @@ -60,7 +84,7 @@ LL | T: Everything + Everything + Everything, = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/duplicate_bounds.rs:39:21 + --> $DIR/duplicate_bounds.rs:52:21 | LL | T: Everything + Everything + Everything, | ^^^^^^^^^^ @@ -68,7 +92,7 @@ LL | T: Everything + Everything + Everything, = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/duplicate_bounds.rs:39:34 + --> $DIR/duplicate_bounds.rs:52:34 | LL | T: Everything + Everything + Everything, | ^^^^^^^^^^ @@ -76,12 +100,12 @@ LL | T: Everything + Everything + Everything, = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/duplicate_bounds.rs:43:8 + --> $DIR/duplicate_bounds.rs:56:8 | LL | U: Everything, | ^^^^^^^^^^ | = help: consider removing this trait bound -error: aborting due to 10 previous errors +error: aborting due to 13 previous errors From 9e7c714631b07ab6ec8569b732b5be829d27ec33 Mon Sep 17 00:00:00 2001 From: ibraheemdev Date: Mon, 16 Aug 2021 18:39:38 -0400 Subject: [PATCH 6/9] extend `duplicate_bounds` to handle region where predicates --- compiler/rustc_lint/src/builtin.rs | 58 ++++++++++++------- src/test/ui/lint/duplicate_bounds.rs | 30 ++++++++-- src/test/ui/lint/duplicate_bounds.stderr | 74 +++++++++++++++++++----- 3 files changed, 123 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index a6a4616702048..9d3d3f8568137 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -3247,27 +3247,43 @@ impl<'tcx> LateLintPass<'tcx> for DuplicateBounds { } for predicate in gen.where_clause.predicates { - if let hir::WherePredicate::BoundPredicate(ref bound_predicate) = predicate { - if let hir::TyKind::Path(hir::QPath::Resolved(_, hir::Path { segments, .. })) = - bound_predicate.bounded_ty.kind - { - if let Some(segment) = segments.first() { - if let Some(bounds) = bounds.get_mut(&segment.ident) { - for res in bound_predicate.bounds.iter().filter_map(Bound::from_generic) - { - let span = res.span.clone(); - let kind = res.kind.as_str(); - if !bounds.insert(res) { - cx.struct_span_lint(DUPLICATE_BOUNDS, span, |lint| { - lint.build(&format!( - "this {} bound has already been specified", - kind - )) - .help(&format!("consider removing this {} bound", kind)) - .emit() - }); - } - } + let res = match &predicate { + hir::WherePredicate::BoundPredicate(hir::WhereBoundPredicate { + bounded_ty: + hir::Ty { + kind: + hir::TyKind::Path(hir::QPath::Resolved(_, hir::Path { segments, .. })), + .. + }, + bounds, + .. + }) => segments.first().map(|s| (s.ident, *bounds)), + hir::WherePredicate::RegionPredicate(hir::WhereRegionPredicate { + lifetime: + hir::Lifetime { + name: hir::LifetimeName::Param(hir::ParamName::Plain(ident)), + .. + }, + bounds, + .. + }) => Some((*ident, *bounds)), + _ => None, + }; + + if let Some((ident, where_predicate_bounds)) = res { + if let Some(bounds) = bounds.get_mut(&ident) { + for res in where_predicate_bounds.iter().filter_map(Bound::from_generic) { + let span = res.span.clone(); + let kind = res.kind.as_str(); + if !bounds.insert(res) { + cx.struct_span_lint(DUPLICATE_BOUNDS, span, |lint| { + lint.build(&format!( + "this {} bound has already been specified", + kind + )) + .help(&format!("consider removing this {} bound", kind)) + .emit() + }); } } } diff --git a/src/test/ui/lint/duplicate_bounds.rs b/src/test/ui/lint/duplicate_bounds.rs index 85b1c2d5803fc..58157896ead7f 100644 --- a/src/test/ui/lint/duplicate_bounds.rs +++ b/src/test/ui/lint/duplicate_bounds.rs @@ -31,15 +31,18 @@ fn not_dup((t, u): (T, U)) { unimplemented!(); } -fn dup_lifetimes<'a, 'b: 'a + 'a>() {} +fn dup_lifetimes<'a, 'b: 'a + 'a>() //~^ ERROR this lifetime bound has already been specified +where + 'b: 'a, + //~^ ERROR this lifetime bound has already been specified +{ +} -fn dup_lifetimes_generic<'a, T: 'a + 'a>() {} +fn dup_lifetimes_generic<'a, T: 'a + 'a>() //~^ ERROR this lifetime bound has already been specified - -fn dup_lifetimes_where() where - T: 'static, + T: 'a, //~^ ERROR this lifetime bound has already been specified { } @@ -59,4 +62,21 @@ where unimplemented!(); } +trait DupStructBound {} +struct DupStruct(T) +//~^ ERROR this trait bound has already been specified +where + T: DupStructBound; +//~^ ERROR this trait bound has already been specified + +impl<'a, T: 'a + DupStructBound + DupStructBound> DupStruct +//~^ ERROR this trait bound has already been specified +where + T: 'a + DupStructBound, + //~^ ERROR this lifetime bound has already been specified + //~| ERROR this trait bound has already been specified +{ + fn _x() {} +} + fn main() {} diff --git a/src/test/ui/lint/duplicate_bounds.stderr b/src/test/ui/lint/duplicate_bounds.stderr index 6cf4441c5b0c2..3cd7704b98f8e 100644 --- a/src/test/ui/lint/duplicate_bounds.stderr +++ b/src/test/ui/lint/duplicate_bounds.stderr @@ -38,29 +38,37 @@ LL | T: DupWhere + DupWhere, error: this lifetime bound has already been specified --> $DIR/duplicate_bounds.rs:34:31 | -LL | fn dup_lifetimes<'a, 'b: 'a + 'a>() {} +LL | fn dup_lifetimes<'a, 'b: 'a + 'a>() | ^^ | = help: consider removing this lifetime bound error: this lifetime bound has already been specified - --> $DIR/duplicate_bounds.rs:37:38 + --> $DIR/duplicate_bounds.rs:37:9 | -LL | fn dup_lifetimes_generic<'a, T: 'a + 'a>() {} +LL | 'b: 'a, + | ^^ + | + = help: consider removing this lifetime bound + +error: this lifetime bound has already been specified + --> $DIR/duplicate_bounds.rs:42:38 + | +LL | fn dup_lifetimes_generic<'a, T: 'a + 'a>() | ^^ | = help: consider removing this lifetime bound error: this lifetime bound has already been specified - --> $DIR/duplicate_bounds.rs:42:8 + --> $DIR/duplicate_bounds.rs:45:8 | -LL | T: 'static, - | ^^^^^^^ +LL | T: 'a, + | ^^ | = help: consider removing this lifetime bound error: this trait bound has already been specified - --> $DIR/duplicate_bounds.rs:48:31 + --> $DIR/duplicate_bounds.rs:51:31 | LL | fn everything((t, u): (T, U)) | ^^^^^^^^^^ @@ -68,7 +76,7 @@ LL | fn everything((t, u = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/duplicate_bounds.rs:48:59 + --> $DIR/duplicate_bounds.rs:51:59 | LL | fn everything((t, u): (T, U)) | ^^^^^^^^^^ @@ -76,7 +84,7 @@ LL | fn everything((t, u = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/duplicate_bounds.rs:52:8 + --> $DIR/duplicate_bounds.rs:55:8 | LL | T: Everything + Everything + Everything, | ^^^^^^^^^^ @@ -84,7 +92,7 @@ LL | T: Everything + Everything + Everything, = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/duplicate_bounds.rs:52:21 + --> $DIR/duplicate_bounds.rs:55:21 | LL | T: Everything + Everything + Everything, | ^^^^^^^^^^ @@ -92,7 +100,7 @@ LL | T: Everything + Everything + Everything, = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/duplicate_bounds.rs:52:34 + --> $DIR/duplicate_bounds.rs:55:34 | LL | T: Everything + Everything + Everything, | ^^^^^^^^^^ @@ -100,12 +108,52 @@ LL | T: Everything + Everything + Everything, = help: consider removing this trait bound error: this trait bound has already been specified - --> $DIR/duplicate_bounds.rs:56:8 + --> $DIR/duplicate_bounds.rs:59:8 | LL | U: Everything, | ^^^^^^^^^^ | = help: consider removing this trait bound -error: aborting due to 13 previous errors +error: this trait bound has already been specified + --> $DIR/duplicate_bounds.rs:66:38 + | +LL | struct DupStruct(T) + | ^^^^^^^^^^^^^^ + | + = help: consider removing this trait bound + +error: this trait bound has already been specified + --> $DIR/duplicate_bounds.rs:69:8 + | +LL | T: DupStructBound; + | ^^^^^^^^^^^^^^ + | + = help: consider removing this trait bound + +error: this trait bound has already been specified + --> $DIR/duplicate_bounds.rs:72:35 + | +LL | impl<'a, T: 'a + DupStructBound + DupStructBound> DupStruct + | ^^^^^^^^^^^^^^ + | + = help: consider removing this trait bound + +error: this lifetime bound has already been specified + --> $DIR/duplicate_bounds.rs:75:8 + | +LL | T: 'a + DupStructBound, + | ^^ + | + = help: consider removing this lifetime bound + +error: this trait bound has already been specified + --> $DIR/duplicate_bounds.rs:75:13 + | +LL | T: 'a + DupStructBound, + | ^^^^^^^^^^^^^^ + | + = help: consider removing this trait bound + +error: aborting due to 19 previous errors From c416b99f2c7bdfa6da77af90213a3c652caf0bbc Mon Sep 17 00:00:00 2001 From: ibraheemdev Date: Mon, 16 Aug 2021 22:08:24 -0400 Subject: [PATCH 7/9] add `clippy::trait_duplication_in_bounds` to deprecated output --- compiler/rustc_lint/src/builtin.rs | 4 ++-- src/tools/clippy/tests/ui/deprecated.stderr | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 9d3d3f8568137..110a345788a4a 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -3144,7 +3144,7 @@ impl<'tcx> LateLintPass<'tcx> for DerefNullPtr { declare_lint! { /// ### What it does - /// Checks for cases where the same trait bound is specified more than once. + /// Checks for cases where the same trait or lifetime bound is specified more than once. /// /// ### Why is this bad? /// Duplicate bounds makes the code less readable than specifing them only once. @@ -3166,7 +3166,7 @@ declare_lint! { /// ``` pub DUPLICATE_BOUNDS, Warn, - "Check if the same bounds is specified more than once" + "Check if the same bound is specified more than once" } declare_lint_pass!(DuplicateBounds => [DUPLICATE_BOUNDS]); diff --git a/src/tools/clippy/tests/ui/deprecated.stderr b/src/tools/clippy/tests/ui/deprecated.stderr index 51048e45c0677..4aa3efe247ec6 100644 --- a/src/tools/clippy/tests/ui/deprecated.stderr +++ b/src/tools/clippy/tests/ui/deprecated.stderr @@ -102,5 +102,11 @@ error: lint `clippy::invalid_atomic_ordering` has been renamed to `invalid_atomi LL | #[warn(clippy::invalid_atomic_ordering)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `invalid_atomic_ordering` +error: lint `clippy::trait_duplication_in_bounds` has been renamed to `duplicate_bounds` + --> $DIR/deprecated.rs:17:8 + | +LL | #[warn(clippy::trait_duplication_in_bounds)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `duplicate_bounds` + error: aborting due to 17 previous errors From e64482613d980bb4adbcdaef73ebe4196544e550 Mon Sep 17 00:00:00 2001 From: ibraheemdev Date: Mon, 16 Aug 2021 22:25:22 -0400 Subject: [PATCH 8/9] remove duplicate trait bound in rustc_data_structures --- compiler/rustc_data_structures/src/transitive_relation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_data_structures/src/transitive_relation.rs b/compiler/rustc_data_structures/src/transitive_relation.rs index ccf8bd69ebd06..76d6a9c2f88a4 100644 --- a/compiler/rustc_data_structures/src/transitive_relation.rs +++ b/compiler/rustc_data_structures/src/transitive_relation.rs @@ -77,7 +77,7 @@ impl TransitiveRelation { pub fn maybe_map(&self, mut f: F) -> Option> where F: FnMut(&T) -> Option, - U: Clone + Debug + Eq + Hash + Clone, + U: Clone + Debug + Eq + Hash, { let mut result = TransitiveRelation::default(); for edge in &self.edges { From ccc2b181c752d28cf3d69efabde3b1c140cd0111 Mon Sep 17 00:00:00 2001 From: ibraheemdev Date: Mon, 16 Aug 2021 22:34:36 -0400 Subject: [PATCH 9/9] remove duplicate trait bound in `rustc_ast::walk_variant` --- compiler/rustc_ast/src/visit.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index a377763983a4b..979c1d0ebbd60 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -350,10 +350,7 @@ pub fn walk_enum_def<'a, V: Visitor<'a>>( walk_list!(visitor, visit_variant, &enum_definition.variants); } -pub fn walk_variant<'a, V: Visitor<'a>>(visitor: &mut V, variant: &'a Variant) -where - V: Visitor<'a>, -{ +pub fn walk_variant<'a, V: Visitor<'a>>(visitor: &mut V, variant: &'a Variant) { visitor.visit_ident(variant.ident); visitor.visit_vis(&variant.vis); visitor.visit_variant_data(&variant.data);