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 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
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,18 @@ pub trait TypeErrCtxtExt<'tcx> {
predicate: ty::Predicate<'tcx>,
call_hir_id: HirId,
);

fn look_for_iterator_item_mistakes(
&self,
assocs_in_this_method: &[Option<(Span, (DefId, Ty<'tcx>))>],
typeck_results: &TypeckResults<'tcx>,
type_diffs: &[TypeError<'tcx>],
param_env: ty::ParamEnv<'tcx>,
path_segment: &hir::PathSegment<'_>,
args: &[hir::Expr<'_>],
err: &mut Diagnostic,
);

fn point_at_chain(
&self,
expr: &hir::Expr<'_>,
Expand All @@ -321,6 +333,7 @@ pub trait TypeErrCtxtExt<'tcx> {
param_env: ty::ParamEnv<'tcx>,
err: &mut Diagnostic,
);

fn probe_assoc_types_at_expr(
&self,
type_diffs: &[TypeError<'tcx>],
Expand Down Expand Up @@ -3592,6 +3605,109 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
}
}

fn look_for_iterator_item_mistakes(
&self,
assocs_in_this_method: &[Option<(Span, (DefId, Ty<'tcx>))>],
typeck_results: &TypeckResults<'tcx>,
type_diffs: &[TypeError<'tcx>],
param_env: ty::ParamEnv<'tcx>,
path_segment: &hir::PathSegment<'_>,
args: &[hir::Expr<'_>],
err: &mut Diagnostic,
) {
let tcx = self.tcx;
// 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,
);
}
}
}
}
}

fn point_at_chain(
&self,
expr: &hir::Expr<'_>,
Expand All @@ -3611,13 +3727,22 @@ 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);
self.look_for_iterator_item_mistakes(
&assocs_in_this_method,
typeck_results,
&type_diffs,
param_env,
path_segment,
args,
err,
);
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()));
}
Loading
Loading