More accurate spans for arg removal suggestion

Author: estebank

compiler/rustc_errors/src/diagnostic.rs

@@ -612,7 +612,7 @@ impl Diagnostic {
     pub fn multipart_suggestion_with_style(
         &mut self,
         msg: impl Into<SubdiagnosticMessage>,
-        suggestion: Vec<(Span, String)>,
+        mut suggestion: Vec<(Span, String)>,
         applicability: Applicability,
         style: SuggestionStyle,
     ) -> &mut Self {
@@ -634,6 +634,7 @@ impl Diagnostic {
             None,
             "suggestion must not have overlapping parts",
         );
+        suggestion.sort_by_key(|(span, _)| (span.lo(), span.hi()));
 
         self.push_suggestion(CodeSuggestion {
             substitutions: vec![Substitution { parts }],

compiler/rustc_errors/src/emitter.rs

@@ -1768,7 +1768,7 @@ impl EmitterWriter {
 
         // Render the replacements for each suggestion
         let suggestions = suggestion.splice_lines(sm);
-        debug!("emit_suggestion_default: suggestions={:?}", suggestions);
+        debug!(?suggestions);
 
         if suggestions.is_empty() {
             // Suggestions coming from macros can have malformed spans. This is a heavy handed
@@ -1797,6 +1797,7 @@ impl EmitterWriter {
         for (complete, parts, highlights, only_capitalization) in
             suggestions.iter().take(MAX_SUGGESTIONS)
         {
+            debug!(?complete, ?parts, ?highlights);
             notice_capitalization |= only_capitalization;
 
             let has_deletion = parts.iter().any(|p| p.is_deletion(sm));

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -755,15 +755,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
 
         errors.drain_filter(|error| {
-                let Error::Invalid(provided_idx, expected_idx, Compatibility::Incompatible(Some(e))) = error else { return false };
-                let (provided_ty, provided_span) = provided_arg_tys[*provided_idx];
-                let trace = mk_trace(provided_span, formal_and_expected_inputs[*expected_idx], provided_ty);
-                if !matches!(trace.cause.as_failure_code(*e), FailureCode::Error0308(_)) {
-                    self.err_ctxt().report_and_explain_type_error(trace, *e).emit();
-                    return true;
-                }
-                false
-            });
+            let Error::Invalid(
+                provided_idx,
+                expected_idx,
+                Compatibility::Incompatible(Some(e)),
+            ) = error else { return false };
+            let (provided_ty, provided_span) = provided_arg_tys[*provided_idx];
+            let trace = mk_trace(
+                provided_span,
+                formal_and_expected_inputs[*expected_idx],
+                provided_ty,
+            );
+            if !matches!(trace.cause.as_failure_code(*e), FailureCode::Error0308(_)) {
+                self.err_ctxt().report_and_explain_type_error(trace, *e).emit();
+                return true;
+            }
+            false
+        });
 
         // We're done if we found errors, but we already emitted them.
         if errors.is_empty() {
@@ -864,7 +872,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
         let mut suggestion_text = SuggestionText::None;
 
+        let ty_to_snippet = |ty: Ty<'tcx>, expected_idx: ExpectedIdx| {
+            if ty.is_unit() {
+                "()".to_string()
+            } else if ty.is_suggestable(tcx, false) {
+                format!("/* {} */", ty)
+            } else if let Some(fn_def_id) = fn_def_id
+                && self.tcx.def_kind(fn_def_id).is_fn_like()
+                && let self_implicit =
+                    matches!(call_expr.kind, hir::ExprKind::MethodCall(..)) as usize
+                && let Some(arg) = self.tcx.fn_arg_names(fn_def_id)
+                    .get(expected_idx.as_usize() + self_implicit)
+                && arg.name != kw::SelfLower
+            {
+                format!("/* {} */", arg.name)
+            } else {
+                "/* value */".to_string()
+            }
+        };
+
         let mut errors = errors.into_iter().peekable();
+        let mut suggestions = vec![];
         while let Some(error) = errors.next() {
             match error {
                 Error::Invalid(provided_idx, expected_idx, compatibility) => {
@@ -906,6 +934,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     };
                     labels
                         .push((provided_span, format!("argument{} unexpected", provided_ty_name)));
+                    let mut span = provided_span;
+                    if let Some((_, next)) = provided_arg_tys.get(
+                        ProvidedIdx::from_usize(arg_idx.index() + 1),
+                    ) {
+                        // Include next comma
+                        span = span.until(*next);
+                    } else if arg_idx.index() > 0
+                        && let Some((_, prev)) = provided_arg_tys
+                            .get(ProvidedIdx::from_usize(arg_idx.index() - 1)
+                    ) {
+                        // Last argument, include previous comma
+                        span = span.with_lo(prev.hi());
+                    }
+                    suggestions.push((span, String::new()));
+
                     suggestion_text = match suggestion_text {
                         SuggestionText::None => SuggestionText::Remove(false),
                         SuggestionText::Remove(_) => SuggestionText::Remove(true),
@@ -1095,6 +1138,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             }
         }
 
+        // Incorporate the argument changes in the removal suggestion.
+        let mut prev = -1;
+        for (expected_idx, provided_idx) in matched_inputs.iter_enumerated() {
+            if let Some(provided_idx) = provided_idx {
+                prev = provided_idx.index() as i64;
+            }
+            let idx = ProvidedIdx::from_usize((prev + 1) as usize);
+            if let None = provided_idx
+                && let Some((_, arg_span)) = provided_arg_tys.get(idx)
+            {
+                let (_, expected_ty) = formal_and_expected_inputs[expected_idx];
+                suggestions.push((*arg_span, ty_to_snippet(expected_ty, expected_idx)));
+            }
+        }
+
         // If we have less than 5 things to say, it would be useful to call out exactly what's wrong
         if labels.len() <= 5 {
             for (span, label) in labels {
@@ -1112,7 +1170,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 Some(format!("provide the argument{}", if plural { "s" } else { "" }))
             }
             SuggestionText::Remove(plural) => {
-                Some(format!("remove the extra argument{}", if plural { "s" } else { "" }))
+                err.multipart_suggestion_verbose(
+                    &format!("remove the extra argument{}", if plural { "s" } else { "" }),
+                    suggestions,
+                    Applicability::HasPlaceholders,
+                );
+                None
             }
             SuggestionText::Swap => Some("swap these arguments".to_string()),
             SuggestionText::Reorder => Some("reorder these arguments".to_string()),
@@ -1151,20 +1214,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 } else {
                     // Propose a placeholder of the correct type
                     let (_, expected_ty) = formal_and_expected_inputs[expected_idx];
-                    if expected_ty.is_unit() {
-                        "()".to_string()
-                    } else if expected_ty.is_suggestable(tcx, false) {
-                        format!("/* {} */", expected_ty)
-                    } else if let Some(fn_def_id) = fn_def_id
-                        && self.tcx.def_kind(fn_def_id).is_fn_like()
-                        && let self_implicit = matches!(call_expr.kind, hir::ExprKind::MethodCall(..)) as usize
-                        && let Some(arg) = self.tcx.fn_arg_names(fn_def_id).get(expected_idx.as_usize() + self_implicit)
-                        && arg.name != kw::SelfLower
-                    {
-                        format!("/* {} */", arg.name)
-                    } else {
-                        "/* value */".to_string()
-                    }
+                    ty_to_snippet(expected_ty, expected_idx)
                 };
                 suggestion += &suggestion_text;
             }

tests/ui/alloc-error/alloc-error-handler-bad-signature-3.stderr

@@ -17,8 +17,10 @@ LL | fn oom() -> ! {
    = note: this error originates in the attribute macro `alloc_error_handler` (in Nightly builds, run with -Z macro-backtrace for more info)
 help: remove the extra argument
    |
-LL | fn oom() -> !() {
-   |              ++
+LL - fn oom() -> ! {
+LL -     loop {}
+LL - }
+   |
 
 error: aborting due to previous error
 

tests/ui/argument-suggestions/basic.stderr

@@ -25,8 +25,9 @@ LL | fn extra() {}
    |    ^^^^^
 help: remove the extra argument
    |
-LL |     extra();
-   |          ~~
+LL -     extra("");
+LL +     extra();
+   |
 
 error[E0061]: this function takes 1 argument but 0 arguments were supplied
   --> $DIR/basic.rs:22:5

tests/ui/argument-suggestions/exotic-calls.stderr

@@ -11,8 +11,9 @@ LL | fn foo<T: Fn()>(t: T) {
    |           ^^^^
 help: remove the extra argument
    |
-LL |     t();
-   |      ~~
+LL -     t(1i32);
+LL +     t();
+   |
 
 error[E0057]: this function takes 0 arguments but 1 argument was supplied
   --> $DIR/exotic-calls.rs:7:5
@@ -27,8 +28,9 @@ LL | fn bar(t: impl Fn()) {
    |           ^^^^^^^^^
 help: remove the extra argument
    |
-LL |     t();
-   |      ~~
+LL -     t(1i32);
+LL +     t();
+   |
 
 error[E0057]: this function takes 0 arguments but 1 argument was supplied
   --> $DIR/exotic-calls.rs:16:5
@@ -43,8 +45,9 @@ LL | fn baz() -> impl Fn() {
    |             ^^^^^^^^^
 help: remove the extra argument
    |
-LL |     baz()()
-   |          ~~
+LL -     baz()(1i32)
+LL +     baz()()
+   |
 
 error[E0057]: this function takes 0 arguments but 1 argument was supplied
   --> $DIR/exotic-calls.rs:22:5
@@ -59,8 +62,9 @@ LL |     let x = || {};
    |             ^^
 help: remove the extra argument
    |
-LL |     x();
-   |      ~~
+LL -     x(1i32);
+LL +     x();
+   |
 
 error: aborting due to 4 previous errors