Auto merge of #11416 - Alexendoo:raw-strings-multipart, r=xFrednet

Use multipart suggestions for raw string lints Should make it slightly easier to see the suggested edit Before/after for `needless_raw_string_hashes`: | Before| After | |--------|--------| | ![before](https://github.com/rust-lang/rust-clippy/assets/1830331/da52a436-d890-4594-9191-819c1af946c7) | ![after](https://github.com/rust-lang/rust-clippy/assets/1830331/9731d790-8efa-42a2-b2e9-0ec51398f8f3) | changelog: none
rust-lang · Sep 1, 2023 · a8b5245 · a8b5245
2 parents acdffd7 + f595f1e
commit a8b5245
Show file tree

Hide file tree

Showing 8 changed files with 289 additions and 52 deletions.
diff --git a/clippy_lints/src/raw_strings.rs b/clippy_lints/src/raw_strings.rs
@@ -1,14 +1,15 @@
 use std::iter::once;
 use std::ops::ControlFlow;
 
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::snippet;
 use rustc_ast::ast::{Expr, ExprKind};
 use rustc_ast::token::LitKind;
 use rustc_errors::Applicability;
 use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::{BytePos, Pos, Span};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -76,14 +77,24 @@ impl EarlyLintPass for RawStrings {
             }
 
             if !str.contains(['\\', '"']) {
-                span_lint_and_sugg(
+                span_lint_and_then(
                     cx,
                     NEEDLESS_RAW_STRINGS,
                     expr.span,
                     "unnecessary raw string literal",
-                    "try",
-                    format!("{}\"{}\"", prefix.replace('r', ""), lit.symbol),
-                    Applicability::MachineApplicable,
+                    |diag| {
+                        let (start, end) = hash_spans(expr.span, prefix, 0, max);
+
+                        // BytePos: skip over the `b` in `br`, we checked the prefix appears in the source text
+                        let r_pos = expr.span.lo() + BytePos::from_usize(prefix.len() - 1);
+                        let start = start.with_lo(r_pos);
+
+                        diag.multipart_suggestion(
+                            "try",
+                            vec![(start, String::new()), (end, String::new())],
+                            Applicability::MachineApplicable,
+                        );
+                    },
                 );
 
                 return;
@@ -96,13 +107,6 @@ impl EarlyLintPass for RawStrings {
                 let num = str.as_bytes().iter().chain(once(&0)).try_fold(0u8, |acc, &b| {
                     match b {
                         b'"' if !following_quote => (following_quote, req) = (true, 1),
-                        // I'm a bit surprised the compiler didn't optimize this out, there's no
-                        // branch but it still ends up doing an unnecessary comparison, it's:
-                        // - cmp r9b,1h
-                        // - sbb cl,-1h
-                        // which will add 1 if it's true. With this change, it becomes:
-                        // - add cl,r9b
-                        // isn't that so much nicer?
                         b'#' => req += u8::from(following_quote),
                         _ => {
                             if following_quote {
@@ -126,18 +130,58 @@ impl EarlyLintPass for RawStrings {
             };
 
             if req < max {
-                let hashes = "#".repeat(req as usize);
-
-                span_lint_and_sugg(
+                span_lint_and_then(
                     cx,
                     NEEDLESS_RAW_STRING_HASHES,
                     expr.span,
                     "unnecessary hashes around raw string literal",
-                    "try",
-                    format!(r#"{prefix}{hashes}"{}"{hashes}"#, lit.symbol),
-                    Applicability::MachineApplicable,
+                    |diag| {
+                        let (start, end) = hash_spans(expr.span, prefix, req, max);
+
+                        let message = match max - req {
+                            _ if req == 0 => "remove all the hashes around the literal".to_string(),
+                            1 => "remove one hash from both sides of the literal".to_string(),
+                            n => format!("remove {n} hashes from both sides of the literal"),
+                        };
+
+                        diag.multipart_suggestion(
+                            message,
+                            vec![(start, String::new()), (end, String::new())],
+                            Applicability::MachineApplicable,
+                        );
+                    },
                 );
             }
         }
     }
 }
+
+/// Returns spans pointing at the unneeded hashes, e.g. for a `req` of `1` and `max` of `3`:
+///
+/// ```ignore
+/// r###".."###
+///   ^^    ^^
+/// ```
+fn hash_spans(literal_span: Span, prefix: &str, req: u8, max: u8) -> (Span, Span) {
+    let literal_span = literal_span.data();
+
+    // BytePos: we checked prefix appears literally in the source text
+    let hash_start = literal_span.lo + BytePos::from_usize(prefix.len());
+    let hash_end = literal_span.hi;
+
+    // BytePos: req/max are counts of the ASCII character #
+    let start = Span::new(
+        hash_start + BytePos(req.into()),
+        hash_start + BytePos(max.into()),
+        literal_span.ctxt,
+        None,
+    );
+    let end = Span::new(
+        hash_end - BytePos(req.into()),
+        hash_end - BytePos(max.into()),
+        literal_span.ctxt,
+        None,
+    );
+
+    (start, end)
+}
diff --git a/tests/ui/needless_raw_string.fixed b/tests/ui/needless_raw_string.fixed
@@ -9,8 +9,13 @@ fn main() {
     b"aaa";
     br#""aaa""#;
     br#"\s"#;
-    // currently disabled: https://github.com/rust-lang/rust/issues/113333
-    // cr#"aaa"#;
-    // cr#""aaa""#;
-    // cr#"\s"#;
+    c"aaa";
+    cr#""aaa""#;
+    cr#"\s"#;
+
+    "
+        a
+        multiline
+        string
+    ";
 }
diff --git a/tests/ui/needless_raw_string.rs b/tests/ui/needless_raw_string.rs
@@ -9,8 +9,13 @@ fn main() {
     br#"aaa"#;
     br#""aaa""#;
     br#"\s"#;
-    // currently disabled: https://github.com/rust-lang/rust/issues/113333
-    // cr#"aaa"#;
-    // cr#""aaa""#;
-    // cr#"\s"#;
+    cr#"aaa"#;
+    cr#""aaa""#;
+    cr#"\s"#;
+
+    r#"
+        a
+        multiline
+        string
+    "#;
 }
diff --git a/tests/ui/needless_raw_string.stderr b/tests/ui/needless_raw_string.stderr
@@ -2,15 +2,57 @@ error: unnecessary raw string literal
   --> $DIR/needless_raw_string.rs:6:5
    |
 LL |     r#"aaa"#;
-   |     ^^^^^^^^ help: try: `"aaa"`
+   |     ^^^^^^^^
    |
    = note: `-D clippy::needless-raw-strings` implied by `-D warnings`
+help: try
+   |
+LL -     r#"aaa"#;
+LL +     "aaa";
+   |
 
 error: unnecessary raw string literal
   --> $DIR/needless_raw_string.rs:9:5
    |
 LL |     br#"aaa"#;
-   |     ^^^^^^^^^ help: try: `b"aaa"`
+   |     ^^^^^^^^^
+   |
+help: try
+   |
+LL -     br#"aaa"#;
+LL +     b"aaa";
+   |
+
+error: unnecessary raw string literal
+  --> $DIR/needless_raw_string.rs:12:5
+   |
+LL |     cr#"aaa"#;
+   |     ^^^^^^^^^
+   |
+help: try
+   |
+LL -     cr#"aaa"#;
+LL +     c"aaa";
+   |
+
+error: unnecessary raw string literal
+  --> $DIR/needless_raw_string.rs:16:5
+   |
+LL | /     r#"
+LL | |         a
+LL | |         multiline
+LL | |         string
+LL | |     "#;
+   | |______^
+   |
+help: try
+   |
+LL ~     "
+LL |         a
+LL |         multiline
+LL |         string
+LL ~     ";
+   |
 
-error: aborting due to 2 previous errors
+error: aborting due to 4 previous errors
 
diff --git a/tests/ui/needless_raw_string_hashes.fixed b/tests/ui/needless_raw_string_hashes.fixed
@@ -3,17 +3,22 @@
 #![feature(c_str_literals)]
 
 fn main() {
-    r#"aaa"#;
+    r"\aaa";
     r#"Hello "world"!"#;
     r####" "### "## "# "####;
     r###" "aa" "# "## "###;
-    br#"aaa"#;
+    br"\aaa";
     br#"Hello "world"!"#;
     br####" "### "## "# "####;
     br###" "aa" "# "## "###;
-    // currently disabled: https://github.com/rust-lang/rust/issues/113333
-    // cr#"aaa"#;
-    // cr##"Hello "world"!"##;
-    // cr######" "### "## "# "######;
-    // cr######" "aa" "# "## "######;
+    cr"\aaa";
+    cr#"Hello "world"!"#;
+    cr####" "### "## "# "####;
+    cr###" "aa" "# "## "###;
+
+    r"
+        \a
+        multiline
+        string
+    ";
 }
diff --git a/tests/ui/needless_raw_string_hashes.rs b/tests/ui/needless_raw_string_hashes.rs
@@ -3,17 +3,22 @@
 #![feature(c_str_literals)]
 
 fn main() {
-    r#"aaa"#;
+    r#"\aaa"#;
     r##"Hello "world"!"##;
     r######" "### "## "# "######;
     r######" "aa" "# "## "######;
-    br#"aaa"#;
+    br#"\aaa"#;
     br##"Hello "world"!"##;
     br######" "### "## "# "######;
     br######" "aa" "# "## "######;
-    // currently disabled: https://github.com/rust-lang/rust/issues/113333
-    // cr#"aaa"#;
-    // cr##"Hello "world"!"##;
-    // cr######" "### "## "# "######;
-    // cr######" "aa" "# "## "######;
+    cr#"\aaa"#;
+    cr##"Hello "world"!"##;
+    cr######" "### "## "# "######;
+    cr######" "aa" "# "## "######;
+
+    r#"
+        \a
+        multiline
+        string
+    "#;
 }