Skip to content

Commit

Permalink
Support trait bounds with type parameters.
Browse files Browse the repository at this point in the history
  • Loading branch information
Allen Hsu committed May 1, 2022
1 parent 2f518f9 commit 9b329ed
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 40 deletions.
118 changes: 79 additions & 39 deletions clippy_lints/src/trait_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use rustc_data_structures::unhash::UnhashMap;
use rustc_errors::Applicability;
use rustc_hir::def::Res;
use rustc_hir::{
GenericBound, Generics, Item, ItemKind, Node, ParamName, Path, PathSegment, QPath, TraitItem, Ty, TyKind,
WherePredicate,
GenericArg, GenericBound, Generics, Item, ItemKind, Node, ParamName, Path, PathSegment, QPath, TraitItem, Ty,
TyKind, WherePredicate,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
Expand Down Expand Up @@ -289,44 +289,11 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
}
}

fn check_bounds_or_where_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
fn rollup_traits(cx: &LateContext<'_>, bounds: &[GenericBound<'_>], msg: &str) {
let mut map = FxHashMap::default();
let mut repeated_spans = false;
for bound in bounds.iter().filter_map(get_trait_info_from_bound) {
let (definition, _, span_direct) = bound;
if map.insert(definition, span_direct).is_some() {
repeated_spans = true;
}
}

if_chain! {
if repeated_spans;
if let Some(first_trait) = bounds.get(0);
if let Some(last_trait) = bounds.iter().last();
then {
let all_trait_span = first_trait.span().to(last_trait.span());

let mut traits = map.values()
.filter_map(|span| snippet_opt(cx, *span))
.collect::<Vec<_>>();
traits.sort_unstable();
let traits = traits.join(" + ");

span_lint_and_sugg(
cx,
REPEATED_WHERE_CLAUSE_OR_TRAIT_BOUND,
all_trait_span,
msg,
"try",
traits,
Applicability::MachineApplicable
);
}
}
}
#[derive(PartialEq, Eq, Hash, Debug)]
struct ComparableBound(Res, Vec<Res>, Vec<ComparableBound>);

if gen.span.from_expansion() || (gen.params.is_empty() && gen.where_clause.predicates.is_empty()) {
fn check_bounds_or_where_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
if gen.span.from_expansion() {
return;
}

Expand Down Expand Up @@ -355,3 +322,76 @@ fn get_trait_info_from_bound<'a>(bound: &'a GenericBound<'_>) -> Option<(Res, &'
None
}
}

fn try_into_comparable_bound(bound: &GenericBound<'_>) -> Option<ComparableBound> {
if let GenericBound::Trait(t, _) = bound {
Some(ComparableBound(
t.trait_ref.path.res,
t.trait_ref
.path
.segments
.iter()
.filter_map(|segment| {
// get trait bound type arguments
Some(segment.args?.args.iter().filter_map(|arg| {
if_chain! {
if let GenericArg::Type(ty) = arg;
if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind;
then { return Some(path.res) }
}
None
}))
})
.flatten()
.collect(),
t.bound_generic_params
.iter()
.flat_map(|param| param.bounds.iter().filter_map(try_into_comparable_bound))
.collect(),
))
} else {
None
}
}

fn rollup_traits(cx: &LateContext<'_>, bounds: &[GenericBound<'_>], msg: &str) {
let mut map = FxHashMap::default();
let mut repeated_spans = false;
for bound in bounds.iter().filter_map(|bound| {
if let GenericBound::Trait(t, _) = bound {
Some((try_into_comparable_bound(bound)?, t.span))
} else {
None
}
}) {
let (comparable_bound, span_direct) = bound;
if map.insert(comparable_bound, span_direct).is_some() {
repeated_spans = true;
}
}

if_chain! {
if repeated_spans;
if let Some(first_trait) = bounds.get(0);
if let Some(last_trait) = bounds.iter().last();
then {
let all_trait_span = first_trait.span().to(last_trait.span());

let mut traits = map.values()
.filter_map(|span| snippet_opt(cx, *span))
.collect::<Vec<_>>();
traits.sort_unstable();
let traits = traits.join(" + ");

span_lint_and_sugg(
cx,
REPEATED_WHERE_CLAUSE_OR_TRAIT_BOUND,
all_trait_span,
msg,
"try",
traits,
Applicability::MachineApplicable
);
}
}
}
10 changes: 10 additions & 0 deletions tests/ui/repeated_where_clause_or_trait_bound.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,14 @@ where

fn no_error_separate_arg_bounds(program: impl AsRef<()>, dir: impl AsRef<()>, args: &[impl AsRef<()>]) {}

trait GenericTrait<T> {}

fn good_generic<T: GenericTrait<u64> + GenericTrait<u32>>(arg0: T) {
unimplemented!();
}

fn bad_generic<T: GenericTrait<u32> + GenericTrait<u64>>(arg0: T) {
unimplemented!();
}

fn main() {}
10 changes: 10 additions & 0 deletions tests/ui/repeated_where_clause_or_trait_bound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,14 @@ where

fn no_error_separate_arg_bounds(program: impl AsRef<()>, dir: impl AsRef<()>, args: &[impl AsRef<()>]) {}

trait GenericTrait<T> {}

fn good_generic<T: GenericTrait<u64> + GenericTrait<u32>>(arg0: T) {
unimplemented!();
}

fn bad_generic<T: GenericTrait<u64> + GenericTrait<u32> + GenericTrait<u64>>(arg0: T) {
unimplemented!();
}

fn main() {}
8 changes: 7 additions & 1 deletion tests/ui/repeated_where_clause_or_trait_bound.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,11 @@ error: this where clause contains repeated elements
LL | T: Clone + Clone + Clone + Copy,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`

error: aborting due to 5 previous errors
error: this trait bound contains repeated elements
--> $DIR/repeated_where_clause_or_trait_bound.rs:101:19
|
LL | fn bad_generic<T: GenericTrait<u64> + GenericTrait<u32> + GenericTrait<u64>>(arg0: T) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `GenericTrait<u32> + GenericTrait<u64>`

error: aborting due to 6 previous errors

0 comments on commit 9b329ed

Please sign in to comment.