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

Special case iterator chain checks for suggestion #116717

Merged
merged 2 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
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`
estebank marked this conversation as resolved.
Show resolved Hide resolved
// 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
Loading