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.

diff --git a/compiler/rustc_typeck/src/check/op.rs b/compiler/rustc_typeck/src/check/op.rs
index 8ebfcdd539b67382d883710ab5c40bf25137309b..838f8b0a61d176dbbb93bd126cb6dfca376b1e9e 100644 (file)
--- a/compiler/rustc_typeck/src/check/op.rs
+++ b/compiler/rustc_typeck/src/check/op.rs
@@ -549,7 +549,6 @@ fn check_str_addition(
         is_assign: IsAssign,
         op: hir::BinOp,
     ) -> bool {
-        let source_map = self.tcx.sess.source_map();
         let remove_borrow_msg = "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";
@@ -574,31 +573,22 @@ fn check_str_addition(
                     ) =>
             {
                 if let IsAssign::No = is_assign { // Do not supply this message if `&str += &str`
-                    err.span_label(
-                        op.span,
-                        "`+` cannot be used to concatenate two `&str` strings",
-                    );
-                    match source_map.span_to_snippet(lhs_expr.span) {
-                        Ok(lstring) => {
-                            err.span_suggestion(
-                                lhs_expr.span,
-                                if lstring.starts_with('&') {
-                                    remove_borrow_msg
-                                } else {
-                                    msg
-                                },
-                                if let Some(stripped) = lstring.strip_prefix('&') {
-                                    // let a = String::new();
-                                    // let _ = &a + "bar";
-                                    stripped.to_string()
-                                } else {
-                                    format!("{}.to_owned()", lstring)
-                                },
-                                Applicability::MachineApplicable,
-                            )
-                        }
-                        _ => err.help(msg),
-                    };
+                    err.span_label(op.span, "`+` cannot be used to concatenate two `&str` strings");
+                    if let hir::ExprKind::AddrOf(_,_,lhs_inner_expr) = lhs_expr.kind {
+                        err.span_suggestion(
+                            lhs_expr.span.until(lhs_inner_expr.span),
+                            remove_borrow_msg,
+                            "".to_owned(),
+                            Applicability::MachineApplicable
+                        );
+                    } else {
+                        err.span_suggestion(
+                            lhs_expr.span.shrink_to_hi(),
+                            msg,
+                            ".to_owned()".to_owned(),
+                            Applicability::MachineApplicable
+                        );
+                    }
                 }
                 true
             }
@@ -609,32 +599,22 @@ fn check_str_addition(
                     op.span,
                     "`+` cannot be used to concatenate a `&str` with a `String`",
                 );
-                match (
-                    source_map.span_to_snippet(lhs_expr.span),
-                    source_map.span_to_snippet(rhs_expr.span),
-                    is_assign,
-                ) {
-                    (Ok(l), Ok(r), IsAssign::No) => {
-                        let to_string = if let Some(stripped) = l.strip_prefix('&') {
-                            // let a = String::new(); let b = String::new();
-                            // let _ = &a + b;
-                            stripped.to_string()
-                        } else {
-                            format!("{}.to_owned()", l)
-                        };
-                        err.multipart_suggestion(
-                            msg,
-                            vec![
-                                (lhs_expr.span, to_string),
-                                (rhs_expr.span, format!("&{}", r)),
-                            ],
-                            Applicability::MachineApplicable,
-                        );
+                match is_assign {
+                    IsAssign::No => {
+                        let suggestions = vec![
+                            if let hir::ExprKind::AddrOf(_, _, lhs_inner_expr) = lhs_expr.kind {
+                                (lhs_expr.span.until(lhs_inner_expr.span), "".to_owned())
+                            } else {
+                                (lhs_expr.span.shrink_to_hi(), ".to_owned()".to_owned())
+                            },
+                            (rhs_expr.span.shrink_to_lo(), "&".to_owned()),
+                        ];
+                        err.multipart_suggestion(msg, suggestions, Applicability::MachineApplicable);
                     }
-                    _ => {
+                    IsAssign::Yes => {
                         err.help(msg);
                     }
-                };
+                }
                 true
             }
             _ => false,

diff --git a/src/test/ui/issues/issue-47377.stderr b/src/test/ui/issues/issue-47377.stderr
index a7b8b1ca86167b732be790005e8c9c9474bc2d7e..2c17a19779f8ff4fb4645dacf147b7c1c3194995 100644 (file)
--- a/src/test/ui/issues/issue-47377.stderr
+++ b/src/test/ui/issues/issue-47377.stderr
@@ -10,7 +10,7 @@ LL |      let _a = b + ", World!";
 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
    |
 LL |      let _a = b.to_owned() + ", World!";
-   |               ~~~~~~~~~~~~
+   |                +++++++++++
 
 error: aborting due to previous error
 

diff --git a/src/test/ui/issues/issue-47380.stderr b/src/test/ui/issues/issue-47380.stderr
index f6222c77e2e251fdf2d869027995b56e2a2d8dd0..417206e6cc4fb19bbb2f66902f1fcb846ae3458f 100644 (file)
--- a/src/test/ui/issues/issue-47380.stderr
+++ b/src/test/ui/issues/issue-47380.stderr
@@ -10,7 +10,7 @@ LL |     println!("🦀🦀🦀🦀🦀"); let _a = b + ", World!";
 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
    |
 LL |     println!("🦀🦀🦀🦀🦀"); let _a = b.to_owned() + ", World!";
-   |                                      ~~~~~~~~~~~~
+   |                                       +++++++++++
 
 error: aborting due to previous error
 

diff --git a/src/test/ui/span/issue-39018.stderr b/src/test/ui/span/issue-39018.stderr
index 92e86bf5d6c91906cde0080801647f136cf18108..444d037571e1885b4daffd36e2ae9a194da782a7 100644 (file)
--- a/src/test/ui/span/issue-39018.stderr
+++ b/src/test/ui/span/issue-39018.stderr
@@ -10,7 +10,7 @@ LL |     let x = "Hello " + "World!";
 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
    |
 LL |     let x = "Hello ".to_owned() + "World!";
-   |             ~~~~~~~~~~~~~~~~~~~
+   |                     +++++++++++
 
 error[E0369]: cannot add `World` to `World`
   --> $DIR/issue-39018.rs:8:26
@@ -49,7 +49,7 @@ LL |     let x = "Hello " + "World!".to_owned();
 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
    |
 LL |     let x = "Hello ".to_owned() + &"World!".to_owned();
-   |             ~~~~~~~~~~~~~~~~~~~   ~~~~~~~~~~~~~~~~~~~~
+   |                     +++++++++++   +
 
 error[E0369]: cannot add `&String` to `&String`
   --> $DIR/issue-39018.rs:26:16
@@ -62,8 +62,9 @@ LL |     let _ = &a + &b;
    |
 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
    |
-LL |     let _ = a + &b;
-   |             ~
+LL -     let _ = &a + &b;
+LL +     let _ = a + &b;
+   | 
 
 error[E0369]: cannot add `String` to `&String`
   --> $DIR/issue-39018.rs:27:16
@@ -76,8 +77,9 @@ LL |     let _ = &a + b;
    |
 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
    |
-LL |     let _ = a + &b;
-   |             ~   ~~
+LL -     let _ = &a + b;
+LL +     let _ = a + &b;
+   | 
 
 error[E0308]: mismatched types
   --> $DIR/issue-39018.rs:29:17
@@ -100,7 +102,7 @@ LL |     let _ = e + b;
 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
    |
 LL |     let _ = e.to_owned() + &b;
-   |             ~~~~~~~~~~~~   ~~
+   |              +++++++++++   +
 
 error[E0369]: cannot add `&String` to `&String`
   --> $DIR/issue-39018.rs:31:15
@@ -114,7 +116,7 @@ LL |     let _ = e + &b;
 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
    |
 LL |     let _ = e.to_owned() + &b;
-   |             ~~~~~~~~~~~~
+   |              +++++++++++
 
 error[E0369]: cannot add `&str` to `&String`
   --> $DIR/issue-39018.rs:32:15
@@ -128,7 +130,7 @@ LL |     let _ = e + d;
 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
    |
 LL |     let _ = e.to_owned() + d;
-   |             ~~~~~~~~~~~~
+   |              +++++++++++
 
 error[E0369]: cannot add `&&str` to `&String`
   --> $DIR/issue-39018.rs:33:15
@@ -142,7 +144,7 @@ LL |     let _ = e + &d;
 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
    |
 LL |     let _ = e.to_owned() + &d;
-   |             ~~~~~~~~~~~~
+   |              +++++++++++
 
 error[E0369]: cannot add `&&str` to `&&str`
   --> $DIR/issue-39018.rs:34:16
@@ -172,7 +174,7 @@ LL |     let _ = c + &d;
 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
    |
 LL |     let _ = c.to_owned() + &d;
-   |             ~~~~~~~~~~~~
+   |              +++++++++++
 
 error[E0369]: cannot add `&str` to `&str`
   --> $DIR/issue-39018.rs:37:15
@@ -186,7 +188,7 @@ LL |     let _ = c + d;
 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
    |
 LL |     let _ = c.to_owned() + d;
-   |             ~~~~~~~~~~~~
+   |              +++++++++++
 
 error: aborting due to 14 previous errors
 

diff --git a/src/test/ui/str/str-concat-on-double-ref.stderr b/src/test/ui/str/str-concat-on-double-ref.stderr
index dee28897f4dddce8aa89722717f08d9760b12aa2..cc9456be901699454db1a9ec9f7beeaf3612b58b 100644 (file)
--- a/src/test/ui/str/str-concat-on-double-ref.stderr
+++ b/src/test/ui/str/str-concat-on-double-ref.stderr
@@ -10,7 +10,7 @@ LL |     let c = a + b;
 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
    |
 LL |     let c = a.to_owned() + b;
-   |             ~~~~~~~~~~~~
+   |              +++++++++++
 
 error: aborting due to previous error
 

diff --git a/src/test/ui/terminal-width/non-1-width-unicode-multiline-label.stderr b/src/test/ui/terminal-width/non-1-width-unicode-multiline-label.stderr
index 480f442fedd2822c7156702e6fa4642ada40b702..2da3db4a7fa8157373dca98005038e592b530283 100644 (file)
--- a/src/test/ui/terminal-width/non-1-width-unicode-multiline-label.stderr
+++ b/src/test/ui/terminal-width/non-1-width-unicode-multiline-label.stderr
@@ -10,7 +10,7 @@ LL | ...ཽཾཿ྄ཱྀྀྂྃ྅྆྇ྈྉྊྋྌྍྎྏྐྑྒྒྷྔ
 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
    |
 LL |     let _ = "ༀ༁༂༃༄༅༆༇༈༉༊་༌།༎༏༐༑༒༓༔༕༖༗༘༙༚༛༜༝༞༟༠༡༢༣༤༥༦༧༨༩༪༫༬༭༮༯༰༱༲༳༴༵༶༷༸༹༺༻༼༽༾༿ཀཁགགྷངཅཆཇ཈ཉཊཋཌཌྷཎཏཐདདྷནཔཕབབྷམཙཚཛཛྷཝཞཟའཡརལཤཥསཧཨཀྵཪཫཬ཭཮཯཰ཱཱཱིིུུྲྀཷླྀཹེཻོཽཾཿ྄ཱྀྀྂྃ྅྆྇ྈྉྊྋྌྍྎྏྐྑྒྒྷྔྕྖྗ྘ྙྚྛྜྜྷྞྟྠྡྡྷྣྤྥྦྦྷྨྩྪྫྫྷྭྮྯྰྱྲླྴྵྶྷྸྐྵྺྻྼ྽྾྿࿀࿁࿂࿃࿄࿅࿆࿇࿈࿉࿊࿋࿌࿍࿎࿏࿐࿑࿒࿓࿔࿕࿖࿗࿘࿙࿚"; let _a = unicode_is_fun.to_owned() + " really fun!";
-   |                                                                                                                                                                           ~~~~~~~~~~~~~~~~~~~~~~~~~
+   |                                                                                                                                                                                         +++++++++++
 
 error: aborting due to previous error