about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJason Newcomb <jsnewcomb@pm.me>2021-05-30 13:25:24 -0400
committerJason Newcomb <jsnewcomb@pm.me>2021-05-30 13:25:24 -0400
commit5fa08eaf53f0c895e73f4841c240389fda951554 (patch)
tree8a7ce90e5477ccecbc23e831c6aefb1ff720ba68
parent898b6a0e072c4f8ba741e83bd7fd12a4c2ccb1a2 (diff)
downloadrust-5fa08eaf53f0c895e73f4841c240389fda951554.tar.gz
rust-5fa08eaf53f0c895e73f4841c240389fda951554.zip
Evaluate constant expressions in `suspicious_splitn`
-rw-r--r--clippy_lints/src/methods/mod.rs5
-rw-r--r--clippy_lints/src/methods/suspicious_splitn.rs16
-rw-r--r--tests/ui/suspicious_splitn.rs4
-rw-r--r--tests/ui/suspicious_splitn.stderr18
4 files changed, 34 insertions, 9 deletions
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 3904572c627..a6e2e0baadb 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -1635,8 +1635,9 @@ declare_clippy_lint! {
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for calls to `splitn` and related functions with
-    /// either zero or one splits.
+    /// **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.
diff --git a/clippy_lints/src/methods/suspicious_splitn.rs b/clippy_lints/src/methods/suspicious_splitn.rs
index 43affe20dc5..a271df60572 100644
--- a/clippy_lints/src/methods/suspicious_splitn.rs
+++ b/clippy_lints/src/methods/suspicious_splitn.rs
@@ -1,3 +1,4 @@
+use clippy_utils::consts::{constant, Constant};
 use clippy_utils::diagnostics::span_lint_and_note;
 use if_chain::if_chain;
 use rustc_ast::LitKind;
@@ -15,18 +16,21 @@ pub(super) fn check(
     count_arg: &Expr<'_>,
 ) {
     if_chain! {
-        // Ignore empty slice literal
-        if !matches!(self_arg.kind, ExprKind::Array([]));
-        // Ignore empty string literal
-        if !matches!(self_arg.kind, ExprKind::Lit(Spanned { node: LitKind::Str(s, _), .. }) if s.is_empty());
-        if let ExprKind::Lit(count_lit) = &count_arg.kind;
-        if let LitKind::Int(count, _) = count_lit.node;
+        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`")
diff --git a/tests/ui/suspicious_splitn.rs b/tests/ui/suspicious_splitn.rs
index a944aa79cfe..a21d94cf20b 100644
--- a/tests/ui/suspicious_splitn.rs
+++ b/tests/ui/suspicious_splitn.rs
@@ -13,4 +13,8 @@ fn main() {
     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, ',');
 }
diff --git a/tests/ui/suspicious_splitn.stderr b/tests/ui/suspicious_splitn.stderr
index 2e4bfe93b94..b6220ae2393 100644
--- a/tests/ui/suspicious_splitn.stderr
+++ b/tests/ui/suspicious_splitn.stderr
@@ -55,5 +55,21 @@ 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: aborting due to 7 previous errors
+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