From 063f8aa094a740b98b0dab49d8361441c8f1d0c4 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 25 Nov 2021 11:50:57 +0300 Subject: [PATCH 1/3] Ignore associated types in traits when considering type complexity --- clippy_lints/src/types/mod.rs | 8 +++++--- tests/ui/type_complexity_issue_1013.rs | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 tests/ui/type_complexity_issue_1013.rs diff --git a/clippy_lints/src/types/mod.rs b/clippy_lints/src/types/mod.rs index 5a7ef760a..69cd49d88 100644 --- a/clippy_lints/src/types/mod.rs +++ b/clippy_lints/src/types/mod.rs @@ -350,7 +350,7 @@ 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( + ImplItemKind::Const(ty, _) => self.check_ty( cx, ty, CheckTyContext { @@ -358,8 +358,10 @@ impl<'tcx> LateLintPass<'tcx> for Types { ..CheckTyContext::default() }, ), - // methods are covered by check_fn - ImplItemKind::Fn(..) => (), + // 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(..) => (), } } diff --git a/tests/ui/type_complexity_issue_1013.rs b/tests/ui/type_complexity_issue_1013.rs new file mode 100644 index 000000000..c68ab3aaf --- /dev/null +++ b/tests/ui/type_complexity_issue_1013.rs @@ -0,0 +1,23 @@ +#![warn(clippy::type_complexity)] +use std::iter::{Filter, Map}; +use std::vec::IntoIter; + +struct S; + +impl IntoIterator for S { + type Item = i32; + // Should not warn since there is no way to simplify this + type IntoIter = Filter, fn(i32) -> i32>, fn(&i32) -> bool>; + + fn into_iter(self) -> Self::IntoIter { + fn m(a: i32) -> i32 { + a + } + fn p(_: &i32) -> bool { + true + } + vec![1i32, 2, 3].into_iter().map(m as fn(_) -> _).filter(p) + } +} + +fn main() {} From 40a6c519b447a45c4ec1cdda4be067207d836306 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 25 Nov 2021 12:47:29 +0300 Subject: [PATCH 2/3] Update tests for type_complexity lint --- tests/ui/type_complexity.rs | 8 ++++++++ tests/ui/type_complexity.stderr | 20 ++++++++++++++++---- tests/ui/type_complexity_issue_1013.rs | 23 ----------------------- 3 files changed, 24 insertions(+), 27 deletions(-) delete mode 100644 tests/ui/type_complexity_issue_1013.rs diff --git a/tests/ui/type_complexity.rs b/tests/ui/type_complexity.rs index 383bbb49d..62d00e007 100644 --- a/tests/ui/type_complexity.rs +++ b/tests/ui/type_complexity.rs @@ -30,6 +30,14 @@ trait T { fn def_method(&self, p: Vec>>) {} } +impl T for () { + const A: Vec>> = vec![]; + + // Should not warn since there is likely no way to simplify this (#1013) + type B = Vec>>; + fn method(&self, p: Vec>>) {} +} + fn test1() -> Vec>> { vec![] } diff --git a/tests/ui/type_complexity.stderr b/tests/ui/type_complexity.stderr index 7879233fd..693f2adcf 100644 --- a/tests/ui/type_complexity.stderr +++ b/tests/ui/type_complexity.stderr @@ -73,22 +73,34 @@ LL | fn def_method(&self, p: Vec>>) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: very complex type used. Consider factoring parts into `type` definitions - --> $DIR/type_complexity.rs:33:15 + --> $DIR/type_complexity.rs:34:14 + | +LL | const A: Vec>> = vec![]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: very complex type used. Consider factoring parts into `type` definitions + --> $DIR/type_complexity.rs:38:25 + | +LL | fn method(&self, p: Vec>>) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: very complex type used. Consider factoring parts into `type` definitions + --> $DIR/type_complexity.rs:41:15 | LL | fn test1() -> Vec>> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: very complex type used. Consider factoring parts into `type` definitions - --> $DIR/type_complexity.rs:37:14 + --> $DIR/type_complexity.rs:45:14 | LL | fn test2(_x: Vec>>) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: very complex type used. Consider factoring parts into `type` definitions - --> $DIR/type_complexity.rs:40:13 + --> $DIR/type_complexity.rs:48:13 | LL | let _y: Vec>> = vec![]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 15 previous errors +error: aborting due to 17 previous errors diff --git a/tests/ui/type_complexity_issue_1013.rs b/tests/ui/type_complexity_issue_1013.rs deleted file mode 100644 index c68ab3aaf..000000000 --- a/tests/ui/type_complexity_issue_1013.rs +++ /dev/null @@ -1,23 +0,0 @@ -#![warn(clippy::type_complexity)] -use std::iter::{Filter, Map}; -use std::vec::IntoIter; - -struct S; - -impl IntoIterator for S { - type Item = i32; - // Should not warn since there is no way to simplify this - type IntoIter = Filter, fn(i32) -> i32>, fn(&i32) -> bool>; - - fn into_iter(self) -> Self::IntoIter { - fn m(a: i32) -> i32 { - a - } - fn p(_: &i32) -> bool { - true - } - vec![1i32, 2, 3].into_iter().map(m as fn(_) -> _).filter(p) - } -} - -fn main() {} From c176568abd9290088ec604be644c021caa81f508 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 8 Dec 2021 14:27:49 +0300 Subject: [PATCH 3/3] Ignore associated items in trait *implementations* when considering type complexity --- clippy_lints/src/types/mod.rs | 40 ++++++++++++++++++++++++--------- tests/ui/type_complexity.rs | 3 ++- tests/ui/type_complexity.stderr | 20 ++++------------- 3 files changed, 35 insertions(+), 28 deletions(-) diff --git a/clippy_lints/src/types/mod.rs b/clippy_lints/src/types/mod.rs index 69cd49d88..481e59574 100644 --- a/clippy_lints/src/types/mod.rs +++ b/clippy_lints/src/types/mod.rs @@ -350,14 +350,24 @@ impl<'tcx> LateLintPass<'tcx> for Types { fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) { match item.kind { - ImplItemKind::Const(ty, _) => self.check_ty( - cx, - ty, - CheckTyContext { - is_in_trait_impl: true, - ..CheckTyContext::default() - }, - ), + 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 @@ -419,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); } @@ -437,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; } diff --git a/tests/ui/type_complexity.rs b/tests/ui/type_complexity.rs index 62d00e007..86a7bd7b6 100644 --- a/tests/ui/type_complexity.rs +++ b/tests/ui/type_complexity.rs @@ -30,11 +30,12 @@ trait T { fn def_method(&self, p: Vec>>) {} } +// Should not warn since there is likely no way to simplify this (#1013) impl T for () { const A: Vec>> = vec![]; - // Should not warn since there is likely no way to simplify this (#1013) type B = Vec>>; + fn method(&self, p: Vec>>) {} } diff --git a/tests/ui/type_complexity.stderr b/tests/ui/type_complexity.stderr index 693f2adcf..9da7edb1c 100644 --- a/tests/ui/type_complexity.stderr +++ b/tests/ui/type_complexity.stderr @@ -73,34 +73,22 @@ LL | fn def_method(&self, p: Vec>>) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: very complex type used. Consider factoring parts into `type` definitions - --> $DIR/type_complexity.rs:34:14 - | -LL | const A: Vec>> = vec![]; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: very complex type used. Consider factoring parts into `type` definitions - --> $DIR/type_complexity.rs:38:25 - | -LL | fn method(&self, p: Vec>>) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: very complex type used. Consider factoring parts into `type` definitions - --> $DIR/type_complexity.rs:41:15 + --> $DIR/type_complexity.rs:42:15 | LL | fn test1() -> Vec>> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: very complex type used. Consider factoring parts into `type` definitions - --> $DIR/type_complexity.rs:45:14 + --> $DIR/type_complexity.rs:46:14 | LL | fn test2(_x: Vec>>) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: very complex type used. Consider factoring parts into `type` definitions - --> $DIR/type_complexity.rs:48:13 + --> $DIR/type_complexity.rs:49:13 | LL | let _y: Vec>> = vec![]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 17 previous errors +error: aborting due to 15 previous errors