From 982124acfafce0c14ace9cf48ab946b4a5901dd3 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Sat, 20 Nov 2021 11:23:49 +0100 Subject: [PATCH] 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 +