Skip to content

Commit f65c6e4

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #106347 - estebank:removal-suggestion, r=TaKO8Ki
More accurate spans for arg removal suggestion Partially address #106304.
2 parents af3c8b2 + dff10d0 commit f65c6e4

24 files changed

+286
-232
lines changed

‎compiler/rustc_errors/src/emitter.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1768,7 +1768,7 @@ impl EmitterWriter {
17681768

17691769
// Render the replacements for each suggestion
17701770
let suggestions = suggestion.splice_lines(sm);
1771-
debug!("emit_suggestion_default: suggestions={:?}", suggestions);
1771+
debug!(?suggestions);
17721772

17731773
if suggestions.is_empty() {
17741774
// Suggestions coming from macros can have malformed spans. This is a heavy handed
@@ -1797,6 +1797,7 @@ impl EmitterWriter {
17971797
for (complete, parts, highlights, only_capitalization) in
17981798
suggestions.iter().take(MAX_SUGGESTIONS)
17991799
{
1800+
debug!(?complete, ?parts, ?highlights);
18001801
notice_capitalization |= only_capitalization;
18011802

18021803
let has_deletion = parts.iter().any(|p| p.is_deletion(sm));

‎compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

+96-25
Original file line numberDiff line numberDiff line change
@@ -755,15 +755,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
755755
}
756756

757757
errors.drain_filter(|error| {
758-
let Error::Invalid(provided_idx, expected_idx, Compatibility::Incompatible(Some(e))) = error else { return false };
759-
let (provided_ty, provided_span) = provided_arg_tys[*provided_idx];
760-
let trace = mk_trace(provided_span, formal_and_expected_inputs[*expected_idx], provided_ty);
761-
if !matches!(trace.cause.as_failure_code(*e), FailureCode::Error0308(_)) {
762-
self.err_ctxt().report_and_explain_type_error(trace, *e).emit();
763-
return true;
764-
}
765-
false
766-
});
758+
let Error::Invalid(
759+
provided_idx,
760+
expected_idx,
761+
Compatibility::Incompatible(Some(e)),
762+
) = error else { return false };
763+
let (provided_ty, provided_span) = provided_arg_tys[*provided_idx];
764+
let trace =
765+
mk_trace(provided_span, formal_and_expected_inputs[*expected_idx], provided_ty);
766+
if !matches!(trace.cause.as_failure_code(*e), FailureCode::Error0308(_)) {
767+
self.err_ctxt().report_and_explain_type_error(trace, *e).emit();
768+
return true;
769+
}
770+
false
771+
});
767772

768773
// We're done if we found errors, but we already emitted them.
769774
if errors.is_empty() {
@@ -864,7 +869,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
864869
}
865870
let mut suggestion_text = SuggestionText::None;
866871

872+
let ty_to_snippet = |ty: Ty<'tcx>, expected_idx: ExpectedIdx| {
873+
if ty.is_unit() {
874+
"()".to_string()
875+
} else if ty.is_suggestable(tcx, false) {
876+
format!("/* {} */", ty)
877+
} else if let Some(fn_def_id) = fn_def_id
878+
&& self.tcx.def_kind(fn_def_id).is_fn_like()
879+
&& let self_implicit =
880+
matches!(call_expr.kind, hir::ExprKind::MethodCall(..)) as usize
881+
&& let Some(arg) = self.tcx.fn_arg_names(fn_def_id)
882+
.get(expected_idx.as_usize() + self_implicit)
883+
&& arg.name != kw::SelfLower
884+
{
885+
format!("/* {} */", arg.name)
886+
} else {
887+
"/* value */".to_string()
888+
}
889+
};
890+
867891
let mut errors = errors.into_iter().peekable();
892+
let mut suggestions = vec![];
868893
while let Some(error) = errors.next() {
869894
match error {
870895
Error::Invalid(provided_idx, expected_idx, compatibility) => {
@@ -905,7 +930,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
905930
"".to_string()
906931
};
907932
labels
908-
.push((provided_span, format!("argument{} unexpected", provided_ty_name)));
933+
.push((provided_span, format!("unexpected argument{}", provided_ty_name)));
934+
let mut span = provided_span;
935+
if arg_idx.index() > 0
936+
&& let Some((_, prev)) = provided_arg_tys
937+
.get(ProvidedIdx::from_usize(arg_idx.index() - 1)
938+
) {
939+
// Include previous comma
940+
span = span.with_lo(prev.hi());
941+
} else if let Some((_, next)) = provided_arg_tys.get(
942+
ProvidedIdx::from_usize(arg_idx.index() + 1),
943+
) {
944+
// Include next comma
945+
span = span.until(*next);
946+
}
947+
suggestions.push((span, String::new()));
948+
909949
suggestion_text = match suggestion_text {
910950
SuggestionText::None => SuggestionText::Remove(false),
911951
SuggestionText::Remove(_) => SuggestionText::Remove(true),
@@ -1095,6 +1135,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10951135
}
10961136
}
10971137

1138+
// Incorporate the argument changes in the removal suggestion.
1139+
// When a type is *missing*, and the rest are additional, we want to suggest these with a
1140+
// multipart suggestion, but in order to do so we need to figure out *where* the arg that
1141+
// was provided but had the wrong type should go, because when looking at `expected_idx`
1142+
// that is the position in the argument list in the definition, while `provided_idx` will
1143+
// not be present. So we have to look at what the *last* provided position was, and point
1144+
// one after to suggest the replacement. FIXME(estebank): This is hacky, and there's
1145+
// probably a better more involved change we can make to make this work.
1146+
// For example, if we have
1147+
// ```
1148+
// fn foo(i32, &'static str) {}
1149+
// foo((), (), ());
1150+
// ```
1151+
// what should be suggested is
1152+
// ```
1153+
// foo(/* i32 */, /* &str */);
1154+
// ```
1155+
// which includes the replacement of the first two `()` for the correct type, and the
1156+
// removal of the last `()`.
1157+
let mut prev = -1;
1158+
for (expected_idx, provided_idx) in matched_inputs.iter_enumerated() {
1159+
// We want to point not at the *current* argument expression index, but rather at the
1160+
// index position where it *should have been*, which is *after* the previous one.
1161+
if let Some(provided_idx) = provided_idx {
1162+
prev = provided_idx.index() as i64;
1163+
}
1164+
let idx = ProvidedIdx::from_usize((prev + 1) as usize);
1165+
if let None = provided_idx
1166+
&& let Some((_, arg_span)) = provided_arg_tys.get(idx)
1167+
{
1168+
// There is a type that was *not* found anywhere, so it isn't a move, but a
1169+
// replacement and we look at what type it should have been. This will allow us
1170+
// To suggest a multipart suggestion when encountering `foo(1, "")` where the def
1171+
// was `fn foo(())`.
1172+
let (_, expected_ty) = formal_and_expected_inputs[expected_idx];
1173+
suggestions.push((*arg_span, ty_to_snippet(expected_ty, expected_idx)));
1174+
}
1175+
}
1176+
10981177
// If we have less than 5 things to say, it would be useful to call out exactly what's wrong
10991178
if labels.len() <= 5 {
11001179
for (span, label) in labels {
@@ -1112,7 +1191,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
11121191
Some(format!("provide the argument{}", if plural { "s" } else { "" }))
11131192
}
11141193
SuggestionText::Remove(plural) => {
1115-
Some(format!("remove the extra argument{}", if plural { "s" } else { "" }))
1194+
err.multipart_suggestion(
1195+
&format!("remove the extra argument{}", if plural { "s" } else { "" }),
1196+
suggestions,
1197+
Applicability::HasPlaceholders,
1198+
);
1199+
None
11161200
}
11171201
SuggestionText::Swap => Some("swap these arguments".to_string()),
11181202
SuggestionText::Reorder => Some("reorder these arguments".to_string()),
@@ -1151,20 +1235,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
11511235
} else {
11521236
// Propose a placeholder of the correct type
11531237
let (_, expected_ty) = formal_and_expected_inputs[expected_idx];
1154-
if expected_ty.is_unit() {
1155-
"()".to_string()
1156-
} else if expected_ty.is_suggestable(tcx, false) {
1157-
format!("/* {} */", expected_ty)
1158-
} else if let Some(fn_def_id) = fn_def_id
1159-
&& self.tcx.def_kind(fn_def_id).is_fn_like()
1160-
&& let self_implicit = matches!(call_expr.kind, hir::ExprKind::MethodCall(..)) as usize
1161-
&& let Some(arg) = self.tcx.fn_arg_names(fn_def_id).get(expected_idx.as_usize() + self_implicit)
1162-
&& arg.name != kw::SelfLower
1163-
{
1164-
format!("/* {} */", arg.name)
1165-
} else {
1166-
"/* value */".to_string()
1167-
}
1238+
ty_to_snippet(expected_ty, expected_idx)
11681239
};
11691240
suggestion += &suggestion_text;
11701241
}

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

