]> git.lizzy.rs Git - rust.git/commitdiff
Rollup merge of #47573 - estebank:closures, r=nikomatsakis
authorGuillaume Gomez <guillaume1.gomez@gmail.com>
Sat, 20 Jan 2018 21:32:49 +0000 (22:32 +0100)
committerGitHub <noreply@github.com>
Sat, 20 Jan 2018 21:32:49 +0000 (22:32 +0100)
Closure argument mismatch tweaks

 - use consistent phrasing for expected and found arguments
 - suggest changing arguments to tuple if possible
 - suggest changing single tuple argument to arguments if possible

Fix #44150.

src/librustc/traits/error_reporting.rs
src/test/ui/mismatched_types/closure-arg-count.rs
src/test/ui/mismatched_types/closure-arg-count.stderr

index 1aca687af2bf3e808bbff8cedaf4741ee28ef9e3..e649f1b49df765052fdd61b6b3b69c0e69c8d2ef 100644 (file)
@@ -717,93 +717,40 @@ pub fn report_selection_error(&self,
                     self.tcx.hir.span_if_local(did)
                 }).map(|sp| self.tcx.sess.codemap().def_span(sp)); // the sp could be an fn def
 
-                let found_ty_count =
-                    match found_trait_ref.skip_binder().substs.type_at(1).sty {
-                        ty::TyTuple(ref tys, _) => tys.len(),
-                        _ => 1,
-                    };
-                let (expected_tys, expected_ty_count) =
-                    match expected_trait_ref.skip_binder().substs.type_at(1).sty {
-                        ty::TyTuple(ref tys, _) =>
-                            (tys.iter().map(|t| &t.sty).collect(), tys.len()),
-                        ref sty => (vec![sty], 1),
-                    };
-                if found_ty_count == expected_ty_count {
+                let found = match found_trait_ref.skip_binder().substs.type_at(1).sty {
+                    ty::TyTuple(ref tys, _) => tys.iter()
+                        .map(|_| ArgKind::empty()).collect::<Vec<_>>(),
+                    _ => vec![ArgKind::empty()],
+                };
+                let expected = match expected_trait_ref.skip_binder().substs.type_at(1).sty {
+                    ty::TyTuple(ref tys, _) => tys.iter()
+                        .map(|t| match t.sty {
+                            ty::TypeVariants::TyTuple(ref tys, _) => ArgKind::Tuple(
+                                span,
+                                tys.iter()
+                                    .map(|ty| ("_".to_owned(), format!("{}", ty.sty)))
+                                    .collect::<Vec<_>>()
+                            ),
+                            _ => ArgKind::Arg("_".to_owned(), format!("{}", t.sty)),
+                        }).collect(),
+                    ref sty => vec![ArgKind::Arg("_".to_owned(), format!("{}", sty))],
+                };
+                if found.len()== expected.len() {
                     self.report_closure_arg_mismatch(span,
                                                      found_span,
                                                      found_trait_ref,
                                                      expected_trait_ref)
                 } else {
-                    let expected_tuple = if expected_ty_count == 1 {
-                        expected_tys.first().and_then(|t| {
-                            if let &&ty::TyTuple(ref tuptys, _) = t {
-                                Some(tuptys.len())
-                            } else {
-                                None
-                            }
-                        })
-                    } else {
-                        None
-                    };
-
-                    // FIXME(#44150): Expand this to "N args expected but a N-tuple found."
-                    // Type of the 1st expected argument is somehow provided as type of a
-                    // found one in that case.
-                    //
-                    // ```
-                    // [1i32, 2, 3].sort_by(|(a, b)| ..)
-                    // //           ^^^^^^^ --------
-                    // // expected_trait_ref:  std::ops::FnMut<(&i32, &i32)>
-                    // //    found_trait_ref:  std::ops::FnMut<(&i32,)>
-                    // ```
-
-                    let (closure_span, closure_args) = found_did
+                    let (closure_span, found) = found_did
                         .and_then(|did| self.tcx.hir.get_if_local(did))
-                        .and_then(|node| {
-                            if let hir::map::NodeExpr(
-                                &hir::Expr {
-                                    node: hir::ExprClosure(_, ref decl, id, span, _),
-                                    ..
-                                }) = node
-                            {
-                                let ty_snips = decl.inputs.iter()
-                                    .map(|ty| {
-                                        self.tcx.sess.codemap().span_to_snippet(ty.span).ok()
-                                            .and_then(|snip| {
-                                                // filter out dummy spans
-                                                if snip == "," || snip == "|" {
-                                                    None
-                                                } else {
-                                                    Some(snip)
-                                                }
-                                            })
-                                    })
-                                    .collect::<Vec<Option<String>>>();
-
-                                let body = self.tcx.hir.body(id);
-                                let pat_snips = body.arguments.iter()
-                                    .map(|arg|
-                                        self.tcx.sess.codemap().span_to_snippet(arg.pat.span).ok())
-                                    .collect::<Option<Vec<String>>>();
-
-                                Some((span, pat_snips, ty_snips))
-                            } else {
-                                None
-                            }
-                        })
-                        .map(|(span, pat, ty)| (Some(span), Some((pat, ty))))
-                        .unwrap_or((None, None));
-                    let closure_args = closure_args.and_then(|(pat, ty)| Some((pat?, ty)));
-
-                    self.report_arg_count_mismatch(
-                        span,
-                        closure_span.or(found_span),
-                        expected_ty_count,
-                        expected_tuple,
-                        found_ty_count,
-                        closure_args,
-                        found_trait_ty.is_closure()
-                    )
+                        .map(|node| self.get_fn_like_arguments(node))
+                        .unwrap_or((found_span.unwrap(), found));
+
+                    self.report_arg_count_mismatch(span,
+                                                   closure_span,
+                                                   expected,
+                                                   found,
+                                                   found_trait_ty.is_closure())
                 }
             }
 
