Skip to content

Commit 6d23ee8

Browse files
estebankestebank
committed
Special case iterator chain checks for suggestion
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 rust-lang#9082.
1 parent df4379b commit 6d23ee8

File tree

5 files changed

+358
-1
lines changed

5 files changed

+358
-1
lines changed

‎compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

+91-1
Original file line numberDiff line numberDiff line change
@@ -3611,13 +3611,103 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
36113611
let mut prev_ty = self.resolve_vars_if_possible(
36123612
typeck_results.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(tcx)),
36133613
);
3614-
while let hir::ExprKind::MethodCall(_path_segment, rcvr_expr, _args, span) = expr.kind {
3614+
while let hir::ExprKind::MethodCall(path_segment, rcvr_expr, args, span) = expr.kind {
36153615
// Point at every method call in the chain with the resulting type.
36163616
// vec![1, 2, 3].iter().map(mapper).sum<i32>()
36173617
// ^^^^^^ ^^^^^^^^^^^
36183618
expr = rcvr_expr;
36193619
let assocs_in_this_method =
36203620
self.probe_assoc_types_at_expr(&type_diffs, span, prev_ty, expr.hir_id, param_env);
3621+
// Special case for iterator chains, we look at potential failures of `Iterator::Item`
3622+
// not being `: Clone` and `Iterator::map` calls with spurious trailing `;`.
3623+
for entry in &assocs_in_this_method {
3624+
let Some((_span, (def_id, ty))) = entry else {
3625+
continue;
3626+
};
3627+
for diff in &type_diffs {
3628+
let Sorts(expected_found) = diff else {
3629+
continue;
3630+
};
3631+
if tcx.is_diagnostic_item(sym::IteratorItem, *def_id)
3632+
&& path_segment.ident.name == sym::map
3633+
&& self.can_eq(param_env, expected_found.found, *ty)
3634+
&& let [arg] = args
3635+
&& let hir::ExprKind::Closure(closure) = arg.kind
3636+
{
3637+
let body = tcx.hir().body(closure.body);
3638+
if let hir::ExprKind::Block(block, None) = body.value.kind
3639+
&& let None = block.expr
3640+
&& let [.., stmt] = block.stmts
3641+
&& let hir::StmtKind::Semi(expr) = stmt.kind
3642+
// FIXME: actually check the expected vs found types, but right now
3643+
// the expected is a projection that we need to resolve.
3644+
// && let Some(tail_ty) = typeck_results.expr_ty_opt(expr)
3645+
&& expected_found.found.is_unit()
3646+
{
3647+
err.span_suggestion_verbose(
3648+
expr.span.shrink_to_hi().with_hi(stmt.span.hi()),
3649+
"consider removing this semicolon",
3650+
String::new(),
3651+
Applicability::MachineApplicable,
3652+
);
3653+
}
3654+
let expr = if let hir::ExprKind::Block(block, None) = body.value.kind
3655+
&& let Some(expr) = block.expr
3656+
{
3657+
expr
3658+
} else {
3659+
body.value
3660+
};
3661+
if let hir::ExprKind::MethodCall(path_segment, rcvr, [], span) = expr.kind
3662+
&& path_segment.ident.name == sym::clone
3663+
&& let Some(expr_ty) = typeck_results.expr_ty_opt(expr)
3664+
&& let Some(rcvr_ty) = typeck_results.expr_ty_opt(rcvr)
3665+
&& self.can_eq(param_env, expr_ty, rcvr_ty)
3666+
&& let ty::Ref(_, ty, _) = expr_ty.kind()
3667+
{
3668+
err.span_label(
3669+
span,
3670+
format!(
3671+
"this method call is cloning the reference `{expr_ty}`, not \
3672+
`{ty}` which doesn't implement `Clone`",
3673+
),
3674+
);
3675+
let ty::Param(..) = ty.kind() else {
3676+
continue;
3677+
};
3678+
let hir = tcx.hir();
3679+
let node = hir.get_by_def_id(hir.get_parent_item(expr.hir_id).def_id);
3680+
3681+
let pred = ty::Binder::dummy(ty::TraitPredicate {
3682+
trait_ref: ty::TraitRef::from_lang_item(
3683+
tcx,
3684+
LangItem::Clone,
3685+
span,
3686+
[*ty],
3687+
),
3688+
polarity: ty::ImplPolarity::Positive,
3689+
});
3690+
let Some(generics) = node.generics() else {
3691+
continue;
3692+
};
3693+
let Some(body_id) = node.body_id() else {
3694+
continue;
3695+
};
3696+
suggest_restriction(
3697+
tcx,
3698+
hir.body_owner_def_id(body_id),
3699+
&generics,
3700+
&format!("type parameter `{ty}`"),
3701+
err,
3702+
node.fn_sig(),
3703+
None,
3704+
pred,
3705+
None,
3706+
);
3707+
}
3708+
}
3709+
}
3710+
}
36213711
assocs.push(assocs_in_this_method);
36223712
prev_ty = self.resolve_vars_if_possible(
36233713
typeck_results.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(tcx)),

‎tests/ui/iterators/invalid-iterator-chain-fixable.fixed

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// run-rustfix
2+
use std::collections::hash_set::Iter;
3+
use std::collections::HashSet;
4+
5+
fn iter_to_vec<'b, X>(i: Iter<'b, X>) -> Vec<X> where X: Clone {
6+
let i = i.map(|x| x.clone());
7+
i.collect() //~ ERROR E0277
8+
}
9+
10+
fn main() {
11+
let v = vec![(0, 0)];
12+
let scores = v
13+
.iter()
14+
.map(|(a, b)| {
15+
a + b
16+
});
17+
println!("{}", scores.sum::<i32>()); //~ ERROR E0277
18+
println!(
19+
"{}",
20+
vec![0, 1]
21+
.iter()
22+
.map(|x| x * 2)
23+
.map(|x| { x })
24+
.map(|x| { x })
25+
.sum::<i32>(), //~ ERROR E0277
26+
);
27+
println!("{}", vec![0, 1].iter().map(|x| { x }).sum::<i32>()); //~ ERROR E0277
28+
let a = vec![0];
29+
let b = a.into_iter();
30+
let c = b.map(|x| x + 1);
31+
let d = c.filter(|x| *x > 10 );
32+
let e = d.map(|x| {
33+
x + 1
34+
});
35+
let f = e.filter(|_| false);
36+
let g: Vec<i32> = f.collect(); //~ ERROR E0277
37+
println!("{g:?}");
38+
39+
let mut s = HashSet::new();
40+
s.insert(1u8);
41+
println!("{:?}", iter_to_vec(s.iter()));
42+
}

‎tests/ui/iterators/invalid-iterator-chain-fixable.rs

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// run-rustfix
2+
use std::collections::hash_set::Iter;
3+
use std::collections::HashSet;
4+
5+
fn iter_to_vec<'b, X>(i: Iter<'b, X>) -> Vec<X> {
6+
let i = i.map(|x| x.clone());
7+
i.collect() //~ ERROR E0277
8+
}
9+
10+
fn main() {
11+
let v = vec![(0, 0)];
12+
let scores = v
13+
.iter()
14+
.map(|(a, b)| {
15+
a + b;
16+
});
17+
println!("{}", scores.sum::<i32>()); //~ ERROR E0277
18+
println!(
19+
"{}",
20+
vec![0, 1]
21+
.iter()
22+
.map(|x| x * 2)
23+
.map(|x| { x; })
24+
.map(|x| { x })
25+
.sum::<i32>(), //~ ERROR E0277
26+
);
27+
println!("{}", vec![0, 1].iter().map(|x| { x; }).sum::<i32>()); //~ ERROR E0277
28+
let a = vec![0];
29+
let b = a.into_iter();
30+
let c = b.map(|x| x + 1);
31+
let d = c.filter(|x| *x > 10 );
32+
let e = d.map(|x| {
33+
x + 1;
34+
});
35+
let f = e.filter(|_| false);
36+
let g: Vec<i32> = f.collect(); //~ ERROR E0277
37+
println!("{g:?}");
38+
39+
let mut s = HashSet::new();
40+
s.insert(1u8);
41+
println!("{:?}", iter_to_vec(s.iter()));
42+
}

