Don't escape unicode escape braces in print_literal

Alexendoo · Alexendoo · commit 97587e87ee8f · 2023-07-31T20:27:34.000Z
diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs
@@ -471,9 +471,9 @@ fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgs, name: &str) {
             && let rustc_ast::ExprKind::Lit(lit) = &arg.expr.kind
             && !arg.expr.span.from_expansion()
             && let Some(value_string) = snippet_opt(cx, arg.expr.span)
-    {
+        {
             let (replacement, replace_raw) = match lit.kind {
-                LitKind::Str | LitKind::StrRaw(_)  => match extract_str_literal(&value_string) {
+                LitKind::Str | LitKind::StrRaw(_) => match extract_str_literal(&value_string) {
                     Some(extracted) => extracted,
                     None => return,
                 },
@@ -519,27 +519,24 @@ fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgs, name: &str) {
                 },
             };
 
-            span_lint_and_then(
-                cx,
-                lint,
-                arg.expr.span,
-                "literal with an empty format string",
-                |diag| {
-                    if let Some(replacement) = replacement
-                        // `format!("{}", "a")`, `format!("{named}", named = "b")
-                        //              ~~~~~                      ~~~~~~~~~~~~~
-                        && let Some(removal_span) = format_arg_removal_span(format_args, index)
-                    {
-                        let replacement = replacement.replace('{', "{{").replace('}', "}}");
-                        diag.multipart_suggestion(
-                            "try",
-                            vec![(*placeholder_span, replacement), (removal_span, String::new())],
-                            Applicability::MachineApplicable,
-                        );
-                    }
-                },
-            );
+            span_lint_and_then(cx, lint, arg.expr.span, "literal with an empty format string", |diag| {
+                if let Some(replacement) = replacement
+                    // `format!("{}", "a")`, `format!("{named}", named = "b")
+                    //              ~~~~~                      ~~~~~~~~~~~~~
+                    && let Some(removal_span) = format_arg_removal_span(format_args, index)
+                {
+                    let replacement = escape_braces(&replacement, !format_string_is_raw && !replace_raw);
 
+                    diag.multipart_suggestion(
+                        "try",
+                        vec![
+                            (*placeholder_span, replacement),
+                            (removal_span, String::new()),
+                        ],
+                        Applicability::MachineApplicable,
+                    );
+                }
+            });
         }
     }
 }
@@ -593,3 +590,46 @@ fn conservative_unescape(literal: &str) -> Result<String, UnescapeErr> {
 
     if err { Err(UnescapeErr::Lint) } else { Ok(unescaped) }
 }
