Skip to content

Commit 234adf8

Browse files
estebankestebank
committed
Handle more string addition cases with appropriate suggestions
1 parent 1962ade commit 234adf8

File tree

4 files changed

+209
-18
lines changed

4 files changed

+209
-18
lines changed

‎src/librustc_typeck/check/op.rs

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
502502
false
503503
}
504504

505+
/// Provide actionable suggestions when trying to add two strings with incorrect types,
506+
/// like `&str + &str`, `String + String` and `&str + &String`.
507+
///
508+
/// If this function returns `true` it means a note was printed, so we don't need
509+
/// to print the normal "implementation of `std::ops::Add` might be missing" note
505510
fn check_str_addition(
506511
&self,
507512
expr: &'gcx hir::Expr,
@@ -514,33 +519,57 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
514519
op: hir::BinOp,
515520
) -> bool {
516521
let source_map = self.tcx.sess.source_map();
522+
let remove_borrow_msg = "String concatenation appends the string on the right to the \
523+
string on the left and may require reallocation. This \
524+
requires ownership of the string on the left";
525+
517526
let msg = "`to_owned()` can be used to create an owned `String` \
518527
from a string reference. String concatenation \
519528
appends the string on the right to the string \
520529
on the left and may require reallocation. This \
521530
requires ownership of the string on the left";
522-
// If this function returns true it means a note was printed, so we don't need
523-
// to print the normal "implementation of `std::ops::Add` might be missing" note
531+
debug!("check_str_addition: {:?} + {:?}", lhs_ty, rhs_ty);
524532
match (&lhs_ty.sty, &rhs_ty.sty) {
525-
(&Ref(_, l_ty, _), &Ref(_, r_ty, _))
526-
if l_ty.sty == Str && r_ty.sty == Str => {
533+
(&Ref(_, l_ty, _), &Ref(_, r_ty, _)) // &str or &String + &str, &String or &&str
534+
if (l_ty.sty == Str || &format!("{:?}", l_ty) == "std::string::String") && (
535+
r_ty.sty == Str ||
536+
&format!("{:?}", r_ty) == "std::string::String" ||
537+
&format!("{:?}", rhs_ty) == "&&str"
538+
) =>
539+
{
527540
if !is_assign {
528-
err.span_label(op.span,
529-
"`+` can't be used to concatenate two `&str` strings");
541+
err.span_label(
542+
op.span,
543+
"`+` can't be used to concatenate two `&str` strings",
544+
);
530545
match source_map.span_to_snippet(lhs_expr.span) {
531-
Ok(lstring) => err.span_suggestion(
532-
lhs_expr.span,
533-
msg,
534-
format!("{}.to_owned()", lstring),
535-
Applicability::MachineApplicable,
536-
),
546+
Ok(lstring) => {
547+
err.span_suggestion(
548+
lhs_expr.span,
549+
if lstring.starts_with("&") {
550+
remove_borrow_msg
551+
} else {
552+
msg
553+
},
554+
if lstring.starts_with("&") {
555+
// let a = String::new();
556+
// let _ = &a + "bar";
557+
format!("{}", &lstring[1..])
558+
} else {
559+
format!("{}.to_owned()", lstring)
560+
},
561+
Applicability::MachineApplicable,
562+
)
563+
}
537564
_ => err.help(msg),
538565
};
539566
}
540567
true
541568
}
542-
(&Ref(_, l_ty, _), &Adt(..))
543-
if l_ty.sty == Str && &format!("{:?}", rhs_ty) == "std::string::String" => {
569+
(&Ref(_, l_ty, _), &Adt(..)) // Handle `&str` & `&String` + `String`
570+
if (l_ty.sty == Str || &format!("{:?}", l_ty) == "std::string::String") &&
571+
&format!("{:?}", rhs_ty) == "std::string::String" =>
572+
{
544573
err.span_label(expr.span,
545574
"`+` can't be used to concatenate a `&str` with a `String`");
546575
match (

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,23 @@ enum World {
1616
Hello,
1717
Goodbye,
1818
}
19+
20+
fn foo() {
21+
let a = String::new();
22+
let b = String::new();
23+
let c = "";
24+
let d = "";
25+
let e = &a;
26+
let _ = &a + &b; //~ ERROR binary operation
27+
let _ = &a + b; //~ ERROR binary operation
28+
let _ = a + &b; // ok
29+
let _ = a + b; //~ ERROR mismatched types
30+
let _ = e + b; //~ ERROR binary operation
31+
let _ = e + &b; //~ ERROR binary operation
32+
let _ = e + d; //~ ERROR binary operation
33+
let _ = e + &d; //~ ERROR binary operation
34+
let _ = &c + &d; //~ ERROR binary operation
35+
let _ = &c + d; //~ ERROR binary operation
36+
let _ = c + &d; //~ ERROR binary operation
37+
let _ = c + d; //~ ERROR binary operation
38+
}

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

Lines changed: 141 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,145 @@ help: `to_owned()` can be used to create an owned `String` from a string referen
3535
LL | let x = "Hello ".to_owned() + &"World!".to_owned();
3636
| ^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^
3737

38-
error: aborting due to 3 previous errors
38+
error[E0369]: binary operation `+` cannot be applied to type `&std::string::String`
39+
--> $DIR/issue-39018.rs:26:16
40+
|
41+
LL | let _ = &a + &b;
42+
| -- ^ -- &std::string::String
43+
| | |
44+
| | `+` can't be used to concatenate two `&str` strings
45+
| &std::string::String
46+
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
47+
|
48+
LL | let _ = a + &b;
49+
| ^
50+
51+
error[E0369]: binary operation `+` cannot be applied to type `&std::string::String`
52+
--> $DIR/issue-39018.rs:27:16
53+
|
54+
LL | let _ = &a + b;
55+
| ---^--
56+
| | |
57+
| | std::string::String
58+
| &std::string::String
59+
| `+` can't be used to concatenate a `&str` with a `String`
60+
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
61+
|
62+
LL | let _ = &a.to_owned() + &b;
63+
| ^^^^^^^^^^^^^ ^^
64+
65+
error[E0308]: mismatched types
66+
--> $DIR/issue-39018.rs:29:17
67+
|
68+
LL | let _ = a + b;
69+
| ^
70+
| |
71+
| expected &str, found struct `std::string::String`
72+
| help: consider borrowing here: `&b`
73+
|
74+
= note: expected type `&str`
75+
found type `std::string::String`
76+
77+
error[E0369]: binary operation `+` cannot be applied to type `&std::string::String`
78+
--> $DIR/issue-39018.rs:30:15
79+
|
80+
LL | let _ = e + b;
81+
| --^--
82+
| | |
83+
| | std::string::String
84+
| &std::string::String
85+
| `+` can't be used to concatenate a `&str` with a `String`
86+
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
87+
|
88+
LL | let _ = e.to_owned() + &b;
89+
| ^^^^^^^^^^^^ ^^
90+
91+
error[E0369]: binary operation `+` cannot be applied to type `&std::string::String`
92+
--> $DIR/issue-39018.rs:31:15
93+
|
94+
LL | let _ = e + &b;
95+
| - ^ -- &std::string::String
96+
| | |
97+
| | `+` can't be used to concatenate two `&str` strings
98+
| &std::string::String
99+
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
100+
|
101+
LL | let _ = e.to_owned() + &b;
102+
| ^^^^^^^^^^^^
103+
104+
error[E0369]: binary operation `+` cannot be applied to type `&std::string::String`
105+
--> $DIR/issue-39018.rs:32:15
106+
|
107+
LL | let _ = e + d;
108+
| - ^ - &str
109+
| | |
110+
| | `+` can't be used to concatenate two `&str` strings
111+
| &std::string::String
112+
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
113+
|
114+
LL | let _ = e.to_owned() + d;
115+
| ^^^^^^^^^^^^
116+
117+
error[E0369]: binary operation `+` cannot be applied to type `&std::string::String`
118+
--> $DIR/issue-39018.rs:33:15
119+
|
120+
LL | let _ = e + &d;
121+
| - ^ -- &&str
122+
| | |
123+
| | `+` can't be used to concatenate two `&str` strings
124+
| &std::string::String
125+
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
126+
|
127+
LL | let _ = e.to_owned() + &d;
128+
| ^^^^^^^^^^^^
129+
130+
error[E0369]: binary operation `+` cannot be applied to type `&&str`
131+
--> $DIR/issue-39018.rs:34:16
132+
|
133+
LL | let _ = &c + &d;
134+
| -- ^ -- &&str
135+
| |
136+
| &&str
137+
|
138+
= note: an implementation of `std::ops::Add` might be missing for `&&str`
139+
140+
error[E0369]: binary operation `+` cannot be applied to type `&&str`
141+
--> $DIR/issue-39018.rs:35:16
142+
|
143+
LL | let _ = &c + d;
144+
| -- ^ - &str
145+
| |
146+
| &&str
147+
|
148+
= note: an implementation of `std::ops::Add` might be missing for `&&str`
149+
150+
error[E0369]: binary operation `+` cannot be applied to type `&str`
151+
--> $DIR/issue-39018.rs:36:15
152+
|
153+
LL | let _ = c + &d;
154+
| - ^ -- &&str
155+
| | |
156+
| | `+` can't be used to concatenate two `&str` strings
157+
| &str
158+
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
159+
|
160+
LL | let _ = c.to_owned() + &d;
161+
| ^^^^^^^^^^^^
162+
163+
error[E0369]: binary operation `+` cannot be applied to type `&str`
164+
--> $DIR/issue-39018.rs:37:15
165+
|
166+
LL | let _ = c + d;
167+
| - ^ - &str
168+
| | |
169+
| | `+` can't be used to concatenate two `&str` strings
170+
| &str
171+
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
172+
|
173+
LL | let _ = c.to_owned() + d;
174+
| ^^^^^^^^^^^^
175+
176+
error: aborting due to 14 previous errors
39177

40-
For more information about this error, try `rustc --explain E0369`.
178+
Some errors have detailed explanations: E0308, E0369.
179+
For more information about an error, try `rustc --explain E0308`.

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@ error[E0369]: binary operation `+` cannot be applied to type `&std::string::Stri
33
|
44
LL | let c = a + b;
55
| - ^ - &str
6-
| |
6+
| | |
7+
| | `+` can't be used to concatenate two `&str` strings
78
| &std::string::String
9+
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
810
|
9-
= note: an implementation of `std::ops::Add` might be missing for `&std::string::String`
11+
LL | let c = a.to_owned() + b;
12+
| ^^^^^^^^^^^^
1013

1114
error: aborting due to previous error
1215

0 commit comments

Comments
 (0)