mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

lint implied bounds in *all* opaque impl Trait types

Browse Source
This commit is contained in:
y21 2024-02-17 21:35:16 +01:00
parent c469cb0023
commit ec29b0d6b8
4 changed files with 145 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

107
clippy_lints/src/implied_bounds_in_impls.rs
View File

@ -2,16 +2,11 @@
use clippy_utils::source::snippet;
use rustc_errors::{Applicability, SuggestionStyle};
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::FnKind;
use rustc_hir::{
Body, FnDecl, FnRetTy, GenericArg, GenericBound, ImplItem, ImplItemKind, ItemKind, OpaqueTy, TraitBoundModifier,
TraitItem, TraitItemKind, TyKind, TypeBinding,
};
use rustc_hir::{GenericArg, GenericBound, GenericBounds, ItemKind, TraitBoundModifier, TyKind, TypeBinding};
use rustc_hir_analysis::hir_ty_to_ty;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{self, ClauseKind, Generics, Ty, TyCtxt};
use rustc_session::declare_lint_pass;
use rustc_span::def_id::LocalDefId;
use rustc_span::Span;
declare_clippy_lint! {
@ -54,7 +49,7 @@
fn emit_lint(
cx: &LateContext<'_>,
poly_trait: &rustc_hir::PolyTraitRef<'_>,
opaque_ty: &rustc_hir::OpaqueTy<'_>,
bounds: GenericBounds<'_>,
index: usize,
// The bindings that were implied, used for suggestion purposes since removing a bound with associated types
// means we might need to then move it to a different bound
@ -73,10 +68,10 @@ fn emit_lint(
// to include the `+` token that is ahead or behind,
// so we don't end up with something like `impl + B` or `impl A + `
let implied_span_extended = if let Some(next_bound) = opaque_ty.bounds.get(index + 1) {
let implied_span_extended = if let Some(next_bound) = bounds.get(index + 1) {
poly_trait.span.to(next_bound.span().shrink_to_lo())
} else if index > 0
&& let Some(prev_bound) = opaque_ty.bounds.get(index - 1)
&& let Some(prev_bound) = bounds.get(index - 1)
{
prev_bound.span().shrink_to_hi().to(poly_trait.span.shrink_to_hi())
} else {
@ -240,9 +235,8 @@ struct ImplTraitBound<'tcx> {
/// For `impl Deref + DerefMut + Eq` this returns `[Deref, PartialEq]`.
/// The `Deref` comes from `DerefMut` because `trait DerefMut: Deref {}`, and `PartialEq` comes from
/// `Eq`.
fn collect_supertrait_bounds<'tcx>(cx: &LateContext<'tcx>, opaque_ty: &OpaqueTy<'tcx>) -> Vec<ImplTraitBound<'tcx>> {
opaque_ty
.bounds
fn collect_supertrait_bounds<'tcx>(cx: &LateContext<'tcx>, bounds: GenericBounds<'tcx>) -> Vec<ImplTraitBound<'tcx>> {
bounds
.iter()
.filter_map(|bound| {
if let GenericBound::Trait(poly_trait, TraitBoundModifier::None) = bound
@ -295,64 +289,49 @@ fn find_bound_in_supertraits<'a, 'tcx>(
})
}
fn check(cx: &LateContext<'_>, decl: &FnDecl<'_>) {
if let FnRetTy::Return(ty) = decl.output
&& let TyKind::OpaqueDef(item_id, ..) = ty.kind
&& let item = cx.tcx.hir().item(item_id)
&& let ItemKind::OpaqueTy(opaque_ty) = item.kind
fn check<'tcx>(cx: &LateContext<'tcx>, bounds: GenericBounds<'tcx>) {
if bounds.len() == 1 {
// Very often there is only a single bound, e.g. `impl Deref<..>`, in which case
// we can avoid doing a bunch of stuff unnecessarily.
&& opaque_ty.bounds.len() > 1
{
let supertraits = collect_supertrait_bounds(cx, opaque_ty);
// we can avoid doing a bunch of stuff unnecessarily; there will trivially be
// no duplicate bounds
return;
}
// Lint all bounds in the `impl Trait` type that we've previously also seen in the set of
// supertraits of each of the bounds.
// This involves some extra logic when generic arguments are present, since
// simply comparing trait `DefId`s won't be enough. We also need to compare the generics.
for (index, bound) in opaque_ty.bounds.iter().enumerate() {
if let GenericBound::Trait(poly_trait, TraitBoundModifier::None) = bound
&& let [.., path] = poly_trait.trait_ref.path.segments
&& let implied_args = path.args.map_or([].as_slice(), |a| a.args)
&& let implied_bindings = path.args.map_or([].as_slice(), |a| a.bindings)
&& let Some(def_id) = poly_trait.trait_ref.path.res.opt_def_id()
&& let Some(bound) = find_bound_in_supertraits(cx, def_id, implied_args, &supertraits)
// If the implied bound has a type binding that also exists in the implied-by trait,
// then we shouldn't lint. See #11880 for an example.
&& let assocs = cx.tcx.associated_items(bound.trait_def_id)
&& !implied_bindings.iter().any(|binding| {
assocs
.filter_by_name_unhygienic(binding.ident.name)
.next()
.is_some_and(|assoc| assoc.kind == ty::AssocKind::Type)
})
{
emit_lint(cx, poly_trait, opaque_ty, index, implied_bindings, bound);
}
let supertraits = collect_supertrait_bounds(cx, bounds);
// Lint all bounds in the `impl Trait` type that we've previously also seen in the set of
// supertraits of each of the bounds.
// This involves some extra logic when generic arguments are present, since
// simply comparing trait `DefId`s won't be enough. We also need to compare the generics.
for (index, bound) in bounds.iter().enumerate() {
if let GenericBound::Trait(poly_trait, TraitBoundModifier::None) = bound
&& let [.., path] = poly_trait.trait_ref.path.segments
&& let implied_args = path.args.map_or([].as_slice(), |a| a.args)
&& let implied_bindings = path.args.map_or([].as_slice(), |a| a.bindings)
&& let Some(def_id) = poly_trait.trait_ref.path.res.opt_def_id()
&& let Some(bound) = find_bound_in_supertraits(cx, def_id, implied_args, &supertraits)
// If the implied bound has a type binding that also exists in the implied-by trait,
// then we shouldn't lint. See #11880 for an example.
&& let assocs = cx.tcx.associated_items(bound.trait_def_id)
&& !implied_bindings.iter().any(|binding| {
assocs
.filter_by_name_unhygienic(binding.ident.name)
.next()
.is_some_and(|assoc| assoc.kind == ty::AssocKind::Type)
})
{
emit_lint(cx, poly_trait, bounds, index, implied_bindings, bound);
}
}
}
impl LateLintPass<'_> for ImpliedBoundsInImpls {
fn check_fn(
&mut self,
cx: &LateContext<'_>,
_: FnKind<'_>,
decl: &FnDecl<'_>,
_: &Body<'_>,
_: Span,
_: LocalDefId,
) {
check(cx, decl);
}
fn check_trait_item(&mut self, cx: &LateContext<'_>, item: &TraitItem<'_>) {
if let TraitItemKind::Fn(sig, ..) = &item.kind {
check(cx, sig.decl);
}
}
fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &ImplItem<'_>) {
if let ImplItemKind::Fn(sig, ..) = &item.kind {
check(cx, sig.decl);
impl<'tcx> LateLintPass<'tcx> for ImpliedBoundsInImpls {
fn check_ty(&mut self, cx: &LateContext<'_>, ty: &rustc_hir::Ty<'_>) {
if let TyKind::OpaqueDef(item_id, ..) = ty.kind
&& let item = cx.tcx.hir().item(item_id)
&& let ItemKind::OpaqueTy(opaque_ty) = item.kind
{
check(cx, opaque_ty.bounds);
}
}
}

23
tests/ui/implied_bounds_in_impls.fixed
View File

@ -1,5 +1,6 @@
#![warn(clippy::implied_bounds_in_impls)]
#![allow(dead_code)]
#![feature(impl_trait_in_assoc_type, type_alias_impl_trait)]
use std::ops::{Deref, DerefMut};
@ -150,4 +151,26 @@ fn issue11880() {
fn f5() -> impl Y<T = u32, U = String> {}
}
fn apit(_: impl Deref + DerefMut) {}
trait Rpitit {
fn f() -> impl DerefMut;
}
trait Atpit {
type Assoc;
fn define() -> Self::Assoc;
}
impl Atpit for () {
type Assoc = impl DerefMut;
fn define() -> Self::Assoc {
&mut [] as &mut [()]
}
}
type Tait = impl DerefMut;
fn define() -> Tait {
&mut [] as &mut [()]
}
fn main() {}

23
tests/ui/implied_bounds_in_impls.rs
View File

@ -1,5 +1,6 @@
#![warn(clippy::implied_bounds_in_impls)]
#![allow(dead_code)]
#![feature(impl_trait_in_assoc_type, type_alias_impl_trait)]
use std::ops::{Deref, DerefMut};
@ -150,4 +151,26 @@ fn f4() -> impl X + Y<T = u32> {}
fn f5() -> impl X<U = String> + Y<T = u32> {}
}
fn apit(_: impl Deref + DerefMut) {}
trait Rpitit {
fn f() -> impl Deref + DerefMut;
}
trait Atpit {
type Assoc;
fn define() -> Self::Assoc;
}
impl Atpit for () {
type Assoc = impl Deref + DerefMut;
fn define() -> Self::Assoc {
&mut [] as &mut [()]
}
}
type Tait = impl Deref + DerefMut;
fn define() -> Tait {
&mut [] as &mut [()]
}
fn main() {}

76
tests/ui/implied_bounds_in_impls.stderr
View File

@ -1,5 +1,5 @@
error: this bound is already specified as the supertrait of `DerefMut<Target = T>`
--> tests/ui/implied_bounds_in_impls.rs:12:36
--> tests/ui/implied_bounds_in_impls.rs:13:36
|
LL | fn deref_derefmut<T>(x: T) -> impl Deref<Target = T> + DerefMut<Target = T> {
| ^^^^^^^^^^^^^^^^^
@ -13,7 +13,7 @@ LL + fn deref_derefmut<T>(x: T) -> impl DerefMut<Target = T> {
|
error: this bound is already specified as the supertrait of `GenericSubtrait<U, W, U>`
--> tests/ui/implied_bounds_in_impls.rs:29:37
--> tests/ui/implied_bounds_in_impls.rs:30:37
|
LL | fn generics_implied<U, W>() -> impl GenericTrait<W> + GenericSubtrait<U, W, U>
| ^^^^^^^^^^^^^^^
@ -25,7 +25,7 @@ LL + fn generics_implied<U, W>() -> impl GenericSubtrait<U, W, U>
|
error: this bound is already specified as the supertrait of `GenericSubtrait<(), i32, V>`
--> tests/ui/implied_bounds_in_impls.rs:35:40
--> tests/ui/implied_bounds_in_impls.rs:36:40
|
LL | fn generics_implied_multi<V>() -> impl GenericTrait<i32> + GenericTrait2<V> + GenericSubtrait<(), i32, V> {}
| ^^^^^^^^^^^^^^^^^
@ -37,7 +37,7 @@ LL + fn generics_implied_multi<V>() -> impl GenericTrait2<V> + GenericSubtrait<(
|
error: this bound is already specified as the supertrait of `GenericSubtrait<(), i32, V>`
--> tests/ui/implied_bounds_in_impls.rs:35:60
--> tests/ui/implied_bounds_in_impls.rs:36:60
|
LL | fn generics_implied_multi<V>() -> impl GenericTrait<i32> + GenericTrait2<V> + GenericSubtrait<(), i32, V> {}
| ^^^^^^^^^^^^^^^^
@ -49,7 +49,7 @@ LL + fn generics_implied_multi<V>() -> impl GenericTrait<i32> + GenericSubtrait<
|
error: this bound is already specified as the supertrait of `GenericSubtrait<(), T, V>`
--> tests/ui/implied_bounds_in_impls.rs:37:44
--> tests/ui/implied_bounds_in_impls.rs:38:44
|
LL | fn generics_implied_multi2<T, V>() -> impl GenericTrait<T> + GenericTrait2<V> + GenericSubtrait<(), T, V>
| ^^^^^^^^^^^^^^^
@ -61,7 +61,7 @@ LL + fn generics_implied_multi2<T, V>() -> impl GenericTrait2<V> + GenericSubtra
|
error: this bound is already specified as the supertrait of `GenericSubtrait<(), T, V>`
--> tests/ui/implied_bounds_in_impls.rs:37:62
--> tests/ui/implied_bounds_in_impls.rs:38:62
|
LL | fn generics_implied_multi2<T, V>() -> impl GenericTrait<T> + GenericTrait2<V> + GenericSubtrait<(), T, V>
| ^^^^^^^^^^^^^^^^
@ -73,7 +73,7 @@ LL + fn generics_implied_multi2<T, V>() -> impl GenericTrait<T> + GenericSubtrai
|
error: this bound is already specified as the supertrait of `GenericSubtrait<(), i32, ()>`
--> tests/ui/implied_bounds_in_impls.rs:47:28
--> tests/ui/implied_bounds_in_impls.rs:48:28
|
LL | fn generics_same() -> impl GenericTrait<i32> + GenericSubtrait<(), i32, ()> {}
| ^^^^^^^^^^^^^^^^^
@ -85,7 +85,7 @@ LL + fn generics_same() -> impl GenericSubtrait<(), i32, ()> {}
|
error: this bound is already specified as the supertrait of `DerefMut<Target = u8>`
--> tests/ui/implied_bounds_in_impls.rs:51:20
--> tests/ui/implied_bounds_in_impls.rs:52:20
|
LL | fn f() -> impl Deref + DerefMut<Target = u8>;
| ^^^^^
@ -97,7 +97,7 @@ LL + fn f() -> impl DerefMut<Target = u8>;
|
error: this bound is already specified as the supertrait of `DerefMut<Target = u8>`
--> tests/ui/implied_bounds_in_impls.rs:56:20
--> tests/ui/implied_bounds_in_impls.rs:57:20
|
LL | fn f() -> impl Deref + DerefMut<Target = u8> {
| ^^^^^
@ -109,7 +109,7 @@ LL + fn f() -> impl DerefMut<Target = u8> {
|
error: this bound is already specified as the supertrait of `DerefMut<Target = u8>`
--> tests/ui/implied_bounds_in_impls.rs:62:20
--> tests/ui/implied_bounds_in_impls.rs:63:20
|
LL | fn f() -> impl Deref + DerefMut<Target = u8> {
| ^^^^^
@ -121,7 +121,7 @@ LL + fn f() -> impl DerefMut<Target = u8> {
|
error: this bound is already specified as the supertrait of `PartialOrd`
--> tests/ui/implied_bounds_in_impls.rs:73:41
--> tests/ui/implied_bounds_in_impls.rs:74:41
|
LL | fn default_generic_param1() -> impl PartialEq + PartialOrd + Debug {}
| ^^^^^^^^^
@ -133,7 +133,7 @@ LL + fn default_generic_param1() -> impl PartialOrd + Debug {}
|
error: this bound is already specified as the supertrait of `PartialOrd`
--> tests/ui/implied_bounds_in_impls.rs:74:54
--> tests/ui/implied_bounds_in_impls.rs:75:54
|
LL | fn default_generic_param2() -> impl PartialOrd + PartialEq + Debug {}
| ^^^^^^^^^
@ -145,7 +145,7 @@ LL + fn default_generic_param2() -> impl PartialOrd + Debug {}
|
error: this bound is already specified as the supertrait of `DoubleEndedIterator`
--> tests/ui/implied_bounds_in_impls.rs:87:26
--> tests/ui/implied_bounds_in_impls.rs:88:26
|
LL | fn my_iter() -> impl Iterator<Item = u32> + DoubleEndedIterator {
| ^^^^^^^^^^^^^^^^^^^^
@ -157,7 +157,7 @@ LL + fn my_iter() -> impl DoubleEndedIterator<Item = u32> {
|
error: this bound is already specified as the supertrait of `Copy`
--> tests/ui/implied_bounds_in_impls.rs:92:27
--> tests/ui/implied_bounds_in_impls.rs:93:27
|
LL | fn f() -> impl Copy + Clone {
| ^^^^^
@ -169,7 +169,7 @@ LL + fn f() -> impl Copy {
|
error: this bound is already specified as the supertrait of `Trait2<i32>`
--> tests/ui/implied_bounds_in_impls.rs:106:21
--> tests/ui/implied_bounds_in_impls.rs:107:21
|
LL | fn f2() -> impl Trait1<i32, U = i64> + Trait2<i32> {}
| ^^^^^^^^^^^^^^^^^^^^
@ -181,7 +181,7 @@ LL + fn f2() -> impl Trait2<i32, U = i64> {}
|
error: this bound is already specified as the supertrait of `Trait4<i8, X = i32>`
--> tests/ui/implied_bounds_in_impls.rs:121:21
--> tests/ui/implied_bounds_in_impls.rs:122:21
|
LL | fn f3() -> impl Trait3<i8, i16, i64, X = i32, Y = i128> + Trait4<i8, X = i32> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -193,7 +193,7 @@ LL + fn f3() -> impl Trait4<i8, X = i32, Y = i128> {}
|
error: this bound is already specified as the supertrait of `Y`
--> tests/ui/implied_bounds_in_impls.rs:148:21
--> tests/ui/implied_bounds_in_impls.rs:149:21
|
LL | fn f3() -> impl X + Y {}
| ^
@ -205,7 +205,7 @@ LL + fn f3() -> impl Y {}
|
error: this bound is already specified as the supertrait of `Y<T = u32>`
--> tests/ui/implied_bounds_in_impls.rs:149:21
--> tests/ui/implied_bounds_in_impls.rs:150:21
|
LL | fn f4() -> impl X + Y<T = u32> {}
| ^
@ -217,7 +217,7 @@ LL + fn f4() -> impl Y<T = u32> {}
|
error: this bound is already specified as the supertrait of `Y<T = u32>`
--> tests/ui/implied_bounds_in_impls.rs:150:21
--> tests/ui/implied_bounds_in_impls.rs:151:21
|
LL | fn f5() -> impl X<U = String> + Y<T = u32> {}
| ^^^^^^^^^^^^^
@ -228,5 +228,41 @@ LL - fn f5() -> impl X<U = String> + Y<T = u32> {}
LL + fn f5() -> impl Y<T = u32, U = String> {}
|
error: aborting due to 19 previous errors
error: this bound is already specified as the supertrait of `DerefMut`
--> tests/ui/implied_bounds_in_impls.rs:157:20
|
LL | fn f() -> impl Deref + DerefMut;
| ^^^^^
|
help: try removing this bound
|
LL - fn f() -> impl Deref + DerefMut;
LL + fn f() -> impl DerefMut;
|
error: this bound is already specified as the supertrait of `DerefMut`
--> tests/ui/implied_bounds_in_impls.rs:165:23
|
LL | type Assoc = impl Deref + DerefMut;
| ^^^^^
|
help: try removing this bound
|
LL - type Assoc = impl Deref + DerefMut;
LL + type Assoc = impl DerefMut;
|
error: this bound is already specified as the supertrait of `DerefMut`
--> tests/ui/implied_bounds_in_impls.rs:171:18
|
LL | type Tait = impl Deref + DerefMut;
| ^^^^^
|
help: try removing this bound
|
LL - type Tait = impl Deref + DerefMut;
LL + type Tait = impl DerefMut;
|
error: aborting due to 22 previous errors