diff options
| -rw-r--r-- | clippy_lints/src/useless_conversion.rs | 97 | ||||
| -rw-r--r-- | tests/ui/useless_conversion.fixed | 18 | ||||
| -rw-r--r-- | tests/ui/useless_conversion.rs | 18 | ||||
| -rw-r--r-- | tests/ui/useless_conversion.stderr | 20 |
4 files changed, 127 insertions, 26 deletions
diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs index 5ac4f0aa46c..05acf034a95 100644 --- a/clippy_lints/src/useless_conversion.rs +++ b/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)| { - if let ty::ClauseKind::Trait(tr) = pred.kind().skip_binder() - && tr.def_id() == into_iter_did - && tr.self_ty().is_param(param_index) +/// 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() { - Some(span) + 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 { - None + 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) + { + 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: diff --git a/tests/ui/useless_conversion.fixed b/tests/ui/useless_conversion.fixed index 52591959905..13463e1782d 100644 --- a/tests/ui/useless_conversion.fixed +++ b/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>; diff --git a/tests/ui/useless_conversion.rs b/tests/ui/useless_conversion.rs index befb2f9a5c3..06f3160b0f3 100644 --- a/tests/ui/useless_conversion.rs +++ b/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>; diff --git a/tests/ui/useless_conversion.stderr b/tests/ui/useless_conversion.stderr index 28e7bb61098..e7466e2fbdb 100644 --- a/tests/ui/useless_conversion.stderr +++ b/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) {} | ^^^^^^^^^^^^^^^^^^^^^^^^ |