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 1 commit
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
131 changes: 131 additions & 0 deletions clippy_lints/src/octal_escapes.rs
Original file line number Diff line number Diff line change
@@ -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
birkenfeld marked this conversation as resolved.
Show resolved Hide resolved
///
/// ### 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;
}
birkenfeld marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}

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",
birkenfeld marked this conversation as resolved.
Show resolved Hide resolved
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,
);
},
);
}
12 changes: 12 additions & 0 deletions tests/ui/octal_escapes.rs
Original file line number Diff line number Diff line change
@@ -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";
xFrednet marked this conversation as resolved.
Show resolved Hide resolved

let _good1 = "\\033[0m";
let _good2 = "\0\\0";
}
67 changes: 67 additions & 0 deletions tests/ui/octal_escapes.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
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 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}";
| ~~~~~~~~~~~
birkenfeld marked this conversation as resolved.
Show resolved Hide resolved
help: if the null character is intended, disambiguate using
|
LL | let _bad4 = "/x001234567";
| ~~~~~~~~~~~~~

error: aborting due to 4 previous errors