+4-5
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,17 @@ LL | fn oom() -> ! {
77
| _-^^^^^^^^^^^^
88
LL | | loop {}
99
LL | | }
10-
| |_- argument of type `core::alloc::Layout` unexpected
10+
| | -
11+
| | |
12+
| |_unexpected argument of type `core::alloc::Layout`
13+
| help: remove the extra argument
1114
|
1215
note: function defined here
1316
--> $DIR/alloc-error-handler-bad-signature-3.rs:10:4
1417
|
1518
LL | fn oom() -> ! {
1619
| ^^^
1720
= note: this error originates in the attribute macro `alloc_error_handler` (in Nightly builds, run with -Z macro-backtrace for more info)
18-
help: remove the extra argument
19-
|
20-
LL | fn oom() -> !() {
21-
| ++
2221

2322
error: aborting due to previous error
2423

‎tests/ui/argument-suggestions/basic.stderr

+4-5
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,16 @@ error[E0061]: this function takes 0 arguments but 1 argument was supplied
1616
--> $DIR/basic.rs:21:5
1717
|
1818
LL | extra("");
19-
| ^^^^^ -- argument of type `&'static str` unexpected
19+
| ^^^^^ --
20+
| |
21+
| unexpected argument of type `&'static str`
22+
| help: remove the extra argument
2023
|
2124
note: function defined here
2225
--> $DIR/basic.rs:14:4
2326
|
2427
LL | fn extra() {}
2528
| ^^^^^
26-
help: remove the extra argument
27-
|
28-
LL | extra();
29-
| ~~
3029

3130
error[E0061]: this function takes 1 argument but 0 arguments were supplied
3231
--> $DIR/basic.rs:22:5

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

+16-20
Original file line numberDiff line numberDiff line change
@@ -2,65 +2,61 @@ error[E0057]: this function takes 0 arguments but 1 argument was supplied
22
--> $DIR/exotic-calls.rs:2:5
33
|
44
LL | t(1i32);
5-
| ^ ---- argument of type `i32` unexpected
5+
| ^ ----
6+
| |
7+
| unexpected argument of type `i32`
8+
| help: remove the extra argument
69
|
710
note: callable defined here
811
--> $DIR/exotic-calls.rs:1:11
912
|
1013
LL | fn foo<T: Fn()>(t: T) {
1114
| ^^^^
12-
help: remove the extra argument
13-
|
14-
LL | t();
15-
| ~~
1615

1716
error[E0057]: this function takes 0 arguments but 1 argument was supplied
1817
--> $DIR/exotic-calls.rs:7:5
1918
|
2019
LL | t(1i32);
21-
| ^ ---- argument of type `i32` unexpected
20+
| ^ ----
21+
| |
22+
| unexpected argument of type `i32`
23+
| help: remove the extra argument
2224
|
2325
note: type parameter defined here
2426
--> $DIR/exotic-calls.rs:6:11
2527
|
2628
LL | fn bar(t: impl Fn()) {
2729
| ^^^^^^^^^
28-
help: remove the extra argument
29-
|
30-
LL | t();
31-
| ~~
3230

3331
error[E0057]: this function takes 0 arguments but 1 argument was supplied
3432
--> $DIR/exotic-calls.rs:16:5
3533
|
3634
LL | baz()(1i32)
37-
| ^^^^^ ---- argument of type `i32` unexpected
35+
| ^^^^^ ----
36+
| |
37+
| unexpected argument of type `i32`
38+
| help: remove the extra argument
3839
|
3940
note: opaque type defined here
4041
--> $DIR/exotic-calls.rs:11:13
4142
|
4243
LL | fn baz() -> impl Fn() {
4344
| ^^^^^^^^^
44-
help: remove the extra argument
45-
|
46-
LL | baz()()
47-
| ~~
4845

4946
error[E0057]: this function takes 0 arguments but 1 argument was supplied
5047
--> $DIR/exotic-calls.rs:22:5
5148
|
5249
LL | x(1i32);
53-
| ^ ---- argument of type `i32` unexpected
50+
| ^ ----
51+
| |
52+
| unexpected argument of type `i32`
53+
| help: remove the extra argument
5454
|
5555
note: closure defined here
5656
--> $DIR/exotic-calls.rs:21:13
5757
|
5858
LL | let x = || {};
5959
| ^^
60-
help: remove the extra argument
61-
|
62-
LL | x();
63-
| ~~
6460

6561
error: aborting due to 4 previous errors
6662

0 commit comments

Comments
 (0)