Skip to content

Commit

Permalink
Special case iterator chain checks for suggestion
Browse files Browse the repository at this point in the history
When encountering method call chains of `Iterator`, check for trailing
`;` in the body of closures passed into `Iterator::map`, as well as
calls to `<T as Clone>::clone` when `T` is a type param and `T: !Clone`.

Fix #9082.
  • Loading branch information
estebank committed Oct 14, 2023
1 parent df4379b commit 6d23ee8
Show file tree
Hide file tree
Showing 5 changed files with 358 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -3611,13 +3611,103 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
let mut prev_ty = self.resolve_vars_if_possible(
typeck_results.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(tcx)),
);
while let hir::ExprKind::MethodCall(_path_segment, rcvr_expr, _args, span) = expr.kind {
while let hir::ExprKind::MethodCall(path_segment, rcvr_expr, args, span) = expr.kind {
// Point at every method call in the chain with the resulting type.
// vec![1, 2, 3].iter().map(mapper).sum<i32>()
// ^^^^^^ ^^^^^^^^^^^
expr = rcvr_expr;
let assocs_in_this_method =
self.probe_assoc_types_at_expr(&type_diffs, span, prev_ty, expr.hir_id, param_env);
// Special case for iterator chains, we look at potential failures of `Iterator::Item`
// not being `: Clone` and `Iterator::map` calls with spurious trailing `;`.
for entry in &assocs_in_this_method {
let Some((_span, (def_id, ty))) = entry else {
continue;
};
for diff in &type_diffs {
let Sorts(expected_found) = diff else {
continue;
};
if tcx.is_diagnostic_item(sym::IteratorItem, *def_id)
&& path_segment.ident.name == sym::map
&& self.can_eq(param_env, expected_found.found, *ty)
&& let [arg] = args
&& let hir::ExprKind::Closure(closure) = arg.kind
{
let body = tcx.hir().body(closure.body);
if let hir::ExprKind::Block(block, None) = body.value.kind
&& let None = block.expr
&& let [.., stmt] = block.stmts
&& let hir::StmtKind::Semi(expr) = stmt.kind
// FIXME: actually check the expected vs found types, but right now
// the expected is a projection that we need to resolve.
// && let Some(tail_ty) = typeck_results.expr_ty_opt(expr)
&& expected_found.found.is_unit()
{
err.span_suggestion_verbose(
expr.span.shrink_to_hi().with_hi(stmt.span.hi()),
"consider removing this semicolon",
String::new(),
Applicability::MachineApplicable,
);
}
let expr = if let hir::ExprKind::Block(block, None) = body.value.kind
&& let Some(expr) = block.expr
{
expr
} else {
body.value
};
if let hir::ExprKind::MethodCall(path_segment, rcvr, [], span) = expr.kind
&& path_segment.ident.name == sym::clone
&& let Some(expr_ty) = typeck_results.expr_ty_opt(expr)
&& let Some(rcvr_ty) = typeck_results.expr_ty_opt(rcvr)
&& self.can_eq(param_env, expr_ty, rcvr_ty)
&& let ty::Ref(_, ty, _) = expr_ty.kind()
{
err.span_label(
span,
format!(
"this method call is cloning the reference `{expr_ty}`, not \
`{ty}` which doesn't implement `Clone`",
),
);
let ty::Param(..) = ty.kind() else {
continue;
};
let hir = tcx.hir();
let node = hir.get_by_def_id(hir.get_parent_item(expr.hir_id).def_id);

let pred = ty::Binder::dummy(ty::TraitPredicate {
trait_ref: ty::TraitRef::from_lang_item(
tcx,
LangItem::Clone,
span,
[*ty],
),
polarity: ty::ImplPolarity::Positive,
});
let Some(generics) = node.generics() else {
continue;
};
let Some(body_id) = node.body_id() else {
continue;
};
suggest_restriction(
tcx,
hir.body_owner_def_id(body_id),
&generics,
&format!("type parameter `{ty}`"),
err,
node.fn_sig(),
None,
pred,
None,
);
}
}
}
}
assocs.push(assocs_in_this_method);
prev_ty = self.resolve_vars_if_possible(
typeck_results.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(tcx)),
Expand Down
42 changes: 42 additions & 0 deletions tests/ui/iterators/invalid-iterator-chain-fixable.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// run-rustfix
use std::collections::hash_set::Iter;
use std::collections::HashSet;

fn iter_to_vec<'b, X>(i: Iter<'b, X>) -> Vec<X> where X: Clone {
let i = i.map(|x| x.clone());
i.collect() //~ ERROR E0277
}

fn main() {
let v = vec![(0, 0)];
let scores = v
.iter()
.map(|(a, b)| {
a + b
});
println!("{}", scores.sum::<i32>()); //~ ERROR E0277
println!(
"{}",
vec![0, 1]
.iter()
.map(|x| x * 2)
.map(|x| { x })
.map(|x| { x })
.sum::<i32>(), //~ ERROR E0277
);
println!("{}", vec![0, 1].iter().map(|x| { x }).sum::<i32>()); //~ ERROR E0277
let a = vec![0];
let b = a.into_iter();
let c = b.map(|x| x + 1);
let d = c.filter(|x| *x > 10 );
let e = d.map(|x| {
x + 1
});
let f = e.filter(|_| false);
let g: Vec<i32> = f.collect(); //~ ERROR E0277
println!("{g:?}");

let mut s = HashSet::new();
s.insert(1u8);
println!("{:?}", iter_to_vec(s.iter()));
}
42 changes: 42 additions & 0 deletions tests/ui/iterators/invalid-iterator-chain-fixable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// run-rustfix
use std::collections::hash_set::Iter;
use std::collections::HashSet;

fn iter_to_vec<'b, X>(i: Iter<'b, X>) -> Vec<X> {
let i = i.map(|x| x.clone());
i.collect() //~ ERROR E0277
}

