From 982124acfafce0c14ace9cf48ab946b4a5901dd3 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Sat, 20 Nov 2021 11:23:49 +0100 Subject: [PATCH 1/4] Add new lint `octal_escapes` This checks for sequences in strings that would be octal character escapes in C, but are not supported in Rust. It suggests either to use the `\x00` escape, or an equivalent hex escape if the octal was intended. --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_suspicious.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/octal_escapes.rs | 131 ++++++++++++++++++++ tests/ui/octal_escapes.rs | 12 ++ tests/ui/octal_escapes.stderr | 67 ++++++++++ 8 files changed, 216 insertions(+) create mode 100644 clippy_lints/src/octal_escapes.rs create mode 100644 tests/ui/octal_escapes.rs create mode 100644 tests/ui/octal_escapes.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index dadf01419f74..401557b3eacd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3056,6 +3056,7 @@ Released 2018-09-13 [`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options [`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces [`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref +[`octal_escapes`]: https://rust-lang.github.io/rust-clippy/master/index.html#octal_escapes [`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect [`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref [`option_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 5ed2729b99b8..612240135ac6 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -219,6 +219,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS), LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY), + LintId::of(octal_escapes::OCTAL_ESCAPES), LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS), LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP), LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 000c72266857..19c35a5e5f40 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -380,6 +380,7 @@ store.register_lints(&[ non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS, non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY, nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES, + octal_escapes::OCTAL_ESCAPES, open_options::NONSENSICAL_OPEN_OPTIONS, option_env_unwrap::OPTION_ENV_UNWRAP, option_if_let_else::OPTION_IF_LET_ELSE, diff --git a/clippy_lints/src/lib.register_suspicious.rs b/clippy_lints/src/lib.register_suspicious.rs index a3f964d15804..414bfc42fdfc 100644 --- a/clippy_lints/src/lib.register_suspicious.rs +++ b/clippy_lints/src/lib.register_suspicious.rs @@ -16,6 +16,7 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec! LintId::of(methods::SUSPICIOUS_MAP), LintId::of(mut_key::MUTABLE_KEY_TYPE), LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY), + LintId::of(octal_escapes::OCTAL_ESCAPES), LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL), LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL), ]) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 449fa8a6311a..3dafdf8f0d5e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -312,6 +312,7 @@ mod non_expressive_names; mod non_octal_unix_permissions; mod non_send_fields_in_send_ty; mod nonstandard_macro_braces; +mod octal_escapes; mod open_options; mod option_env_unwrap; mod option_if_let_else; @@ -849,6 +850,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(match_str_case_mismatch::MatchStrCaseMismatch)); store.register_late_pass(move || Box::new(format_args::FormatArgs)); store.register_late_pass(|| Box::new(trailing_empty_array::TrailingEmptyArray)); + store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/octal_escapes.rs b/clippy_lints/src/octal_escapes.rs new file mode 100644 index 000000000000..9ae269bc7d99 --- /dev/null +++ b/clippy_lints/src/octal_escapes.rs @@ -0,0 +1,131 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use rustc_ast::ast::{Expr, ExprKind}; +use rustc_ast::token::{Lit, LitKind}; +use rustc_errors::Applicability; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_middle::lint::in_external_macro; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::Span; + +declare_clippy_lint! { + /// ### What it does + /// Checks for `\0` escapes in string and byte literals that look like octal character + /// escapes in C + /// + /// ### Why is this bad? + /// Rust does not support octal notation for character escapes. `\0` is always a + /// null byte/character, and any following digits do not form part of the escape + /// sequence. + /// + /// ### Known problems + /// The actual meaning can be the intended one. `\x00` can be used in these + /// cases to be unambigious. + /// + /// # Example + /// ```rust + /// // Bad + /// let one = "\033[1m Bold? \033[0m"; // \033 intended as escape + /// let two = "\033\0"; // \033 intended as null-3-3 + /// + /// // Good + /// let one = "\x1b[1mWill this be bold?\x1b[0m"; + /// let two = "\x0033\x00"; + /// ``` + #[clippy::version = "1.58.0"] + pub OCTAL_ESCAPES, + suspicious, + "string escape sequences looking like octal characters" +} + +declare_lint_pass!(OctalEscapes => [OCTAL_ESCAPES]); + +impl EarlyLintPass for OctalEscapes { + fn check_expr(&mut self, cx: &EarlyContext<'tcx>, expr: &Expr) { + if in_external_macro(cx.sess, expr.span) { + return; + } + + if let ExprKind::Lit(lit) = &expr.kind { + if matches!(lit.token.kind, LitKind::Str) { + check_lit(cx, &lit.token, lit.span, true); + } else if matches!(lit.token.kind, LitKind::ByteStr) { + check_lit(cx, &lit.token, lit.span, false); + } + } + } +} + +fn check_lit(cx: &EarlyContext<'tcx>, lit: &Lit, span: Span, is_string: bool) { + let contents = lit.symbol.as_str(); + let mut iter = contents.char_indices(); + + // go through the string, looking for \0[0-7] + while let Some((from, ch)) = iter.next() { + if ch == '\\' { + if let Some((mut to, '0')) = iter.next() { + // collect all further potentially octal digits + while let Some((j, '0'..='7')) = iter.next() { + to = j + 1; + } + // if it's more than just `\0` we have a match + if to > from + 2 { + emit(cx, &contents, from, to, span, is_string); + return; + } + } + } + } +} + +fn emit(cx: &EarlyContext<'tcx>, contents: &str, from: usize, to: usize, span: Span, is_string: bool) { + // construct a replacement escape for that case that octal was intended + let escape = &contents[from + 1..to]; + let literal_suggestion = if is_string { + u32::from_str_radix(escape, 8).ok().and_then(|n| { + if n < 256 { + Some(format!("\\x{:02x}", n)) + } else if n <= std::char::MAX as u32 { + Some(format!("\\u{{{:x}}}", n)) + } else { + None + } + }) + } else { + u8::from_str_radix(escape, 8).ok().map(|n| format!("\\x{:02x}", n)) + }; + + span_lint_and_then( + cx, + OCTAL_ESCAPES, + span, + &format!( + "octal-looking escape in {} literal", + if is_string { "string" } else { "byte string" } + ), + |diag| { + diag.help(&format!( + "octal escapes are not supported, `\\0` is always a null {}", + if is_string { "character" } else { "byte" } + )); + // suggestion 1: equivalent hex escape + if let Some(sugg) = literal_suggestion { + diag.span_suggestion( + span, + "if an octal escape is intended, use", + format!("\"{}{}{}\"", &contents[..from], sugg, &contents[to..]), + Applicability::MaybeIncorrect, + ); + } + // suggestion 2: unambiguous null byte + diag.span_suggestion( + span, + &format!( + "if the null {} is intended, disambiguate using", + if is_string { "character" } else { "byte" } + ), + format!("\"{}\\x00{}\"", &contents[..from], &contents[from + 2..]), + Applicability::MaybeIncorrect, + ); + }, + ); +} diff --git a/tests/ui/octal_escapes.rs b/tests/ui/octal_escapes.rs new file mode 100644 index 000000000000..5ddee73c020e --- /dev/null +++ b/tests/ui/octal_escapes.rs @@ -0,0 +1,12 @@ +#![warn(clippy::octal_escapes)] + +fn main() { + let _bad1 = "\033[0m"; + let _bad2 = b"\033[0m"; + let _bad3 = "\\\033[0m"; + let _bad4 = "\01234567"; + let _bad5 = "\0\03"; + + let _good1 = "\\033[0m"; + let _good2 = "\0\\0"; +} diff --git a/tests/ui/octal_escapes.stderr b/tests/ui/octal_escapes.stderr new file mode 100644 index 000000000000..8a93e781bed2 --- /dev/null +++ b/tests/ui/octal_escapes.stderr @@ -0,0 +1,67 @@ +error: octal-looking escape in string literal + --> $DIR/octal_escapes.rs:4:17 + | +LL | let _bad1 = "/033[0m"; + | ^^^^^^^^^ + | + = note: `-D clippy::octal-escapes` implied by `-D warnings` + = help: octal escapes are not supported, `/0` is always a null character +help: if an octal escape is intended, use + | +LL | let _bad1 = "/x1b[0m"; + | ~~~~~~~~~ +help: if the null character is intended, disambiguate using + | +LL | let _bad1 = "/x0033[0m"; + | ~~~~~~~~~~~ + +error: octal-looking escape in byte string literal + --> $DIR/octal_escapes.rs:5:17 + | +LL | let _bad2 = b"/033[0m"; + | ^^^^^^^^^^ + | + = help: octal escapes are not supported, `/0` is always a null byte +help: if an octal escape is intended, use + | +LL | let _bad2 = "/x1b[0m"; + | ~~~~~~~~~ +help: if the null byte is intended, disambiguate using + | +LL | let _bad2 = "/x0033[0m"; + | ~~~~~~~~~~~ + +error: octal-looking escape in string literal + --> $DIR/octal_escapes.rs:6:17 + | +LL | let _bad3 = "//033[0m"; + | ^^^^^^^^^^^ + | + = help: octal escapes are not supported, `/0` is always a null character +help: if an octal escape is intended, use + | +LL | let _bad3 = "//x1b[0m"; + | ~~~~~~~~~~~ +help: if the null character is intended, disambiguate using + | +LL | let _bad3 = "//x0033[0m"; + | ~~~~~~~~~~~~~ + +error: octal-looking escape in string literal + --> $DIR/octal_escapes.rs:7:17 + | +LL | let _bad4 = "/01234567"; + | ^^^^^^^^^^^ + | + = help: octal escapes are not supported, `/0` is always a null character +help: if an octal escape is intended, use + | +LL | let _bad4 = "/u{53977}"; + | ~~~~~~~~~~~ +help: if the null character is intended, disambiguate using + | +LL | let _bad4 = "/x001234567"; + | ~~~~~~~~~~~~~ + +error: aborting due to 4 previous errors + From 850e7f533e8dc8ed37957550e88be0b0e34d7170 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Sun, 21 Nov 2021 16:06:19 +0100 Subject: [PATCH 2/4] `octal_escapes`: updates from review, fix byte string prefix --- clippy_lints/src/octal_escapes.rs | 44 ++++------- tests/ui/octal_escapes.rs | 8 ++ tests/ui/octal_escapes.stderr | 118 +++++++++++++++++++++++++++--- 3 files changed, 131 insertions(+), 39 deletions(-) diff --git a/clippy_lints/src/octal_escapes.rs b/clippy_lints/src/octal_escapes.rs index 9ae269bc7d99..b3a4ab121e1a 100644 --- a/clippy_lints/src/octal_escapes.rs +++ b/clippy_lints/src/octal_escapes.rs @@ -9,8 +9,8 @@ use rustc_span::Span; declare_clippy_lint! { /// ### What it does - /// Checks for `\0` escapes in string and byte literals that look like octal character - /// escapes in C + /// Checks for `\0` escapes in string and byte literals that look like octal + /// character escapes in C. /// /// ### Why is this bad? /// Rust does not support octal notation for character escapes. `\0` is always a @@ -57,20 +57,18 @@ impl EarlyLintPass for OctalEscapes { fn check_lit(cx: &EarlyContext<'tcx>, lit: &Lit, span: Span, is_string: bool) { let contents = lit.symbol.as_str(); - let mut iter = contents.char_indices(); + let mut iter = contents.char_indices().peekable(); // go through the string, looking for \0[0-7] while let Some((from, ch)) = iter.next() { if ch == '\\' { - if let Some((mut to, '0')) = iter.next() { - // collect all further potentially octal digits - while let Some((j, '0'..='7')) = iter.next() { - to = j + 1; - } - // if it's more than just `\0` we have a match - if to > from + 2 { - emit(cx, &contents, from, to, span, is_string); - return; + if let Some((_, '0')) = iter.next() { + // collect up to two further octal digits + if let Some((mut to, '0'..='7')) = iter.next() { + if let Some((_, '0'..='7')) = iter.peek() { + to += 1; + } + emit(cx, &contents, from, to + 1, span, is_string); } } } @@ -80,19 +78,9 @@ fn check_lit(cx: &EarlyContext<'tcx>, lit: &Lit, span: Span, is_string: bool) { fn emit(cx: &EarlyContext<'tcx>, contents: &str, from: usize, to: usize, span: Span, is_string: bool) { // construct a replacement escape for that case that octal was intended let escape = &contents[from + 1..to]; - let literal_suggestion = if is_string { - u32::from_str_radix(escape, 8).ok().and_then(|n| { - if n < 256 { - Some(format!("\\x{:02x}", n)) - } else if n <= std::char::MAX as u32 { - Some(format!("\\u{{{:x}}}", n)) - } else { - None - } - }) - } else { - u8::from_str_radix(escape, 8).ok().map(|n| format!("\\x{:02x}", n)) - }; + // the maximum value is \077, or \x3f + let literal_suggestion = u8::from_str_radix(escape, 8).ok().map(|n| format!("\\x{:02x}", n)); + let prefix = if is_string { "" } else { "b" }; span_lint_and_then( cx, @@ -111,8 +99,8 @@ fn emit(cx: &EarlyContext<'tcx>, contents: &str, from: usize, to: usize, span: S if let Some(sugg) = literal_suggestion { diag.span_suggestion( span, - "if an octal escape is intended, use", - format!("\"{}{}{}\"", &contents[..from], sugg, &contents[to..]), + "if an octal escape was intended, use the hexadecimal representation instead", + format!("{}\"{}{}{}\"", prefix, &contents[..from], sugg, &contents[to..]), Applicability::MaybeIncorrect, ); } @@ -123,7 +111,7 @@ fn emit(cx: &EarlyContext<'tcx>, contents: &str, from: usize, to: usize, span: S "if the null {} is intended, disambiguate using", if is_string { "character" } else { "byte" } ), - format!("\"{}\\x00{}\"", &contents[..from], &contents[from + 2..]), + format!("{}\"{}\\x00{}\"", prefix, &contents[..from], &contents[from + 2..]), Applicability::MaybeIncorrect, ); }, diff --git a/tests/ui/octal_escapes.rs b/tests/ui/octal_escapes.rs index 5ddee73c020e..53145ef0fd20 100644 --- a/tests/ui/octal_escapes.rs +++ b/tests/ui/octal_escapes.rs @@ -4,9 +4,17 @@ fn main() { let _bad1 = "\033[0m"; let _bad2 = b"\033[0m"; let _bad3 = "\\\033[0m"; + // maximum 3 digits (\012 is the escape) let _bad4 = "\01234567"; let _bad5 = "\0\03"; + let _bad6 = "Text-\055\077-MoreText"; + let _bad7 = "EvenMoreText-\01\02-ShortEscapes"; + let _bad8 = "锈\01锈"; + let _bad9 = "锈\011锈"; let _good1 = "\\033[0m"; let _good2 = "\0\\0"; + let _good3 = "\0\0"; + let _good4 = "X\0\0X"; + let _good5 = "锈\0锈"; } diff --git a/tests/ui/octal_escapes.stderr b/tests/ui/octal_escapes.stderr index 8a93e781bed2..8c27dc0cf05d 100644 --- a/tests/ui/octal_escapes.stderr +++ b/tests/ui/octal_escapes.stderr @@ -6,7 +6,7 @@ LL | let _bad1 = "/033[0m"; | = note: `-D clippy::octal-escapes` implied by `-D warnings` = help: octal escapes are not supported, `/0` is always a null character -help: if an octal escape is intended, use +help: if an octal escape was intended, use the hexadecimal representation instead | LL | let _bad1 = "/x1b[0m"; | ~~~~~~~~~ @@ -22,14 +22,14 @@ LL | let _bad2 = b"/033[0m"; | ^^^^^^^^^^ | = help: octal escapes are not supported, `/0` is always a null byte -help: if an octal escape is intended, use +help: if an octal escape was intended, use the hexadecimal representation instead | -LL | let _bad2 = "/x1b[0m"; - | ~~~~~~~~~ +LL | let _bad2 = b"/x1b[0m"; + | ~~~~~~~~~~ help: if the null byte is intended, disambiguate using | -LL | let _bad2 = "/x0033[0m"; - | ~~~~~~~~~~~ +LL | let _bad2 = b"/x0033[0m"; + | ~~~~~~~~~~~~ error: octal-looking escape in string literal --> $DIR/octal_escapes.rs:6:17 @@ -38,7 +38,7 @@ LL | let _bad3 = "//033[0m"; | ^^^^^^^^^^^ | = help: octal escapes are not supported, `/0` is always a null character -help: if an octal escape is intended, use +help: if an octal escape was intended, use the hexadecimal representation instead | LL | let _bad3 = "//x1b[0m"; | ~~~~~~~~~~~ @@ -48,20 +48,116 @@ LL | let _bad3 = "//x0033[0m"; | ~~~~~~~~~~~~~ error: octal-looking escape in string literal - --> $DIR/octal_escapes.rs:7:17 + --> $DIR/octal_escapes.rs:8:17 | LL | let _bad4 = "/01234567"; | ^^^^^^^^^^^ | = help: octal escapes are not supported, `/0` is always a null character -help: if an octal escape is intended, use +help: if an octal escape was intended, use the hexadecimal representation instead | -LL | let _bad4 = "/u{53977}"; +LL | let _bad4 = "/x0a34567"; | ~~~~~~~~~~~ help: if the null character is intended, disambiguate using | LL | let _bad4 = "/x001234567"; | ~~~~~~~~~~~~~ -error: aborting due to 4 previous errors +error: octal-looking escape in string literal + --> $DIR/octal_escapes.rs:10:17 + | +LL | let _bad6 = "Text-/055/077-MoreText"; + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: octal escapes are not supported, `/0` is always a null character +help: if an octal escape was intended, use the hexadecimal representation instead + | +LL | let _bad6 = "Text-/x2d/077-MoreText"; + | ~~~~~~~~~~~~~~~~~~~~~~~~ +help: if the null character is intended, disambiguate using + | +LL | let _bad6 = "Text-/x0055/077-MoreText"; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: octal-looking escape in string literal + --> $DIR/octal_escapes.rs:10:17 + | +LL | let _bad6 = "Text-/055/077-MoreText"; + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: octal escapes are not supported, `/0` is always a null character +help: if an octal escape was intended, use the hexadecimal representation instead + | +LL | let _bad6 = "Text-/055/x3f-MoreText"; + | ~~~~~~~~~~~~~~~~~~~~~~~~ +help: if the null character is intended, disambiguate using + | +LL | let _bad6 = "Text-/055/x0077-MoreText"; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: octal-looking escape in string literal + --> $DIR/octal_escapes.rs:11:17 + | +LL | let _bad7 = "EvenMoreText-/01/02-ShortEscapes"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: octal escapes are not supported, `/0` is always a null character +help: if an octal escape was intended, use the hexadecimal representation instead + | +LL | let _bad7 = "EvenMoreText-/x01/02-ShortEscapes"; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: if the null character is intended, disambiguate using + | +LL | let _bad7 = "EvenMoreText-/x001/02-ShortEscapes"; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: octal-looking escape in string literal + --> $DIR/octal_escapes.rs:11:17 + | +LL | let _bad7 = "EvenMoreText-/01/02-ShortEscapes"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: octal escapes are not supported, `/0` is always a null character +help: if an octal escape was intended, use the hexadecimal representation instead + | +LL | let _bad7 = "EvenMoreText-/01/x02-ShortEscapes"; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: if the null character is intended, disambiguate using + | +LL | let _bad7 = "EvenMoreText-/01/x002-ShortEscapes"; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: octal-looking escape in string literal + --> $DIR/octal_escapes.rs:12:17 + | +LL | let _bad8 = "锈/01锈"; + | ^^^^^^^^^ + | + = help: octal escapes are not supported, `/0` is always a null character +help: if an octal escape was intended, use the hexadecimal representation instead + | +LL | let _bad8 = "锈/x01锈"; + | ~~~~~~~~~~ +help: if the null character is intended, disambiguate using + | +LL | let _bad8 = "锈/x001锈"; + | ~~~~~~~~~~~ + +error: octal-looking escape in string literal + --> $DIR/octal_escapes.rs:13:17 + | +LL | let _bad9 = "锈/011锈"; + | ^^^^^^^^^^ + | + = help: octal escapes are not supported, `/0` is always a null character +help: if an octal escape was intended, use the hexadecimal representation instead + | +LL | let _bad9 = "锈/x09锈"; + | ~~~~~~~~~~ +help: if the null character is intended, disambiguate using + | +LL | let _bad9 = "锈/x0011锈"; + | ~~~~~~~~~~~~ + +error: aborting due to 10 previous errors From 0bc25d04c6aed8a40f415b9a8eaaeb136feda0e3 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Mon, 22 Nov 2021 18:18:16 +0100 Subject: [PATCH 3/4] octal_escapes: emit only one lint for all cases found each literal --- clippy_lints/src/octal_escapes.rs | 69 +++++++++++++++++++++---------- tests/ui/octal_escapes.stderr | 46 ++++----------------- 2 files changed, 55 insertions(+), 60 deletions(-) diff --git a/clippy_lints/src/octal_escapes.rs b/clippy_lints/src/octal_escapes.rs index b3a4ab121e1a..0e27be259856 100644 --- a/clippy_lints/src/octal_escapes.rs +++ b/clippy_lints/src/octal_escapes.rs @@ -6,6 +6,7 @@ use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::Span; +use std::fmt::Write; declare_clippy_lint! { /// ### What it does @@ -13,9 +14,14 @@ declare_clippy_lint! { /// character escapes in C. /// /// ### Why is this bad? - /// Rust does not support octal notation for character escapes. `\0` is always a - /// null byte/character, and any following digits do not form part of the escape - /// sequence. + /// + /// C and other languages support octal character escapes in strings, where + /// a backslash is followed by up to three octal digits. For example, `\033` + /// stands for the ASCII character 27 (ESC). Rust does not support this + /// notation, but has the escape code `\0` which stands for a null + /// byte/character, and any following digits do not form part of the escape + /// sequence. Therefore, `\033` is not a compiler error but the result may + /// be surprising. /// /// ### Known problems /// The actual meaning can be the intended one. `\x00` can be used in these @@ -58,8 +64,9 @@ impl EarlyLintPass for OctalEscapes { fn check_lit(cx: &EarlyContext<'tcx>, lit: &Lit, span: Span, is_string: bool) { let contents = lit.symbol.as_str(); let mut iter = contents.char_indices().peekable(); + let mut found = vec![]; - // go through the string, looking for \0[0-7] + // go through the string, looking for \0[0-7][0-7]? while let Some((from, ch)) = iter.next() { if ch == '\\' { if let Some((_, '0')) = iter.next() { @@ -68,19 +75,41 @@ fn check_lit(cx: &EarlyContext<'tcx>, lit: &Lit, span: Span, is_string: bool) { if let Some((_, '0'..='7')) = iter.peek() { to += 1; } - emit(cx, &contents, from, to + 1, span, is_string); + found.push((from, to + 1)); } } } } -} -fn emit(cx: &EarlyContext<'tcx>, contents: &str, from: usize, to: usize, span: Span, is_string: bool) { - // construct a replacement escape for that case that octal was intended - let escape = &contents[from + 1..to]; - // the maximum value is \077, or \x3f - let literal_suggestion = u8::from_str_radix(escape, 8).ok().map(|n| format!("\\x{:02x}", n)); - let prefix = if is_string { "" } else { "b" }; + if found.is_empty() { + return; + } + + // construct two suggestion strings, one with \x escapes with octal meaning + // as in C, and one with \x00 for null bytes. + let mut suggest_1 = if is_string { "\"" } else { "b\"" }.to_string(); + let mut suggest_2 = suggest_1.clone(); + let mut index = 0; + for (from, to) in found { + suggest_1.push_str(&contents[index..from]); + suggest_2.push_str(&contents[index..from]); + + // construct a replacement escape + // the maximum value is \077, or \x3f, so u8 is sufficient here + if let Ok(n) = u8::from_str_radix(&contents[from + 1..to], 8) { + write!(&mut suggest_1, "\\x{:02x}", n).unwrap(); + } + + // append the null byte as \x00 and the following digits literally + suggest_2.push_str("\\x00"); + suggest_2.push_str(&contents[from + 2..to]); + + index = to; + } + suggest_1.push_str(&contents[index..]); + suggest_1.push('"'); + suggest_2.push_str(&contents[index..]); + suggest_2.push('"'); span_lint_and_then( cx, @@ -96,14 +125,12 @@ fn emit(cx: &EarlyContext<'tcx>, contents: &str, from: usize, to: usize, span: S if is_string { "character" } else { "byte" } )); // suggestion 1: equivalent hex escape - if let Some(sugg) = literal_suggestion { - diag.span_suggestion( - span, - "if an octal escape was intended, use the hexadecimal representation instead", - format!("{}\"{}{}{}\"", prefix, &contents[..from], sugg, &contents[to..]), - Applicability::MaybeIncorrect, - ); - } + diag.span_suggestion( + span, + "if an octal escape was intended, use the hexadecimal representation instead", + suggest_1, + Applicability::MaybeIncorrect, + ); // suggestion 2: unambiguous null byte diag.span_suggestion( span, @@ -111,7 +138,7 @@ fn emit(cx: &EarlyContext<'tcx>, contents: &str, from: usize, to: usize, span: S "if the null {} is intended, disambiguate using", if is_string { "character" } else { "byte" } ), - format!("{}\"{}\\x00{}\"", prefix, &contents[..from], &contents[from + 2..]), + suggest_2, Applicability::MaybeIncorrect, ); }, diff --git a/tests/ui/octal_escapes.stderr b/tests/ui/octal_escapes.stderr index 8c27dc0cf05d..54f5bbb0fc43 100644 --- a/tests/ui/octal_escapes.stderr +++ b/tests/ui/octal_escapes.stderr @@ -72,28 +72,12 @@ LL | let _bad6 = "Text-/055/077-MoreText"; = help: octal escapes are not supported, `/0` is always a null character help: if an octal escape was intended, use the hexadecimal representation instead | -LL | let _bad6 = "Text-/x2d/077-MoreText"; +LL | let _bad6 = "Text-/x2d/x3f-MoreText"; | ~~~~~~~~~~~~~~~~~~~~~~~~ help: if the null character is intended, disambiguate using | -LL | let _bad6 = "Text-/x0055/077-MoreText"; - | ~~~~~~~~~~~~~~~~~~~~~~~~~~ - -error: octal-looking escape in string literal - --> $DIR/octal_escapes.rs:10:17 - | -LL | let _bad6 = "Text-/055/077-MoreText"; - | ^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: octal escapes are not supported, `/0` is always a null character -help: if an octal escape was intended, use the hexadecimal representation instead - | -LL | let _bad6 = "Text-/055/x3f-MoreText"; - | ~~~~~~~~~~~~~~~~~~~~~~~~ -help: if the null character is intended, disambiguate using - | -LL | let _bad6 = "Text-/055/x0077-MoreText"; - | ~~~~~~~~~~~~~~~~~~~~~~~~~~ +LL | let _bad6 = "Text-/x0055/x0077-MoreText"; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: octal-looking escape in string literal --> $DIR/octal_escapes.rs:11:17 @@ -104,28 +88,12 @@ LL | let _bad7 = "EvenMoreText-/01/02-ShortEscapes"; = help: octal escapes are not supported, `/0` is always a null character help: if an octal escape was intended, use the hexadecimal representation instead | -LL | let _bad7 = "EvenMoreText-/x01/02-ShortEscapes"; - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -help: if the null character is intended, disambiguate using - | -LL | let _bad7 = "EvenMoreText-/x001/02-ShortEscapes"; +LL | let _bad7 = "EvenMoreText-/x01/x02-ShortEscapes"; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -error: octal-looking escape in string literal - --> $DIR/octal_escapes.rs:11:17 - | -LL | let _bad7 = "EvenMoreText-/01/02-ShortEscapes"; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: octal escapes are not supported, `/0` is always a null character -help: if an octal escape was intended, use the hexadecimal representation instead - | -LL | let _bad7 = "EvenMoreText-/01/x02-ShortEscapes"; - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ help: if the null character is intended, disambiguate using | -LL | let _bad7 = "EvenMoreText-/01/x002-ShortEscapes"; - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +LL | let _bad7 = "EvenMoreText-/x001/x002-ShortEscapes"; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: octal-looking escape in string literal --> $DIR/octal_escapes.rs:12:17 @@ -159,5 +127,5 @@ help: if the null character is intended, disambiguate using LL | let _bad9 = "锈/x0011锈"; | ~~~~~~~~~~~~ -error: aborting due to 10 previous errors +error: aborting due to 8 previous errors From 1210bb40d35829672744c1d58f3614723a7e1bad Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Mon, 22 Nov 2021 21:02:03 +0100 Subject: [PATCH 4/4] octal_escapes: note on print!() format strings --- clippy_lints/src/octal_escapes.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clippy_lints/src/octal_escapes.rs b/clippy_lints/src/octal_escapes.rs index 0e27be259856..9c9714376454 100644 --- a/clippy_lints/src/octal_escapes.rs +++ b/clippy_lints/src/octal_escapes.rs @@ -27,6 +27,10 @@ declare_clippy_lint! { /// The actual meaning can be the intended one. `\x00` can be used in these /// cases to be unambigious. /// + /// The lint does not trigger for format strings in `print!()`, `write!()` + /// and friends since the string is already preprocessed when Clippy lints + /// can see it. + /// /// # Example /// ```rust /// // Bad