Better method call error messages

compiler/rustc_typeck/src/check/expr.rs

@@ -221,7 +221,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         ty
     }
 
-    fn check_expr_kind(
+    pub(super) fn check_expr_kind(
         &self,
         expr: &'tcx hir::Expr<'tcx>,
         expected: Expectation<'tcx>,

compiler/rustc_typeck/src/check/fn_ctxt/checks.rs

@@ -235,6 +235,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let check_compatible = |arg_idx, input_idx| {
             let formal_input_ty: Ty<'tcx> = formal_input_tys[input_idx];
             let expected_input_ty: Ty<'tcx> = expected_input_tys[input_idx];
+
 
             // If either is an error type, we defy the usual convention and consider them to *not* be
             // coercible.  This prevents our error message heuristic from trying to pass errors into
@@ -244,25 +245,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             }
 
             let provided_arg: &hir::Expr<'tcx> = &provided_args[arg_idx];
-            let tables = self.in_progress_typeck_results.map(|t| t.borrow()).unwrap();
-            let maybe_ty = tables.node_type_opt(provided_arg.hir_id);
-            if let Some(checked_ty) = maybe_ty {
-                let expectation = Expectation::rvalue_hint(self, expected_input_ty);
-                let coerced_ty = expectation.only_has_type(self).unwrap_or(formal_input_ty);
-                let can_coerce = self.can_coerce(checked_ty, coerced_ty);
-
-                let is_super = self
-                    .at(&self.misc(provided_arg.span), self.param_env)
-                    .sup(formal_input_ty, coerced_ty)
-                    .is_ok();
-                // Same as above: if either the coerce type or the checked type is an error type,
-                // consider them *not* compatible.
-                return !coerced_ty.references_error()
-                    && !checked_ty.references_error()
-                    && can_coerce
-                    && is_super;
-            }
-            return false;
+            let expectation = Expectation::rvalue_hint(self, expected_input_ty);
+            // TODO: check that this is safe; I don't believe this commits any of the obligations, but I can't be sure.
+            //
+            //   I had another method of "soft" type checking before,
+            //   but it was failing to find the type of some expressions (like "")
+            //   so I prodded this method and made it pub(super) so I could call it, and it seems to work well. 
+            let checked_ty = self.check_expr_kind(provided_arg, expectation);
+
+            let coerced_ty = expectation.only_has_type(self).unwrap_or(formal_input_ty);
+            let can_coerce = self.can_coerce(checked_ty, coerced_ty);
+
+            let is_super = self
+                .at(&self.misc(provided_arg.span), self.param_env)
+                .sup(formal_input_ty, coerced_ty)
+                .is_ok();
+            // Same as above: if either the coerce type or the checked type is an error type,
+            // consider them *not* compatible.
+            return !coerced_ty.references_error()
+                && !checked_ty.references_error()
+                && can_coerce
+                && is_super;
         };
 
         // Check each argument, to satisfy the input it was provided for
@@ -379,6 +382,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 eliminate_arg(mat, ai, arg_idx);
             };
 
+            let eliminate_satisfied = |mat: &mut Vec<Vec<bool>>, ii: &mut Vec<usize>, ai: &mut Vec<usize>| {
+                let mut i = cmp::min(ii.len(), ai.len());
+                while i > 0 {
+                    let idx = i - 1;
+                    if mat[idx][idx] {
+                        satisfy_input(
+                            mat,
+                            ii,
+                            ai,
+                            idx,
+                            idx,
+                        );
+                    }
+                    i -= 1;
+                }
+            };
+
             // A list of the issues we might find
             enum Issue {
                 Invalid(usize),
@@ -528,6 +548,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 return None;
             };
 
+            // Before we start looking for issues, eliminate any arguments that are already satisfied,
+            // so that an argument which is already spoken for by the input it's in doesn't
+            // spill over into another similarly typed input
+            // ex:
+            //   fn some_func(_a: i32, _a: i32) {}
+            //   some_func(1, "");
+            // Without this elimination, the first argument causes the second argument
+            // to show up as both a missing input and extra argument, rather than
+            // just an invalid type.
+            eliminate_satisfied(&mut compatibility_matrix, &mut input_indexes, &mut arg_indexes);
+
             // As we encounter issues, we'll transcribe them to their actual indices
             let mut issues: Vec<Issue> = vec![];
             // Until we've elimineated / satisfied all arguments/inputs
@@ -601,20 +632,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     None => {
                         // We didn't find any issues, so we need to push the algorithm forward
                         // First, eliminate any arguments that currently satisfy their inputs
-                        let mut i = cmp::min(arg_indexes.len(), input_indexes.len());
-                        while i > 0 {
-                            let idx = i - 1;
-                            if compatibility_matrix[idx][idx] {
-                                satisfy_input(
-                                    &mut compatibility_matrix,
-                                    &mut input_indexes,
-                                    &mut arg_indexes,
-                                    idx,
-                                    idx,
-                                );
-                            }
-                            i -= 1;
-                        }
+                        eliminate_satisfied(&mut compatibility_matrix, &mut input_indexes, &mut arg_indexes);
                     }
                 };
             }
