From bc27d1492d60ecfb0673629e28bc4bbe0b7fd886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20Jos=C3=A9=20Pereira?= Date: Thu, 8 Oct 2020 00:19:56 -0300 Subject: [PATCH] Add string_from_utf8_as_bytes linter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Patrick José Pereira --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 3 + clippy_lints/src/strings.rs | 68 ++++++++++++++++++++++- clippy_lints/src/utils/paths.rs | 1 + src/lintlist/mod.rs | 7 +++ tests/ui/string_from_utf8_as_bytes.fixed | 6 ++ tests/ui/string_from_utf8_as_bytes.rs | 6 ++ tests/ui/string_from_utf8_as_bytes.stderr | 10 ++++ 8 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 tests/ui/string_from_utf8_as_bytes.fixed create mode 100644 tests/ui/string_from_utf8_as_bytes.rs create mode 100644 tests/ui/string_from_utf8_as_bytes.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b63dbb7e..aeccf6cf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5c3af014e..d29ba3306 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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, @@ -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), @@ -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), diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index 3783bd78d..a55df1e55 100644 --- a/clippy_lints/src/strings.rs +++ b/clippy_lints/src/strings.rs @@ -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}; @@ -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 @@ -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); diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index cd72fdd61..0d0ecf9ae 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -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", "", "ends_with"]; +pub const STR_FROM_UTF8: [&str; 4] = ["core", "str", "converts", "from_utf8"]; pub const STR_LEN: [&str; 4] = ["core", "str", "", "len"]; pub const STR_STARTS_WITH: [&str; 4] = ["core", "str", "", "starts_with"]; pub const SYNTAX_CONTEXT: [&str; 3] = ["rustc_span", "hygiene", "SyntaxContext"]; diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 016bda77e..4d824cf94 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -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", diff --git a/tests/ui/string_from_utf8_as_bytes.fixed b/tests/ui/string_from_utf8_as_bytes.fixed new file mode 100644 index 000000000..6e665cdd5 --- /dev/null +++ b/tests/ui/string_from_utf8_as_bytes.fixed @@ -0,0 +1,6 @@ +// run-rustfix +#![warn(clippy::string_from_utf8_as_bytes)] + +fn main() { + let _ = Some(&"Hello World!"[6..11]); +} diff --git a/tests/ui/string_from_utf8_as_bytes.rs b/tests/ui/string_from_utf8_as_bytes.rs new file mode 100644 index 000000000..670d206d3 --- /dev/null +++ b/tests/ui/string_from_utf8_as_bytes.rs @@ -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]); +} diff --git a/tests/ui/string_from_utf8_as_bytes.stderr b/tests/ui/string_from_utf8_as_bytes.stderr new file mode 100644 index 000000000..bf5e5d33e --- /dev/null +++ b/tests/ui/string_from_utf8_as_bytes.stderr @@ -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 +