Skip to content

Commit

Permalink
Auto merge of #8007 - birkenfeld:octal_escapes, r=xFrednet
Browse files Browse the repository at this point in the history
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.

Fixes #7981

---

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: Add new lint [`octal_escapes`], which checks for literals like `"\033[0m"`.
  • Loading branch information
bors committed Nov 22, 2021
2 parents 4027594 + 1210bb4 commit 57a8804
Show file tree
Hide file tree
Showing 8 changed files with 307 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_suspicious.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
])
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
150 changes: 150 additions & 0 deletions clippy_lints/src/octal_escapes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
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;
use std::fmt::Write;

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?
///
/// 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
/// 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
/// 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().peekable();
let mut found = vec![];

// 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() {
// 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;
}
found.push((from, to + 1));
}
}
}
}

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,
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
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,
&format!(
"if the null {} is intended, disambiguate using",
if is_string { "character" } else { "byte" }
),
suggest_2,
Applicability::MaybeIncorrect,
);
},
);
}
20 changes: 20 additions & 0 deletions tests/ui/octal_escapes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#![warn(clippy::octal_escapes)]

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锈";
}
131 changes: 131 additions & 0 deletions tests/ui/octal_escapes.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
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 was intended, use the hexadecimal representation instead
|
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 was intended, use the hexadecimal representation instead
|
LL | let _bad2 = b"/x1b[0m";
| ~~~~~~~~~~
help: if the null byte is intended, disambiguate using
|
LL | let _bad2 = b"/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 was intended, use the hexadecimal representation instead
|
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:8:17
|
LL | let _bad4 = "/01234567";
| ^^^^^^^^^^^
|
= 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 _bad4 = "/x0a34567";
| ~~~~~~~~~~~
help: if the null character is intended, disambiguate using
|
LL | let _bad4 = "/x001234567";
| ~~~~~~~~~~~~~

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/x3f-MoreText";
| ~~~~~~~~~~~~~~~~~~~~~~~~
help: if the null character is intended, disambiguate using
|
LL | let _bad6 = "Text-/x0055/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/x02-ShortEscapes";
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
help: if the null character is intended, disambiguate using
|
LL | let _bad7 = "EvenMoreText-/x001/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 8 previous errors

0 comments on commit 57a8804

Please sign in to comment.