diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index a985346b3c05..872544db6457 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -634,6 +634,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio * [`match_like_matches_macro`](https://rust-lang.github.io/rust-clippy/master/index.html#match_like_matches_macro) * [`mem_replace_with_default`](https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_default) * [`missing_const_for_fn`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn) +* [`multiple_bound_locations`](https://rust-lang.github.io/rust-clippy/master/index.html#multiple_bound_locations) * [`needless_borrow`](https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow) * [`option_as_ref_deref`](https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref) * [`option_map_unwrap_or`](https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unwrap_or) diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index a453386154d9..8d968968a5df 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -502,7 +502,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::module_style::MOD_MODULE_FILES_INFO, crate::module_style::SELF_NAMED_MODULE_FILES_INFO, crate::multi_assignments::MULTI_ASSIGNMENTS_INFO, - crate::multiple_bound_locations::MULTIPLE_BOUND_LOCATIONS_INFO, crate::multiple_unsafe_ops_per_block::MULTIPLE_UNSAFE_OPS_PER_BLOCK_INFO, crate::mut_key::MUTABLE_KEY_TYPE_INFO, crate::mut_mut::MUT_MUT_INFO, @@ -669,6 +668,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::to_digit_is_some::TO_DIGIT_IS_SOME_INFO, crate::to_string_trait_impl::TO_STRING_TRAIT_IMPL_INFO, crate::trailing_empty_array::TRAILING_EMPTY_ARRAY_INFO, + crate::trait_bounds::MULTIPLE_BOUND_LOCATIONS_INFO, crate::trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS_INFO, crate::trait_bounds::TYPE_REPETITION_IN_BOUNDS_INFO, crate::transmute::CROSSPOINTER_TRANSMUTE_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 76e759683145..a9f9c636607f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -231,7 +231,6 @@ mod missing_trait_methods; mod mixed_read_write_in_expression; mod module_style; mod multi_assignments; -mod multiple_bound_locations; mod multiple_unsafe_ops_per_block; mod mut_key; mod mut_mut; @@ -1117,7 +1116,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { }); store.register_late_pass(move |_| Box::new(incompatible_msrv::IncompatibleMsrv::new(msrv()))); store.register_late_pass(|_| Box::new(to_string_trait_impl::ToStringTraitImpl)); - store.register_early_pass(|| Box::new(multiple_bound_locations::MultipleBoundLocations)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/multiple_bound_locations.rs b/clippy_lints/src/multiple_bound_locations.rs deleted file mode 100644 index 191b32408efe..000000000000 --- a/clippy_lints/src/multiple_bound_locations.rs +++ /dev/null @@ -1,84 +0,0 @@ -use rustc_ast::visit::FnKind; -use rustc_ast::{NodeId, WherePredicate}; -use rustc_data_structures::fx::FxHashMap; -use rustc_lint::{EarlyContext, EarlyLintPass}; -use rustc_session::declare_lint_pass; -use rustc_span::Span; - -use clippy_utils::diagnostics::span_lint; -use clippy_utils::source::snippet_opt; - -declare_clippy_lint! { - /// ### What it does - /// Check if a generic is defined both in the bound predicate and in the `where` clause. - /// - /// ### Why is this bad? - /// It can be confusing for developers when seeing bounds for a generic in multiple places. - /// - /// ### Example - /// ```no_run - /// fn ty(a: F) - /// where - /// F: Sized, - /// {} - /// ``` - /// Use instead: - /// ```no_run - /// fn ty(a: F) - /// where - /// F: Sized + std::fmt::Debug, - /// {} - /// ``` - #[clippy::version = "1.77.0"] - pub MULTIPLE_BOUND_LOCATIONS, - suspicious, - "defining generic bounds in multiple locations" -} - -declare_lint_pass!(MultipleBoundLocations => [MULTIPLE_BOUND_LOCATIONS]); - -impl EarlyLintPass for MultipleBoundLocations { - fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, _: Span, _: NodeId) { - if let FnKind::Fn(_, _, _, _, generics, _) = kind - && !generics.params.is_empty() - && !generics.where_clause.predicates.is_empty() - { - let mut generic_params_with_bounds = FxHashMap::default(); - - for param in &generics.params { - if !param.bounds.is_empty() { - generic_params_with_bounds.insert(param.ident.name.as_str(), param.ident.span); - } - } - for clause in &generics.where_clause.predicates { - match clause { - WherePredicate::BoundPredicate(pred) => { - if (!pred.bound_generic_params.is_empty() || !pred.bounds.is_empty()) - && let Some(name) = snippet_opt(cx, pred.bounded_ty.span) - && let Some(bound_span) = generic_params_with_bounds.get(name.as_str()) - { - emit_lint(cx, *bound_span, pred.bounded_ty.span); - } - }, - WherePredicate::RegionPredicate(pred) => { - if !pred.bounds.is_empty() - && let Some(bound_span) = generic_params_with_bounds.get(&pred.lifetime.ident.name.as_str()) - { - emit_lint(cx, *bound_span, pred.lifetime.ident.span); - } - }, - WherePredicate::EqPredicate(_) => {}, - } - } - } - } -} - -fn emit_lint(cx: &EarlyContext<'_>, bound_span: Span, where_span: Span) { - span_lint( - cx, - MULTIPLE_BOUND_LOCATIONS, - vec![bound_span, where_span], - "bound is defined in more than one place", - ); -} diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 768623b5d034..9060b6fa3ed0 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -1,5 +1,5 @@ use clippy_config::msrvs::{self, Msrv}; -use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; +use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg}; use clippy_utils::source::{snippet, snippet_opt, snippet_with_applicability}; use clippy_utils::{is_from_proc_macro, SpanlessEq, SpanlessHash}; use core::hash::{Hash, Hasher}; @@ -86,6 +86,33 @@ declare_clippy_lint! { "check if the same trait bounds are specified more than once during a generic declaration" } +declare_clippy_lint! { + /// ### What it does + /// Check if a generic is defined both in the bound predicate and in the `where` clause. + /// + /// ### Why is this bad? + /// It can be confusing for developers when seeing bounds for a generic in multiple places. + /// + /// ### Example + /// ```no_run + /// fn ty(a: F) + /// where + /// F: Sized, + /// {} + /// ``` + /// Use instead: + /// ```no_run + /// fn ty(a: F) + /// where + /// F: Sized + std::fmt::Debug, + /// {} + /// ``` + #[clippy::version = "1.77.0"] + pub MULTIPLE_BOUND_LOCATIONS, + suspicious, + "defining generic bounds in multiple locations" +} + #[derive(Clone)] pub struct TraitBounds { max_trait_bounds: u64, @@ -99,10 +126,11 @@ impl TraitBounds { } } -impl_lint_pass!(TraitBounds => [TYPE_REPETITION_IN_BOUNDS, TRAIT_DUPLICATION_IN_BOUNDS]); +impl_lint_pass!(TraitBounds => [TYPE_REPETITION_IN_BOUNDS, TRAIT_DUPLICATION_IN_BOUNDS, MULTIPLE_BOUND_LOCATIONS]); impl<'tcx> LateLintPass<'tcx> for TraitBounds { fn check_generics(&mut self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) { + self.check_multiple_bound_location(cx, gen); self.check_type_repetition(cx, gen); check_trait_bound_duplication(cx, gen); } @@ -237,6 +265,55 @@ impl TraitBounds { } } + fn check_multiple_bound_location<'tcx>(&self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) { + let mut generic_params_with_bounds = FxHashMap::default(); + + for bound in gen.predicates { + if let WherePredicate::BoundPredicate(p) = bound + && p.origin == PredicateOrigin::GenericParam + && p.bounds + .iter() + .any(|b| !self.cannot_combine_maybe_bound(cx, b)) + && let Some((_, ident)) = p.bounded_ty.as_generic_param() + { + generic_params_with_bounds.insert(ident.name, ident.span); + } else if let WherePredicate::RegionPredicate(p) = bound + && !p.in_where_clause + && !p.bounds.is_empty() + { + let ident = p.lifetime.ident; + generic_params_with_bounds.insert(ident.name, ident.span); + } + } + + fn emit_lint(cx: &LateContext<'_>, bound_span: Span, where_span: Span) { + span_lint( + cx, + MULTIPLE_BOUND_LOCATIONS, + vec![bound_span, where_span], + "bound is defined in more than one place", + ); + } + + for bound in gen.predicates { + if let WherePredicate::BoundPredicate(p) = bound + && p.origin == PredicateOrigin::WhereClause + && !p.bounds.is_empty() + && let Some((_, ident)) = p.bounded_ty.as_generic_param() + && let Some(bound_span) = generic_params_with_bounds.get(&ident.name) + { + emit_lint(cx, *bound_span, ident.span); + } else if let WherePredicate::RegionPredicate(p) = bound + && p.in_where_clause + && !p.bounds.is_empty() + && let ident = p.lifetime.ident + && let Some(bound_span) = generic_params_with_bounds.get(&ident.name) + { + emit_lint(cx, *bound_span, ident.span); + } + } + } + fn check_type_repetition<'tcx>(&self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) { struct SpanlessTy<'cx, 'tcx> { ty: &'tcx Ty<'tcx>, diff --git a/tests/ui/multiple_bound_locations.rs b/tests/ui/multiple_bound_locations.rs index de9d994782ec..428be37a8e86 100644 --- a/tests/ui/multiple_bound_locations.rs +++ b/tests/ui/multiple_bound_locations.rs @@ -57,4 +57,26 @@ impl C { } } +#[clippy::msrv = "1.14.0"] +mod issue12370_fail { + trait Trait {} + + fn f() + where + T: Trait, + { + } +} + +#[clippy::msrv = "1.15.0"] +mod issue12370_pass { + trait Trait {} + + fn f() + where + T: Trait, + { + } +} + fn main() {} diff --git a/tests/ui/multiple_bound_locations.stderr b/tests/ui/multiple_bound_locations.stderr index 22dd2e0a5524..751f4bb6de6d 100644 --- a/tests/ui/multiple_bound_locations.stderr +++ b/tests/ui/multiple_bound_locations.stderr @@ -55,5 +55,14 @@ LL | fn ty_pred() LL | for<'a> F: Send + 'a, | ^ -error: aborting due to 6 previous errors +error: bound is defined in more than one place + --> tests/ui/multiple_bound_locations.rs:75:10 + | +LL | fn f() + | ^ +LL | where +LL | T: Trait, + | ^ + +error: aborting due to 7 previous errors