@@ -637,33 +655,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 let mut labels = vec![];
                 let mut suggestions = vec![];
                 let mut suggestion_type = NoSuggestion;
+                let mut eliminated_args = vec![false; provided_arg_count];
                 let source_map = self.sess().source_map();
                 for issue in issues {
                     match issue {
                         Issue::Invalid(arg) => {
                             let span = provided_args[arg].span;
                             let expected_type = expected_input_tys[arg];
-                            let found_type = final_arg_types[arg].unwrap().1;
+                            let found_type = final_arg_types[arg].unwrap().0;
                             labels.push((span, format!("expected {}, found {}", expected_type, found_type)));
                             suggestion_type = match suggestion_type {
                                 NoSuggestion | Provide => Provide,
                                 _ => Changes,
                             };
-                            suggestions.push((span, format!(" {{{}}},", expected_type)));
+                            suggestions.push((span, format!("{{{}}}", expected_type)));
                         }
                         Issue::Extra(arg) => {
-                            // FIXME: This could probably be a lot cleaner, but I dunno how
                             let span = provided_args[arg].span;
-                            let hungry_span = if arg < provided_args.len() - 1 {
-                                // Eat the comma
-                                Span::new(
-                                    span.lo(),
-                                    span.hi() + BytePos(2),
-                                    span.ctxt(),
-                                )
-                            } else {
-                                span
-                            };
+
+                            // We want to suggest deleting this arg,
+                            // but dealing with formatting before and after is tricky
+                            // so we mark it as deleted, so we can do a scan afterwards
+                            eliminated_args[arg] = true;
                             let found_type = self.check_expr(&provided_args[arg]);
                             // TODO: if someone knows the method-chain-foo to achieve this, let me know
                             // but I didn't have the patience for it lol
@@ -673,23 +686,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                                 let descr = def_kind.descr(def_id);
                                 if let Some(node) = node {
                                     if let Some(ident) = node.ident() {
-                                        format!("{}", ident)
+                                        format!("for {}", ident)
                                     } else {
-                                        format!("this {}", descr)
+                                        format!("for this {}", descr)
                                     }
                                 } else {
-                                    format!("this {}", descr)
+                                    format!("for this {}", descr)
                                 }
                             } else {
                                 "here".to_string()
                             };
 
-                            labels.push((span, format!("no parameter of type {} is needed {}", found_type, fn_name)));
+                            labels.push((span, format!("this parameter of type {} isn't needed {}", found_type, fn_name)));
                             suggestion_type = match suggestion_type {
                                 NoSuggestion | Remove => Remove,
                                 _ => Changes,
                             };
-                            suggestions.push((hungry_span, format!("")));
                         }
                         Issue::Missing(arg) => {
                             // FIXME: The spans here are all kinds of wrong for multiple missing arguments etc.
@@ -793,15 +805,80 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 for (span, label) in labels {
                     err.span_label(span, label);
                 }
+
+                // Before constructing our multipart suggestions, we need to add in all the suggestions
+                // to eliminate extra parameters
+                // We can't overlap spans in a suggestion or weird things happen
+                let mut lo = None;
+                let mut hi = None;
+                let mut in_block = false;
+                for (idx, eliminated) in eliminated_args.iter().enumerate() {
+                    match (in_block, eliminated, idx == eliminated_args.len() - 1) {
+                        (false, true, false) => {
+                            // We just encountered the start of a block of eliminated parameters
+                            lo = Some(idx);
+                            in_block = true;
+                        },
+                        (false, true, true) => {
+                            // We encountered a single eliminated arg at the end of the arg list
+                            lo = Some(idx);
+                            hi = Some(idx);
+                        }
+                        (true, false, _) => {
+                            // We encountered the end of a block, set the hi so the logic below kicks in
+                            hi = Some(idx - 1);
+                            in_block = false;
+                        },
+                        (true, true, true) => {
+                            hi = Some(idx);
+                            in_block = false;
+                        }
+                        _ => {}
+                    }
+                    if lo.is_some() && hi.is_some() {
+                        let (lo_idx, hi_idx) = (lo.unwrap(), hi.unwrap());
+                        // We've found a contiguous block, so emit the elimination
+                        // be careful abound the boundaries
+                        let ctxt = provided_args[0].span.ctxt();
+                        let (lo_bp, hi_bp) = match (lo_idx == 0, hi_idx == provided_arg_count - 1) {
+                            // If we have an arg to our left, chop off it's comma
+                            // a, xx, xx, xx
+                            //  [-----------)
+                            (false, _) => {
+                                (provided_args[lo_idx - 1].span.hi(), provided_args[hi_idx].span.hi())
+                            },
+                            // If we start from the first arg, and we have an arg to our right, chop of the last params comma
+                            // xx, xx, xx, a
+                            // [-----------)
+                            (true, false) => {
+                                (provided_args[lo_idx].span.lo(), provided_args[hi_idx + 1].span.lo())
+                            },
+                            // If every argument was eliminated, don't need to worry about commas before or after
+                            // xx, xx, xx, xx
+                            // [-------------)
+                            (true, true) => {
+                                (provided_args[lo_idx].span.lo(), provided_args[hi_idx].span.hi())
+                            },
+                        };
+                        let eliminated_span = Span::new(
+                            lo_bp,
+                            hi_bp,
+                            ctxt,
+                        );
+                        suggestions.push((eliminated_span, "".to_string()));
+                        lo = None;
+                        hi = None;
+                    }
+                }
 
                 // And add a series of suggestions
                 // FIXME: for simpler cases, this might be overkill
                 if suggestions.len() > 0 {
                     let suggestion_text = match (suggestion_type, issue_count) {
                         (Remove, 1) => Some("removing this argument may help"),
-                        (Remove, _) => Some("removing these argument may help"),
-                        (Provide, 1) => None,
-                        (Provide, _) => Some("providing these parameters may help"),
+                        (Remove, _) => Some("removing these arguments may help"),
+                        (Provide, 1) => Some("provideing a parameter of the correct type here may help"),
+                        (Provide, _) => Some("providing parameters of these types may help"),
                         (Swap, 1) => Some("swapping these two arguments might help"),
                         (Swap, _) => Some("swapping these sets of arguments might help"),
                         (Reorder, _) => Some("reordering these parameters might help"),

src/test/ui/argument-suggestions/basic.stderr

@@ -2,21 +2,25 @@ error[E0059]: arguments to this function are incorrect
   --> $DIR/basic.rs:20:13
    |
 LL |     invalid(1.0);
-   |             ^^^ expected u32, found u32
+   |             ^^^ expected u32, found {float}
    |
 note: function defined here
   --> $DIR/basic.rs:13:4
    |
 LL | fn invalid(_i: u32) {}
    |    ^^^^^^^ -------
+help: provideing a parameter of the correct type here may help
+   |
+LL |     invalid({u32});
+   |             ^^^^^
 
 error[E0059]: arguments to this function are incorrect
   --> $DIR/basic.rs:21:11
    |
 LL |     extra("");
    |           ^^
    |           |
-   |           no parameter of type &'static str is needed extra
+   |           this parameter of type &'static str isn't needed for extra
    |           help: removing this argument may help
    |
 note: function defined here
@@ -36,6 +40,10 @@ note: function defined here
    |
 LL | fn missing(_i: u32) {}
    |    ^^^^^^^ -------
+help: provideing a parameter of the correct type here may help
+   |
+LL |     missing( {u32},);
+   |              ^^^^^^
 
 error[E0059]: arguments to this function are incorrect
   --> $DIR/basic.rs:23:13

src/test/ui/argument-suggestions/complex.stderr

@@ -9,8 +9,8 @@ LL |   complex(1.0, H {}, &"", G{}, F::X2, Z {}, X {}, Y {});
    |           |    |         ||    expected G, found G
    |           |    |         |expected F, found F
    |           |    |         missing argument of type E
-   |           |    no parameter of type H is needed complex
-   |           expected u32, found u32
+   |           |    this parameter of type H isn't needed for complex
+   |           expected u32, found {float}
    |
 note: function defined here
   --> $DIR/complex.rs:11:4
@@ -19,8 +19,8 @@ LL | fn complex(_i: u32, _s: &str, _e: E, _f: F, _g: G, _x: X, _y: Y, _z: Z ) {}
    |    ^^^^^^^ -------  --------  -----  -----  -----  -----  -----  ------
 help: the following changes might help
    |
-LL |   complex( {u32},, &"", {E}, F::X2, G{}, X {}, Y {}, Z {});
-   |            ^^^^^^ --    ^^^^ ^^^^^  ^^^  ^^^^  ^^^^  ^^^^
+LL |   complex({u32}, &"", {E}, F::X2, G{}, X {}, Y {}, Z {});
+   |           ^^--^       ^^^^ ^^^^^  ^^^  ^^^^  ^^^^  ^^^^
 
 error: aborting due to previous error
 

src/test/ui/argument-suggestions/extra_arguments.rs

@@ -17,4 +17,19 @@ fn main() {
   two_arg_diff(1, "", ""); //~ ERROR arguments to this function are incorrect
   two_arg_diff(1, 1, "", ""); //~ ERROR arguments to this function are incorrect
   two_arg_diff(1, "", 1, ""); //~ ERROR arguments to this function are incorrect
+
+  // Check with weird spacing and newlines
+  two_arg_same(1, 1,     ""); //~ ERROR arguments to this function are incorrect
+  two_arg_diff(1, 1,     ""); //~ ERROR arguments to this function are incorrect
+  two_arg_same(
+    1,
+    1,
+    "" //~ ERROR arguments to this function are incorrect
+  );
+
+  two_arg_diff(
+    1,
+    1, //~ ERROR arguments to this function are incorrect
+    ""
+  );
 }