‎tests/ui/iterators/invalid-iterator-chain-fixable.stderr

+157
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
error[E0277]: a value of type `Vec<X>` cannot be built from an iterator over elements of type `&X`
2+
--> $DIR/invalid-iterator-chain-fixable.rs:7:7
3+
|
4+
LL | let i = i.map(|x| x.clone());
5+
| ------- this method call is cloning the reference `&X`, not `X` which doesn't implement `Clone`
6+
LL | i.collect()
7+
| ^^^^^^^ value of type `Vec<X>` cannot be built from `std::iter::Iterator<Item=&X>`
8+
|
9+
= help: the trait `FromIterator<&X>` is not implemented for `Vec<X>`
10+
= help: the trait `FromIterator<X>` is implemented for `Vec<X>`
11+
= help: for that trait implementation, expected `X`, found `&X`
12+
note: the method call chain might not have had the expected associated types
13+
--> $DIR/invalid-iterator-chain-fixable.rs:5:26
14+
|
15+
LL | fn iter_to_vec<'b, X>(i: Iter<'b, X>) -> Vec<X> {
16+
| ^^^^^^^^^^^ `Iterator::Item` is `&X` here
17+
LL | let i = i.map(|x| x.clone());
18+
| ------------------ `Iterator::Item` remains `&X` here
19+
note: required by a bound in `collect`
20+
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
21+
help: consider further restricting type parameter `X`
22+
|
23+
LL | fn iter_to_vec<'b, X>(i: Iter<'b, X>) -> Vec<X> where X: Clone {
24+
| ++++++++++++++
25+
26+
error[E0277]: a value of type `i32` cannot be made by summing an iterator over elements of type `()`
27+
--> $DIR/invalid-iterator-chain-fixable.rs:17:33
28+
|
29+
LL | println!("{}", scores.sum::<i32>());
30+
| --- ^^^ value of type `i32` cannot be made by summing a `std::iter::Iterator<Item=()>`
31+
| |
32+
| required by a bound introduced by this call
33+
|
34+
= help: the trait `Sum<()>` is not implemented for `i32`
35+
= help: the following other types implement trait `Sum<A>`:
36+
<i32 as Sum>
37+
<i32 as Sum<&'a i32>>
38+
note: the method call chain might not have had the expected associated types
39+
--> $DIR/invalid-iterator-chain-fixable.rs:14:10
40+
|
41+
LL | let v = vec![(0, 0)];
42+
| ------------ this expression has type `Vec<({integer}, {integer})>`
43+
LL | let scores = v
44+
LL | .iter()
45+
| ------ `Iterator::Item` is `&({integer}, {integer})` here
46+
LL | .map(|(a, b)| {
47+
| __________^
48+
LL | | a + b;
49+
LL | | });
50+
| |__________^ `Iterator::Item` changed to `()` here
51+
note: required by a bound in `std::iter::Iterator::sum`
52+
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
53+
help: consider removing this semicolon
54+
|
55+
LL - a + b;
56+
LL + a + b
57+
|
58+
59+
error[E0277]: a value of type `i32` cannot be made by summing an iterator over elements of type `()`
60+
--> $DIR/invalid-iterator-chain-fixable.rs:25:20
61+
|
62+
LL | .sum::<i32>(),
63+
| --- ^^^ value of type `i32` cannot be made by summing a `std::iter::Iterator<Item=()>`
64+
| |
65+
| required by a bound introduced by this call
66+
|
67+
= help: the trait `Sum<()>` is not implemented for `i32`
68+
= help: the following other types implement trait `Sum<A>`:
69+
<i32 as Sum>
70+
<i32 as Sum<&'a i32>>
71+
note: the method call chain might not have had the expected associated types
72+
--> $DIR/invalid-iterator-chain-fixable.rs:23:14
73+
|
74+
LL | vec![0, 1]
75+
| ---------- this expression has type `Vec<{integer}>`
76+
LL | .iter()
77+
| ------ `Iterator::Item` is `&{integer}` here
78+
LL | .map(|x| x * 2)
79+
| -------------- `Iterator::Item` changed to `{integer}` here
80+
LL | .map(|x| { x; })
81+
| ^^^^^^^^^^^^^^^ `Iterator::Item` changed to `()` here
82+
LL | .map(|x| { x })
83+
| -------------- `Iterator::Item` remains `()` here
84+
note: required by a bound in `std::iter::Iterator::sum`
85+
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
86+
help: consider removing this semicolon
87+
|
88+
LL - .map(|x| { x; })
89+
LL + .map(|x| { x })
90+
|
91+
92+
error[E0277]: a value of type `i32` cannot be made by summing an iterator over elements of type `()`
93+
--> $DIR/invalid-iterator-chain-fixable.rs:27:60
94+
|
95+
LL | println!("{}", vec![0, 1].iter().map(|x| { x; }).sum::<i32>());
96+
| --- ^^^ value of type `i32` cannot be made by summing a `std::iter::Iterator<Item=()>`
97+
| |
98+
| required by a bound introduced by this call
99+
|
100+
= help: the trait `Sum<()>` is not implemented for `i32`
101+
= help: the following other types implement trait `Sum<A>`:
102+
<i32 as Sum>
103+
<i32 as Sum<&'a i32>>
104+
note: the method call chain might not have had the expected associated types
105+
--> $DIR/invalid-iterator-chain-fixable.rs:27:38
106+
|
107+
LL | println!("{}", vec![0, 1].iter().map(|x| { x; }).sum::<i32>());
108+
| ---------- ------ ^^^^^^^^^^^^^^^ `Iterator::Item` changed to `()` here
109+
| | |
110+
| | `Iterator::Item` is `&{integer}` here
111+
| this expression has type `Vec<{integer}>`
112+
note: required by a bound in `std::iter::Iterator::sum`
113+
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
114+
help: consider removing this semicolon
115+
|
116+
LL - println!("{}", vec![0, 1].iter().map(|x| { x; }).sum::<i32>());
117+
LL + println!("{}", vec![0, 1].iter().map(|x| { x }).sum::<i32>());
118+
|
119+
120+
error[E0277]: a value of type `Vec<i32>` cannot be built from an iterator over elements of type `()`
121+
--> $DIR/invalid-iterator-chain-fixable.rs:36:25
122+
|
123+
LL | let g: Vec<i32> = f.collect();
124+
| ^^^^^^^ value of type `Vec<i32>` cannot be built from `std::iter::Iterator<Item=()>`
125+
|
126+
= help: the trait `FromIterator<()>` is not implemented for `Vec<i32>`
127+
= help: the trait `FromIterator<i32>` is implemented for `Vec<i32>`
128+
= help: for that trait implementation, expected `i32`, found `()`
129+
note: the method call chain might not have had the expected associated types
130+
--> $DIR/invalid-iterator-chain-fixable.rs:32:15
131+
|
132+
LL | let a = vec![0];
133+
| ------- this expression has type `Vec<{integer}>`
134+
LL | let b = a.into_iter();
135+
| ----------- `Iterator::Item` is `{integer}` here
136+
LL | let c = b.map(|x| x + 1);
137+
| -------------- `Iterator::Item` remains `{integer}` here
138+
LL | let d = c.filter(|x| *x > 10 );
139+
| -------------------- `Iterator::Item` remains `{integer}` here
140+
LL | let e = d.map(|x| {
141+
| _______________^
142+
LL | | x + 1;
143+
LL | | });
144+
| |______^ `Iterator::Item` changed to `()` here
145+
LL | let f = e.filter(|_| false);
146+
| ----------------- `Iterator::Item` remains `()` here
147+
note: required by a bound in `collect`
148+
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
149+
help: consider removing this semicolon
150+
|
151+
LL - x + 1;
152+
LL + x + 1
153+
|
154+
155+
error: aborting due to 5 previous errors
156+
157+
For more information about this error, try `rustc --explain E0277`.

0 commit comments

Comments
 (0)