useless_conversion: don't lint if ty param has multiple bounds

clippy_lints/src/useless_conversion.rs

@@ -1,15 +1,19 @@
+use std::ops::ControlFlow;
+
 use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_context};
 use clippy_utils::sugg::Sugg;
 use clippy_utils::ty::{is_copy, is_type_diagnostic_item, same_type_and_consts};
-use clippy_utils::{get_parent_expr, is_trait_method, is_ty_alias, match_def_path, path_to_local, paths};
+use clippy_utils::{
+    get_parent_expr, is_diag_trait_item, is_trait_method, is_ty_alias, match_def_path, path_to_local, paths,
+};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::DefId;
 use rustc_hir::{BindingAnnotation, Expr, ExprKind, HirId, MatchSource, Node, PatKind};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty;
+use rustc_middle::ty::{self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::{sym, Span};
 
@@ -61,22 +65,83 @@ impl MethodOrFunction {
     }
 }
 
-/// Returns the span of the `IntoIterator` trait bound in the function pointed to by `fn_did`
-fn into_iter_bound(cx: &LateContext<'_>, fn_did: DefId, into_iter_did: DefId, param_index: u32) -> Option<Span> {
-    cx.tcx
-        .predicates_of(fn_did)
-        .predicates
-        .iter()
-        .find_map(|&(ref pred, span)| {
+/// Returns the span of the `IntoIterator` trait bound in the function pointed to by `fn_did`,
+/// iff the `IntoIterator` bound is the only bound on the type parameter.
+///
+/// This last part is important because it might be that the type the user is calling `.into_iter()`
+/// on might not satisfy those other bounds and would result in compile errors:
+/// ```ignore
+/// pub fn foo<I>(i: I)
+/// where I: IntoIterator<Item=i32> + ExactSizeIterator
+///                                   ^^^^^^^^^^^^^^^^^ this extra bound stops us from suggesting to remove `.into_iter()` ...
+/// {
+///     assert_eq!(i.len(), 3);
+/// }
+///
+/// pub fn bar() {
+///     foo([1, 2, 3].into_iter());
+///                  ^^^^^^^^^^^^ ... here, because `[i32; 3]` is not `ExactSizeIterator`
+/// }
+/// ```
+fn exclusive_into_iter_bound(
+    cx: &LateContext<'_>,
+    fn_did: DefId,
+    into_iter_did: DefId,
+    param_index: u32,
+) -> Option<Span> {
+    #[derive(Clone)]
+    struct ExplicitlyUsedTyParam<'a, 'tcx> {
+        cx: &'a LateContext<'tcx>,
+        param_index: u32,
+    }
+
+    impl<'a, 'tcx> TypeVisitor<TyCtxt<'tcx>> for ExplicitlyUsedTyParam<'a, 'tcx> {
+        type BreakTy = ();
+        fn visit_predicate(&mut self, p: ty::Predicate<'tcx>) -> ControlFlow<Self::BreakTy> {
+            // Ignore implicit `T: Sized` bound
+            if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(tr)) = p.kind().skip_binder()
+                && let Some(sized_trait_did) = self.cx.tcx.lang_items().sized_trait()
+                && sized_trait_did == tr.def_id()
+            {
+                return ControlFlow::Continue(());
+            }
+
+            // Ignore `<T as IntoIterator>::Item` projection, this use of the ty param specifically is fine
+            // because it's what we're already looking for
+            if let ty::PredicateKind::Clause(ty::ClauseKind::Projection(proj)) = p.kind().skip_binder()
+                && is_diag_trait_item(self.cx,proj.projection_ty.def_id, sym::IntoIterator)
+            {
+                return ControlFlow::Continue(());
+            }
+
+            p.super_visit_with(self)
+        }
+
+        fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
+            if t.is_param(self.param_index) {
+                ControlFlow::Break(())
+            } else {
+                ControlFlow::Continue(())
+            }
+        }
+    }
+
+    let mut into_iter_span = None;
+
+    for (pred, span) in cx.tcx.explicit_predicates_of(fn_did).predicates {
         if let ty::ClauseKind::Trait(tr) = pred.kind().skip_binder()
             && tr.def_id() == into_iter_did
             && tr.self_ty().is_param(param_index)
         {
-                Some(span)
-            } else {
-                None
+            into_iter_span = Some(*span);
+        } else if pred.visit_with(&mut ExplicitlyUsedTyParam { cx, param_index }).is_break() {
+            // Found another reference of the type parameter; conservatively assume
+            // that we can't remove the bound.
+            return None;
         }
-        })
+    }
+
+    into_iter_span
 }
 
 /// Extracts the receiver of a `.into_iter()` method call.
@@ -175,7 +240,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
                             && let Some(arg_pos) = args.iter().position(|x| x.hir_id == e.hir_id)
                             && let Some(&into_iter_param) = sig.inputs().get(kind.param_pos(arg_pos))
                             && let ty::Param(param) = into_iter_param.kind()
