Skip to content

Commit 2835ace

Browse files
camelidcamelid
committed
Improve suggestions for type errors with string concatenation
Now, multipart suggestions are used instead of `span_to_snippet`, which improves code quality, makes the suggestion work even without access to source code, and, most importantly, improves the rendering of the suggestion.
1 parent 124555a commit 2835ace

File tree

6 files changed

+47
-65
lines changed

6 files changed

+47
-65
lines changed

‎compiler/rustc_typeck/src/check/op.rs‎

Lines changed: 29 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
549549
is_assign: IsAssign,
550550
op: hir::BinOp,
551551
) -> bool {
552-
let source_map = self.tcx.sess.source_map();
553552
let remove_borrow_msg = "String concatenation appends the string on the right to the \
554553
string on the left and may require reallocation. This \
555554
requires ownership of the string on the left";
@@ -574,31 +573,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
574573
) =>
575574
{
576575
if let IsAssign::No = is_assign { // Do not supply this message if `&str += &str`
577-
err.span_label(
578-
op.span,
579-
"`+` cannot be used to concatenate two `&str` strings",
580-
);
581-
match source_map.span_to_snippet(lhs_expr.span) {
582-
Ok(lstring) => {
583-
err.span_suggestion(
584-
lhs_expr.span,
585-
if lstring.starts_with('&') {
586-
remove_borrow_msg
587-
} else {
588-
msg
589-
},
590-
if let Some(stripped) = lstring.strip_prefix('&') {
591-
// let a = String::new();
592-
// let _ = &a + "bar";
593-
stripped.to_string()
594-
} else {
595-
format!("{}.to_owned()", lstring)
596-
},
597-
Applicability::MachineApplicable,
598-
)
599-
}
600-
_ => err.help(msg),
601-
};
576+
err.span_label(op.span, "`+` cannot be used to concatenate two `&str` strings");
577+
if let hir::ExprKind::AddrOf(_,_,lhs_inner_expr) = lhs_expr.kind {
578+
err.span_suggestion(
579+
lhs_expr.span.until(lhs_inner_expr.span),
580+
remove_borrow_msg,
581+
"".to_owned(),
582+
Applicability::MachineApplicable
583+
);
584+
} else {
585+
err.span_suggestion(
586+
lhs_expr.span.shrink_to_hi(),
587+
msg,
588+
".to_owned()".to_owned(),
589+
Applicability::MachineApplicable
590+
);
591+
}
602592
}
603593
true
604594
}
@@ -609,32 +599,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
609599
op.span,
610600
"`+` cannot be used to concatenate a `&str` with a `String`",
611601
);
612-
match (
613-
source_map.span_to_snippet(lhs_expr.span),
614-
source_map.span_to_snippet(rhs_expr.span),
615-
is_assign,
616-
) {
617-
(Ok(l), Ok(r), IsAssign::No) => {
618-
let to_string = if let Some(stripped) = l.strip_prefix('&') {
619-
// let a = String::new(); let b = String::new();
620-
// let _ = &a + b;
621-
stripped.to_string()
622-
} else {
623-
format!("{}.to_owned()", l)
624-
};
625-
err.multipart_suggestion(
626-
msg,
627-
vec![
628-
(lhs_expr.span, to_string),
629-
(rhs_expr.span, format!("&{}", r)),
630-
],
631-
Applicability::MachineApplicable,
632-
);
602+
match is_assign {
603+
IsAssign::No => {
604+
let suggestions = vec![
605+
if let hir::ExprKind::AddrOf(_, _, lhs_inner_expr) = lhs_expr.kind {
606+
(lhs_expr.span.until(lhs_inner_expr.span), "".to_owned())
607+
} else {
608+
(lhs_expr.span.shrink_to_hi(), ".to_owned()".to_owned())
609+
},
610+
(rhs_expr.span.shrink_to_lo(), "&".to_owned()),
611+
];
612+
err.multipart_suggestion(msg, suggestions, Applicability::MachineApplicable);
633613
}
634-
_ => {
614+
IsAssign::Yes => {
635615
err.help(msg);
636616
}
637-
};
617+
}
638618
true
639619
}
640620
_ => false,

‎src/test/ui/issues/issue-47377.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ LL | let _a = b + ", World!";
1010
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
1111
|
1212
LL | let _a = b.to_owned() + ", World!";
13-
| ~~~~~~~~~~~~
13+
| +++++++++++
1414

1515
error: aborting due to previous error
1616

‎src/test/ui/issues/issue-47380.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ LL | println!("🦀🦀🦀🦀🦀"); let _a = b + ", World!";
1010
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
1111
|
1212
LL | println!("🦀🦀🦀🦀🦀"); let _a = b.to_owned() + ", World!";
13-
| ~~~~~~~~~~~~
13+
| +++++++++++
1414

1515
error: aborting due to previous error
1616

‎src/test/ui/span/issue-39018.stderr‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ LL | let x = "Hello " + "World!";
1010
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
1111
|
1212
LL | let x = "Hello ".to_owned() + "World!";
13-
| ~~~~~~~~~~~~~~~~~~~
13+
| +++++++++++
1414

1515
error[E0369]: cannot add `World` to `World`
1616
--> $DIR/issue-39018.rs:8:26
@@ -49,7 +49,7 @@ LL | let x = "Hello " + "World!".to_owned();
4949
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
5050
|
5151
LL | let x = "Hello ".to_owned() + &"World!".to_owned();
52-
| ~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~
52+
| +++++++++++ +
5353

5454
error[E0369]: cannot add `&String` to `&String`
5555
--> $DIR/issue-39018.rs:26:16
@@ -62,8 +62,9 @@ LL | let _ = &a + &b;
6262
|
6363
help: String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
6464
|
65-
LL | let _ = a + &b;
66-
| ~
65+
LL - let _ = &a + &b;
66+
LL + let _ = a + &b;
67+
|
6768

6869
error[E0369]: cannot add `String` to `&String`
6970
--> $DIR/issue-39018.rs:27:16
@@ -76,8 +77,9 @@ LL | let _ = &a + b;
7677
|
7778
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
7879
|
79-
LL | let _ = a + &b;
80-
| ~ ~~
80+
LL - let _ = &a + b;
81+
LL + let _ = a + &b;
82+
|
8183

8284
error[E0308]: mismatched types
8385
--> $DIR/issue-39018.rs:29:17
@@ -100,7 +102,7 @@ LL | let _ = e + b;
100102
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
101103
|
102104
LL | let _ = e.to_owned() + &b;
103-
| ~~~~~~~~~~~~ ~~
105+
| +++++++++++ +
104106

105107
error[E0369]: cannot add `&String` to `&String`
106108
--> $DIR/issue-39018.rs:31:15
@@ -114,7 +116,7 @@ LL | let _ = e + &b;
114116
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
115117
|
116118
LL | let _ = e.to_owned() + &b;
117-
| ~~~~~~~~~~~~
119+
| +++++++++++
118120

119121
error[E0369]: cannot add `&str` to `&String`
120122
--> $DIR/issue-39018.rs:32:15
@@ -128,7 +130,7 @@ LL | let _ = e + d;
128130
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
129131
|
130132
LL | let _ = e.to_owned() + d;
131-
| ~~~~~~~~~~~~
133+
| +++++++++++
132134

133135
error[E0369]: cannot add `&&str` to `&String`
134136
--> $DIR/issue-39018.rs:33:15
@@ -142,7 +144,7 @@ LL | let _ = e + &d;
142144
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
143145
|
144146
LL | let _ = e.to_owned() + &d;
145-
| ~~~~~~~~~~~~
147+
| +++++++++++
146148

147149
error[E0369]: cannot add `&&str` to `&&str`
148150
--> $DIR/issue-39018.rs:34:16
@@ -172,7 +174,7 @@ LL | let _ = c + &d;
172174
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
173175
|
174176
LL | let _ = c.to_owned() + &d;
175-
| ~~~~~~~~~~~~
177+
| +++++++++++
176178

177179
error[E0369]: cannot add `&str` to `&str`
178180
--> $DIR/issue-39018.rs:37:15
@@ -186,7 +188,7 @@ LL | let _ = c + d;
186188
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
187189
|
188190
LL | let _ = c.to_owned() + d;
189-
| ~~~~~~~~~~~~
191+
| +++++++++++
190192

191193
error: aborting due to 14 previous errors
192194

‎src/test/ui/str/str-concat-on-double-ref.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ LL | let c = a + b;
1010
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
1111
|
1212
LL | let c = a.to_owned() + b;
13-
| ~~~~~~~~~~~~
13+
| +++++++++++
1414

1515
error: aborting due to previous error
1616

‎src/test/ui/terminal-width/non-1-width-unicode-multiline-label.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ LL | ...ཽཾཿ྄ཱྀྀྂྃ྅྆྇ྈྉྊྋྌྍྎྏྐྑྒྒྷྔ
1010
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
1111
|
1212
LL | let _ = "ༀ༁༂༃༄༅༆༇༈༉༊་༌།༎༏༐༑༒༓༔༕༖༗༘༙༚༛༜༝༞༟༠༡༢༣༤༥༦༧༨༩༪༫༬༭༮༯༰༱༲༳༴༵༶༷༸༹༺༻༼༽༾༿ཀཁགགྷངཅཆཇ཈ཉཊཋཌཌྷཎཏཐདདྷནཔཕབབྷམཙཚཛཛྷཝཞཟའཡརལཤཥསཧཨཀྵཪཫཬ཭཮཯཰ཱཱཱིིུུྲྀཷླྀཹེཻོཽཾཿ྄ཱྀྀྂྃ྅྆྇ྈྉྊྋྌྍྎྏྐྑྒྒྷྔྕྖྗ྘ྙྚྛྜྜྷྞྟྠྡྡྷྣྤྥྦྦྷྨྩྪྫྫྷྭྮྯྰྱྲླྴྵྶྷྸྐྵྺྻྼ྽྾྿࿀࿁࿂࿃࿄࿅࿆࿇࿈࿉࿊࿋࿌࿍࿎࿏࿐࿑࿒࿓࿔࿕࿖࿗࿘࿙࿚"; let _a = unicode_is_fun.to_owned() + " really fun!";
13-
| ~~~~~~~~~~~~~~~~~~~~~~~~~
13+
| +++++++++++
1414

1515
error: aborting due to previous error
1616

0 commit comments

Comments
 (0)