@@ -845,94 +792,135 @@ fn suggest_borrow_on_unsized_slice(&self,
         }
     }
 
+    fn get_fn_like_arguments(&self, node: hir::map::Node) -> (Span, Vec<ArgKind>) {
+        if let hir::map::NodeExpr(&hir::Expr {
+            node: hir::ExprClosure(_, ref _decl, id, span, _),
+            ..
+        }) = node {
+            (self.tcx.sess.codemap().def_span(span), self.tcx.hir.body(id).arguments.iter()
+                .map(|arg| {
+                    if let hir::Pat {
+                        node: hir::PatKind::Tuple(args, _),
+                        span,
+                        ..
+                    } = arg.pat.clone().into_inner() {
+                        ArgKind::Tuple(
+                            span,
+                            args.iter().map(|pat| {
+                                let snippet = self.tcx.sess.codemap()
+                                    .span_to_snippet(pat.span).unwrap();
+                                (snippet, "_".to_owned())
+                            }).collect::<Vec<_>>(),
+                        )
+                    } else {
+                        let name = self.tcx.sess.codemap().span_to_snippet(arg.pat.span).unwrap();
+                        ArgKind::Arg(name, "_".to_owned())
+                    }
+                })
+                .collect::<Vec<ArgKind>>())
+        } else if let hir::map::NodeItem(&hir::Item {
+            span,
+            node: hir::ItemFn(ref decl, ..),
+            ..
+        }) = node {
+            (self.tcx.sess.codemap().def_span(span), decl.inputs.iter()
+                    .map(|arg| match arg.clone().into_inner().node {
+                hir::TyTup(ref tys) => ArgKind::Tuple(
+                    arg.span,
+                    tys.iter()
+                        .map(|_| ("_".to_owned(), "_".to_owned()))
+                        .collect::<Vec<_>>(),
+                ),
+                _ => ArgKind::Arg("_".to_owned(), "_".to_owned())
+            }).collect::<Vec<ArgKind>>())
+        } else {
+            panic!("non-FnLike node found: {:?}", node);
+        }
+    }
+
     fn report_arg_count_mismatch(
         &self,
         span: Span,
-        found_span: Option<Span>,
-        expected: usize,
-        expected_tuple: Option<usize>,
-        found: usize,
-        closure_args: Option<(Vec<String>, Vec<Option<String>>)>,
-        is_closure: bool
+        found_span: Span,
+        expected_args: Vec<ArgKind>,
+        found_args: Vec<ArgKind>,
+        is_closure: bool,
     ) -> DiagnosticBuilder<'tcx> {
-        use std::borrow::Cow;
-
         let kind = if is_closure { "closure" } else { "function" };
 