-                            && let Some(span) = into_iter_bound(cx, parent_fn_did, into_iter_did, param.index)
+                            && let Some(span) = exclusive_into_iter_bound(cx, parent_fn_did, into_iter_did, param.index)
                             && self.expn_depth == 0
                         {
                             // Get the "innermost" `.into_iter()` call, e.g. given this expression:

tests/ui/useless_conversion.fixed

@@ -151,6 +151,8 @@ fn main() {
     let _ = s3;
     let s4: Foo<'a'> = Foo;
     let _ = vec![s4, s4, s4].into_iter();
+
+    issue11300::bar();
 }
 
 #[allow(dead_code)]
@@ -196,6 +198,22 @@ fn explicit_into_iter_fn_arg() {
     b(macro_generated!());
 }
 
+mod issue11300 {
+    pub fn foo<I>(i: I)
+    where
+        I: IntoIterator<Item = i32> + ExactSizeIterator,
+    {
+        assert_eq!(i.len(), 3);
+    }
+
+    pub fn bar() {
+        // This should not trigger the lint:
+        // `[i32, 3]` does not satisfy the `ExactSizeIterator` bound, so the into_iter call cannot be
+        // removed and is not useless.
+        foo([1, 2, 3].into_iter());
+    }
+}
+
 #[derive(Copy, Clone)]
 struct Foo<const C: char>;
 

tests/ui/useless_conversion.rs

@@ -151,6 +151,8 @@ fn main() {
     let _ = Foo::<'a'>::from(s3);
     let s4: Foo<'a'> = Foo;
     let _ = vec![s4, s4, s4].into_iter().into_iter();
+
+    issue11300::bar();
 }
 
 #[allow(dead_code)]
@@ -196,6 +198,22 @@ fn explicit_into_iter_fn_arg() {
     b(macro_generated!());
 }
 
+mod issue11300 {
+    pub fn foo<I>(i: I)
+    where
+        I: IntoIterator<Item = i32> + ExactSizeIterator,
+    {
+        assert_eq!(i.len(), 3);
+    }
+
+    pub fn bar() {
+        // This should not trigger the lint:
+        // `[i32, 3]` does not satisfy the `ExactSizeIterator` bound, so the into_iter call cannot be
+        // removed and is not useless.
+        foo([1, 2, 3].into_iter());
+    }
+}
+
 #[derive(Copy, Clone)]
 struct Foo<const C: char>;
 

tests/ui/useless_conversion.stderr

@@ -119,61 +119,61 @@ LL |     let _ = vec![s4, s4, s4].into_iter().into_iter();
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![s4, s4, s4].into_iter()`
 
 error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
-  --> $DIR/useless_conversion.rs:183:7
+  --> $DIR/useless_conversion.rs:185:7
    |
 LL |     b(vec![1, 2].into_iter());
    |       ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `vec![1, 2]`
    |
 note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
-  --> $DIR/useless_conversion.rs:173:13
+  --> $DIR/useless_conversion.rs:175:13
    |
 LL |     fn b<T: IntoIterator<Item = i32>>(_: T) {}
    |             ^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
-  --> $DIR/useless_conversion.rs:184:7
+  --> $DIR/useless_conversion.rs:186:7
    |
 LL |     c(vec![1, 2].into_iter());
    |       ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `vec![1, 2]`
    |
 note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
-  --> $DIR/useless_conversion.rs:174:18
+  --> $DIR/useless_conversion.rs:176:18
    |
 LL |     fn c(_: impl IntoIterator<Item = i32>) {}
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
-  --> $DIR/useless_conversion.rs:185:7
+  --> $DIR/useless_conversion.rs:187:7
    |
 LL |     d(vec![1, 2].into_iter());
    |       ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `vec![1, 2]`
    |
 note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
-  --> $DIR/useless_conversion.rs:177:12
+  --> $DIR/useless_conversion.rs:179:12
    |
 LL |         T: IntoIterator<Item = i32>,
    |            ^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
-  --> $DIR/useless_conversion.rs:188:7
+  --> $DIR/useless_conversion.rs:190:7
    |
 LL |     b(vec![1, 2].into_iter().into_iter());
    |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`s: `vec![1, 2]`
    |
 note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
-  --> $DIR/useless_conversion.rs:173:13
+  --> $DIR/useless_conversion.rs:175:13
    |
 LL |     fn b<T: IntoIterator<Item = i32>>(_: T) {}
    |             ^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
-  --> $DIR/useless_conversion.rs:189:7
+  --> $DIR/useless_conversion.rs:191:7
    |
 LL |     b(vec![1, 2].into_iter().into_iter().into_iter());
    |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`s: `vec![1, 2]`
    |
 note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
-  --> $DIR/useless_conversion.rs:173:13
+  --> $DIR/useless_conversion.rs:175:13
    |
 LL |     fn b<T: IntoIterator<Item = i32>>(_: T) {}
    |             ^^^^^^^^^^^^^^^^^^^^^^^^