From 2cc38a23227e743aa24b7656ccbcee768ebef5ac Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Wed, 5 Jan 2022 13:46:35 -0500 Subject: [PATCH 1/2] Lint `iter_not_returning_iterator` on the trait definition rather than the implementation --- .../src/iter_not_returning_iterator.rs | 61 +++++++++++++------ tests/ui/iter_not_returning_iterator.rs | 12 ++++ tests/ui/iter_not_returning_iterator.stderr | 8 ++- 3 files changed, 60 insertions(+), 21 deletions(-) diff --git a/clippy_lints/src/iter_not_returning_iterator.rs b/clippy_lints/src/iter_not_returning_iterator.rs index 0af6b3b7d46..91d3c00039a 100644 --- a/clippy_lints/src/iter_not_returning_iterator.rs +++ b/clippy_lints/src/iter_not_returning_iterator.rs @@ -1,8 +1,7 @@ -use clippy_utils::{diagnostics::span_lint, return_ty, ty::implements_trait}; -use rustc_hir::{ImplItem, ImplItemKind}; +use clippy_utils::{diagnostics::span_lint, get_parent_node, ty::implements_trait}; +use rustc_hir::{def_id::LocalDefId, FnSig, ImplItem, ImplItemKind, Item, ItemKind, Node, TraitItem, TraitItemKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::symbol::kw; use rustc_span::symbol::sym; declare_clippy_lint! { @@ -41,25 +40,47 @@ declare_clippy_lint! { declare_lint_pass!(IterNotReturningIterator => [ITER_NOT_RETURNING_ITERATOR]); impl LateLintPass<'_> for IterNotReturningIterator { - fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'tcx>) { - let name = impl_item.ident.name.as_str(); - if_chain! { - if let ImplItemKind::Fn(fn_sig, _) = &impl_item.kind; - let ret_ty = return_ty(cx, impl_item.hir_id()); - if matches!(name, "iter" | "iter_mut"); - if let [param] = cx.tcx.fn_arg_names(impl_item.def_id); - if param.name == kw::SelfLower; - if let Some(iter_trait_id) = cx.tcx.get_diagnostic_item(sym::Iterator); - if !implements_trait(cx, ret_ty, iter_trait_id, &[]); + fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) { + let name = item.ident.name.as_str(); + if matches!(name, "iter" | "iter_mut") { + if let TraitItemKind::Fn(fn_sig, _) = &item.kind { + check_sig(cx, name, fn_sig, item.def_id); + } + } + } - then { - span_lint( - cx, - ITER_NOT_RETURNING_ITERATOR, - fn_sig.span, - &format!("this method is named `{}` but its return type does not implement `Iterator`", name), - ); + fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'tcx>) { + let name = item.ident.name.as_str(); + if matches!(name, "iter" | "iter_mut") + && !matches!( + get_parent_node(cx.tcx, item.hir_id()), + Some(Node::Item(Item { kind: ItemKind::Impl(i), .. })) if i.of_trait.is_some() + ) + { + if let ImplItemKind::Fn(fn_sig, _) = &item.kind { + check_sig(cx, name, fn_sig, item.def_id); } } } } + +fn check_sig(cx: &LateContext<'_>, name: &str, sig: &FnSig<'_>, fn_id: LocalDefId) { + if sig.decl.implicit_self.has_implicit_self() { + let ret_ty = cx.tcx.fn_sig(fn_id).skip_binder().output(); + if cx + .tcx + .get_diagnostic_item(sym::Iterator) + .map_or(false, |iter_id| !implements_trait(cx, ret_ty, iter_id, &[])) + { + span_lint( + cx, + ITER_NOT_RETURNING_ITERATOR, + sig.span, + &format!( + "this method is named `{}` but its return type does not implement `Iterator`", + name + ), + ); + } + } +} diff --git a/tests/ui/iter_not_returning_iterator.rs b/tests/ui/iter_not_returning_iterator.rs index 377f760b3c4..f5ee044576c 100644 --- a/tests/ui/iter_not_returning_iterator.rs +++ b/tests/ui/iter_not_returning_iterator.rs @@ -44,4 +44,16 @@ impl Iterator for Counter { } } +trait Iter { + type I; + fn iter(&self) -> Self::I; +} + +impl Iter for () { + type I = core::slice::Iter<'static, ()>; + fn iter(&self) -> Self::I { + [].iter() + } +} + fn main() {} diff --git a/tests/ui/iter_not_returning_iterator.stderr b/tests/ui/iter_not_returning_iterator.stderr index 2273cd0be66..ddb2b88d5f9 100644 --- a/tests/ui/iter_not_returning_iterator.stderr +++ b/tests/ui/iter_not_returning_iterator.stderr @@ -12,5 +12,11 @@ error: this method is named `iter_mut` but its return type does not implement `I LL | fn iter_mut(&self) -> Counter2 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 2 previous errors +error: this method is named `iter` but its return type does not implement `Iterator` + --> $DIR/iter_not_returning_iterator.rs:49:5 + | +LL | fn iter(&self) -> Self::I; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors From d98339d3e002b8b1bddf225e98c3140d7f1cecc7 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Wed, 5 Jan 2022 13:57:45 -0500 Subject: [PATCH 2/2] Handle type projections in `iter_not_returning_iterator` --- clippy_lints/src/iter_not_returning_iterator.rs | 4 ++++ tests/ui/iter_not_returning_iterator.rs | 8 ++++++++ tests/ui/iter_not_returning_iterator.stderr | 2 +- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/iter_not_returning_iterator.rs b/clippy_lints/src/iter_not_returning_iterator.rs index 91d3c00039a..017a8a779d9 100644 --- a/clippy_lints/src/iter_not_returning_iterator.rs +++ b/clippy_lints/src/iter_not_returning_iterator.rs @@ -67,6 +67,10 @@ impl LateLintPass<'_> for IterNotReturningIterator { fn check_sig(cx: &LateContext<'_>, name: &str, sig: &FnSig<'_>, fn_id: LocalDefId) { if sig.decl.implicit_self.has_implicit_self() { let ret_ty = cx.tcx.fn_sig(fn_id).skip_binder().output(); + let ret_ty = cx + .tcx + .try_normalize_erasing_regions(cx.param_env, ret_ty) + .unwrap_or(ret_ty); if cx .tcx .get_diagnostic_item(sym::Iterator) diff --git a/tests/ui/iter_not_returning_iterator.rs b/tests/ui/iter_not_returning_iterator.rs index f5ee044576c..2c91e02e842 100644 --- a/tests/ui/iter_not_returning_iterator.rs +++ b/tests/ui/iter_not_returning_iterator.rs @@ -44,6 +44,7 @@ impl Iterator for Counter { } } +// Issue #8225 trait Iter { type I; fn iter(&self) -> Self::I; @@ -56,4 +57,11 @@ impl Iter for () { } } +struct S; +impl S { + fn iter(&self) -> <() as Iter>::I { + ().iter() + } +} + fn main() {} diff --git a/tests/ui/iter_not_returning_iterator.stderr b/tests/ui/iter_not_returning_iterator.stderr index ddb2b88d5f9..44f02955836 100644 --- a/tests/ui/iter_not_returning_iterator.stderr +++ b/tests/ui/iter_not_returning_iterator.stderr @@ -13,7 +13,7 @@ LL | fn iter_mut(&self) -> Counter2 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this method is named `iter` but its return type does not implement `Iterator` - --> $DIR/iter_not_returning_iterator.rs:49:5 + --> $DIR/iter_not_returning_iterator.rs:50:5 | LL | fn iter(&self) -> Self::I; | ^^^^^^^^^^^^^^^^^^^^^^^^^^