Skip to content

Commit

Permalink
Auto merge of #11639 - y21:issue11635, r=llogiq
Browse files Browse the repository at this point in the history
[`into_iter_without_iter`]: walk up deref impl chain to find `iter` methods

Fixes #11635

changelog: [`into_iter_without_iter`]: walk up deref impl chain to find `iter` methods
  • Loading branch information
bors committed Oct 7, 2023
2 parents 3662bd8 + 1c6fa29 commit 7624045
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 3 deletions.
25 changes: 22 additions & 3 deletions clippy_lints/src/iter_without_into_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{self, Ty};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{sym, Symbol};
use std::iter;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -52,7 +53,8 @@ declare_clippy_lint! {
declare_clippy_lint! {
/// ### What it does
/// This is the opposite of the `iter_without_into_iter` lint.
/// It looks for `IntoIterator for (&|&mut) Type` implementations without an inherent `iter` or `iter_mut` method.
/// It looks for `IntoIterator for (&|&mut) Type` implementations without an inherent `iter` or `iter_mut` method
/// on the type or on any of the types in its `Deref` chain.
///
/// ### Why is this bad?
/// It's not bad, but having them is idiomatic and allows the type to be used in iterator chains
Expand Down Expand Up @@ -102,7 +104,20 @@ fn is_nameable_in_impl_trait(ty: &rustc_hir::Ty<'_>) -> bool {
!matches!(ty.kind, TyKind::OpaqueDef(..))
}

fn type_has_inherent_method(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> bool {
/// Returns the deref chain of a type, starting with the type itself.
fn deref_chain<'cx, 'tcx>(cx: &'cx LateContext<'tcx>, ty: Ty<'tcx>) -> impl Iterator<Item = Ty<'tcx>> + 'cx {
iter::successors(Some(ty), |&ty| {
if let Some(deref_did) = cx.tcx.lang_items().deref_trait()
&& implements_trait(cx, ty, deref_did, &[])
{
make_normalized_projection(cx.tcx, cx.param_env, deref_did, sym::Target, [ty])
} else {
None
}
})
}

fn adt_has_inherent_method(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> bool {
if let Some(ty_did) = ty.ty_adt_def().map(ty::AdtDef::did) {
cx.tcx.inherent_impls(ty_did).iter().any(|&did| {
cx.tcx
Expand All @@ -127,7 +142,11 @@ impl LateLintPass<'_> for IterWithoutIntoIter {
Mutability::Mut => sym::iter_mut,
Mutability::Not => sym::iter,
}
&& !type_has_inherent_method(cx, ty, expected_method_name)
&& !deref_chain(cx, ty)
.any(|ty| {
// We can't check inherent impls for slices, but we know that they have an `iter(_mut)` method
ty.peel_refs().is_slice() || adt_has_inherent_method(cx, ty, expected_method_name)
})
&& let Some(iter_assoc_span) = imp.items.iter().find_map(|item| {
if item.ident.name == sym!(IntoIter) {
Some(cx.tcx.hir().impl_item(item.id).expect_type().span)
Expand Down
34 changes: 34 additions & 0 deletions tests/ui/into_iter_without_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,37 @@ fn main() {
}
}
}

fn issue11635() {
// A little more involved than the original repro in the issue, but this tests that it correctly
// works for more than one deref step

use std::ops::Deref;

pub struct Thing(Vec<u8>);
pub struct Thing2(Thing);

impl Deref for Thing {
type Target = [u8];

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl Deref for Thing2 {
type Target = Thing;
fn deref(&self) -> &Self::Target {
&self.0
}
}

impl<'a> IntoIterator for &'a Thing2 {
type Item = &'a u8;
type IntoIter = <&'a [u8] as IntoIterator>::IntoIter;

fn into_iter(self) -> Self::IntoIter {
self.0.iter()
}
}
}

0 comments on commit 7624045

Please sign in to comment.