diff options
| -rw-r--r-- | clippy_lints/src/useless_conversion.rs | 121 | ||||
| -rw-r--r-- | tests/ui/useless_conversion.fixed | 32 | ||||
| -rw-r--r-- | tests/ui/useless_conversion.rs | 32 | ||||
| -rw-r--r-- | tests/ui/useless_conversion.stderr | 26 |
4 files changed, 148 insertions, 63 deletions
diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs index 05acf034a95..5afb1f76df8 100644 --- a/clippy_lints/src/useless_conversion.rs +++ b/clippy_lints/src/useless_conversion.rs @@ -1,21 +1,21 @@ -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_diag_trait_item, is_trait_method, is_ty_alias, match_def_path, path_to_local, paths, -}; +use clippy_utils::{get_parent_expr, 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_infer::infer::TyCtxtInferExt; +use rustc_infer::traits::Obligation; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::{self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor}; +use rustc_middle::traits::ObligationCause; +use rustc_middle::ty::{self, EarlyBinder, GenericArg, GenericArgsRef, Ty}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::{sym, Span}; +use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; declare_clippy_lint! { /// ### What it does @@ -66,10 +66,7 @@ impl MethodOrFunction { } /// 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: +/// iff all of the bounds also hold for the type of the `.into_iter()` receiver. /// ```ignore /// pub fn foo<I>(i: I) /// where I: IntoIterator<Item=i32> + ExactSizeIterator @@ -83,61 +80,42 @@ impl MethodOrFunction { /// ^^^^^^^^^^^^ ... here, because `[i32; 3]` is not `ExactSizeIterator` /// } /// ``` -fn exclusive_into_iter_bound( - cx: &LateContext<'_>, +fn into_iter_bound<'tcx>( + cx: &LateContext<'tcx>, fn_did: DefId, into_iter_did: DefId, + into_iter_receiver: Ty<'tcx>, param_index: u32, + node_args: GenericArgsRef<'tcx>, ) -> 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 param_env = cx.tcx.param_env(fn_did); 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; + if let ty::ClauseKind::Trait(tr) = pred.kind().skip_binder() { + if tr.def_id() == into_iter_did && tr.self_ty().is_param(param_index) { + into_iter_span = Some(*span); + } else { + // Substitute generics in the predicate and replace the IntoIterator type parameter with the + // `.into_iter()` receiver to see if the bound also holds for that type. + let args = cx.tcx.mk_args_from_iter(node_args.iter().enumerate().map(|(i, arg)| { + if i == param_index as usize { + GenericArg::from(into_iter_receiver) + } else { + arg + } + })); + let predicate = EarlyBinder::bind(tr).instantiate(cx.tcx, args); + let obligation = Obligation::new(cx.tcx, ObligationCause::dummy(), param_env, predicate); + if !cx + .tcx + .infer_ctxt() + .build() + .predicate_must_hold_modulo_regions(&obligation) + { + return None; + } + } } } @@ -225,22 +203,41 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { // `fn_sig` does not ICE. (see #11065) && cx.tcx.opt_def_kind(did).is_some_and(DefKind::is_fn_like) => { - Some((did, args, MethodOrFunction::Function)) + Some(( + did, + args, + cx.typeck_results().node_args(recv.hir_id), + MethodOrFunction::Function + )) } ExprKind::MethodCall(.., args, _) => { cx.typeck_results().type_dependent_def_id(parent.hir_id) - .map(|did| (did, args, MethodOrFunction::Method)) + .map(|did| { + return ( + did, + args, + cx.typeck_results().node_args(recv.hir_id), + MethodOrFunction::Method + ); + }) } _ => None, }; - if let Some((parent_fn_did, args, kind)) = parent_fn + if let Some((parent_fn_did, args, node_args, kind)) = parent_fn && let Some(into_iter_did) = cx.tcx.get_diagnostic_item(sym::IntoIterator) && let sig = cx.tcx.fn_sig(parent_fn_did).skip_binder().skip_binder() && 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) = exclusive_into_iter_bound(cx, parent_fn_did, into_iter_did, param.index) + && let Some(span) = into_iter_bound( + cx, + parent_fn_did, + into_iter_did, + cx.typeck_results().expr_ty(into_iter_recv), + param.index, + node_args + ) && 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 13463e1782d..f3504bc62b4 100644 --- a/tests/ui/useless_conversion.fixed +++ b/tests/ui/useless_conversion.fixed @@ -206,11 +206,43 @@ mod issue11300 { assert_eq!(i.len(), 3); } + trait Helper<T: ?Sized> {} + impl Helper<i32> for [i32; 3] {} + impl Helper<i32> for std::array::IntoIter<i32, 3> {} + impl Helper<()> for std::array::IntoIter<i32, 3> {} + + fn foo2<X: ?Sized, I>(_: I) + where + I: IntoIterator<Item = i32> + Helper<X>, + { + } + + trait Helper2<T> {} + impl Helper2<std::array::IntoIter<i32, 3>> for i32 {} + impl Helper2<[i32; 3]> for i32 {} + fn foo3<I>(_: I) + where + I: IntoIterator<Item = i32>, + i32: Helper2<I>, + { + } + 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()); + + // This should trigger the lint, receiver type [i32; 3] also implements `Helper` + foo2::<i32, _>([1, 2, 3]); + + // This again should *not* lint, since X = () and I = std::array::IntoIter<i32, 3>, + // and `[i32; 3]: Helper<()>` is not true (only `std::array::IntoIter<i32, 3>: Helper<()>` is). + foo2::<(), _>([1, 2, 3].into_iter()); + + // This should lint. Removing the `.into_iter()` means that `I` gets substituted with `[i32; 3]`, + // and `i32: Helper2<[i32, 3]>` is true, so this call is indeed unncessary. + foo3([1, 2, 3]); } } diff --git a/tests/ui/useless_conversion.rs b/tests/ui/useless_conversion.rs index 06f3160b0f3..a4f4722927f 100644 --- a/tests/ui/useless_conversion.rs +++ b/tests/ui/useless_conversion.rs @@ -206,11 +206,43 @@ mod issue11300 { assert_eq!(i.len(), 3); } + trait Helper<T: ?Sized> {} + impl Helper<i32> for [i32; 3] {} + impl Helper<i32> for std::array::IntoIter<i32, 3> {} + impl Helper<()> for std::array::IntoIter<i32, 3> {} + + fn foo2<X: ?Sized, I>(_: I) + where + I: IntoIterator<Item = i32> + Helper<X>, + { + } + + trait Helper2<T> {} + impl Helper2<std::array::IntoIter<i32, 3>> for i32 {} + impl Helper2<[i32; 3]> for i32 {} + fn foo3<I>(_: I) + where + I: IntoIterator<Item = i32>, + i32: Helper2<I>, + { + } + 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()); + + // This should trigger the lint, receiver type [i32; 3] also implements `Helper` + foo2::<i32, _>([1, 2, 3].into_iter()); + + // This again should *not* lint, since X = () and I = std::array::IntoIter<i32, 3>, + // and `[i32; 3]: Helper<()>` is not true (only `std::array::IntoIter<i32, 3>: Helper<()>` is). + foo2::<(), _>([1, 2, 3].into_iter()); + + // This should lint. Removing the `.into_iter()` means that `I` gets substituted with `[i32; 3]`, + // and `i32: Helper2<[i32, 3]>` is true, so this call is indeed unncessary. + foo3([1, 2, 3].into_iter()); } } diff --git a/tests/ui/useless_conversion.stderr b/tests/ui/useless_conversion.stderr index e7466e2fbdb..81a0898bdf6 100644 --- a/tests/ui/useless_conversion.stderr +++ b/tests/ui/useless_conversion.stderr @@ -178,5 +178,29 @@ note: this parameter accepts any `IntoIterator`, so you don't need to call `.int LL | fn b<T: IntoIterator<Item = i32>>(_: T) {} | ^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 24 previous errors +error: explicit call to `.into_iter()` in function argument accepting `IntoIterator` + --> $DIR/useless_conversion.rs:237:24 + | +LL | foo2::<i32, _>([1, 2, 3].into_iter()); + | ^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `[1, 2, 3]` + | +note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()` + --> $DIR/useless_conversion.rs:216:12 + | +LL | I: IntoIterator<Item = i32> + Helper<X>, + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: explicit call to `.into_iter()` in function argument accepting `IntoIterator` + --> $DIR/useless_conversion.rs:245:14 + | +LL | foo3([1, 2, 3].into_iter()); + | ^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `[1, 2, 3]` + | +note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()` + --> $DIR/useless_conversion.rs:225:12 + | +LL | I: IntoIterator<Item = i32>, + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 26 previous errors |