+
+/// Replaces `{` with `{{` and `}` with `}}`. If `preserve_unicode_escapes` is `true` the braces in
+/// `\u{xxxx}` are left unmodified
+fn escape_braces(literal: &str, preserve_unicode_escapes: bool) -> String {
+    #[derive(Clone, Copy)]
+    enum State {
+        Normal,
+        Backslash,
+        UnicodeEscape,
+    }
+
+    let mut escaped = String::with_capacity(literal.len());
+    let mut state = State::Normal;
+
+    for ch in literal.chars() {
+        state = match (ch, state) {
+            // Escape braces outside of unicode escapes by doubling them up
+            ('{' | '}', State::Normal) => {
+                escaped.push(ch);
+                State::Normal
+            },
+            // If `preserve_unicode_escapes` isn't enabled stay in `State::Normal`, otherwise:
+            //
+            // \u{aaaa} \\ \x01
+            // ^        ^  ^
+            ('\\', State::Normal) if preserve_unicode_escapes => State::Backslash,
+            // \u{aaaa}
+            //  ^
+            ('u', State::Backslash) => State::UnicodeEscape,
+            // \xAA \\
+            //  ^    ^
+            (_, State::Backslash) => State::Normal,
+            // \u{aaaa}
+            //        ^
+            ('}', State::UnicodeEscape) => State::Normal,
+            _ => state,
+        };
+
+        escaped.push(ch);
+    }
+
+    escaped
+}
diff --git a/tests/compile-test.rs b/tests/compile-test.rs
@@ -381,7 +381,6 @@ const RUSTFIX_COVERAGE_KNOWN_EXCEPTIONS: &[&str] = &[
     "needless_borrow_pat.rs",
     "needless_for_each_unfixable.rs",
     "nonminimal_bool.rs",
-    "print_literal.rs",
     "redundant_static_lifetimes_multiple.rs",
     "ref_binding_to_reference.rs",
     "repl_uninit.rs",
diff --git a/tests/ui/print_literal.fixed b/tests/ui/print_literal.fixed
@@ -0,0 +1,59 @@
+//@run-rustfix
+
+#![warn(clippy::print_literal)]
+#![allow(clippy::uninlined_format_args)]
+
+fn main() {
+    // these should be fine
+    print!("Hello");
+    println!("Hello");
+    let world = "world";
+    println!("Hello {}", world);
+    println!("Hello {world}", world = world);
+    println!("3 in hex is {:X}", 3);
+    println!("2 + 1 = {:.4}", 3);
+    println!("2 + 1 = {:5.4}", 3);
+    println!("Debug test {:?}", "hello, world");
+    println!("{0:8} {1:>8}", "hello", "world");
+    println!("{1:8} {0:>8}", "hello", "world");
+    println!("{foo:8} {bar:>8}", foo = "hello", bar = "world");
+    println!("{bar:8} {foo:>8}", foo = "hello", bar = "world");
+    println!("{number:>width$}", number = 1, width = 6);
+    println!("{number:>0width$}", number = 1, width = 6);
+    println!("{} of {:b} people know binary, the other half doesn't", 1, 2);
+    println!("10 / 4 is {}", 2.5);
+    println!("2 + 1 = {}", 3);
+    println!("From expansion {}", stringify!(not a string literal));
+
+    // these should throw warnings
+    print!("Hello world");
+    println!("Hello {} world", world);
+    println!("Hello world");
+    println!("a literal {:.4}", 5);
+
+    // positional args don't change the fact
+    // that we're using a literal -- this should
+    // throw a warning
+    println!("hello world");
+    println!("world hello");
+
+    // named args shouldn't change anything either
+    println!("hello world");
+    println!("world hello");
+
+    // The string literal from `file!()` has a callsite span that isn't marked as coming from an
+    // expansion
+    println!("file: {}", file!());
+
+    // Braces in unicode escapes should not be escaped
+    println!("{{}} \x00 \u{ab123} \\\u{ab123} {{:?}}");
+    // This does not lint because it would have to suggest unescaping the character
+    println!(r"{}", "\u{ab123}");
+    // These are not unicode escapes
+    println!("\\u{{ab123}} \\u{{{{");
+    println!(r"\u{{ab123}} \u{{{{");
+    println!("\\{{ab123}} \\u{{{{");
+    println!("\\u{{ab123}}");
+
+    println!("mixed: {{hello}} {world}");
+}
diff --git a/tests/ui/print_literal.rs b/tests/ui/print_literal.rs
@@ -1,3 +1,5 @@
+//@run-rustfix
+
 #![warn(clippy::print_literal)]
 #![allow(clippy::uninlined_format_args)]
 
@@ -42,4 +44,16 @@ fn main() {
     // The string literal from `file!()` has a callsite span that isn't marked as coming from an
     // expansion
     println!("file: {}", file!());
+
+    // Braces in unicode escapes should not be escaped
+    println!("{}", "{} \x00 \u{ab123} \\\u{ab123} {:?}");
+    // This does not lint because it would have to suggest unescaping the character
+    println!(r"{}", "\u{ab123}");
+    // These are not unicode escapes
+    println!("{}", r"\u{ab123} \u{{");
+    println!(r"{}", r"\u{ab123} \u{{");
+    println!("{}", r"\{ab123} \u{{");
+    println!("{}", "\\u{ab123}");
+
+    println!("mixed: {} {world}", "{hello}");
 }
diff --git a/tests/ui/print_literal.stderr b/tests/ui/print_literal.stderr
@@ -1,5 +1,5 @@
 error: literal with an empty format string
-  --> $DIR/print_literal.rs:27:24
+  --> $DIR/print_literal.rs:29:24
    |
 LL |     print!("Hello {}", "world");
    |                        ^^^^^^^
@@ -12,7 +12,7 @@ LL +     print!("Hello world");
    |
 
 error: literal with an empty format string
-  --> $DIR/print_literal.rs:28:36
+  --> $DIR/print_literal.rs:30:36
    |
 LL |     println!("Hello {} {}", world, "world");
    |                                    ^^^^^^^
@@ -24,7 +24,7 @@ LL +     println!("Hello {} world", world);
    |
 
 error: literal with an empty format string
-  --> $DIR/print_literal.rs:29:26
+  --> $DIR/print_literal.rs:31:26
    |
 LL |     println!("Hello {}", "world");
    |                          ^^^^^^^
@@ -36,7 +36,7 @@ LL +     println!("Hello world");
    |
 
 error: literal with an empty format string
-  --> $DIR/print_literal.rs:30:26
+  --> $DIR/print_literal.rs:32:26
    |
 LL |     println!("{} {:.4}", "a literal", 5);
    |                          ^^^^^^^^^^^
@@ -48,7 +48,7 @@ LL +     println!("a literal {:.4}", 5);
    |
 
 error: literal with an empty format string
-  --> $DIR/print_literal.rs:35:25
+  --> $DIR/print_literal.rs:37:25
    |
 LL |     println!("{0} {1}", "hello", "world");
    |                         ^^^^^^^
@@ -60,7 +60,7 @@ LL +     println!("hello {1}", "world");
    |
 
 error: literal with an empty format string
-  --> $DIR/print_literal.rs:35:34
+  --> $DIR/print_literal.rs:37:34
    |
 LL |     println!("{0} {1}", "hello", "world");
    |                                  ^^^^^^^
@@ -72,7 +72,7 @@ LL +     println!("{0} world", "hello");
    |
 
 error: literal with an empty format string
-  --> $DIR/print_literal.rs:36:34
+  --> $DIR/print_literal.rs:38:34
    |
 LL |     println!("{1} {0}", "hello", "world");
    |                                  ^^^^^^^
@@ -84,7 +84,7 @@ LL +     println!("world {0}", "hello");
    |
 
 error: literal with an empty format string
-  --> $DIR/print_literal.rs:36:25
+  --> $DIR/print_literal.rs:38:25
    |
 LL |     println!("{1} {0}", "hello", "world");
    |                         ^^^^^^^
@@ -96,7 +96,7 @@ LL +     println!("{1} hello", "world");
    |
 
 error: literal with an empty format string
-  --> $DIR/print_literal.rs:39:35
+  --> $DIR/print_literal.rs:41:35
    |
 LL |     println!("{foo} {bar}", foo = "hello", bar = "world");
    |                                   ^^^^^^^
@@ -108,7 +108,7 @@ LL +     println!("hello {bar}", bar = "world");
    |
 
 error: literal with an empty format string
-  --> $DIR/print_literal.rs:39:50
+  --> $DIR/print_literal.rs:41:50
    |
 LL |     println!("{foo} {bar}", foo = "hello", bar = "world");
    |                                                  ^^^^^^^
@@ -120,7 +120,7 @@ LL +     println!("{foo} world", foo = "hello");
    |
 
 error: literal with an empty format string
-  --> $DIR/print_literal.rs:40:50
+  --> $DIR/print_literal.rs:42:50
    |
 LL |     println!("{bar} {foo}", foo = "hello", bar = "world");
    |                                                  ^^^^^^^
@@ -132,7 +132,7 @@ LL +     println!("world {foo}", foo = "hello");
    |
 
 error: literal with an empty format string
-  --> $DIR/print_literal.rs:40:35
+  --> $DIR/print_literal.rs:42:35
    |
 LL |     println!("{bar} {foo}", foo = "hello", bar = "world");
    |                                   ^^^^^^^
@@ -143,5 +143,77 @@ LL -     println!("{bar} {foo}", foo = "hello", bar = "world");
 LL +     println!("{bar} hello", bar = "world");
    |
 
-error: aborting due to 12 previous errors
+error: literal with an empty format string
+  --> $DIR/print_literal.rs:49:20
+   |
+LL |     println!("{}", "{} /x00 /u{ab123} ///u{ab123} {:?}");
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try
+   |
+LL -     println!("{}", "{} /x00 /u{ab123} ///u{ab123} {:?}");
+LL +     println!("{{}} /x00 /u{ab123} ///u{ab123} {{:?}}");
+   |
+
+error: literal with an empty format string
+  --> $DIR/print_literal.rs:53:20
+   |
+LL |     println!("{}", r"/u{ab123} /u{{");
+   |                    ^^^^^^^^^^^^^^^^^
+   |
+help: try
+   |
+LL -     println!("{}", r"/u{ab123} /u{{");
+LL +     println!("//u{{ab123}} //u{{{{");
+   |
+
+error: literal with an empty format string
+  --> $DIR/print_literal.rs:54:21
+   |
+LL |     println!(r"{}", r"/u{ab123} /u{{");
+   |                     ^^^^^^^^^^^^^^^^^
+   |
+help: try
+   |
+LL -     println!(r"{}", r"/u{ab123} /u{{");
+LL +     println!(r"/u{{ab123}} /u{{{{");
+   |
+
+error: literal with an empty format string
+  --> $DIR/print_literal.rs:55:20
+   |
+LL |     println!("{}", r"/{ab123} /u{{");
+   |                    ^^^^^^^^^^^^^^^^
+   |
+help: try
+   |
+LL -     println!("{}", r"/{ab123} /u{{");
+LL +     println!("//{{ab123}} //u{{{{");
+   |
+
+error: literal with an empty format string
+  --> $DIR/print_literal.rs:56:20
+   |
+LL |     println!("{}", "//u{ab123}");
+   |                    ^^^^^^^^^^^^
+   |
+help: try
+   |
+LL -     println!("{}", "//u{ab123}");
+LL +     println!("//u{{ab123}}");
+   |
+
+error: literal with an empty format string
+  --> $DIR/print_literal.rs:58:35
+   |
+LL |     println!("mixed: {} {world}", "{hello}");
+   |                                   ^^^^^^^^^
+   |
+help: try
+   |
+LL -     println!("mixed: {} {world}", "{hello}");
+LL +     println!("mixed: {{hello}} {world}");
+   |
+
+error: aborting due to 18 previous errors
 
