about summary refs log tree commit diff
diff options
context:
space:
mode:
authorPatrick José Pereira <patrickelectric@gmail.com>2020-10-08 00:19:56 -0300
committerPatrick José Pereira <patrickelectric@gmail.com>2020-11-04 21:17:46 -0300
commitbc27d1492d60ecfb0673629e28bc4bbe0b7fd886 (patch)
tree3636989d25dd14aa4220952af1cca73f5abf2612
parente298c830af37e48a7778b3e6a487a2107230e8a6 (diff)
downloadrust-bc27d1492d60ecfb0673629e28bc4bbe0b7fd886.tar.gz
rust-bc27d1492d60ecfb0673629e28bc4bbe0b7fd886.zip
Add string_from_utf8_as_bytes linter
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/lib.rs3
-rw-r--r--clippy_lints/src/strings.rs68
-rw-r--r--clippy_lints/src/utils/paths.rs1
-rw-r--r--src/lintlist/mod.rs7
-rw-r--r--tests/ui/string_from_utf8_as_bytes.fixed6
-rw-r--r--tests/ui/string_from_utf8_as_bytes.rs6
-rw-r--r--tests/ui/string_from_utf8_as_bytes.stderr10
8 files changed, 99 insertions, 3 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6b63dbb7eff..aeccf6cf83b 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 5c3af014ee1..d29ba33064c 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 3783bd78de2..a55df1e5502 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,10 +176,30 @@ 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<'_>) {
@@ -184,6 +207,45 @@ impl<'tcx> LateLintPass<'tcx> for StringLitAsBytes {
         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);
             if let ExprKind::Lit(lit) = &args[0].kind;
diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs
index cd72fdd61fd..0d0ecf9ae9f 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", "<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"];
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index 016bda77ef5..4d824cf94be 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -2231,6 +2231,13 @@ vec![
         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",
         desc: "calling `as_bytes` on a string literal instead of using a byte string literal",
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 00000000000..6e665cdd563
--- /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 00000000000..670d206d367
--- /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 00000000000..bf5e5d33e8f
--- /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
+