From bd4b755f574296e0ad7ea49f230d14bf30121282 Mon Sep 17 00:00:00 2001 From: Sasha Pourcelot Date: Tue, 19 Aug 2025 20:02:16 +0200 Subject: [PATCH] Malformed call function call: do multiple suggestions when possible --- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 204 ++++++++++++------ compiler/rustc_span/src/source_map.rs | 10 +- .../argument-suggestions/issue-100478.stderr | 18 +- .../missing_arguments.stderr | 90 ++++---- .../fn-arg-count-mismatch-diagnostics.stderr | 5 +- tests/ui/fn/issue-3044.stderr | 6 +- tests/ui/typeck/cyclic_type_ice.stderr | 5 +- 7 files changed, 188 insertions(+), 150 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index b80a2af31007e..41e930c9cd271 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/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 == ", " { + // FIXME(scrabsha): explain what this condition is and why + // it is needed. + // FIXME(scrabsha): 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() diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs index d93151497986c..5000af8c25781 100644 --- a/compiler/rustc_span/src/source_map.rs +++ b/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(&self, sp: Span, extract_source: F) -> Result + pub fn span_to_source( + &self, + sp: Span, + mut extract_source: F, + ) -> Result where - F: Fn(&str, usize, usize) -> Result, + F: FnMut(&str, usize, usize) -> Result, { 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 { 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); diff --git a/tests/ui/argument-suggestions/issue-100478.stderr b/tests/ui/argument-suggestions/issue-100478.stderr index 81dbff2f00089..b480292118a53 100644 --- a/tests/ui/argument-suggestions/issue-100478.stderr +++ b/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, p3: T3, p4: Arc, p5: T5, p6: T6, p7: T7, p8 | ^^^ ----------- help: provide the argument | -LL ~ foo( -LL + p1, -LL + /* Arc */, -LL + p3, -LL + p4, -LL + p5, -LL + p6, -LL + p7, -LL + p8, -LL ~ ); - | +LL | p1, /* Arc */ + | +++++++++++++ error: aborting due to 4 previous errors diff --git a/tests/ui/argument-suggestions/missing_arguments.stderr b/tests/ui/argument-suggestions/missing_arguments.stderr index 9dc13c41113b6..ab8cf5b37d889 100644 --- a/tests/ui/argument-suggestions/missing_arguments.stderr +++ b/tests/ui/argument-suggestions/missing_arguments.stderr @@ -27,9 +27,8 @@ LL | fn two_same(_a: i32, _b: i32) {} | ^^^^^^^^ ------- ------- help: provide the arguments | -LL - two_same( ); -LL + two_same(/* i32 */, /* i32 */); - | +LL | two_same(/* i32 */, /* i32 */ ); + | ++++++++++++++++++++ error[E0061]: this function takes 2 arguments but 1 argument was supplied --> $DIR/missing_arguments.rs:15:3 @@ -44,9 +43,8 @@ LL | fn two_same(_a: i32, _b: i32) {} | ^^^^^^^^ ------- help: provide the argument | -LL - two_same( 1 ); -LL + two_same(1, /* i32 */); - | +LL | two_same( 1 , /* i32 */); + | +++++++++++ error[E0061]: this function takes 2 arguments but 0 arguments were supplied --> $DIR/missing_arguments.rs:16:3 @@ -61,9 +59,8 @@ LL | fn two_diff(_a: i32, _b: f32) {} | ^^^^^^^^ ------- ------- help: provide the arguments | -LL - two_diff( ); -LL + two_diff(/* i32 */, /* f32 */); - | +LL | two_diff(/* i32 */, /* f32 */ ); + | ++++++++++++++++++++ error[E0061]: this function takes 2 arguments but 1 argument was supplied --> $DIR/missing_arguments.rs:17:3 @@ -78,9 +75,8 @@ LL | fn two_diff(_a: i32, _b: f32) {} | ^^^^^^^^ ------- help: provide the argument | -LL - two_diff( 1 ); -LL + two_diff(1, /* f32 */); - | +LL | two_diff( 1 , /* f32 */); + | +++++++++++ error[E0061]: this function takes 2 arguments but 1 argument was supplied --> $DIR/missing_arguments.rs:18:3 @@ -95,9 +91,8 @@ LL | fn two_diff(_a: i32, _b: f32) {} | ^^^^^^^^ ------- help: provide the argument | -LL - two_diff( 1.0 ); -LL + two_diff(/* i32 */, 1.0); - | +LL | two_diff(/* i32 */, 1.0 ); + | ++++++++++ error[E0061]: this function takes 3 arguments but 0 arguments were supplied --> $DIR/missing_arguments.rs:21:3 @@ -112,9 +107,8 @@ LL | fn three_same(_a: i32, _b: i32, _c: i32) {} | ^^^^^^^^^^ ------- ------- ------- help: provide the arguments | -LL - three_same( ); -LL + three_same(/* i32 */, /* i32 */, /* i32 */); - | +LL | three_same(/* i32 */, /* i32 */, /* i32 */ ); + | +++++++++++++++++++++++++++++++ error[E0061]: this function takes 3 arguments but 1 argument was supplied --> $DIR/missing_arguments.rs:22:3 @@ -129,9 +123,8 @@ LL | fn three_same(_a: i32, _b: i32, _c: i32) {} | ^^^^^^^^^^ ------- ------- help: provide the arguments | -LL - three_same( 1 ); -LL + three_same(1, /* i32 */, /* i32 */); - | +LL | three_same( 1 , /* i32 */, /* i32 */); + | ++++++++++++++++++++++ error[E0061]: this function takes 3 arguments but 2 arguments were supplied --> $DIR/missing_arguments.rs:23:3 @@ -146,9 +139,8 @@ LL | fn three_same(_a: i32, _b: i32, _c: i32) {} | ^^^^^^^^^^ ------- help: provide the argument | -LL - three_same( 1, 1 ); -LL + three_same(1, 1, /* i32 */); - | +LL | three_same( 1, 1 , /* i32 */); + | +++++++++++ error[E0061]: this function takes 3 arguments but 2 arguments were supplied --> $DIR/missing_arguments.rs:26:3 @@ -163,9 +155,8 @@ LL | fn three_diff(_a: i32, _b: f32, _c: &str) {} | ^^^^^^^^^^ ------- help: provide the argument | -LL - three_diff( 1.0, "" ); -LL + three_diff(/* i32 */, 1.0, ""); - | +LL | three_diff(/* i32 */, 1.0, "" ); + | ++++++++++ error[E0061]: this function takes 3 arguments but 2 arguments were supplied --> $DIR/missing_arguments.rs:27:3 @@ -180,9 +171,8 @@ LL | fn three_diff(_a: i32, _b: f32, _c: &str) {} | ^^^^^^^^^^ ------- help: provide the argument | -LL - three_diff( 1, "" ); -LL + three_diff(1, /* f32 */, ""); - | +LL | three_diff( 1, /* f32 */, "" ); + | ++++++++++ error[E0061]: this function takes 3 arguments but 2 arguments were supplied --> $DIR/missing_arguments.rs:28:3 @@ -197,9 +187,8 @@ LL | fn three_diff(_a: i32, _b: f32, _c: &str) {} | ^^^^^^^^^^ -------- help: provide the argument | -LL - three_diff( 1, 1.0 ); -LL + three_diff(1, 1.0, /* &str */); - | +LL | three_diff( 1, 1.0 , /* &str */); + | ++++++++++++ error[E0061]: this function takes 3 arguments but 1 argument was supplied --> $DIR/missing_arguments.rs:29:3 @@ -214,9 +203,8 @@ LL | fn three_diff(_a: i32, _b: f32, _c: &str) {} | ^^^^^^^^^^ ------- ------- help: provide the arguments | -LL - three_diff( "" ); -LL + three_diff(/* i32 */, /* f32 */, ""); - | +LL | three_diff(/* i32 */, /* f32 */, "" ); + | +++++++++++++++++++++ error[E0061]: this function takes 3 arguments but 1 argument was supplied --> $DIR/missing_arguments.rs:30:3 @@ -234,9 +222,8 @@ LL | fn three_diff(_a: i32, _b: f32, _c: &str) {} | ^^^^^^^^^^ ------- -------- help: provide the arguments | -LL - three_diff( 1.0 ); -LL + three_diff(/* i32 */, 1.0, /* &str */); - | +LL | three_diff(/* i32 */, 1.0 , /* &str */); + | ++++++++++ ++++++++++++ error[E0061]: this function takes 3 arguments but 1 argument was supplied --> $DIR/missing_arguments.rs:31:3 @@ -251,9 +238,8 @@ LL | fn three_diff(_a: i32, _b: f32, _c: &str) {} | ^^^^^^^^^^ ------- -------- help: provide the arguments | -LL - three_diff( 1 ); -LL + three_diff(1, /* f32 */, /* &str */); - | +LL | three_diff( 1 , /* f32 */, /* &str */); + | +++++++++++++++++++++++ error[E0061]: this function takes 4 arguments but 0 arguments were supplied --> $DIR/missing_arguments.rs:34:3 @@ -268,9 +254,8 @@ LL | fn four_repeated(_a: i32, _b: f32, _c: f32, _d: &str) {} | ^^^^^^^^^^^^^ ------- ------- ------- -------- help: provide the arguments | -LL - four_repeated( ); -LL + four_repeated(/* i32 */, /* f32 */, /* f32 */, /* &str */); - | +LL | four_repeated(/* i32 */, /* f32 */, /* f32 */, /* &str */ ); + | +++++++++++++++++++++++++++++++++++++++++++ error[E0061]: this function takes 4 arguments but 2 arguments were supplied --> $DIR/missing_arguments.rs:35:3 @@ -285,9 +270,8 @@ LL | fn four_repeated(_a: i32, _b: f32, _c: f32, _d: &str) {} | ^^^^^^^^^^^^^ ------- ------- help: provide the arguments | -LL - four_repeated( 1, "" ); -LL + four_repeated(1, /* f32 */, /* f32 */, ""); - | +LL | four_repeated( 1, /* f32 */, /* f32 */, "" ); + | +++++++++++++++++++++ error[E0061]: this function takes 5 arguments but 0 arguments were supplied --> $DIR/missing_arguments.rs:38:3 @@ -302,9 +286,8 @@ LL | fn complex(_a: i32, _b: f32, _c: i32, _d: f32, _e: &str) {} | ^^^^^^^ ------- ------- ------- ------- -------- help: provide the arguments | -LL - complex( ); -LL + complex(/* i32 */, /* f32 */, /* i32 */, /* f32 */, /* &str */); - | +LL | complex(/* i32 */, /* f32 */, /* i32 */, /* f32 */, /* &str */ ); + | ++++++++++++++++++++++++++++++++++++++++++++++++++++++ error[E0061]: this function takes 5 arguments but 2 arguments were supplied --> $DIR/missing_arguments.rs:39:3 @@ -319,9 +302,8 @@ LL | fn complex(_a: i32, _b: f32, _c: i32, _d: f32, _e: &str) {} | ^^^^^^^ ------- ------- ------- help: provide the arguments | -LL - complex( 1, "" ); -LL + complex(1, /* f32 */, /* i32 */, /* f32 */, ""); - | +LL | complex( 1, /* f32 */, /* i32 */, /* f32 */, "" ); + | ++++++++++++++++++++++++++++++++ error: aborting due to 19 previous errors diff --git a/tests/ui/fn/fn-arg-count-mismatch-diagnostics.stderr b/tests/ui/fn/fn-arg-count-mismatch-diagnostics.stderr index dda9b398a833c..4053adebfcb37 100644 --- a/tests/ui/fn/fn-arg-count-mismatch-diagnostics.stderr +++ b/tests/ui/fn/fn-arg-count-mismatch-diagnostics.stderr @@ -99,10 +99,7 @@ LL | fn function_with_lots_of_arguments(a: i32, b: char, c: i32, d: i32, e: i32, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ------- help: provide the argument | -LL | function_with_lots_of_arguments( -LL | variable_name, -LL ~ /* char */, -LL ~ variable_name, +LL + /* char */ | error: aborting due to 6 previous errors diff --git a/tests/ui/fn/issue-3044.stderr b/tests/ui/fn/issue-3044.stderr index 787eeec09cc2f..4a6c285c0579e 100644 --- a/tests/ui/fn/issue-3044.stderr +++ b/tests/ui/fn/issue-3044.stderr @@ -11,10 +11,8 @@ note: method defined here --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL help: provide the argument | -LL | needlesArr.iter().fold(|x, y| { -LL | -LL ~ }, /* f */); - | +LL | }, /* f */); + | +++++++++ error: aborting due to 1 previous error diff --git a/tests/ui/typeck/cyclic_type_ice.stderr b/tests/ui/typeck/cyclic_type_ice.stderr index 645766becbf72..4e76bebcba991 100644 --- a/tests/ui/typeck/cyclic_type_ice.stderr +++ b/tests/ui/typeck/cyclic_type_ice.stderr @@ -22,9 +22,8 @@ LL | let f = |_, _| (); | ^^^^^^ help: provide the argument | -LL - f(f); -LL + f(/* _ */, /* _ */); - | +LL | f(/* _ */, /* _ */f); + | ++++++++++++++++ error: aborting due to 2 previous errors