-        let args_str = |n, distinct| format!(
-                "{} {}argument{}",
-                n,
-                if distinct && n >= 2 { "distinct " } else { "" },
-                if n == 1 { "" } else { "s" },
-            );
-
-        let expected_str = if let Some(n) = expected_tuple {
-            assert!(expected == 1);
-            if closure_args.as_ref().map(|&(ref pats, _)| pats.len()) == Some(n) {
-                Cow::from("a single tuple as argument")
-            } else {
-                // be verbose when numbers differ
-                Cow::from(format!("a single {}-tuple as argument", n))
+        let args_str = |arguments: &Vec<ArgKind>, other: &Vec<ArgKind>| {
+            let arg_length = arguments.len();
+            let distinct = match &other[..] {
+                &[ArgKind::Tuple(..)] => true,
+                _ => false,
+            };
+            match (arg_length, arguments.get(0)) {
+                (1, Some(&ArgKind::Tuple(_, ref fields))) => {
+                    format!("a single {}-tuple as argument", fields.len())
+                }
+                _ => format!("{} {}argument{}",
+                             arg_length,
+                             if distinct && arg_length > 1 { "distinct " } else { "" },
+                             if arg_length == 1 { "" } else { "s" }),
             }
-        } else {
-            Cow::from(args_str(expected, false))
-        };
-
-        let found_str = if expected_tuple.is_some() {
-            args_str(found, true)
-        } else {
-            args_str(found, false)
         };
 
+        let expected_str = args_str(&expected_args, &found_args);
+        let found_str = args_str(&found_args, &expected_args);
 
-        let mut err = struct_span_err!(self.tcx.sess, span, E0593,
+        let mut err = struct_span_err!(
+            self.tcx.sess,
+            span,
+            E0593,
             "{} is expected to take {}, but it takes {}",
             kind,
             expected_str,
             found_str,
         );
 
-        err.span_label(
-            span,
-            format!(
-                "expected {} that takes {}",
-                kind,
-                expected_str,
-            )
-        );
-
-        if let Some(span) = found_span {
-            if let (Some(expected_tuple), Some((pats, tys))) = (expected_tuple, closure_args) {
-                if expected_tuple != found || pats.len() != found {
-                    err.span_label(span, format!("takes {}", found_str));
-                } else {
-                    let sugg = format!(
-                        "|({}){}|",
-                        pats.join(", "),
-
-                        // add type annotations if available
-                        if tys.iter().any(|ty| ty.is_some()) {
-                            Cow::from(format!(
-                                ": ({})",
-                                tys.into_iter().map(|ty| if let Some(ty) = ty {
-                                    ty
-                                } else {
-                                    "_".to_string()
-                                }).collect::<Vec<String>>().join(", ")
-                            ))
-                        } else {
-                            Cow::from("")
-                        },
-                    );
-
-                    err.span_suggestion(
-                        span,
-                        "consider changing the closure to accept a tuple",
-                        sugg
-                    );
-                }
-            } else {
-                err.span_label(span, format!("takes {}", found_str));
+        err.span_label(span, format!( "expected {} that takes {}", kind, expected_str));
+        err.span_label(found_span, format!("takes {}", found_str));
+
+        if let &[ArgKind::Tuple(_, ref fields)] = &found_args[..] {
+            if fields.len() == expected_args.len() {
+                let sugg = fields.iter()
+                    .map(|(name, _)| name.to_owned())
+                    .collect::<Vec<String>>().join(", ");
+                err.span_suggestion(found_span,
+                                    "change the closure to take multiple arguments instead of \
+                                     a single tuple",
+                                    format!("|{}|", sugg));
+            }
+        }
+        if let &[ArgKind::Tuple(_, ref fields)] = &expected_args[..] {
+            if fields.len() == found_args.len() && is_closure {
+                let sugg = format!(
+                    "|({}){}|",
+                    found_args.iter()
+                        .map(|arg| match arg {
+                            ArgKind::Arg(name, _) => name.to_owned(),
+                            _ => "_".to_owned(),
+                        })
+                        .collect::<Vec<String>>()
+                        .join(", "),
+                    // add type annotations if available
+                    if found_args.iter().any(|arg| match arg {
+                        ArgKind::Arg(_, ty) => ty != "_",
+                        _ => false,
+                    }) {
+                        format!(": ({})",
+                                fields.iter()
+                                    .map(|(_, ty)| ty.to_owned())
+                                    .collect::<Vec<String>>()
+                                    .join(", "))
+                    } else {
+                        "".to_owned()
+                    },
+                );
+                err.span_suggestion(found_span,
+                                    "change the closure to accept a tuple instead of individual \
+                                     arguments",
+                                    sugg);
             }
         }
 
@@ -1331,3 +1319,14 @@ fn suggest_new_overflow_limit(&self, err: &mut DiagnosticBuilder) {
                           suggested_limit));
     }
 }
+
+enum ArgKind {
+    Arg(String, String),
+    Tuple(Span, Vec<(String, String)>),
+}
+
+impl ArgKind {
+    fn empty() -> ArgKind {
+        ArgKind::Arg("_".to_owned(), "_".to_owned())
+    }
+}
index 1ee24e398520bcfde582f57396bb68c2f6f72c51..96e5201716c7173f2be48a72aac924ed94d650ab 100644 (file)
@@ -18,6 +18,8 @@ fn main() {
     //~^ ERROR closure is expected to take
     [1, 2, 3].sort_by(|(tuple, tuple2)| panic!());
     //~^ ERROR closure is expected to take
+    [1, 2, 3].sort_by(|(tuple, tuple2): (usize, _)| panic!());
+    //~^ ERROR closure is expected to take
     f(|| panic!());
     //~^ ERROR closure is expected to take
 
@@ -32,6 +34,9 @@ fn main() {
     let bar = |i, x, y| i;
     let _it = vec![1, 2, 3].into_iter().enumerate().map(bar);
     //~^ ERROR closure is expected to take
+    let _it = vec![1, 2, 3].into_iter().enumerate().map(qux);
+    //~^ ERROR function is expected to take
 }
 
 fn foo() {}
+fn qux(x: usize, y: usize) {}
index ba25d67d76ef2fc7fb0fff65e6caf7a04ff9fa05..be00ee4d74e7eededd2590ec6468ed33a62e651d 100644 (file)
@@ -14,18 +14,34 @@ error[E0593]: closure is expected to take 2 arguments, but it takes 1 argument
    |               |
    |               expected closure that takes 2 arguments
 
-error[E0593]: closure is expected to take 2 arguments, but it takes 1 argument
+error[E0593]: closure is expected to take 2 distinct arguments, but it takes a single 2-tuple as argument
   --> $DIR/closure-arg-count.rs:19:15
    |
 19 |     [1, 2, 3].sort_by(|(tuple, tuple2)| panic!());
-   |               ^^^^^^^ ----------------- takes 1 argument
+   |               ^^^^^^^ ----------------- takes a single 2-tuple as argument
    |               |
-   |               expected closure that takes 2 arguments
+   |               expected closure that takes 2 distinct arguments
+help: change the closure to take multiple arguments instead of a single tuple
+   |
+19 |     [1, 2, 3].sort_by(|tuple, tuple2| panic!());
+   |                       ^^^^^^^^^^^^^^^
+
+error[E0593]: closure is expected to take 2 distinct arguments, but it takes a single 2-tuple as argument
+  --> $DIR/closure-arg-count.rs:21:15
+   |
+21 |     [1, 2, 3].sort_by(|(tuple, tuple2): (usize, _)| panic!());
+   |               ^^^^^^^ ----------------------------- takes a single 2-tuple as argument
+   |               |
+   |               expected closure that takes 2 distinct arguments
+help: change the closure to take multiple arguments instead of a single tuple
+   |
+21 |     [1, 2, 3].sort_by(|tuple, tuple2| panic!());
+   |                       ^^^^^^^^^^^^^^^
 
 error[E0593]: closure is expected to take 1 argument, but it takes 0 arguments
-  --> $DIR/closure-arg-count.rs:21:5
+  --> $DIR/closure-arg-count.rs:23:5
    |
-21 |     f(|| panic!());
+23 |     f(|| panic!());
    |     ^ -- takes 0 arguments
    |     |
    |     expected closure that takes 1 argument
@@ -36,46 +52,63 @@ note: required by `f`
 13 | fn f<F: Fn<usize>>(_: F) {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^
 
-error[E0593]: closure is expected to take a single tuple as argument, but it takes 2 distinct arguments
-  --> $DIR/closure-arg-count.rs:24:53
+error[E0593]: closure is expected to take a single 2-tuple as argument, but it takes 2 distinct arguments
+  --> $DIR/closure-arg-count.rs:26:53
    |
-24 |     let _it = vec![1, 2, 3].into_iter().enumerate().map(|i, x| i);
-   |                                                     ^^^ ------ help: consider changing the closure to accept a tuple: `|(i, x)|`
+26 |     let _it = vec![1, 2, 3].into_iter().enumerate().map(|i, x| i);
+   |                                                     ^^^ ------ takes 2 distinct arguments
    |                                                     |
-   |                                                     expected closure that takes a single tuple as argument
+   |                                                     expected closure that takes a single 2-tuple as argument
+help: change the closure to accept a tuple instead of individual arguments
+   |
+26 |     let _it = vec![1, 2, 3].into_iter().enumerate().map(|(i, x)| i);
+   |                                                         ^^^^^^^^
 
-error[E0593]: closure is expected to take a single tuple as argument, but it takes 2 distinct arguments
-  --> $DIR/closure-arg-count.rs:26:53
+error[E0593]: closure is expected to take a single 2-tuple as argument, but it takes 2 distinct arguments
+  --> $DIR/closure-arg-count.rs:28:53
    |
-26 |     let _it = vec![1, 2, 3].into_iter().enumerate().map(|i: usize, x| i);
-   |                                                     ^^^ ------------- help: consider changing the closure to accept a tuple: `|(i, x): (usize, _)|`
+28 |     let _it = vec![1, 2, 3].into_iter().enumerate().map(|i: usize, x| i);
+   |                                                     ^^^ ------------- takes 2 distinct arguments
    |                                                     |
-   |                                                     expected closure that takes a single tuple as argument
+   |                                                     expected closure that takes a single 2-tuple as argument
+help: change the closure to accept a tuple instead of individual arguments
+   |
+28 |     let _it = vec![1, 2, 3].into_iter().enumerate().map(|(i, x)| i);
+   |                                                         ^^^^^^^^
 
 error[E0593]: closure is expected to take a single 2-tuple as argument, but it takes 3 distinct arguments
-  --> $DIR/closure-arg-count.rs:28:53
+  --> $DIR/closure-arg-count.rs:30:53
    |
-28 |     let _it = vec![1, 2, 3].into_iter().enumerate().map(|i, x, y| i);
+30 |     let _it = vec![1, 2, 3].into_iter().enumerate().map(|i, x, y| i);
    |                                                     ^^^ --------- takes 3 distinct arguments
    |                                                     |
    |                                                     expected closure that takes a single 2-tuple as argument
 
 error[E0593]: function is expected to take a single 2-tuple as argument, but it takes 0 arguments
-  --> $DIR/closure-arg-count.rs:30:53
+  --> $DIR/closure-arg-count.rs:32:53
    |
-30 |     let _it = vec![1, 2, 3].into_iter().enumerate().map(foo);
+32 |     let _it = vec![1, 2, 3].into_iter().enumerate().map(foo);
    |                                                     ^^^ expected function that takes a single 2-tuple as argument
 ...
-37 | fn foo() {}
+41 | fn foo() {}
    | -------- takes 0 arguments
 
 error[E0593]: closure is expected to take a single 2-tuple as argument, but it takes 3 distinct arguments
-  --> $DIR/closure-arg-count.rs:33:53
+  --> $DIR/closure-arg-count.rs:35:53
    |
-32 |     let bar = |i, x, y| i;
+34 |     let bar = |i, x, y| i;
    |               --------- takes 3 distinct arguments
-33 |     let _it = vec![1, 2, 3].into_iter().enumerate().map(bar);
+35 |     let _it = vec![1, 2, 3].into_iter().enumerate().map(bar);
    |                                                     ^^^ expected closure that takes a single 2-tuple as argument
 
-error: aborting due to 9 previous errors
+error[E0593]: function is expected to take a single 2-tuple as argument, but it takes 2 distinct arguments
+  --> $DIR/closure-arg-count.rs:37:53
+   |
+37 |     let _it = vec![1, 2, 3].into_iter().enumerate().map(qux);
+   |                                                     ^^^ expected function that takes a single 2-tuple as argument
+...
+42 | fn qux(x: usize, y: usize) {}
+   | -------------------------- takes 2 distinct arguments
+
+error: aborting due to 11 previous errors