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

Add new lint octal_escapes #8007

Merged
merged 4 commits into from
Nov 22, 2021
Merged
Show file tree
Hide file tree
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
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";
xFrednet marked this conversation as resolved.
Show resolved Hide resolved
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";
birkenfeld marked this conversation as resolved.
Show resolved Hide resolved
| ^^^^^^^^^
|
= 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