Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't escape unicode escape braces in print_literal #11265

Merged
merged 1 commit into from
Oct 1, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 47 additions & 3 deletions clippy_lints/src/write.rs
Original file line number Diff line number Diff line change
@@ -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<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
#[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
}
14 changes: 14 additions & 0 deletions tests/ui/print_literal.fixed
Original file line number Diff line number Diff line change
@@ -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}");
}
14 changes: 14 additions & 0 deletions tests/ui/print_literal.rs
Original file line number Diff line number Diff line change
@@ -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}");
}
98 changes: 97 additions & 1 deletion tests/ui/print_literal.stderr
Original file line number Diff line number Diff line change
@@ -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