Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore associated types in traits when considering type complexity #8030

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 33 additions & 13 deletions clippy_lints/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,16 +350,28 @@ impl<'tcx> LateLintPass<'tcx> for Types {

fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) {
match item.kind {
ImplItemKind::Const(ty, _) | ImplItemKind::TyAlias(ty) => self.check_ty(
cx,
ty,
CheckTyContext {
is_in_trait_impl: true,
..CheckTyContext::default()
},
),
// methods are covered by check_fn
ImplItemKind::Fn(..) => (),
ImplItemKind::Const(ty, _) => {
let is_in_trait_impl = if let Some(hir::Node::Item(item)) =
cx.tcx.hir().find(cx.tcx.hir().get_parent_item(item.hir_id()))
{
matches!(item.kind, ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }))
} else {
false
};

self.check_ty(
cx,
ty,
CheckTyContext {
is_in_trait_impl,
..CheckTyContext::default()
},
);
},
// Methods are covered by check_fn.
// Type aliases are ignored because oftentimes it's impossible to
// make type alias declaration in trait simpler, see #1013
ImplItemKind::Fn(..) | ImplItemKind::TyAlias(..) => (),
}
}

Expand Down Expand Up @@ -417,6 +429,14 @@ impl Types {
}

fn check_fn_decl(&mut self, cx: &LateContext<'_>, decl: &FnDecl<'_>, context: CheckTyContext) {
// Ignore functions in trait implementations as they are usually forced by the trait definition.
//
// FIXME: idially we would like to warn *if the compicated type can be simplified*, but it's hard to
// check.
if context.is_in_trait_impl {
return;
}

for input in decl.inputs {
self.check_ty(cx, input, context);
}
Expand All @@ -435,12 +455,12 @@ impl Types {
return;
}

if !context.is_nested_call && type_complexity::check(cx, hir_ty, self.type_complexity_threshold) {
// Skip trait implementations; see issue #605.
if context.is_in_trait_impl {
return;
}

// Skip trait implementations; see issue #605.
if context.is_in_trait_impl {
if !context.is_nested_call && type_complexity::check(cx, hir_ty, self.type_complexity_threshold) {
return;
}

Expand Down
9 changes: 9 additions & 0 deletions tests/ui/type_complexity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ trait T {
fn def_method(&self, p: Vec<Vec<Box<(u32, u32, u32, u32)>>>) {}
}

// Should not warn since there is likely no way to simplify this (#1013)
impl T for () {
const A: Vec<Vec<Box<(u32, u32, u32, u32)>>> = vec![];

type B = Vec<Vec<Box<(u32, u32, u32, u32)>>>;

fn method(&self, p: Vec<Vec<Box<(u32, u32, u32, u32)>>>) {}
}

fn test1() -> Vec<Vec<Box<(u32, u32, u32, u32)>>> {
vec![]
}
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/type_complexity.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,19 @@ LL | fn def_method(&self, p: Vec<Vec<Box<(u32, u32, u32, u32)>>>) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: very complex type used. Consider factoring parts into `type` definitions
--> $DIR/type_complexity.rs:33:15
--> $DIR/type_complexity.rs:42:15
|
LL | fn test1() -> Vec<Vec<Box<(u32, u32, u32, u32)>>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: very complex type used. Consider factoring parts into `type` definitions
--> $DIR/type_complexity.rs:37:14
--> $DIR/type_complexity.rs:46:14
|
LL | fn test2(_x: Vec<Vec<Box<(u32, u32, u32, u32)>>>) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: very complex type used. Consider factoring parts into `type` definitions
--> $DIR/type_complexity.rs:40:13
--> $DIR/type_complexity.rs:49:13
|
LL | let _y: Vec<Vec<Box<(u32, u32, u32, u32)>>> = vec![];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down