Preserve suggestion formatting or safely unescape in write_literal

Author: Alexendoo

clippy_lints/src/write.rs

@@ -290,7 +290,7 @@ impl<'tcx> LateLintPass<'tcx> for Write {
         }
 
         let Some(args) = format_args.args(cx) else { return };
-        check_literal(cx, &args, name, format_args.is_raw(cx));
+        check_literal(cx, &format_args, &args, name);
 
         if !self.in_debug_impl {
             for arg in args {
@@ -426,7 +426,7 @@ fn check_empty_string(cx: &LateContext<'_>, format_args: &FormatArgsExpn<'_>, ma
     }
 }
 
-fn check_literal(cx: &LateContext<'_>, args: &[FormatArgsArg<'_>], name: &str, raw: bool) {
+fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgsExpn<'_>, args: &[FormatArgsArg<'_>], name: &str) {
     let mut counts = HirIdMap::<usize>::default();
     for arg in args {
         *counts.entry(arg.value.hir_id).or_default() += 1;
@@ -436,14 +436,24 @@ fn check_literal(cx: &LateContext<'_>, args: &[FormatArgsArg<'_>], name: &str, r
         if_chain! {
             if counts[&arg.value.hir_id] == 1;
             if arg.format_trait == sym::Display;
+            if let ExprKind::Lit(lit) = &arg.value.kind;
             if !arg.has_primitive_formatting();
             if !arg.value.span.from_expansion();
-            if let ExprKind::Lit(lit) = &arg.value.kind;
+            if let Some(value_string) = snippet_opt(cx, arg.value.span);
+            if let Some(format_string) = snippet_opt(cx, format_args.format_string_span);
             then {
-                let replacement = match lit.node {
-                    LitKind::Str(s, _) => s.to_string(),
-                    LitKind::Char(c) => c.to_string(),
-                    LitKind::Bool(b) => b.to_string(),
+                let (replacement, replace_raw) = match lit.node {
+                    LitKind::Str(..) => extract_str_literal(&value_string),
+                    LitKind::Char(ch) => (
+                        match ch {
+                            '"' => "\\\"",
+                            '\'' => "'",
+                            _ => &value_string[1..value_string.len() - 1],
+                        }
+                        .to_string(),
+                        false,
+                    ),
+                    LitKind::Bool(b) => (b.to_string(), false),
                     _ => continue,
                 };
 
@@ -453,40 +463,95 @@ fn check_literal(cx: &LateContext<'_>, args: &[FormatArgsArg<'_>], name: &str, r
                     PRINT_LITERAL
                 };
 
+                let replacement = match (format_string.starts_with('r'), replace_raw) {
+                    (false, false) => Some(replacement),
+                    (false, true) => Some(replacement.replace('"', "\\\"").replace('\\', "\\\\")),
+                    (true, false) => match conservative_unescape(&replacement) {
+                        Ok(unescaped) => Some(unescaped),
+                        Err(UnescapeErr::Lint) => None,
+                        Err(UnescapeErr::Ignore) => continue,
+                    },
+                    (true, true) => {
+                        if replacement.contains(['#', '"']) {
+                            None
+                        } else {
+                            Some(replacement)
+                        }
+                    },
+                };
+
                 span_lint_and_then(cx, lint, arg.value.span, "literal with an empty format string", |diag| {
-                    if raw && replacement.contains(&['"', '#']) {
-                        return;
-                    }
+                    if let Some(replacement) = replacement {
+                        // `format!("{}", "a")`, `format!("{named}", named = "b")
+                        //              ~~~~~                      ~~~~~~~~~~~~~
+                        let value_span = expand_past_previous_comma(cx, arg.value.span);
 
-                    let backslash = if raw {
-                        r"\"
-                    } else {
-                        r"\\"
-                    };
-                    let replacement = replacement
-                        .replace('{', "{{")
-                        .replace('}', "}}")
-                        .replace('"', "\\\"")
-                        .replace('\\', backslash);
-
-                    // `format!("{}", "a")`, `format!("{named}", named = "b")
-                    //              ~~~~~                      ~~~~~~~~~~~~~
-                    let value_span = expand_past_previous_comma(cx, arg.value.span);
-
-                    diag.multipart_suggestion(
-                        "try this",
-                        vec![
-                            (arg.span, replacement),
-                            (value_span, String::new()),
-                        ],
-                        Applicability::MachineApplicable,
-                    );
+                        let replacement = replacement.replace('{', "{{").replace('}', "}}");
+                        diag.multipart_suggestion(
+                            "try this",
+                            vec![
+                                (arg.span, replacement),
+                                (value_span, String::new()),
+                            ],
+                            Applicability::MachineApplicable,
+                        );
+                    }
                 });
             }
         }
     }
 }
 