fn main() {
let v = vec![(0, 0)];
let scores = v
.iter()
.map(|(a, b)| {
a + b;
});
println!("{}", scores.sum::<i32>()); //~ ERROR E0277
println!(
"{}",
vec![0, 1]
.iter()
.map(|x| x * 2)
.map(|x| { x; })
.map(|x| { x })
.sum::<i32>(), //~ ERROR E0277
);
println!("{}", vec![0, 1].iter().map(|x| { x; }).sum::<i32>()); //~ ERROR E0277
let a = vec![0];
let b = a.into_iter();
let c = b.map(|x| x + 1);
let d = c.filter(|x| *x > 10 );
let e = d.map(|x| {
x + 1;
});
let f = e.filter(|_| false);
let g: Vec<i32> = f.collect(); //~ ERROR E0277
println!("{g:?}");

let mut s = HashSet::new();
s.insert(1u8);
println!("{:?}", iter_to_vec(s.iter()));
}
157 changes: 157 additions & 0 deletions tests/ui/iterators/invalid-iterator-chain-fixable.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
error[E0277]: a value of type `Vec<X>` cannot be built from an iterator over elements of type `&X`
--> $DIR/invalid-iterator-chain-fixable.rs:7:7
|
LL | let i = i.map(|x| x.clone());
| ------- this method call is cloning the reference `&X`, not `X` which doesn't implement `Clone`
LL | i.collect()
| ^^^^^^^ value of type `Vec<X>` cannot be built from `std::iter::Iterator<Item=&X>`
|
= help: the trait `FromIterator<&X>` is not implemented for `Vec<X>`
= help: the trait `FromIterator<X>` is implemented for `Vec<X>`
= help: for that trait implementation, expected `X`, found `&X`
note: the method call chain might not have had the expected associated types
--> $DIR/invalid-iterator-chain-fixable.rs:5:26
|
LL | fn iter_to_vec<'b, X>(i: Iter<'b, X>) -> Vec<X> {
| ^^^^^^^^^^^ `Iterator::Item` is `&X` here
LL | let i = i.map(|x| x.clone());
| ------------------ `Iterator::Item` remains `&X` here
note: required by a bound in `collect`
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
help: consider further restricting type parameter `X`
|
LL | fn iter_to_vec<'b, X>(i: Iter<'b, X>) -> Vec<X> where X: Clone {
| ++++++++++++++

error[E0277]: a value of type `i32` cannot be made by summing an iterator over elements of type `()`
--> $DIR/invalid-iterator-chain-fixable.rs:17:33
|
LL | println!("{}", scores.sum::<i32>());
| --- ^^^ value of type `i32` cannot be made by summing a `std::iter::Iterator<Item=()>`
| |
| required by a bound introduced by this call
|
= help: the trait `Sum<()>` is not implemented for `i32`
= help: the following other types implement trait `Sum<A>`:
<i32 as Sum>
<i32 as Sum<&'a i32>>
note: the method call chain might not have had the expected associated types
--> $DIR/invalid-iterator-chain-fixable.rs:14:10
|
LL | let v = vec![(0, 0)];
| ------------ this expression has type `Vec<({integer}, {integer})>`
LL | let scores = v
LL | .iter()
| ------ `Iterator::Item` is `&({integer}, {integer})` here
LL | .map(|(a, b)| {
| __________^
LL | | a + b;
LL | | });
| |__________^ `Iterator::Item` changed to `()` here
note: required by a bound in `std::iter::Iterator::sum`
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
help: consider removing this semicolon
|
LL - a + b;
LL + a + b
|

error[E0277]: a value of type `i32` cannot be made by summing an iterator over elements of type `()`
--> $DIR/invalid-iterator-chain-fixable.rs:25:20
|
LL | .sum::<i32>(),
| --- ^^^ value of type `i32` cannot be made by summing a `std::iter::Iterator<Item=()>`
| |
| required by a bound introduced by this call
|
= help: the trait `Sum<()>` is not implemented for `i32`
= help: the following other types implement trait `Sum<A>`:
<i32 as Sum>
<i32 as Sum<&'a i32>>
note: the method call chain might not have had the expected associated types
--> $DIR/invalid-iterator-chain-fixable.rs:23:14
|
LL | vec![0, 1]
| ---------- this expression has type `Vec<{integer}>`
LL | .iter()
| ------ `Iterator::Item` is `&{integer}` here
LL | .map(|x| x * 2)
| -------------- `Iterator::Item` changed to `{integer}` here
LL | .map(|x| { x; })
| ^^^^^^^^^^^^^^^ `Iterator::Item` changed to `()` here
LL | .map(|x| { x })
| -------------- `Iterator::Item` remains `()` here
note: required by a bound in `std::iter::Iterator::sum`
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
help: consider removing this semicolon
|
LL - .map(|x| { x; })
LL + .map(|x| { x })
|

error[E0277]: a value of type `i32` cannot be made by summing an iterator over elements of type `()`
--> $DIR/invalid-iterator-chain-fixable.rs:27:60
|
LL | println!("{}", vec![0, 1].iter().map(|x| { x; }).sum::<i32>());
| --- ^^^ value of type `i32` cannot be made by summing a `std::iter::Iterator<Item=()>`
| |
| required by a bound introduced by this call
|
= help: the trait `Sum<()>` is not implemented for `i32`
= help: the following other types implement trait `Sum<A>`:
<i32 as Sum>
<i32 as Sum<&'a i32>>
note: the method call chain might not have had the expected associated types
--> $DIR/invalid-iterator-chain-fixable.rs:27:38
|
LL | println!("{}", vec![0, 1].iter().map(|x| { x; }).sum::<i32>());
| ---------- ------ ^^^^^^^^^^^^^^^ `Iterator::Item` changed to `()` here
| | |
| | `Iterator::Item` is `&{integer}` here
| this expression has type `Vec<{integer}>`
note: required by a bound in `std::iter::Iterator::sum`
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
help: consider removing this semicolon
|
LL - println!("{}", vec![0, 1].iter().map(|x| { x; }).sum::<i32>());
LL + println!("{}", vec![0, 1].iter().map(|x| { x }).sum::<i32>());
|

error[E0277]: a value of type `Vec<i32>` cannot be built from an iterator over elements of type `()`
--> $DIR/invalid-iterator-chain-fixable.rs:36:25
|
LL | let g: Vec<i32> = f.collect();
| ^^^^^^^ value of type `Vec<i32>` cannot be built from `std::iter::Iterator<Item=()>`
|
= help: the trait `FromIterator<()>` is not implemented for `Vec<i32>`
= help: the trait `FromIterator<i32>` is implemented for `Vec<i32>`
= help: for that trait implementation, expected `i32`, found `()`
note: the method call chain might not have had the expected associated types
--> $DIR/invalid-iterator-chain-fixable.rs:32:15
|
LL | let a = vec![0];
| ------- this expression has type `Vec<{integer}>`
LL | let b = a.into_iter();
| ----------- `Iterator::Item` is `{integer}` here
LL | let c = b.map(|x| x + 1);
| -------------- `Iterator::Item` remains `{integer}` here
LL | let d = c.filter(|x| *x > 10 );
| -------------------- `Iterator::Item` remains `{integer}` here
LL | let e = d.map(|x| {
| _______________^
LL | | x + 1;
LL | | });
| |______^ `Iterator::Item` changed to `()` here
LL | let f = e.filter(|_| false);
| ----------------- `Iterator::Item` remains `()` here
note: required by a bound in `collect`
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
help: consider removing this semicolon
|
LL - x + 1;
LL + x + 1
|

error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0277`.
Loading

0 comments on commit 6d23ee8

Please sign in to comment.