Auto merge of #6134 - patrickelectric:as_utf8, r=llogiq

Check when `from_utf8` is called from sliced byte array from string

---

*Please keep the line below*
changelog: Fix #5487: Add linter to check when `from_utf8` is called from sliced byte array from string.

CHANGELOG.md

@@ -1956,6 +1956,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

clippy_lints/src/lib.rs

@@ -832,6 +832,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,
@@ -1527,6 +1528,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),
@@ -1752,6 +1754,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),

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};
@@ -9,7 +9,10 @@ use rustc_span::sym;
 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
@@ -174,16 +177,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);

clippy_lints/src/utils/paths.rs

@@ -122,6 +122,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"];

src/lintlist/mod.rs

@@ -2258,6 +2258,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",

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]);
+}

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]);
+}

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
+