From 6ea7cd8ec700c4fbf1397eeef65fad5478965564 Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Mon, 5 Jun 2023 14:38:38 -0500 Subject: [PATCH 1/6] Fix #10504, don't lint on derived code --- clippy_lints/src/trait_bounds.rs | 3 +++ tests/ui/type_repetition_in_bounds.rs | 15 +++++++++++++++ tests/ui/type_repetition_in_bounds.stderr | 8 ++++---- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 4ccda15068b..913ef7644aa 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -260,6 +260,9 @@ impl TraitBounds { SpanlessTy { ty: p.bounded_ty, cx }, p.bounds.iter().collect::<Vec<_>>() ); + if let TyKind::Path(qpath) = p.bounded_ty.kind; + if format!("{}:", rustc_hir_pretty::qpath_to_string(&qpath)) + == format!("{}:", snippet(cx, p.bounded_ty.span, "_")); then { let trait_bounds = v diff --git a/tests/ui/type_repetition_in_bounds.rs b/tests/ui/type_repetition_in_bounds.rs index 8b4613b3f6e..1d36f4b3c7b 100644 --- a/tests/ui/type_repetition_in_bounds.rs +++ b/tests/ui/type_repetition_in_bounds.rs @@ -1,6 +1,7 @@ #![deny(clippy::type_repetition_in_bounds)] #![allow(clippy::extra_unused_type_parameters)] +use serde::Deserialize; use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign}; pub fn foo<T>(_t: T) @@ -70,6 +71,20 @@ mod issue4326 { } } +// Extern macros shouldn't lint, again (see #10504) +mod issue10504 { + use serde::{Deserialize, Serialize}; + use std::fmt::Debug; + use std::hash::Hash; + + #[derive(Debug, Serialize, Deserialize)] + #[serde(bound( + serialize = "T: Serialize + Hash + Eq", + deserialize = "Box<T>: serde::de::DeserializeOwned + Hash + Eq" + ))] + struct OpaqueParams<T: ?Sized + Debug>(std::marker::PhantomData<T>); +} + // Issue #7360 struct Foo<T, U> where diff --git a/tests/ui/type_repetition_in_bounds.stderr b/tests/ui/type_repetition_in_bounds.stderr index a90df03c04f..56867f75b07 100644 --- a/tests/ui/type_repetition_in_bounds.stderr +++ b/tests/ui/type_repetition_in_bounds.stderr @@ -1,5 +1,5 @@ error: this type has already been used as a bound predicate - --> $DIR/type_repetition_in_bounds.rs:9:5 + --> $DIR/type_repetition_in_bounds.rs:10:5 | LL | T: Clone, | ^^^^^^^^ @@ -12,7 +12,7 @@ LL | #![deny(clippy::type_repetition_in_bounds)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this type has already been used as a bound predicate - --> $DIR/type_repetition_in_bounds.rs:26:5 + --> $DIR/type_repetition_in_bounds.rs:27:5 | LL | Self: Copy + Default + Ord, | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -20,7 +20,7 @@ LL | Self: Copy + Default + Ord, = help: consider combining the bounds: `Self: Clone + Copy + Default + Ord` error: this type has already been used as a bound predicate - --> $DIR/type_repetition_in_bounds.rs:86:5 + --> $DIR/type_repetition_in_bounds.rs:101:5 | LL | T: Clone, | ^^^^^^^^ @@ -28,7 +28,7 @@ LL | T: Clone, = help: consider combining the bounds: `T: ?Sized + Clone` error: this type has already been used as a bound predicate - --> $DIR/type_repetition_in_bounds.rs:91:5 + --> $DIR/type_repetition_in_bounds.rs:106:5 | LL | T: ?Sized, | ^^^^^^^^^ From 0250c518449f535a4d38bf17f5b0ba5ecda3f3c9 Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Mon, 5 Jun 2023 14:46:47 -0500 Subject: [PATCH 2/6] refactor --- clippy_lints/src/trait_bounds.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 913ef7644aa..90c6429ff38 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -260,9 +260,9 @@ impl TraitBounds { SpanlessTy { ty: p.bounded_ty, cx }, p.bounds.iter().collect::<Vec<_>>() ); + let bounded_ty = snippet(cx, p.bounded_ty.span, "_"); if let TyKind::Path(qpath) = p.bounded_ty.kind; - if format!("{}:", rustc_hir_pretty::qpath_to_string(&qpath)) - == format!("{}:", snippet(cx, p.bounded_ty.span, "_")); + if format!("{}:", rustc_hir_pretty::qpath_to_string(&qpath)) == format!("{}:", bounded_ty); then { let trait_bounds = v @@ -272,10 +272,7 @@ impl TraitBounds { .filter_map(get_trait_info_from_bound) .map(|(_, _, span)| snippet_with_applicability(cx, span, "..", &mut applicability)) .join(" + "); - let hint_string = format!( - "consider combining the bounds: `{}: {trait_bounds}`", - snippet(cx, p.bounded_ty.span, "_"), - ); + let hint_string = format!("consider combining the bounds: `{}: {trait_bounds}`", bounded_ty); span_lint_and_help( cx, TYPE_REPETITION_IN_BOUNDS, @@ -345,7 +342,7 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) { "this trait bound is already specified in the where clause", None, "consider removing this trait bound", - ); + ); } } } From f68ee79864fb51232fb186fda6bf218be50c7e4a Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Mon, 5 Jun 2023 14:53:22 -0500 Subject: [PATCH 3/6] Update trait_bounds.rs --- clippy_lints/src/trait_bounds.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 90c6429ff38..2e6108d6b24 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -262,7 +262,7 @@ impl TraitBounds { ); let bounded_ty = snippet(cx, p.bounded_ty.span, "_"); if let TyKind::Path(qpath) = p.bounded_ty.kind; - if format!("{}:", rustc_hir_pretty::qpath_to_string(&qpath)) == format!("{}:", bounded_ty); + if format!("{}:", rustc_hir_pretty::qpath_to_string(&qpath)) == format!("{bounded_ty}:"); then { let trait_bounds = v @@ -272,7 +272,7 @@ impl TraitBounds { .filter_map(get_trait_info_from_bound) .map(|(_, _, span)| snippet_with_applicability(cx, span, "..", &mut applicability)) .join(" + "); - let hint_string = format!("consider combining the bounds: `{}: {trait_bounds}`", bounded_ty); + let hint_string = format!("consider combining the bounds: `{bounded_ty}: {trait_bounds}`"); span_lint_and_help( cx, TYPE_REPETITION_IN_BOUNDS, From e97f190a9dab40aa1fe2e5ec72af5d52f5fcf1fd Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Tue, 6 Jun 2023 18:05:15 -0500 Subject: [PATCH 4/6] `ty_search_pat` --- clippy_utils/src/check_proc_macro.rs | 42 ++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index 7e954898460..7f79b8f87dd 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -19,8 +19,8 @@ use rustc_ast::{ }; use rustc_hir::{ intravisit::FnKind, Block, BlockCheckMode, Body, Closure, Destination, Expr, ExprKind, FieldDef, FnHeader, HirId, - Impl, ImplItem, ImplItemKind, IsAuto, Item, ItemKind, LoopSource, MatchSource, Node, QPath, TraitItem, - TraitItemKind, UnOp, UnsafeSource, Unsafety, Variant, VariantData, YieldSource, + Impl, ImplItem, ImplItemKind, IsAuto, Item, ItemKind, LoopSource, MatchSource, MutTy, Node, QPath, TraitItem, + TraitItemKind, Ty, TyKind, UnOp, UnsafeSource, Unsafety, Variant, VariantData, YieldSource, }; use rustc_lint::{LateContext, LintContext}; use rustc_middle::ty::TyCtxt; @@ -319,6 +319,43 @@ fn attr_search_pat(attr: &Attribute) -> (Pat, Pat) { } } +// TODO: Waiting on `ty_search_pat`. +// fn where_pred_search_pat(where_pred: &WherePredicate<'_>) -> (Pat, Pat) { +// match where_pred { +// WherePredicate::BoundPredicate(bound) => { +// todo!(); +// }, +// WherePredicate::RegionPredicate(region) => { +// +// }, +// WherePredicate::EqPredicate(..) => unimplemented!(), +// } +// } + +fn ty_search_pat(ty: &Ty<'_>) -> (Pat, Pat) { + match ty.kind { + TyKind::Slice(..) | TyKind::Array(..) => (Pat::Str("["), Pat::Str("]")), + TyKind::Ptr(MutTy { mutbl, ty }) => ( + if mutbl.is_mut() { + Pat::Str("*const") + } else { + Pat::Str("*mut") + }, + ty_search_pat(ty).1, + ), + TyKind::Ref(_, MutTy { ty, .. }) => (Pat::Str("&"), ty_search_pat(ty).1), + TyKind::BareFn(bare_fn) => ( + Pat::OwnedStr(format!("{}{} fn", bare_fn.unsafety.prefix_str(), bare_fn.abi.name())), + ty_search_pat(ty).1, + ), + TyKind::Never => (Pat::Str("!"), Pat::Str("")), + TyKind::Tup(..) => (Pat::Str("("), Pat::Str(")")), + TyKind::OpaqueDef(..) => (Pat::Str("impl"), Pat::Str("")), + // NOTE: This is missing `TraitObject` and `Path` here. It always return true then. + _ => (Pat::Str(""), Pat::Str("")), + } +} + pub trait WithSearchPat { type Context: LintContext; fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat); @@ -345,6 +382,7 @@ impl_with_search_pat!(LateContext: TraitItem with trait_item_search_pat); impl_with_search_pat!(LateContext: ImplItem with impl_item_search_pat); impl_with_search_pat!(LateContext: FieldDef with field_def_search_pat); impl_with_search_pat!(LateContext: Variant with variant_search_pat); +impl_with_search_pat!(LateContext: Ty with ty_search_pat); impl<'cx> WithSearchPat for (&FnKind<'cx>, &Body<'cx>, HirId, Span) { type Context = LateContext<'cx>; From a434a7715d205959c3a07c9a7fe43e56a128e0c4 Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Tue, 6 Jun 2023 22:16:02 -0500 Subject: [PATCH 5/6] `impl WithSearchPat for Ty` --- clippy_lints/src/trait_bounds.rs | 12 ++++++------ clippy_utils/src/check_proc_macro.rs | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 2e6108d6b24..f4554fbcf9b 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; use clippy_utils::source::{snippet, snippet_opt, snippet_with_applicability}; -use clippy_utils::{SpanlessEq, SpanlessHash}; +use clippy_utils::{is_from_proc_macro, SpanlessEq, SpanlessHash}; use core::hash::{Hash, Hasher}; use if_chain::if_chain; use itertools::Itertools; @@ -260,10 +260,7 @@ impl TraitBounds { SpanlessTy { ty: p.bounded_ty, cx }, p.bounds.iter().collect::<Vec<_>>() ); - let bounded_ty = snippet(cx, p.bounded_ty.span, "_"); - if let TyKind::Path(qpath) = p.bounded_ty.kind; - if format!("{}:", rustc_hir_pretty::qpath_to_string(&qpath)) == format!("{bounded_ty}:"); - + if !is_from_proc_macro(cx, p.bounded_ty); then { let trait_bounds = v .iter() @@ -272,7 +269,10 @@ impl TraitBounds { .filter_map(get_trait_info_from_bound) .map(|(_, _, span)| snippet_with_applicability(cx, span, "..", &mut applicability)) .join(" + "); - let hint_string = format!("consider combining the bounds: `{bounded_ty}: {trait_bounds}`"); + let hint_string = format!( + "consider combining the bounds: `{}: {trait_bounds}`", + snippet(cx, p.bounded_ty.span, "_"), + ); span_lint_and_help( cx, TYPE_REPETITION_IN_BOUNDS, diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index 7f79b8f87dd..05b34986274 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -351,7 +351,8 @@ fn ty_search_pat(ty: &Ty<'_>) -> (Pat, Pat) { TyKind::Never => (Pat::Str("!"), Pat::Str("")), TyKind::Tup(..) => (Pat::Str("("), Pat::Str(")")), TyKind::OpaqueDef(..) => (Pat::Str("impl"), Pat::Str("")), - // NOTE: This is missing `TraitObject` and `Path` here. It always return true then. + TyKind::Path(qpath) => qpath_search_pat(&qpath), + // NOTE: This is missing `TraitObject`. It always return true then. _ => (Pat::Str(""), Pat::Str("")), } } From 4191de3303687b54f9ed5114170c9985b8962fe5 Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Sun, 11 Jun 2023 14:52:26 -0500 Subject: [PATCH 6/6] Update check_proc_macro.rs --- clippy_utils/src/check_proc_macro.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index 05b34986274..bcfac527c8a 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -319,19 +319,6 @@ fn attr_search_pat(attr: &Attribute) -> (Pat, Pat) { } } -// TODO: Waiting on `ty_search_pat`. -// fn where_pred_search_pat(where_pred: &WherePredicate<'_>) -> (Pat, Pat) { -// match where_pred { -// WherePredicate::BoundPredicate(bound) => { -// todo!(); -// }, -// WherePredicate::RegionPredicate(region) => { -// -// }, -// WherePredicate::EqPredicate(..) => unimplemented!(), -// } -// } - fn ty_search_pat(ty: &Ty<'_>) -> (Pat, Pat) { match ty.kind { TyKind::Slice(..) | TyKind::Array(..) => (Pat::Str("["), Pat::Str("]")),