+/// Removes the raw marker, `#`s and quotes from a str, and returns if the literal is raw
+///
+/// `r#"a"#` -> (`a`, true)
+///
+/// `"b"` -> (`b`, false)
+fn extract_str_literal(literal: &str) -> (String, bool) {
+    let (literal, raw) = match literal.strip_prefix('r') {
+        Some(stripped) => (stripped.trim_matches('#'), true),
+        None => (literal, false),
+    };
+
+    (literal[1..literal.len() - 1].to_string(), raw)
+}
+
+enum UnescapeErr {
+    /// Should still be linted, can be manually resolved by author, e.g.
+    ///
+    /// ```ignore
+    /// print!(r"{}", '"');
+    /// ```
+    Lint,
+    /// Should not be linted, e.g.
+    ///
+    /// ```ignore
+    /// print!(r"{}", '\r');
+    /// ```
+    Ignore,
+}
+
+/// Unescape a normal string into a raw string
+fn conservative_unescape(literal: &str) -> Result<String, UnescapeErr> {
+    let mut unescaped = String::with_capacity(literal.len());
+    let mut chars = literal.chars();
+    let mut err = false;
+
+    while let Some(ch) = chars.next() {
+        match ch {
+            '#' => err = true,
+            '\\' => match chars.next() {
+                Some('\\') => unescaped.push('\\'),
+                Some('"') => err = true,
+                _ => return Err(UnescapeErr::Ignore),
+            },
+            _ => unescaped.push(ch),
+        }
+    }
+
+    if err { Err(UnescapeErr::Lint) } else { Ok(unescaped) }
+}
+
 // Expand from `writeln!(o, "")` to `writeln!(o, "")`
 //                          ^^                 ^^^^
 fn expand_past_previous_comma(cx: &LateContext<'_>, span: Span) -> Span {

clippy_utils/src/macros.rs

@@ -511,14 +511,6 @@ impl<'tcx> FormatArgsExpn<'tcx> {
             .collect()
     }
 
-    pub fn is_raw(&self, cx: &LateContext<'tcx>) -> bool {
-        if let Some(format_string) = snippet_opt(cx, self.format_string_span) {
-            format_string.starts_with('r')
-        } else {
-            false
-        }
-    }
-
     /// Source callsite span of all inputs
     pub fn inputs_span(&self) -> Span {
         match *self.value_args {

tests/ui/write_literal_2.rs

@@ -28,4 +28,7 @@ fn main() {
     writeln!(v, r"{}", "\\");
     writeln!(v, r#"{}"#, "\\");
     writeln!(v, "{}", r"\");
+    writeln!(v, "{}", "\r");
+    writeln!(v, r#"{}{}"#, '#', '"'); // hard mode
+    writeln!(v, r"{}", "\r"); // should not lint
 }

tests/ui/write_literal_2.stderr

@@ -74,9 +74,9 @@ LL | |         world!"
    |
 help: try this
    |
-LL -         "some {}",
-LL +         "some hello world!"
-   | 
+LL ~         "some hello /
+LL ~         world!"
+   |
 
 error: literal with an empty format string
   --> $DIR/write_literal_2.rs:25:9
@@ -162,5 +162,29 @@ LL -     writeln!(v, "{}", r"/");
 LL +     writeln!(v, "/");
    | 
 
-error: aborting due to 14 previous errors
+error: literal with an empty format string
+  --> $DIR/write_literal_2.rs:31:23
+   |
+LL |     writeln!(v, "{}", "/r");
+   |                       ^^^^
+   |
+help: try this
+   |
+LL -     writeln!(v, "{}", "/r");
+LL +     writeln!(v, "/r");
+   | 
+
+error: literal with an empty format string
+  --> $DIR/write_literal_2.rs:32:28
+   |
+LL |     writeln!(v, r#"{}{}"#, '#', '"'); // hard mode
+   |                            ^^^
+
+error: literal with an empty format string
+  --> $DIR/write_literal_2.rs:32:33
+   |
+LL |     writeln!(v, r#"{}{}"#, '#', '"'); // hard mode
+   |                                 ^^^
+
+error: aborting due to 17 previous errors