diff options
| -rw-r--r-- | clippy_lints/src/loops.rs | 31 | ||||
| -rw-r--r-- | tests/ui/for_loop_fixable.fixed | 37 | ||||
| -rw-r--r-- | tests/ui/for_loop_fixable.rs | 37 | ||||
| -rw-r--r-- | tests/ui/for_loop_fixable.stderr | 20 |
4 files changed, 108 insertions, 17 deletions
diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index c4061a3e330..5ce35fe89e7 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -12,8 +12,7 @@ use rustc_session::declare_tool_lint; // use rustc::middle::region::CodeExtent; use crate::consts::{constant, Constant}; use crate::utils::usage::mutated_variables; -use crate::utils::{is_type_diagnostic_item, qpath_res, sext, sugg}; -use rustc::ty::subst::Subst; +use crate::utils::{is_type_diagnostic_item, qpath_res, same_tys, sext, sugg}; use rustc::ty::{self, Ty}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::Applicability; @@ -1344,20 +1343,9 @@ fn check_for_loop_arg(cx: &LateContext<'_, '_>, pat: &Pat<'_>, arg: &Expr<'_>, e lint_iter_method(cx, args, arg, method_name); } } else if method_name == "into_iter" && match_trait_method(cx, arg, &paths::INTO_ITERATOR) { - let def_id = cx.tables.type_dependent_def_id(arg.hir_id).unwrap(); - let substs = cx.tables.node_substs(arg.hir_id); - let method_type = cx.tcx.type_of(def_id).subst(cx.tcx, substs); - - let fn_arg_tys = method_type.fn_sig(cx.tcx).inputs(); - assert_eq!(fn_arg_tys.skip_binder().len(), 1); - if fn_arg_tys.skip_binder()[0].is_region_ptr() { - match cx.tables.expr_ty(&args[0]).kind { - // If the length is greater than 32 no traits are implemented for array and - // therefore we cannot use `&`. - ty::Array(_, size) if size.eval_usize(cx.tcx, cx.param_env) > 32 => {}, - _ => lint_iter_method(cx, args, arg, method_name), - }; - } else { + let receiver_ty = cx.tables.expr_ty(&args[0]); + let receiver_ty_adjusted = cx.tables.expr_ty_adjusted(&args[0]); + if same_tys(cx, receiver_ty, receiver_ty_adjusted) { let mut applicability = Applicability::MachineApplicable; let object = snippet_with_applicability(cx, args[0].span, "_", &mut applicability); span_lint_and_sugg( @@ -1370,6 +1358,17 @@ fn check_for_loop_arg(cx: &LateContext<'_, '_>, pat: &Pat<'_>, arg: &Expr<'_>, e object.to_string(), applicability, ); + } else { + let ref_receiver_ty = cx.tcx.mk_ref( + cx.tcx.lifetimes.re_erased, + ty::TypeAndMut { + ty: receiver_ty, + mutbl: Mutability::Not, + }, + ); + if same_tys(cx, receiver_ty_adjusted, ref_receiver_ty) { + lint_iter_method(cx, args, arg, method_name) + } } } else if method_name == "next" && match_trait_method(cx, arg, &paths::ITERATOR) { span_lint( diff --git a/tests/ui/for_loop_fixable.fixed b/tests/ui/for_loop_fixable.fixed index ec5ff1aeeef..6717899ed09 100644 --- a/tests/ui/for_loop_fixable.fixed +++ b/tests/ui/for_loop_fixable.fixed @@ -299,3 +299,40 @@ mod issue_2496 { unimplemented!() } } + +// explicit_into_iter_loop bad suggestions +#[warn(clippy::explicit_into_iter_loop, clippy::explicit_iter_loop)] +mod issue_4958 { + fn takes_iterator<T>(iterator: &T) + where + for<'a> &'a T: IntoIterator<Item = &'a String>, + { + for i in iterator { + println!("{}", i); + } + } + + struct T; + impl IntoIterator for &T { + type Item = (); + type IntoIter = std::vec::IntoIter<Self::Item>; + fn into_iter(self) -> Self::IntoIter { + vec![].into_iter() + } + } + + fn more_tests() { + let t = T; + let r = &t; + let rr = &&t; + + // This case is handled by `explicit_iter_loop`. No idea why. + for _ in &t {} + + for _ in r {} + + // No suggestion for this. + // We'd have to suggest `for _ in *rr {}` which is less clear. + for _ in rr.into_iter() {} + } +} diff --git a/tests/ui/for_loop_fixable.rs b/tests/ui/for_loop_fixable.rs index 2f42ea3ca41..7c08d383420 100644 --- a/tests/ui/for_loop_fixable.rs +++ b/tests/ui/for_loop_fixable.rs @@ -299,3 +299,40 @@ mod issue_2496 { unimplemented!() } } + +// explicit_into_iter_loop bad suggestions +#[warn(clippy::explicit_into_iter_loop, clippy::explicit_iter_loop)] +mod issue_4958 { + fn takes_iterator<T>(iterator: &T) + where + for<'a> &'a T: IntoIterator<Item = &'a String>, + { + for i in iterator.into_iter() { + println!("{}", i); + } + } + + struct T; + impl IntoIterator for &T { + type Item = (); + type IntoIter = std::vec::IntoIter<Self::Item>; + fn into_iter(self) -> Self::IntoIter { + vec![].into_iter() + } + } + + fn more_tests() { + let t = T; + let r = &t; + let rr = &&t; + + // This case is handled by `explicit_iter_loop`. No idea why. + for _ in t.into_iter() {} + + for _ in r.into_iter() {} + + // No suggestion for this. + // We'd have to suggest `for _ in *rr {}` which is less clear. + for _ in rr.into_iter() {} + } +} diff --git a/tests/ui/for_loop_fixable.stderr b/tests/ui/for_loop_fixable.stderr index 485ba1ee7b3..71a2334170e 100644 --- a/tests/ui/for_loop_fixable.stderr +++ b/tests/ui/for_loop_fixable.stderr @@ -130,5 +130,23 @@ error: it is more concise to loop over references to containers instead of using LL | for _v in bs.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&bs` -error: aborting due to 17 previous errors +error: it is more concise to loop over containers instead of using explicit iteration methods` + --> $DIR/for_loop_fixable.rs:310:18 + | +LL | for i in iterator.into_iter() { + | ^^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `iterator` + +error: it is more concise to loop over references to containers instead of using explicit iteration methods + --> $DIR/for_loop_fixable.rs:330:18 + | +LL | for _ in t.into_iter() {} + | ^^^^^^^^^^^^^ help: to write this more concisely, try: `&t` + +error: it is more concise to loop over containers instead of using explicit iteration methods` + --> $DIR/for_loop_fixable.rs:332:18 + | +LL | for _ in r.into_iter() {} + | ^^^^^^^^^^^^^ help: to write this more concisely, try: `r` + +error: aborting due to 20 previous errors |