Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bad explicit_into_iter_loop suggestion #4978

Merged
merged 1 commit into from Jan 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 15 additions & 16 deletions clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down
37 changes: 37 additions & 0 deletions tests/ui/for_loop_fixable.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
}
}
37 changes: 37 additions & 0 deletions tests/ui/for_loop_fixable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
}
}
20 changes: 19 additions & 1 deletion tests/ui/for_loop_fixable.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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