Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Suggest giving return type to closures on E0282
  • Loading branch information
estebank committed Aug 13, 2019
1 parent 939c1cb commit 806476c
Show file tree
Hide file tree
Showing 13 changed files with 199 additions and 90 deletions.
2 changes: 1 addition & 1 deletion src/librustc/hir/mod.rs
Expand Up @@ -1541,7 +1541,7 @@ pub enum ExprKind {
Match(P<Expr>, HirVec<Arm>, MatchSource),
/// A closure (e.g., `move |a, b, c| {a + b + c}`).
///
/// The final span is the span of the argument block `|...|`.
/// The `Span` is the argument block `|...|`.
///
/// This may also be a generator literal or an `async block` as indicated by the
/// `Option<GeneratorMovability>`.
Expand Down
211 changes: 141 additions & 70 deletions src/librustc/infer/error_reporting/need_type_info.rs
@@ -1,13 +1,13 @@
use crate::hir::def::Namespace;
use crate::hir::{self, Local, Pat, Body, HirId};
use crate::hir::{self, Body, FunctionRetTy, Expr, ExprKind, HirId, Local, Pat};
use crate::hir::intravisit::{self, Visitor, NestedVisitorMap};
use crate::infer::InferCtxt;
use crate::infer::type_variable::TypeVariableOriginKind;
use crate::ty::{self, Ty, Infer, TyVar};
use crate::ty::print::Print;
use syntax::source_map::DesugaringKind;
use syntax_pos::Span;
use errors::DiagnosticBuilder;
use errors::{Applicability, DiagnosticBuilder};

struct FindLocalByTypeVisitor<'a, 'tcx> {
infcx: &'a InferCtxt<'a, 'tcx>,
Expand All @@ -16,9 +16,26 @@ struct FindLocalByTypeVisitor<'a, 'tcx> {
found_local_pattern: Option<&'tcx Pat>,
found_arg_pattern: Option<&'tcx Pat>,
found_ty: Option<Ty<'tcx>>,
found_closure: Option<&'tcx ExprKind>,
}

impl<'a, 'tcx> FindLocalByTypeVisitor<'a, 'tcx> {
fn new(
infcx: &'a InferCtxt<'a, 'tcx>,
target_ty: Ty<'tcx>,
hir_map: &'a hir::map::Map<'tcx>,
) -> FindLocalByTypeVisitor<'a, 'tcx> {
FindLocalByTypeVisitor {
infcx,
target_ty,
hir_map,
found_local_pattern: None,
found_arg_pattern: None,
found_ty: None,
found_closure: None,
}
}

fn node_matches_type(&mut self, hir_id: HirId) -> Option<Ty<'tcx>> {
let ty_opt = self.infcx.in_progress_tables.and_then(|tables| {
tables.borrow().node_type_opt(hir_id)
Expand Down Expand Up @@ -72,6 +89,16 @@ impl<'a, 'tcx> Visitor<'tcx> for FindLocalByTypeVisitor<'a, 'tcx> {
}
intravisit::walk_body(self, body);
}

fn visit_expr(&mut self, expr: &'tcx Expr) {
if let (ExprKind::Closure(_, _fn_decl, _id, _sp, _), Some(_)) = (
&expr.node,
self.node_matches_type(expr.hir_id),
) {
self.found_closure = Some(&expr.node);
}
intravisit::walk_expr(self, expr);
}
}

impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
Expand Down Expand Up @@ -106,16 +133,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let ty = self.resolve_vars_if_possible(&ty);
let name = self.extract_type_name(&ty, None);

let mut err_span = span;

let mut local_visitor = FindLocalByTypeVisitor {
infcx: &self,
target_ty: ty,
hir_map: &self.tcx.hir(),
found_local_pattern: None,
found_arg_pattern: None,
found_ty: None,
};
let mut local_visitor = FindLocalByTypeVisitor::new(&self, ty, &self.tcx.hir());
let ty_to_string = |ty: Ty<'tcx>| -> String {
let mut s = String::new();
let mut printer = ty::print::FmtPrinter::new(self.tcx, &mut s, Namespace::TypeNS);
Expand All @@ -136,6 +154,35 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let expr = self.tcx.hir().expect_expr(body_id.hir_id);
local_visitor.visit_expr(expr);
}
let err_span = if let Some(pattern) = local_visitor.found_arg_pattern {
pattern.span
} else {
span
};

let ty_msg = match local_visitor.found_ty {
Some(ty::TyS { sty: ty::Closure(def_id, substs), .. }) => {
let fn_sig = substs.closure_sig(*def_id, self.tcx);
let args = fn_sig.inputs()
.skip_binder()
.iter()
.next()
.map(|args| args.tuple_fields()
.map(|arg| arg.to_string())
.collect::<Vec<_>>().join(", "))
.unwrap_or_default();
let ret = fn_sig.output().skip_binder().to_string();
format!(" for the closure `fn({}) -> {}`", args, ret)
}
Some(ty) if &ty.to_string() != "_" &&
// FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527
(!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) =>
{
let ty = ty_to_string(ty);
format!(" for `{}`", ty)
}
_ => String::new(),
};

// When `name` corresponds to a type argument, show the path of the full type we're
// trying to infer. In the following example, `ty_msg` contains
Expand All @@ -150,34 +197,47 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
// | consider giving `b` the explicit type `std::result::Result<i32, E>`, where
// | the type parameter `E` is specified
// ```
let (ty_msg, suffix) = match &local_visitor.found_ty {
Some(ty) if &ty.to_string() != "_" &&
name == "_" &&
// FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527
(!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) &&
!ty.is_closure() => // The suggestion doesn't make sense for closures.
{
let ty = ty_to_string(ty);
(format!(" for `{}`", ty),
format!("the explicit type `{}`, with the type parameters specified", ty))
}
Some(ty) if &ty.to_string() != "_" &&
ty.to_string() != name &&
// FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527
(!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) &&
!ty.is_closure() => // The suggestion doesn't make sense for closures.
{
let ty = ty_to_string(ty);
(format!(" for `{}`", ty),
format!(
"the explicit type `{}`, where the type parameter `{}` is specified",
ty,
name,
))
}
let mut err = struct_span_err!(
self.tcx.sess,
err_span,
E0282,
"type annotations needed{}",
ty_msg,
);

let suffix = match local_visitor.found_ty {
Some(ty::TyS { sty: ty::Closure(def_id, substs), .. }) => {
let msg = " for the closure".to_string();
let fn_sig = substs.closure_sig(*def_id, self.tcx);
let ret = fn_sig.output().skip_binder().to_string();

if let Some(ExprKind::Closure(_, decl, body_id, ..)) = local_visitor.found_closure {
let (arrow, post) = match decl.output {
FunctionRetTy::DefaultReturn(_) => ("-> ", " "),
_ => ("", ""),
};
if let Some(body) = self.tcx.hir().krate().bodies.get(body_id) {
let suggestion = match body.value.node {
ExprKind::Block(..) => {
vec![(decl.output.span(), format!("{}{}{}", arrow, ret, post))]
}
_ => {
vec![
(decl.output.span(), format!("{}{}{}{{ ", arrow, ret, post)),
(body.value.span.shrink_to_hi(), " }".to_string()),
]
}
};
err.multipart_suggestion(
"give this closure an explicit return type without `_` placeholders",
suggestion,
Applicability::HasPlaceholders,
);
err.span_label(span, InferCtxt::missing_type_msg(&name));
return err;
}
}

// This shouldn't be reachable, but just in case we leave a reasonable fallback.
let args = fn_sig.inputs()
.skip_binder()
.iter()
Expand All @@ -189,18 +249,32 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
// This suggestion is incomplete, as the user will get further type inference
// errors due to the `_` placeholders and the introduction of `Box`, but it does
// nudge them in the right direction.
(msg, format!(
"a boxed closure type like `Box<dyn Fn({}) -> {}>`",
args,
fn_sig.output().skip_binder().to_string(),
))
format!("a boxed closure type like `Box<dyn Fn({}) -> {}>`", args, ret)
}
Some(ty) if &ty.to_string() != "_" &&
name == "_" &&
// FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527
(!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) =>
{
let ty = ty_to_string(ty);
format!("the explicit type `{}`, with the type parameters specified", ty)
}
_ => (String::new(), "a type".to_owned()),
Some(ty) if &ty.to_string() != "_" &&
ty.to_string() != name &&
// FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527
(!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) =>
{
let ty = ty_to_string(ty);
format!(
"the explicit type `{}`, where the type parameter `{}` is specified",
ty,
name,
)
}
_ => "a type".to_string(),
};
let mut labels = vec![(span, InferCtxt::missing_type_msg(&name))];

if let Some(pattern) = local_visitor.found_arg_pattern {
err_span = pattern.span;
// We don't want to show the default label for closures.
//
// So, before clearing, the output would look something like this:
Expand All @@ -217,39 +291,36 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
// ^ consider giving this closure parameter the type `[_; 0]`
// with the type parameter `_` specified
// ```
labels.clear();
labels.push((
err.span_label(
pattern.span,
format!("consider giving this closure parameter {}", suffix),
));
);
} else if let Some(pattern) = local_visitor.found_local_pattern {
if let Some(simple_ident) = pattern.simple_ident() {
match pattern.span.desugaring_kind() {
None => labels.push((
pattern.span,
format!("consider giving `{}` {}", simple_ident, suffix),
)),
Some(DesugaringKind::ForLoop) => labels.push((
pattern.span,
"the element type for this iterator is not specified".to_owned(),
)),
None => {
err.span_label(
pattern.span,
format!("consider giving `{}` {}", simple_ident, suffix),
);
}
Some(DesugaringKind::ForLoop) => {
err.span_label(
pattern.span,
"the element type for this iterator is not specified".to_string(),
);
}
_ => {}
}
} else {
labels.push((pattern.span, format!("consider giving this pattern {}", suffix)));
err.span_label(pattern.span, format!("consider giving this pattern {}", suffix));
}
};

let mut err = struct_span_err!(
self.tcx.sess,
err_span,
E0282,
"type annotations needed{}",
ty_msg,
);

for (target_span, label_message) in labels {
err.span_label(target_span, label_message);
}
if !err.span.span_labels().iter().any(|span_label| {
span_label.label.is_some() && span_label.span == span
}) && local_visitor.found_arg_pattern.is_none()
{ // Avoid multiple labels pointing at `span`.
err.span_label(span, InferCtxt::missing_type_msg(&name));
}

err
Expand Down
5 changes: 1 addition & 4 deletions src/test/ui/error-codes/E0282.stderr
Expand Up @@ -2,10 +2,7 @@ error[E0282]: type annotations needed
--> $DIR/E0282.rs:2:9
|
LL | let x = "hello".chars().rev().collect();
| ^
| |
| cannot infer type
| consider giving `x` a type
| ^ consider giving `x` a type

error: aborting due to previous error

Expand Down
5 changes: 1 addition & 4 deletions src/test/ui/for/for-loop-unconstrained-element-type.stderr
Expand Up @@ -2,10 +2,7 @@ error[E0282]: type annotations needed
--> $DIR/for-loop-unconstrained-element-type.rs:8:14
|
LL | for i in Vec::new() { }
| ^^^^^^^^^^
| |
| cannot infer type
| the element type for this iterator is not specified
| ^^^^^^^^^^ the element type for this iterator is not specified

error: aborting due to previous error

Expand Down
8 changes: 5 additions & 3 deletions src/test/ui/inference/cannot-infer-closure.stderr
@@ -1,10 +1,12 @@
error[E0282]: type annotations needed for the closure
error[E0282]: type annotations needed for the closure `fn((), ()) -> std::result::Result<(), _>`
--> $DIR/cannot-infer-closure.rs:3:9
|
LL | let x = |a: (), b: ()| {
| - consider giving `x` a boxed closure type like `Box<dyn Fn((), ()) -> std::result::Result<(), _>>`
LL | Err(a)?;
| ^^^^^^^ cannot infer type
help: give this closure an explicit return type without `_` placeholders
|
LL | let x = |a: (), b: ()| -> std::result::Result<(), _> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Expand Down
5 changes: 1 addition & 4 deletions src/test/ui/issues/issue-18159.stderr
Expand Up @@ -2,10 +2,7 @@ error[E0282]: type annotations needed
--> $DIR/issue-18159.rs:2:9
|
LL | let x;
| ^
| |
| cannot infer type
| consider giving `x` a type
| ^ consider giving `x` a type

error: aborting due to previous error

Expand Down
5 changes: 1 addition & 4 deletions src/test/ui/match/match-unresolved-one-arm.stderr
Expand Up @@ -2,10 +2,7 @@ error[E0282]: type annotations needed
--> $DIR/match-unresolved-one-arm.rs:4:9
|
LL | let x = match () {
| ^
| |
| cannot infer type
| consider giving `x` a type
| ^ consider giving `x` a type

error: aborting due to previous error

Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/suggestions/suggest-closure-return-type-1.rs
@@ -0,0 +1,3 @@
fn main() {
let _v = || -> _ { [] }; //~ ERROR type annotations needed for the closure
}
13 changes: 13 additions & 0 deletions src/test/ui/suggestions/suggest-closure-return-type-1.stderr
@@ -0,0 +1,13 @@
error[E0282]: type annotations needed for the closure `fn() -> [_; 0]`
--> $DIR/suggest-closure-return-type-1.rs:2:24
|
LL | let _v = || -> _ { [] };
| ^^ cannot infer type
help: give this closure an explicit return type without `_` placeholders
|
LL | let _v = || -> [_; 0] { [] };
| ^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0282`.
3 changes: 3 additions & 0 deletions src/test/ui/suggestions/suggest-closure-return-type-2.rs
@@ -0,0 +1,3 @@
fn main() {
let _v = || { [] }; //~ ERROR type annotations needed for the closure
}
13 changes: 13 additions & 0 deletions src/test/ui/suggestions/suggest-closure-return-type-2.stderr
@@ -0,0 +1,13 @@
error[E0282]: type annotations needed for the closure `fn() -> [_; 0]`
--> $DIR/suggest-closure-return-type-2.rs:2:19
|
LL | let _v = || { [] };
| ^^ cannot infer type
help: give this closure an explicit return type without `_` placeholders
|
LL | let _v = || -> [_; 0] { [] };
| ^^^^^^^^^

error: aborting due to previous error

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

0 comments on commit 806476c

Please sign in to comment.