Auto merge of #7292 - Jarcho:suspicious_splitn, r=flip1995

Add lint `suspicious_splitn`

fixes: #7245
changelog: Add lint `suspicious_splitn`

Author: bors

CHANGELOG.md

@@ -2671,6 +2671,7 @@ Released 2018-09-13
 [`suspicious_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_map
 [`suspicious_op_assign_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_op_assign_impl
 [`suspicious_operation_groupings`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_operation_groupings
+[`suspicious_splitn`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_splitn
 [`suspicious_unary_op_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_unary_op_formatting
 [`tabs_in_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#tabs_in_doc_comments
 [`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment

clippy_lints/src/lib.rs

@@ -779,6 +779,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         methods::SKIP_WHILE_NEXT,
         methods::STRING_EXTEND_CHARS,
         methods::SUSPICIOUS_MAP,
+        methods::SUSPICIOUS_SPLITN,
         methods::UNINIT_ASSUMED_INIT,
         methods::UNNECESSARY_FILTER_MAP,
         methods::UNNECESSARY_FOLD,
@@ -1312,6 +1313,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(methods::SKIP_WHILE_NEXT),
         LintId::of(methods::STRING_EXTEND_CHARS),
         LintId::of(methods::SUSPICIOUS_MAP),
+        LintId::of(methods::SUSPICIOUS_SPLITN),
         LintId::of(methods::UNINIT_ASSUMED_INIT),
         LintId::of(methods::UNNECESSARY_FILTER_MAP),
         LintId::of(methods::UNNECESSARY_FOLD),
@@ -1688,6 +1690,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(mem_replace::MEM_REPLACE_WITH_UNINIT),
         LintId::of(methods::CLONE_DOUBLE_REF),
         LintId::of(methods::ITERATOR_STEP_BY_ZERO),
+        LintId::of(methods::SUSPICIOUS_SPLITN),
         LintId::of(methods::UNINIT_ASSUMED_INIT),
         LintId::of(methods::ZST_OFFSET),
         LintId::of(minmax::MIN_MAX),

clippy_lints/src/methods/mod.rs

@@ -48,6 +48,7 @@ mod single_char_push_string;
 mod skip_while_next;
 mod string_extend_chars;
 mod suspicious_map;
+mod suspicious_splitn;
 mod uninit_assumed_init;
 mod unnecessary_filter_map;
 mod unnecessary_fold;
@@ -1633,6 +1634,36 @@ declare_clippy_lint! {
     "replace `.iter().count()` with `.len()`"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for calls to [`splitn`]
+    /// (https://doc.rust-lang.org/std/primitive.str.html#method.splitn) and
+    /// related functions with either zero or one splits.
+    ///
+    /// **Why is this bad?** These calls don't actually split the value and are
+    /// likely to be intended as a different number.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// // Bad
+    /// let s = "";
+    /// for x in s.splitn(1, ":") {
+    ///     // use x
+    /// }
+    ///
+    /// // Good
+    /// let s = "";
+    /// for x in s.splitn(2, ":") {
+    ///     // use x
+    /// }
+    /// ```
+    pub SUSPICIOUS_SPLITN,
+    correctness,
+    "checks for `.splitn(0, ..)` and `.splitn(1, ..)`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Option<RustcVersion>,
@@ -1705,7 +1736,8 @@ impl_lint_pass!(Methods => [
     MAP_COLLECT_RESULT_UNIT,
     FROM_ITER_INSTEAD_OF_COLLECT,
     INSPECT_FOR_EACH,
-    IMPLICIT_CLONE
+    IMPLICIT_CLONE,
+    SUSPICIOUS_SPLITN
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -2024,6 +2056,9 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
                     unnecessary_lazy_eval::check(cx, expr, recv, arg, "or");
                 }
             },
+            ("splitn" | "splitn_mut" | "rsplitn" | "rsplitn_mut", [count_arg, _]) => {
+                suspicious_splitn::check(cx, name, expr, recv, count_arg);
+            },
             ("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg),
             ("to_os_string" | "to_owned" | "to_path_buf" | "to_vec", []) => {
                 implicit_clone::check(cx, name, expr, recv, span);

clippy_lints/src/methods/suspicious_splitn.rs

@@ -0,0 +1,56 @@
+use clippy_utils::consts::{constant, Constant};
+use clippy_utils::diagnostics::span_lint_and_note;
+use if_chain::if_chain;
+use rustc_ast::LitKind;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_span::source_map::Spanned;
+
+use super::SUSPICIOUS_SPLITN;
+
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    method_name: &str,
+    expr: &Expr<'_>,
+    self_arg: &Expr<'_>,
+    count_arg: &Expr<'_>,
+) {
+    if_chain! {
+        if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg);
+        if count <= 1;
+        if let Some(call_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
+        if let Some(impl_id) = cx.tcx.impl_of_method(call_id);
+        let lang_items = cx.tcx.lang_items();
+        if lang_items.slice_impl() == Some(impl_id) || lang_items.str_impl() == Some(impl_id);
+        then {
+            // Ignore empty slice and string literals when used with a literal count.
+            if (matches!(self_arg.kind, ExprKind::Array([]))
+                || matches!(self_arg.kind, ExprKind::Lit(Spanned { node: LitKind::Str(s, _), .. }) if s.is_empty())
+            ) && matches!(count_arg.kind, ExprKind::Lit(_))
+            {
+                return;
+            }
+
+            let (msg, note_msg) = if count == 0 {
+                (format!("`{}` called with `0` splits", method_name),
+                "the resulting iterator will always return `None`")
+            } else {
+                (format!("`{}` called with `1` split", method_name),
+                if lang_items.slice_impl() == Some(impl_id) {
+                    "the resulting iterator will always return the entire slice followed by `None`"
+                } else {
+                    "the resulting iterator will always return the entire string followed by `None`"
+                })
+            };
+
+            span_lint_and_note(
+                cx,
+                SUSPICIOUS_SPLITN,
+                expr.span,
+                &msg,
+                None,
+                note_msg,
+            );
+        }
+    }
+}

tests/ui/single_char_pattern.fixed

@@ -25,8 +25,8 @@ fn main() {
     x.rsplit('x');
     x.split_terminator('x');
     x.rsplit_terminator('x');
-    x.splitn(0, 'x');
-    x.rsplitn(0, 'x');
+    x.splitn(2, 'x');
+    x.rsplitn(2, 'x');
     x.matches('x');
     x.rmatches('x');
     x.match_indices('x');

tests/ui/single_char_pattern.rs

@@ -25,8 +25,8 @@ fn main() {
     x.rsplit("x");
     x.split_terminator("x");
     x.rsplit_terminator("x");
-    x.splitn(0, "x");
-    x.rsplitn(0, "x");
+    x.splitn(2, "x");
+    x.rsplitn(2, "x");
     x.matches("x");
     x.rmatches("x");
     x.match_indices("x");

tests/ui/single_char_pattern.stderr

@@ -75,13 +75,13 @@ LL |     x.rsplit_terminator("x");
 error: single-character string constant used as pattern
   --> $DIR/single_char_pattern.rs:28:17
    |
-LL |     x.splitn(0, "x");
+LL |     x.splitn(2, "x");
    |                 ^^^ help: try using a `char` instead: `'x'`
 
 error: single-character string constant used as pattern
   --> $DIR/single_char_pattern.rs:29:18
    |
-LL |     x.rsplitn(0, "x");
+LL |     x.rsplitn(2, "x");
    |                  ^^^ help: try using a `char` instead: `'x'`
 
 error: single-character string constant used as pattern

tests/ui/suspicious_splitn.rs

@@ -0,0 +1,20 @@
+#![warn(clippy::suspicious_splitn)]
+
+fn main() {
+    let _ = "a,b,c".splitn(3, ',');
+    let _ = [0, 1, 2, 1, 3].splitn(3, |&x| x == 1);
+    let _ = "".splitn(0, ',');
+    let _ = [].splitn(0, |&x: &u32| x == 1);
+
+    let _ = "a,b".splitn(0, ',');
+    let _ = "a,b".rsplitn(0, ',');
+    let _ = "a,b".splitn(1, ',');
+    let _ = [0, 1, 2].splitn(0, |&x| x == 1);
+    let _ = [0, 1, 2].splitn_mut(0, |&x| x == 1);
+    let _ = [0, 1, 2].splitn(1, |&x| x == 1);
+    let _ = [0, 1, 2].rsplitn_mut(1, |&x| x == 1);
+
+    const X: usize = 0;
+    let _ = "a,b".splitn(X + 1, ',');
+    let _ = "a,b".splitn(X, ',');
+}

tests/ui/suspicious_splitn.stderr

@@ -0,0 +1,75 @@
+error: `splitn` called with `0` splits
+  --> $DIR/suspicious_splitn.rs:9:13
+   |
+LL |     let _ = "a,b".splitn(0, ',');
+   |             ^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::suspicious-splitn` implied by `-D warnings`
+   = note: the resulting iterator will always return `None`
+
+error: `rsplitn` called with `0` splits
+  --> $DIR/suspicious_splitn.rs:10:13
+   |
+LL |     let _ = "a,b".rsplitn(0, ',');
+   |             ^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the resulting iterator will always return `None`
+
+error: `splitn` called with `1` split
+  --> $DIR/suspicious_splitn.rs:11:13
+   |
+LL |     let _ = "a,b".splitn(1, ',');
+   |             ^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the resulting iterator will always return the entire string followed by `None`
+
+error: `splitn` called with `0` splits
+  --> $DIR/suspicious_splitn.rs:12:13
+   |
+LL |     let _ = [0, 1, 2].splitn(0, |&x| x == 1);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the resulting iterator will always return `None`
+
+error: `splitn_mut` called with `0` splits
+  --> $DIR/suspicious_splitn.rs:13:13
+   |
+LL |     let _ = [0, 1, 2].splitn_mut(0, |&x| x == 1);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the resulting iterator will always return `None`
+
+error: `splitn` called with `1` split
+  --> $DIR/suspicious_splitn.rs:14:13
+   |
+LL |     let _ = [0, 1, 2].splitn(1, |&x| x == 1);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the resulting iterator will always return the entire slice followed by `None`
+
+error: `rsplitn_mut` called with `1` split
+  --> $DIR/suspicious_splitn.rs:15:13
+   |
+LL |     let _ = [0, 1, 2].rsplitn_mut(1, |&x| x == 1);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the resulting iterator will always return the entire slice followed by `None`
+
+error: `splitn` called with `1` split
+  --> $DIR/suspicious_splitn.rs:18:13
+   |
+LL |     let _ = "a,b".splitn(X + 1, ',');
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the resulting iterator will always return the entire string followed by `None`
+
+error: `splitn` called with `0` splits
+  --> $DIR/suspicious_splitn.rs:19:13
+   |
+LL |     let _ = "a,b".splitn(X, ',');
+   |             ^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the resulting iterator will always return `None`
+
+error: aborting due to 9 previous errors
+