Auto merge of #11871 - GuillaumeGomez:UNNECESSARY_TO_OWNED-split, r=llogiq

Extend `UNNECESSARY_TO_OWNED` to handle `split`

Fixes #9965.

When you have `to_string().split('a')` or equivalent, it'll suggest to remove the `to_owned`/`to_string` part.

r? `@flip1995`

changelog: Extend `UNNECESSARY_TO_OWNED` to handle `split`

Author: bors

clippy_lints/src/methods/unnecessary_to_owned.rs

@@ -7,6 +7,7 @@ use clippy_utils::ty::{get_iterator_item_ty, implements_trait, is_copy, peel_mid
 use clippy_utils::visitors::find_all_ret_expressions;
 use clippy_utils::{fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item, return_ty};
 use rustc_errors::Applicability;
+use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::DefId;
 use rustc_hir::{BorrowKind, Expr, ExprKind, ItemKind, Node};
 use rustc_hir_typeck::{FnCtxt, Inherited};
@@ -37,6 +38,9 @@ pub fn check<'tcx>(
         if is_cloned_or_copied(cx, method_name, method_def_id) {
             unnecessary_iter_cloned::check(cx, expr, method_name, receiver);
         } else if is_to_owned_like(cx, expr, method_name, method_def_id) {
+            if check_split_call_arg(cx, expr, method_name, receiver) {
+                return;
+            }
             // At this point, we know the call is of a `to_owned`-like function. The functions
             // `check_addr_of_expr` and `check_call_arg` determine whether the call is unnecessary
             // based on its context, that is, whether it is a referent in an `AddrOf` expression, an
@@ -233,6 +237,58 @@ fn check_into_iter_call_arg(
     false
 }
 
+/// Checks whether `expr` is an argument in an `into_iter` call and, if so, determines whether its
+/// call of a `to_owned`-like function is unnecessary.
+fn check_split_call_arg(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: Symbol, receiver: &Expr<'_>) -> bool {
+    if let Some(parent) = get_parent_expr(cx, expr)
+        && let Some((fn_name, argument_expr)) = get_fn_name_and_arg(cx, parent)
+        && fn_name.as_str() == "split"
+        && let Some(receiver_snippet) = snippet_opt(cx, receiver.span)
+        && let Some(arg_snippet) = snippet_opt(cx, argument_expr.span)
+    {
+        // The next suggestion may be incorrect because the removal of the `to_owned`-like
+        // function could cause the iterator to hold a reference to a resource that is used
+        // mutably. See https://github.com/rust-lang/rust-clippy/issues/8148.
+        span_lint_and_sugg(
+            cx,
+            UNNECESSARY_TO_OWNED,
+            parent.span,
+            &format!("unnecessary use of `{method_name}`"),
+            "use",
+            format!("{receiver_snippet}.split({arg_snippet})"),
+            Applicability::MaybeIncorrect,
+        );
+        return true;
+    }
+    false
+}
+
+fn get_fn_name_and_arg<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<(Symbol, Expr<'tcx>)> {
+    match &expr.kind {
+        ExprKind::MethodCall(path, _, [arg_expr], ..) => Some((path.ident.name, *arg_expr)),
+        ExprKind::Call(
+            Expr {
+                kind: ExprKind::Path(qpath),
+                hir_id: path_hir_id,
+                ..
+            },
+            [arg_expr],
+        ) => {
+            // Only return Fn-like DefIds, not the DefIds of statics/consts/etc that contain or
+            // deref to fn pointers, dyn Fn, impl Fn - #8850
+            if let Res::Def(DefKind::Fn | DefKind::Ctor(..) | DefKind::AssocFn, def_id) =
+                cx.typeck_results().qpath_res(qpath, *path_hir_id)
+                && let Some(fn_name) = cx.tcx.opt_item_name(def_id)
+            {
+                Some((fn_name, *arg_expr))
+            } else {
+                None
+            }
+        },
+        _ => None,
+    }
+}
+
 /// Checks whether `expr` is an argument in a function call and, if so, determines whether its call
 /// of a `to_owned`-like function is unnecessary.
 fn check_other_call_arg<'tcx>(

tests/ui/unnecessary_to_owned_on_split.fixed

@@ -0,0 +1,21 @@
+#[allow(clippy::single_char_pattern)]
+
+fn main() {
+    let _ = "a".split('a').next().unwrap();
+    //~^ ERROR: unnecessary use of `to_string`
+    let _ = "a".split("a").next().unwrap();
+    //~^ ERROR: unnecessary use of `to_string`
+    let _ = "a".split('a').next().unwrap();
+    //~^ ERROR: unnecessary use of `to_owned`
+    let _ = "a".split("a").next().unwrap();
+    //~^ ERROR: unnecessary use of `to_owned`
+
+    let _ = [1].split(|x| *x == 2).next().unwrap();
+    //~^ ERROR: unnecessary use of `to_vec`
+    let _ = [1].split(|x| *x == 2).next().unwrap();
+    //~^ ERROR: unnecessary use of `to_vec`
+    let _ = [1].split(|x| *x == 2).next().unwrap();
+    //~^ ERROR: unnecessary use of `to_owned`
+    let _ = [1].split(|x| *x == 2).next().unwrap();
+    //~^ ERROR: unnecessary use of `to_owned`
+}

tests/ui/unnecessary_to_owned_on_split.rs

@@ -0,0 +1,21 @@
+#[allow(clippy::single_char_pattern)]
+
+fn main() {
+    let _ = "a".to_string().split('a').next().unwrap();
+    //~^ ERROR: unnecessary use of `to_string`
+    let _ = "a".to_string().split("a").next().unwrap();
+    //~^ ERROR: unnecessary use of `to_string`
+    let _ = "a".to_owned().split('a').next().unwrap();
+    //~^ ERROR: unnecessary use of `to_owned`
+    let _ = "a".to_owned().split("a").next().unwrap();
+    //~^ ERROR: unnecessary use of `to_owned`
+
+    let _ = [1].to_vec().split(|x| *x == 2).next().unwrap();
+    //~^ ERROR: unnecessary use of `to_vec`
+    let _ = [1].to_vec().split(|x| *x == 2).next().unwrap();
+    //~^ ERROR: unnecessary use of `to_vec`
+    let _ = [1].to_owned().split(|x| *x == 2).next().unwrap();
+    //~^ ERROR: unnecessary use of `to_owned`
+    let _ = [1].to_owned().split(|x| *x == 2).next().unwrap();
+    //~^ ERROR: unnecessary use of `to_owned`
+}

tests/ui/unnecessary_to_owned_on_split.stderr

@@ -0,0 +1,53 @@
+error: unnecessary use of `to_string`
+  --> $DIR/unnecessary_to_owned_on_split.rs:4:13
+   |
+LL |     let _ = "a".to_string().split('a').next().unwrap();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `"a".split('a')`
+   |
+   = note: `-D clippy::unnecessary-to-owned` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::unnecessary_to_owned)]`
+
+error: unnecessary use of `to_string`
+  --> $DIR/unnecessary_to_owned_on_split.rs:6:13
+   |
+LL |     let _ = "a".to_string().split("a").next().unwrap();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `"a".split("a")`
+
+error: unnecessary use of `to_owned`
+  --> $DIR/unnecessary_to_owned_on_split.rs:8:13
+   |
+LL |     let _ = "a".to_owned().split('a').next().unwrap();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `"a".split('a')`
+
+error: unnecessary use of `to_owned`
+  --> $DIR/unnecessary_to_owned_on_split.rs:10:13
+   |
+LL |     let _ = "a".to_owned().split("a").next().unwrap();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `"a".split("a")`
+
+error: unnecessary use of `to_vec`
+  --> $DIR/unnecessary_to_owned_on_split.rs:13:13
+   |
+LL |     let _ = [1].to_vec().split(|x| *x == 2).next().unwrap();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[1].split(|x| *x == 2)`
+
+error: unnecessary use of `to_vec`
+  --> $DIR/unnecessary_to_owned_on_split.rs:15:13
+   |
+LL |     let _ = [1].to_vec().split(|x| *x == 2).next().unwrap();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[1].split(|x| *x == 2)`
+
+error: unnecessary use of `to_owned`
+  --> $DIR/unnecessary_to_owned_on_split.rs:17:13
+   |
+LL |     let _ = [1].to_owned().split(|x| *x == 2).next().unwrap();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[1].split(|x| *x == 2)`
+
+error: unnecessary use of `to_owned`
+  --> $DIR/unnecessary_to_owned_on_split.rs:19:13
+   |
+LL |     let _ = [1].to_owned().split(|x| *x == 2).next().unwrap();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[1].split(|x| *x == 2)`
+
+error: aborting due to 8 previous errors
+