about summary refs log tree commit diff
diff options
context:
space:
mode:
authorGuillaume Gomez <guillaume1.gomez@gmail.com>2023-11-25 22:35:39 +0100
committerGuillaume Gomez <guillaume1.gomez@gmail.com>2023-12-18 16:46:41 +0100
commit71ea36b5391be5e19ae31e8d576fe7b31f6c8760 (patch)
tree2bce3b40b220bbbd3215b9b90c7090382080893c
parent7e650b761006b9dfb5b26a05c2f725a06e9cb14d (diff)
Extend `UNNECESSARY_TO_OWNED` to handle `split`
-rw-r--r--clippy_lints/src/methods/unnecessary_to_owned.rs56
1 files changed, 56 insertions, 0 deletions
diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs
index c4775b6bd04..637368e9361 100644
--- a/clippy_lints/src/methods/unnecessary_to_owned.rs
+++ b/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>(