Skip to content

Commit d368e3f

Browse files
scrabshascrabsha
committed
Malformed call function call: do multiple suggestions when possible
1 parent 16ad385 commit d368e3f

File tree

7 files changed

+200
-159
lines changed

7 files changed

+200
-159
lines changed

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

Lines changed: 148 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,7 +1154,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
11541154
Reorder,
11551155
DidYouMean,
11561156
}
1157-
let mut suggestion_text = SuggestionText::None;
1157+
let mut suggestion_kind = SuggestionText::None;
11581158

11591159
let ty_to_snippet = |ty: Ty<'tcx>, expected_idx: ExpectedIdx| {
11601160
if ty.is_unit() {
@@ -1179,10 +1179,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
11791179
let mut only_extras_so_far = errors
11801180
.peek()
11811181
.is_some_and(|first| matches!(first, Error::Extra(arg_idx) if arg_idx.index() == 0));
1182+
let mut only_missing_so_far = true;
11821183
let mut prev_extra_idx = None;
11831184
let mut suggestions = vec![];
11841185
while let Some(error) = errors.next() {
11851186
only_extras_so_far &= matches!(error, Error::Extra(_));
1187+
only_missing_so_far &= matches!(error, Error::Missing(_));
11861188

11871189
match error {
11881190
Error::Invalid(provided_idx, expected_idx, compatibility) => {
@@ -1290,7 +1292,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
12901292

12911293
suggestions.push((span, String::new()));
12921294

1293-
suggestion_text = match suggestion_text {
1295+
suggestion_kind = match suggestion_kind {
12941296
SuggestionText::None => SuggestionText::Remove(false),
12951297
SuggestionText::Remove(_) => SuggestionText::Remove(true),
12961298
_ => SuggestionText::DidYouMean,
@@ -1343,7 +1345,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
13431345
),
13441346
));
13451347

1346-
suggestion_text = match suggestion_text {
1348+
suggestion_kind = match suggestion_kind {
13471349
SuggestionText::None => SuggestionText::Provide(false),
13481350
SuggestionText::Provide(_) => SuggestionText::Provide(true),
13491351
_ => SuggestionText::DidYouMean,
@@ -1369,7 +1371,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
13691371
"".to_string()
13701372
};
13711373
labels.push((span, format!("two arguments{rendered} are missing")));
1372-
suggestion_text = match suggestion_text {
1374+
suggestion_kind = match suggestion_kind {
13731375
SuggestionText::None | SuggestionText::Provide(_) => {
13741376
SuggestionText::Provide(true)
13751377
}
@@ -1400,7 +1402,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
14001402
"".to_string()
14011403
};
14021404
labels.push((span, format!("three arguments{rendered} are missing")));
1403-
suggestion_text = match suggestion_text {
1405+
suggestion_kind = match suggestion_kind {
14041406
SuggestionText::None | SuggestionText::Provide(_) => {
14051407
SuggestionText::Provide(true)
14061408
}
@@ -1422,7 +1424,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
14221424
args_span
14231425
};
14241426
labels.push((span, "multiple arguments are missing".to_string()));
1425-
suggestion_text = match suggestion_text {
1427+
suggestion_kind = match suggestion_kind {
14261428
SuggestionText::None | SuggestionText::Provide(_) => {
14271429
SuggestionText::Provide(true)
14281430
}
@@ -1461,7 +1463,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
14611463
format!("expected `{second_expected_ty}`{second_provided_ty_name}"),
14621464
));
14631465

1464-
suggestion_text = match suggestion_text {
1466+
suggestion_kind = match suggestion_kind {
14651467
SuggestionText::None => SuggestionText::Swap,
14661468
_ => SuggestionText::DidYouMean,
14671469
};
@@ -1481,7 +1483,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
14811483
));
14821484
}
14831485

1484-
suggestion_text = match suggestion_text {
1486+
suggestion_kind = match suggestion_kind {
14851487
SuggestionText::None => SuggestionText::Reorder,
14861488
_ => SuggestionText::DidYouMean,
14871489
};
@@ -1559,7 +1561,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
15591561
);
15601562

15611563
// And add a suggestion block for all of the parameters
1562-
let suggestion_text = match suggestion_text {
1564+
let suggestion_text = match suggestion_kind {
15631565
SuggestionText::None => None,
15641566
SuggestionText::Provide(plural) => {
15651567
Some(format!("provide the argument{}", if plural { "s" } else { "" }))
@@ -1580,80 +1582,149 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
15801582
&& !full_call_span.in_external_macro(self.sess().source_map())
15811583
{
15821584
let source_map = self.sess().source_map();
1583-
let suggestion_span = if let Some(args_span) = error_span.trim_start(full_call_span) {
1584-
// Span of the braces, e.g. `(a, b, c)`.
1585-
args_span
1586-
} else {
1587-
// The arg span of a function call that wasn't even given braces
1588-
// like what might happen with delegation reuse.
1589-
// e.g. `reuse HasSelf::method;` should suggest `reuse HasSelf::method($args);`.
1590-
full_call_span.shrink_to_hi()
1591-
};
1592-
1593-
// Controls how the arguments should be listed in the suggestion.
1594-
enum ArgumentsFormatting {
1595-
SingleLine,
1596-
Multiline { fallback_indent: String, brace_indent: String },
1597-
}
1598-
let arguments_formatting = {
1599-
let mut provided_inputs = matched_inputs.iter().filter_map(|a| *a);
1600-
if let Some(brace_indent) = source_map.indentation_before(suggestion_span)
1601-
&& let Some(first_idx) = provided_inputs.by_ref().next()
1602-
&& let Some(last_idx) = provided_inputs.by_ref().next()
1603-
&& let (_, first_span) = provided_arg_tys[first_idx]
1604-
&& let (_, last_span) = provided_arg_tys[last_idx]
1605-
&& source_map.is_multiline(first_span.to(last_span))
1606-
&& let Some(fallback_indent) = source_map.indentation_before(first_span)
1607-
{
1608-
ArgumentsFormatting::Multiline { fallback_indent, brace_indent }
1585+
let (suggestion_span, has_paren) =
1586+
if let Some(args_span) = error_span.trim_start(full_call_span) {
1587+
// Span of the braces, e.g. `(a, b, c)`.
1588+
(args_span, true)
16091589
} else {
1610-
ArgumentsFormatting::SingleLine
1611-
}
1612-
};
1590+
// The arg span of a function call that wasn't even given braces
1591+
// like what might happen with delegation reuse.
1592+
// e.g. `reuse HasSelf::method;` should suggest `reuse HasSelf::method($args);`.
1593+
(full_call_span.shrink_to_hi(), false)
1594+
};
16131595

1614-
let mut suggestion = "(".to_owned();
1615-
let mut needs_comma = false;
1616-
for (expected_idx, provided_idx) in matched_inputs.iter_enumerated() {
1617-
if needs_comma {
1618-
suggestion += ",";
1619-
}
1620-
match &arguments_formatting {
1621-
ArgumentsFormatting::SingleLine if needs_comma => suggestion += " ",
1622-
ArgumentsFormatting::SingleLine => {}
1623-
ArgumentsFormatting::Multiline { .. } => suggestion += "\n",
1596+
if matches!(suggestion_kind, SuggestionText::Provide(_))
1597+
&& has_paren
1598+
&& only_missing_so_far
1599+
{
1600+
// Controls how the arguments should be listed in the suggestion.
1601+
enum ArgumentsFormatting {
1602+
SingleLine,
1603+
Multiline { fallback_indent: String },
16241604
}
1625-
needs_comma = true;
1626-
let (suggestion_span, suggestion_text) = if let Some(provided_idx) = provided_idx
1627-
&& let (_, provided_span) = provided_arg_tys[*provided_idx]
1628-
&& let Ok(arg_text) = source_map.span_to_snippet(provided_span)
1629-
{
1630-
(Some(provided_span), arg_text)
1631-
} else {
1632-
// Propose a placeholder of the correct type
1633-
let (_, expected_ty) = formal_and_expected_inputs[expected_idx];
1634-
(None, ty_to_snippet(expected_ty, expected_idx))
1605+
let arguments_formatting = {
1606+
let mut provided_inputs = matched_inputs.iter().filter_map(|a| *a);
1607+
if let Some(first_idx) = provided_inputs.by_ref().next()
1608+
&& let Some(last_idx) = provided_inputs.by_ref().next()
1609+
&& let (_, first_span) = provided_arg_tys[first_idx]
1610+
&& let (_, last_span) = provided_arg_tys[last_idx]
1611+
&& source_map.is_multiline(first_span.to(last_span))
1612+
&& let Some(fallback_indent) = source_map.indentation_before(first_span)
1613+
{
1614+
ArgumentsFormatting::Multiline { fallback_indent }
1615+
} else {
1616+
ArgumentsFormatting::SingleLine
1617+
}
16351618
};
1636-
if let ArgumentsFormatting::Multiline { fallback_indent, .. } =
1637-
&arguments_formatting
1638-
{
1639-
let indent = suggestion_span
1640-
.and_then(|span| source_map.indentation_before(span))
1641-
.unwrap_or_else(|| fallback_indent.clone());
1642-
suggestion += &indent;
1619+
1620+
let mut suggestions: Vec<(Span, String)> = Vec::new();
1621+
let mut previous_span = source_map.start_point(suggestion_span).shrink_to_hi();
1622+
let mut first = true;
1623+
for (expected_idx, provided_idx) in matched_inputs.iter_enumerated() {
1624+
if let Some(provided_idx) = provided_idx {
1625+
let (_, provided_span) = provided_arg_tys[*provided_idx];
1626+
previous_span = provided_span;
1627+
} else {
1628+
// Propose a placeholder of the correct type
1629+
let (insertion_span, last_arg) = if first {
1630+
(previous_span, matched_inputs.len() - 1 == expected_idx.as_usize())
1631+
} else {
1632+
let mut comma_hit = false;
1633+
let mut closing_paren_hit = false;
1634+
let after_arg = previous_span.shrink_to_hi();
1635+
let after_previous_comma = source_map
1636+
.span_extend_while(after_arg, |c| {
1637+
closing_paren_hit = c == ')';
1638+
if comma_hit || closing_paren_hit {
1639+
false
1640+
} else {
1641+
comma_hit = c == ',';
1642+
true
1643+
}
1644+
})
1645+
.unwrap()
1646+
.shrink_to_hi();
1647+
let span = if closing_paren_hit {
1648+
after_previous_comma
1649+
} else {
1650+
source_map.next_point(after_previous_comma).shrink_to_hi()
1651+
};
1652+
(span, closing_paren_hit)
1653+
};
1654+
let (_, expected_ty) = formal_and_expected_inputs[expected_idx];
1655+
let expected_ty = ty_to_snippet(expected_ty, expected_idx);
1656+
let indent = match arguments_formatting {
1657+
ArgumentsFormatting::SingleLine => "",
1658+
ArgumentsFormatting::Multiline { ref fallback_indent, .. } => {
1659+
fallback_indent
1660+
}
1661+
};
1662+
let prefix = if last_arg && !first {
1663+
// `call(a)` -> `call(a, b)` requires adding a comma.
1664+
", "
1665+
} else {
1666+
""
1667+
};
1668+
let suffix = match arguments_formatting {
1669+
ArgumentsFormatting::SingleLine if !last_arg => ", ",
1670+
ArgumentsFormatting::SingleLine => "",
1671+
ArgumentsFormatting::Multiline { .. } => "\n",
1672+
};
1673+
let suggestion = format!("{indent}{prefix}{expected_ty}{suffix}");
1674+
if let Some((last_sugg_span, last_sugg_msg)) = suggestions.last_mut()
1675+
&& *last_sugg_span == insertion_span
1676+
{
1677+
// More than one suggestion to insert at a given span.
1678+
// Merge them into one.
1679+
if last_sugg_msg.ends_with(", ") && prefix == ", " {
1680+
// TODO: explain what this condition is and why
1681+
// it is needed.
1682+
// TODO: find a better way to express this
1683+
// condition.
1684+
last_sugg_msg.truncate(last_sugg_msg.len() - 2);
1685+
}
1686+
last_sugg_msg.push_str(&suggestion);
1687+
} else {
1688+
suggestions.push((insertion_span, suggestion));
1689+
};
1690+
};
1691+
first = false;
16431692
}
1644-
suggestion += &suggestion_text;
1645-
}
1646-
if let ArgumentsFormatting::Multiline { brace_indent, .. } = arguments_formatting {
1647-
suggestion += ",\n";
1648-
suggestion += &brace_indent;
1693+
err.multipart_suggestion_verbose(
1694+
suggestion_text,
1695+
suggestions,
1696+
Applicability::HasPlaceholders,
1697+
);
1698+
} else {
1699+
// FIXME: make this multiline-aware.
1700+
let mut suggestion = "(".to_owned();
1701+
let mut needs_comma = false;
1702+
for (expected_idx, provided_idx) in matched_inputs.iter_enumerated() {
1703+
if needs_comma {
1704+
suggestion += ", ";
1705+
} else {
1706+
needs_comma = true;
1707+
}
1708+
let suggestion_text = if let Some(provided_idx) = provided_idx
1709+
&& let (_, provided_span) = provided_arg_tys[*provided_idx]
1710+
&& let Ok(arg_text) = source_map.span_to_snippet(provided_span)
1711+
{
1712+
arg_text
1713+
} else {
1714+
// Propose a placeholder of the correct type
1715+
let (_, expected_ty) = formal_and_expected_inputs[expected_idx];
1716+
ty_to_snippet(expected_ty, expected_idx)
1717+
};
1718+
suggestion += &suggestion_text;
1719+
}
1720+
suggestion += ")";
1721+
err.span_suggestion_verbose(
1722+
suggestion_span,
1723+
suggestion_text,
1724+
suggestion,
1725+
Applicability::HasPlaceholders,
1726+
);
16491727
}
1650-
suggestion += ")";
1651-
err.span_suggestion_verbose(
1652-
suggestion_span,
1653-
suggestion_text,
1654-
suggestion,
1655-
Applicability::HasPlaceholders,
1656-
);
16571728
}
16581729

16591730
err.emit()

‎compiler/rustc_span/src/source_map.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -543,9 +543,13 @@ impl SourceMap {
543543
/// Extracts the source surrounding the given `Span` using the `extract_source` function. The
544544
/// extract function takes three arguments: a string slice containing the source, an index in
545545
/// the slice for the beginning of the span and an index in the slice for the end of the span.
546-
pub fn span_to_source<F, T>(&self, sp: Span, extract_source: F) -> Result<T, SpanSnippetError>
546+
pub fn span_to_source<F, T>(
547+
&self,
548+
sp: Span,
549+
mut extract_source: F,
550+
) -> Result<T, SpanSnippetError>
547551
where
548-
F: Fn(&str, usize, usize) -> Result<T, SpanSnippetError>,
552+
F: FnMut(&str, usize, usize) -> Result<T, SpanSnippetError>,
549553
{
550554
let local_begin = self.lookup_byte_offset(sp.lo());
551555
let local_end = self.lookup_byte_offset(sp.hi());
@@ -700,7 +704,7 @@ impl SourceMap {
700704
pub fn span_extend_while(
701705
&self,
702706
span: Span,
703-
f: impl Fn(char) -> bool,
707+
mut f: impl FnMut(char) -> bool,
704708
) -> Result<Span, SpanSnippetError> {
705709
self.span_to_source(span, |s, _start, end| {
706710
let n = s[end..].char_indices().find(|&(_, c)| !f(c)).map_or(s.len() - end, |(i, _)| i);

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

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,8 @@ LL | fn three_diff(_a: T1, _b: T2, _c: T3) {}
1414
| ^^^^^^^^^^ ------ ------
1515
help: provide the arguments
1616
|
17-
LL - three_diff(T2::new(0));
18-
LL + three_diff(/* T1 */, T2::new(0), /* T3 */);
19-
|
17+
LL | three_diff(/* T1 */, T2::new(0), /* T3 */);
18+
| +++++++++ ++++++++++
2019

2120
error[E0308]: arguments to this function are incorrect
2221
--> $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
7574
| ^^^ -----------
7675
help: provide the argument
7776
|
78-
LL ~ foo(
79-
LL + p1,
80-
LL + /* Arc<T2> */,
81-
LL + p3,
82-
LL + p4,
83-
LL + p5,
84-
LL + p6,
85-
LL + p7,
86-
LL + p8,
87-
LL ~ );
88-
|
77+
LL | p1, /* Arc<T2> */
78+
| +++++++++++++
8979

9080
error: aborting due to 4 previous errors
9181

0 commit comments

Comments
 (0)