Skip to content

Check when from_utf8 is called from sliced byte array from string #6134

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

Merged
merged 1 commit into from
Nov 7, 2020
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 @@ -1952,6 +1952,7 @@ Released 2018-09-13
[`string_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add
[`string_add_assign`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add_assign
[`string_extend_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_extend_chars
[`string_from_utf8_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_from_utf8_as_bytes
[`string_lit_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes
[`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string
[`struct_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#struct_excessive_bools
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&stable_sort_primitive::STABLE_SORT_PRIMITIVE,
&strings::STRING_ADD,
&strings::STRING_ADD_ASSIGN,
&strings::STRING_FROM_UTF8_AS_BYTES,
&strings::STRING_LIT_AS_BYTES,
&suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL,
&suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL,
Expand Down Expand Up @@ -1515,6 +1516,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
LintId::of(&slow_vector_initialization::SLOW_VECTOR_INITIALIZATION),
LintId::of(&stable_sort_primitive::STABLE_SORT_PRIMITIVE),
LintId::of(&strings::STRING_FROM_UTF8_AS_BYTES),
LintId::of(&suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
LintId::of(&suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
LintId::of(&swap::ALMOST_SWAPPED),
Expand Down Expand Up @@ -1738,6 +1740,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&reference::DEREF_ADDROF),
LintId::of(&reference::REF_IN_DEREF),
LintId::of(&repeat_once::REPEAT_ONCE),
LintId::of(&strings::STRING_FROM_UTF8_AS_BYTES),
LintId::of(&swap::MANUAL_SWAP),
LintId::of(&temporary_assignment::TEMPORARY_ASSIGNMENT),
LintId::of(&transmute::CROSSPOINTER_TRANSMUTE),
Expand Down
68 changes: 65 additions & 3 deletions clippy_lints/src/strings.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, LangItem, QPath};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand All @@ -8,7 +8,10 @@ use rustc_span::source_map::Spanned;
use if_chain::if_chain;

use crate::utils::SpanlessEq;
use crate::utils::{get_parent_expr, is_allowed, is_type_diagnostic_item, span_lint, span_lint_and_sugg};
use crate::utils::{
get_parent_expr, is_allowed, is_type_diagnostic_item, match_function_call, method_calls, paths, span_lint,
span_lint_and_sugg,
};

declare_clippy_lint! {
/// **What it does:** Checks for string appends of the form `x = x + y` (without
Expand Down Expand Up @@ -173,16 +176,75 @@ fn is_add(cx: &LateContext<'_>, src: &Expr<'_>, target: &Expr<'_>) -> bool {
}
}

declare_clippy_lint! {
/// **What it does:** Check if the string is transformed to byte array and casted back to string.
///
/// **Why is this bad?** It's unnecessary, the string can be used directly.
///
/// **Known problems:** None
///
/// **Example:**
/// ```rust
/// let _ = std::str::from_utf8(&"Hello World!".as_bytes()[6..11]).unwrap();
/// ```
/// could be written as
/// ```rust
/// let _ = &"Hello World!"[6..11];
/// ```
pub STRING_FROM_UTF8_AS_BYTES,
complexity,
"casting string slices to byte slices and back"
}

// Max length a b"foo" string can take
const MAX_LENGTH_BYTE_STRING_LIT: usize = 32;

declare_lint_pass!(StringLitAsBytes => [STRING_LIT_AS_BYTES]);
declare_lint_pass!(StringLitAsBytes => [STRING_LIT_AS_BYTES, STRING_FROM_UTF8_AS_BYTES]);

impl<'tcx> LateLintPass<'tcx> for StringLitAsBytes {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
use crate::utils::{snippet, snippet_with_applicability};
use rustc_ast::LitKind;

if_chain! {
// Find std::str::converts::from_utf8
if let Some(args) = match_function_call(cx, e, &paths::STR_FROM_UTF8);

// Find string::as_bytes
if let ExprKind::AddrOf(BorrowKind::Ref, _, ref args) = args[0].kind;
if let ExprKind::Index(ref left, ref right) = args.kind;
let (method_names, expressions, _) = method_calls(left, 1);
if method_names.len() == 1;
if expressions.len() == 1;
if expressions[0].len() == 1;
if method_names[0] == sym!(as_bytes);

// Check for slicer
if let ExprKind::Struct(ref path, _, _) = right.kind;
if let QPath::LangItem(LangItem::Range, _) = path;

then {
let mut applicability = Applicability::MachineApplicable;
let string_expression = &expressions[0][0];

let snippet_app = snippet_with_applicability(
cx,
string_expression.span, "..",
&mut applicability,
);

span_lint_and_sugg(
cx,
STRING_FROM_UTF8_AS_BYTES,
e.span,
"calling a slice of `as_bytes()` with `from_utf8` should be not necessary",
"try",
format!("Some(&{}[{}])", snippet_app, snippet(cx, right.span, "..")),
applicability
)
}
}

if_chain! {
if let ExprKind::MethodCall(path, _, args, _) = &e.kind;
if path.ident.name == sym!(as_bytes);
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ pub const STRING: [&str; 3] = ["alloc", "string", "String"];
pub const STRING_AS_MUT_STR: [&str; 4] = ["alloc", "string", "String", "as_mut_str"];
pub const STRING_AS_STR: [&str; 4] = ["alloc", "string", "String", "as_str"];
pub const STR_ENDS_WITH: [&str; 4] = ["core", "str", "<impl str>", "ends_with"];
pub const STR_FROM_UTF8: [&str; 4] = ["core", "str", "converts", "from_utf8"];
pub const STR_LEN: [&str; 4] = ["core", "str", "<impl str>", "len"];
pub const STR_STARTS_WITH: [&str; 4] = ["core", "str", "<impl str>", "starts_with"];
pub const SYNTAX_CONTEXT: [&str; 3] = ["rustc_span", "hygiene", "SyntaxContext"];
Expand Down
7 changes: 7 additions & 0 deletions src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2230,6 +2230,13 @@ vec![
deprecation: None,
module: "methods",
},
Lint {
name: "string_from_utf8_as_bytes",
group: "complexity",
desc: "casting string slices to byte slices and back",
deprecation: None,
module: "strings",
},
Lint {
name: "string_lit_as_bytes",
group: "nursery",
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/string_from_utf8_as_bytes.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// run-rustfix
#![warn(clippy::string_from_utf8_as_bytes)]

fn main() {
let _ = Some(&"Hello World!"[6..11]);
}
6 changes: 6 additions & 0 deletions tests/ui/string_from_utf8_as_bytes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// run-rustfix
#![warn(clippy::string_from_utf8_as_bytes)]

fn main() {
let _ = std::str::from_utf8(&"Hello World!".as_bytes()[6..11]);
}
10 changes: 10 additions & 0 deletions tests/ui/string_from_utf8_as_bytes.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: calling a slice of `as_bytes()` with `from_utf8` should be not necessary
--> $DIR/string_from_utf8_as_bytes.rs:5:13
|
LL | let _ = std::str::from_utf8(&"Hello World!".as_bytes()[6..11]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(&"Hello World!"[6..11])`
|
= note: `-D clippy::string-from-utf8-as-bytes` implied by `-D warnings`

error: aborting due to previous error