Skip to content

Commit ba0e251

Browse files
ManishearthManishearth
authored
Rollup merge of rust-lang#60901 - estebank:str-str-str, r=Centril
Handle more string addition cases with appropriate suggestions
2 parents 7269117 + 8895fb9 commit ba0e251

File tree

7 files changed

+231
-36
lines changed

7 files changed

+231
-36
lines changed

‎src/librustc_typeck/check/op.rs

Lines changed: 60 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
305305
};
306306
if let Some(missing_trait) = missing_trait {
307307
if op.node == hir::BinOpKind::Add &&
308-
self.check_str_addition(expr, lhs_expr, rhs_expr, lhs_ty,
309-
rhs_ty, &mut err, true, op) {
308+
self.check_str_addition(
309+
lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, true, op) {
310310
// This has nothing here because it means we did string
311311
// concatenation (e.g., "Hello " += "World!"). This means
312312
// we don't want the note in the else clause to be emitted
@@ -400,8 +400,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
400400
};
401401
if let Some(missing_trait) = missing_trait {
402402
if op.node == hir::BinOpKind::Add &&
403-
self.check_str_addition(expr, lhs_expr, rhs_expr, lhs_ty,
404-
rhs_ty, &mut err, false, op) {
403+
self.check_str_addition(
404+
lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, false, op) {
405405
// This has nothing here because it means we did string
406406
// concatenation (e.g., "Hello " + "World!"). This means
407407
// we don't want the note in the else clause to be emitted
@@ -502,9 +502,13 @@ 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,
507-
expr: &'gcx hir::Expr,
508512
lhs_expr: &'gcx hir::Expr,
509513
rhs_expr: &'gcx hir::Expr,
510514
lhs_ty: Ty<'tcx>,
@@ -514,45 +518,78 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
514518
op: hir::BinOp,
515519
) -> bool {
516520
let source_map = self.tcx.sess.source_map();
521+
let remove_borrow_msg = "String concatenation appends the string on the right to the \
522+
string on the left and may require reallocation. This \
523+
requires ownership of the string on the left";
524+
517525
let msg = "`to_owned()` can be used to create an owned `String` \
518526
from a string reference. String concatenation \
519527
appends the string on the right to the string \
520528
on the left and may require reallocation. This \
521529
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
530+
531+
let is_std_string = |ty| &format!("{:?}", ty) == "std::string::String";
532+
524533
match (&lhs_ty.sty, &rhs_ty.sty) {
525-
(&Ref(_, l_ty, _), &Ref(_, r_ty, _))
526-
if l_ty.sty == Str && r_ty.sty == Str => {
527-
if !is_assign {
528-
err.span_label(op.span,
529-
"`+` can't be used to concatenate two `&str` strings");
534+
(&Ref(_, l_ty, _), &Ref(_, r_ty, _)) // &str or &String + &str, &String or &&str
535+
if (l_ty.sty == Str || is_std_string(l_ty)) && (
536+
r_ty.sty == Str || is_std_string(r_ty) ||
537+
&format!("{:?}", rhs_ty) == "&&str"
538+
) =>
539+
{
540+
if !is_assign { // Do not supply this message if `&str += &str`
541+
err.span_label(
542+
op.span,
543+
"`+` cannot 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" => {
544-
err.span_label(expr.span,
545-
"`+` can't be used to concatenate a `&str` with a `String`");
569+
(&Ref(_, l_ty, _), &Adt(..)) // Handle `&str` & `&String` + `String`
570+
if (l_ty.sty == Str || is_std_string(l_ty)) && is_std_string(rhs_ty) =>
571+
{
572+
err.span_label(
573+
op.span,
574+
"`+` cannot be used to concatenate a `&str` with a `String`",
575+
);
546576
match (
547577
source_map.span_to_snippet(lhs_expr.span),
548578
source_map.span_to_snippet(rhs_expr.span),
549579
is_assign,
550580
) {
551581
(Ok(l), Ok(r), false) => {
582+
let to_string = if l.starts_with("&") {
583+
// let a = String::new(); let b = String::new();
584+
// let _ = &a + b;
585+
format!("{}", &l[1..])
586+
} else {
587+
format!("{}.to_owned()", l)
588+
};
552589
err.multipart_suggestion(
553590
msg,
554591
vec![
555-
(lhs_expr.span, format!("{}.to_owned()", l)),
592+
(lhs_expr.span, to_string),
556593
(rhs_expr.span, format!("&{}", r)),
557594
],
558595
Applicability::MachineApplicable,

‎src/libsyntax/parse/parser.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3497,8 +3497,7 @@ impl<'a> Parser<'a> {
34973497
let binary = self.mk_binary(source_map::respan(cur_op_span, ast_op), lhs, rhs);
34983498
self.mk_expr(span, binary, ThinVec::new())
34993499
}
3500-
AssocOp::Assign =>
3501-
self.mk_expr(span, ExprKind::Assign(lhs, rhs), ThinVec::new()),
3500+
AssocOp::Assign => self.mk_expr(span, ExprKind::Assign(lhs, rhs), ThinVec::new()),
35023501
AssocOp::ObsoleteInPlace =>
35033502
self.mk_expr(span, ExprKind::ObsoleteInPlace(lhs, rhs), ThinVec::new()),
35043503
AssocOp::AssignOp(k) => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ error[E0369]: binary operation `+` cannot be applied to type `&str`
44
LL | let _a = b + ", World!";
55
| - ^ ---------- &str
66
| | |
7-
| | `+` can't be used to concatenate two `&str` strings
7+
| | `+` cannot be used to concatenate two `&str` strings
88
| &str
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
1010
|

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ error[E0369]: binary operation `+` cannot be applied to type `&str`
44
LL | println!("🦀🦀🦀🦀🦀"); let _a = b + ", World!";
55
| - ^ ---------- &str
66
| | |
7-
| | `+` can't be used to concatenate two `&str` strings
7+
| | `+` cannot be used to concatenate two `&str` strings
88
| &str
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
1010
|

‎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: 143 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ error[E0369]: binary operation `+` cannot be applied to type `&str`
44
LL | let x = "Hello " + "World!";
55
| -------- ^ -------- &str
66
| | |
7-
| | `+` can't be used to concatenate two `&str` strings
7+
| | `+` cannot be used to concatenate two `&str` strings
88
| &str
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
1010
|
@@ -25,16 +25,152 @@ error[E0369]: binary operation `+` cannot be applied to type `&str`
2525
--> $DIR/issue-39018.rs:11:22
2626
|
2727
LL | let x = "Hello " + "World!".to_owned();
28-
| ---------^--------------------
29-
| | |
30-
| | std::string::String
28+
| -------- ^ ------------------- std::string::String
29+
| | |
30+
| | `+` cannot be used to concatenate a `&str` with a `String`
3131
| &str
32-
| `+` can't be used to concatenate a `&str` with a `String`
3332
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
3433
|
3534
LL | let x = "Hello ".to_owned() + &"World!".to_owned();
3635
| ^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^
3736

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

40-
For more information about this error, try `rustc --explain E0369`.
175+
Some errors have detailed explanations: E0308, E0369.
176+
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+
| | `+` cannot 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)