Skip to content

Commit

Permalink
Rollup merge of #126588 - linyihai:trim-extra-comma, r=petrochenkov
Browse files Browse the repository at this point in the history
Added more scenarios where comma to be removed in the function arg

This is an attempt to address the problem methion in #106304 (comment).

Copy the annotation to explain the fix

If the next Error::Extra ("next") doesn't next to current ("current")

```
fn foo(_: (), _: u32) {}
- foo("current", (), 1u32, "next")
+ foo((), 1u32)
```

If the previous error is not a `Error::Extra`, then do not trim the next comma

```
- foo((), "current", 42u32, "next")
+ foo((), 42u32)
```

Frankly, this is a fix from a test case and may not cover all scenarios
  • Loading branch information
matthiaskrgr authored Oct 21, 2024
2 parents fb32dd4 + f107082 commit 9ff4dab
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 9 deletions.
30 changes: 25 additions & 5 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,7 @@ 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 prev_extra_idx = None;
let mut suggestions = vec![];
while let Some(error) = errors.next() {
only_extras_so_far &= matches!(error, Error::Extra(_));
Expand Down Expand Up @@ -1165,11 +1166,29 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// fn f() {}
// - f(0, 1,)
// + f()
if only_extras_so_far
&& !errors
.peek()
.is_some_and(|next_error| matches!(next_error, Error::Extra(_)))
{
let trim_next_comma = match errors.peek() {
Some(Error::Extra(provided_idx))
if only_extras_so_far
&& provided_idx.index() > arg_idx.index() + 1 =>
// If the next Error::Extra ("next") doesn't next to current ("current"),
// fn foo(_: (), _: u32) {}
// - foo("current", (), 1u32, "next")
// + foo((), 1u32)
// If the previous error is not a `Error::Extra`, then do not trim the next comma
// - foo((), "current", 42u32, "next")
// + foo((), 42u32)
{
prev_extra_idx.map_or(true, |prev_extra_idx| {
prev_extra_idx + 1 == arg_idx.index()
})
}
// If no error left, we need to delete the next comma
None if only_extras_so_far => true,
// Not sure if other error type need to be handled as well
_ => false,
};

if trim_next_comma {
let next = provided_arg_tys
.get(arg_idx + 1)
.map(|&(_, sp)| sp)
Expand All @@ -1192,6 +1211,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
SuggestionText::Remove(_) => SuggestionText::Remove(true),
_ => SuggestionText::DidYouMean,
};
prev_extra_idx = Some(arg_idx.index())
}
}
Error::Missing(expected_idx) => {
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/argument-suggestions/issue-109425.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ fn main() {
//~^ error: this function takes 1 argument but 3 arguments were supplied
is(0, ""); // is(0, "")
//~^ error: this function takes 2 arguments but 4 arguments were supplied
is(1, "");
//~^ error: this function takes 2 arguments but 4 arguments were supplied
is(1, "");
//~^ error: this function takes 2 arguments but 4 arguments were supplied
s(""); // s("")
//~^ error: this function takes 1 argument but 3 arguments were supplied
}
4 changes: 4 additions & 0 deletions tests/ui/argument-suggestions/issue-109425.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ fn main() {
//~^ error: this function takes 1 argument but 3 arguments were supplied
is(0, 1, 2, ""); // is(0, "")
//~^ error: this function takes 2 arguments but 4 arguments were supplied
is((), 1, "", ());
//~^ error: this function takes 2 arguments but 4 arguments were supplied
is(1, (), "", ());
//~^ error: this function takes 2 arguments but 4 arguments were supplied
s(0, 1, ""); // s("")
//~^ error: this function takes 1 argument but 3 arguments were supplied
}
42 changes: 40 additions & 2 deletions tests/ui/argument-suggestions/issue-109425.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,47 @@ LL - is(0, 1, 2, ""); // is(0, "")
LL + is(0, ""); // is(0, "")
|

error[E0061]: this function takes 1 argument but 3 arguments were supplied
error[E0061]: this function takes 2 arguments but 4 arguments were supplied
--> $DIR/issue-109425.rs:18:5
|
LL | is((), 1, "", ());
| ^^ -- -- unexpected argument #4 of type `()`
| |
| unexpected argument #1 of type `()`
|
note: function defined here
--> $DIR/issue-109425.rs:5:4
|
LL | fn is(_: u32, _: &str) {}
| ^^ ------ -------
help: remove the extra arguments
|
LL - is((), 1, "", ());
LL + is(1, "");
|

error[E0061]: this function takes 2 arguments but 4 arguments were supplied
--> $DIR/issue-109425.rs:20:5
|
LL | is(1, (), "", ());
| ^^ -- -- unexpected argument #4 of type `()`
| |
| unexpected argument #2 of type `()`
|
note: function defined here
--> $DIR/issue-109425.rs:5:4
|
LL | fn is(_: u32, _: &str) {}
| ^^ ------ -------
help: remove the extra arguments
|
LL - is(1, (), "", ());
LL + is(1, "");
|

error[E0061]: this function takes 1 argument but 3 arguments were supplied
--> $DIR/issue-109425.rs:22:5
|
LL | s(0, 1, ""); // s("")
| ^ - - unexpected argument #2 of type `{integer}`
| |
Expand All @@ -93,6 +131,6 @@ LL - s(0, 1, ""); // s("")
LL + s(""); // s("")
|

error: aborting due to 5 previous errors
error: aborting due to 7 previous errors

For more information about this error, try `rustc --explain E0061`.
5 changes: 3 additions & 2 deletions tests/ui/argument-suggestions/issue-112507.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ LL | Float(Option<f64>),
| ^^^^^
help: remove the extra arguments
|
LL ~ ,
LL ~ None);
LL - 0,
LL - None,
LL + None);
|

error: aborting due to 1 previous error
Expand Down

0 comments on commit 9ff4dab

Please sign in to comment.