From 258b9a856273a0e6bbf6becd1dd769cb20228815 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Mon, 31 Jul 2023 20:22:39 +0000 Subject: [PATCH] Don't escape unicode escape braces in print_literal --- clippy_lints/src/write.rs | 50 ++++++++++++++++-- tests/ui/print_literal.fixed | 14 +++++ tests/ui/print_literal.rs | 14 +++++ tests/ui/print_literal.stderr | 98 ++++++++++++++++++++++++++++++++++- 4 files changed, 172 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index 21b675ff679..855aefa70cb 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -486,9 +486,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, }, @@ -538,7 +538,7 @@ fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgs, name: &str) { // `format!("{}", "a")`, `format!("{named}", named = "b") // ~~~~~ ~~~~~~~~~~~~~ && let Some(removal_span) = format_arg_removal_span(format_args, index) { - let replacement = replacement.replace('{', "{{").replace('}', "}}"); + let replacement = escape_braces(&replacement, !format_string_is_raw && !replace_raw); suggestion.push((*placeholder_span, replacement)); suggestion.push((removal_span, String::new())); } @@ -631,3 +631,47 @@ fn conservative_unescape(literal: &str) -> Result { 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 +#[expect(clippy::match_same_arms)] +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/ui/print_literal.fixed b/tests/ui/print_literal.fixed index 04a484258a4..a7157c07f8a 100644 --- a/tests/ui/print_literal.fixed +++ b/tests/ui/print_literal.fixed @@ -51,4 +51,18 @@ 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} {{:?}}"); + println!("\\\u{1234}"); + // 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!("\\\\u{{1234}}"); + + println!("mixed: {{hello}} {world}"); } diff --git a/tests/ui/print_literal.rs b/tests/ui/print_literal.rs index 38c036f56c5..4b04b42744c 100644 --- a/tests/ui/print_literal.rs +++ b/tests/ui/print_literal.rs @@ -51,4 +51,18 @@ 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} {:?}"); + println!("{}", "\\\u{1234}"); + // 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!("{}", "\\\\u{1234}"); + + println!("mixed: {} {world}", "{hello}"); } diff --git a/tests/ui/print_literal.stderr b/tests/ui/print_literal.stderr index b2f85be9d94..8c011d7bc0a 100644 --- a/tests/ui/print_literal.stderr +++ b/tests/ui/print_literal.stderr @@ -96,5 +96,101 @@ LL - println!("{bar} {foo}", foo = "hello", bar = "world"); LL + println!("world hello"); | -error: aborting due to 8 previous errors +error: literal with an empty format string + --> $DIR/print_literal.rs:56: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:57:20 + | +LL | println!("{}", "\\\u{1234}"); + | ^^^^^^^^^^^^ + | +help: try + | +LL - println!("{}", "\\\u{1234}"); +LL + println!("\\\u{1234}"); + | + +error: literal with an empty format string + --> $DIR/print_literal.rs:61: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:62: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:63: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:64: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:65:20 + | +LL | println!("{}", "\\\\u{1234}"); + | ^^^^^^^^^^^^^ + | +help: try + | +LL - println!("{}", "\\\\u{1234}"); +LL + println!("\\\\u{{1234}}"); + | + +error: literal with an empty format string + --> $DIR/print_literal.rs:67:35 + | +LL | println!("mixed: {} {world}", "{hello}"); + | ^^^^^^^^^ + | +help: try + | +LL - println!("mixed: {} {world}", "{hello}"); +LL + println!("mixed: {{hello}} {world}"); + | + +error: aborting due to 16 previous errors