Malformed call function call: do multiple suggestions when possible

Author: scrabsha

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -1179,10 +1179,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let mut only_extras_so_far = errors
             .peek()
             .is_some_and(|first| matches!(first, Error::Extra(arg_idx) if arg_idx.index() == 0));
+        let mut only_missing_so_far = true;
         let mut prev_extra_idx = None;
         let mut suggestions = vec![];
         while let Some(error) = errors.next() {
             only_extras_so_far &= matches!(error, Error::Extra(_));
+            only_missing_so_far &= matches!(error, Error::Missing(_));
 
             match error {
                 Error::Invalid(provided_idx, expected_idx, compatibility) => {
@@ -1580,80 +1582,146 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             && !full_call_span.in_external_macro(self.sess().source_map())
         {
             let source_map = self.sess().source_map();
-            let suggestion_span = if let Some(args_span) = error_span.trim_start(full_call_span) {
-                // Span of the braces, e.g. `(a, b, c)`.
-                args_span
-            } else {
-                // The arg span of a function call that wasn't even given braces
-                // like what might happen with delegation reuse.
-                // e.g. `reuse HasSelf::method;` should suggest `reuse HasSelf::method($args);`.
-                full_call_span.shrink_to_hi()
-            };
-
-            // Controls how the arguments should be listed in the suggestion.
-            enum ArgumentsFormatting {
-                SingleLine,
-                Multiline { fallback_indent: String, brace_indent: String },
-            }
-            let arguments_formatting = {
-                let mut provided_inputs = matched_inputs.iter().filter_map(|a| *a);
-                if let Some(brace_indent) = source_map.indentation_before(suggestion_span)
-                    && let Some(first_idx) = provided_inputs.by_ref().next()
-                    && let Some(last_idx) = provided_inputs.by_ref().next()
-                    && let (_, first_span) = provided_arg_tys[first_idx]
-                    && let (_, last_span) = provided_arg_tys[last_idx]
-                    && source_map.is_multiline(first_span.to(last_span))
-                    && let Some(fallback_indent) = source_map.indentation_before(first_span)
-                {
-                    ArgumentsFormatting::Multiline { fallback_indent, brace_indent }
+            let (suggestion_span, has_paren) =
+                if let Some(args_span) = error_span.trim_start(full_call_span) {
+                    // Span of the braces, e.g. `(a, b, c)`.
+                    (args_span, true)
                 } else {
-                    ArgumentsFormatting::SingleLine
-                }
-            };
+                    // The arg span of a function call that wasn't even given braces
+                    // like what might happen with delegation reuse.
+                    // e.g. `reuse HasSelf::method;` should suggest `reuse HasSelf::method($args);`.
+                    (full_call_span.shrink_to_hi(), false)
+                };
 
-            let mut suggestion = "(".to_owned();
-            let mut needs_comma = false;
-            for (expected_idx, provided_idx) in matched_inputs.iter_enumerated() {
-                if needs_comma {
-                    suggestion += ",";
-                }
-                match &arguments_formatting {
-                    ArgumentsFormatting::SingleLine if needs_comma => suggestion += " ",
-                    ArgumentsFormatting::SingleLine => {}
-                    ArgumentsFormatting::Multiline { .. } => suggestion += "\n",
+            if has_paren && only_missing_so_far {
+                // Controls how the arguments should be listed in the suggestion.
+                enum ArgumentsFormatting {
+                    SingleLine,
+                    Multiline { fallback_indent: String },
                 }
-                needs_comma = true;
-                let (suggestion_span, suggestion_text) = if let Some(provided_idx) = provided_idx
-                    && let (_, provided_span) = provided_arg_tys[*provided_idx]
-                    && let Ok(arg_text) = source_map.span_to_snippet(provided_span)
-                {
-                    (Some(provided_span), arg_text)
-                } else {
-                    // Propose a placeholder of the correct type
-                    let (_, expected_ty) = formal_and_expected_inputs[expected_idx];
-                    (None, ty_to_snippet(expected_ty, expected_idx))
+                let arguments_formatting = {
+                    let mut provided_inputs = matched_inputs.iter().filter_map(|a| *a);
+                    if let Some(first_idx) = provided_inputs.by_ref().next()
+                        && let Some(last_idx) = provided_inputs.by_ref().next()
+                        && let (_, first_span) = provided_arg_tys[first_idx]
+                        && let (_, last_span) = provided_arg_tys[last_idx]
+                        && source_map.is_multiline(first_span.to(last_span))
+                        && let Some(fallback_indent) = source_map.indentation_before(first_span)
+                    {
+                        ArgumentsFormatting::Multiline { fallback_indent }
+                    } else {
+                        ArgumentsFormatting::SingleLine
+                    }
                 };
-                if let ArgumentsFormatting::Multiline { fallback_indent, .. } =
-                    &arguments_formatting
-                {
-                    let indent = suggestion_span
-                        .and_then(|span| source_map.indentation_before(span))
-                        .unwrap_or_else(|| fallback_indent.clone());
-                    suggestion += &indent;
+
+                let mut suggestions: Vec<(Span, String)> = Vec::new();
+                let mut previous_span = source_map.start_point(suggestion_span).shrink_to_hi();
+                let mut no_provided_arg_yet = true;
+                for (expected_idx, provided_idx) in matched_inputs.iter_enumerated() {
+                    if let Some(provided_idx) = provided_idx {
+                        let (_, provided_span) = provided_arg_tys[*provided_idx];
+                        previous_span = provided_span;
+                        no_provided_arg_yet = false;
+                    } else {
+                        // Propose a placeholder of the correct type
+                        let (insertion_span, last_arg) = if no_provided_arg_yet {
+                            (previous_span, matched_inputs.len() - 1 == expected_idx.as_usize())
+                        } else {
+                            let mut comma_hit = false;
+                            let mut closing_paren_hit = false;
+                            let after_arg = previous_span.shrink_to_hi();
+                            let after_previous_comma = source_map
+                                .span_extend_while(after_arg, |c| {
+                                    closing_paren_hit = c == ')';
+                                    if comma_hit || closing_paren_hit {
+                                        false
+                                    } else {
+                                        comma_hit = c == ',';
+                                        true
+                                    }
+                                })
+                                .unwrap()
+                                .shrink_to_hi();
+                            let span = if closing_paren_hit {
+                                after_previous_comma
+                            } else {
+                                source_map.next_point(after_previous_comma).shrink_to_hi()
+                            };
+                            (span, closing_paren_hit)
+                        };
+                        let (_, expected_ty) = formal_and_expected_inputs[expected_idx];
+                        let expected_ty = ty_to_snippet(expected_ty, expected_idx);
+                        let indent = match arguments_formatting {
+                            ArgumentsFormatting::SingleLine => "",
+                            ArgumentsFormatting::Multiline { ref fallback_indent, .. } => {
+                                fallback_indent
+                            }
+                        };
+                        let prefix = if last_arg && !no_provided_arg_yet {
+                            // `call(a)` -> `call(a, b)` requires adding a comma.
+                            ", "
+                        } else {
+                            ""
+                        };
+                        let suffix = match arguments_formatting {
+                            ArgumentsFormatting::SingleLine if !last_arg => ", ",
+                            ArgumentsFormatting::SingleLine => "",
+                            ArgumentsFormatting::Multiline { .. } => "\n",
+                        };
+                        let suggestion = format!("{indent}{prefix}{expected_ty}{suffix}");
+                        if let Some((last_sugg_span, last_sugg_msg)) = suggestions.last_mut()
+                            && *last_sugg_span == insertion_span
+                        {
+                            // More than one suggestion to insert at a given span.
+                            // Merge them into one.
+                            if last_sugg_msg.ends_with(", ") && prefix == ", " {
+                                // TODO: explain what this condition is and why
+                                // it is needed.
+                                // TODO: find a better way to express this
+                                // condition.
+                                last_sugg_msg.truncate(last_sugg_msg.len() - 2);
+                            }
+                            last_sugg_msg.push_str(&suggestion);
+                        } else {
+                            suggestions.push((insertion_span, suggestion));
+                        };
+                    };
                 }
-                suggestion += &suggestion_text;
-            }
-            if let ArgumentsFormatting::Multiline { brace_indent, .. } = arguments_formatting {
-                suggestion += ",\n";
-                suggestion += &brace_indent;
+                err.multipart_suggestion_verbose(
+                    suggestion_text,
+                    suggestions,
+                    Applicability::HasPlaceholders,
+                );
+            } else {
+                // FIXME: make this multiline-aware.
+                let mut suggestion = "(".to_owned();
+                let mut needs_comma = false;
+                for (expected_idx, provided_idx) in matched_inputs.iter_enumerated() {
+                    if needs_comma {
+                        suggestion += ", ";
+                    } else {
+                        needs_comma = true;
+                    }
+                    let suggestion_text = if let Some(provided_idx) = provided_idx
+                        && let (_, provided_span) = provided_arg_tys[*provided_idx]
+                        && let Ok(arg_text) = source_map.span_to_snippet(provided_span)
+                    {
+                        arg_text
+                    } else {
+                        // Propose a placeholder of the correct type
+                        let (_, expected_ty) = formal_and_expected_inputs[expected_idx];
+                        ty_to_snippet(expected_ty, expected_idx)
+                    };
+                    suggestion += &suggestion_text;
+                }
+                suggestion += ")";
+                err.span_suggestion_verbose(
+                    suggestion_span,
+                    suggestion_text,
+                    suggestion,
+                    Applicability::HasPlaceholders,
+                );
             }
-            suggestion += ")";
-            err.span_suggestion_verbose(
-                suggestion_span,
-                suggestion_text,
-                suggestion,
-                Applicability::HasPlaceholders,
-            );
         }
 
         err.emit()

compiler/rustc_span/src/source_map.rs

@@ -543,9 +543,13 @@ impl SourceMap {
     /// Extracts the source surrounding the given `Span` using the `extract_source` function. The
     /// extract function takes three arguments: a string slice containing the source, an index in
     /// the slice for the beginning of the span and an index in the slice for the end of the span.
-    pub fn span_to_source<F, T>(&self, sp: Span, extract_source: F) -> Result<T, SpanSnippetError>
+    pub fn span_to_source<F, T>(
+        &self,
+        sp: Span,
+        mut extract_source: F,
+    ) -> Result<T, SpanSnippetError>
     where
-        F: Fn(&str, usize, usize) -> Result<T, SpanSnippetError>,
+        F: FnMut(&str, usize, usize) -> Result<T, SpanSnippetError>,
     {
         let local_begin = self.lookup_byte_offset(sp.lo());
         let local_end = self.lookup_byte_offset(sp.hi());
@@ -700,7 +704,7 @@ impl SourceMap {
     pub fn span_extend_while(
         &self,
         span: Span,
-        f: impl Fn(char) -> bool,
+        mut f: impl FnMut(char) -> bool,
     ) -> Result<Span, SpanSnippetError> {
         self.span_to_source(span, |s, _start, end| {
             let n = s[end..].char_indices().find(|&(_, c)| !f(c)).map_or(s.len() - end, |(i, _)| i);

tests/ui/argument-suggestions/issue-100478.stderr

@@ -14,9 +14,8 @@ LL | fn three_diff(_a: T1, _b: T2, _c: T3) {}
    |    ^^^^^^^^^^ ------          ------
 help: provide the arguments
    |
-LL -     three_diff(T2::new(0));
-LL +     three_diff(/* T1 */, T2::new(0), /* T3 */);
-   |
+LL |     three_diff(/* T1 */, T2::new(0), /* T3 */);
+   |                +++++++++           ++++++++++
 
 error[E0308]: arguments to this function are incorrect
   --> $DIR/issue-100478.rs:35:5
@@ -75,17 +74,8 @@ LL | fn foo(p1: T1, p2: Arc<T2>, p3: T3, p4: Arc<T4>, p5: T5, p6: T6, p7: T7, p8
    |    ^^^         -----------
 help: provide the argument
    |
-LL ~     foo(
-LL +         p1,
-LL +         /* Arc<T2> */,
-LL +         p3,
-LL +         p4,
-LL +         p5,
-LL +         p6,
-LL +         p7,
-LL +         p8,
-LL ~     );
-   |
+LL |         p1,         /* Arc<T2> */
+   |                     +++++++++++++
 
 error: aborting due to 4 previous errors