From e75841397365ff207553551f14ab35122774f23c Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Thu, 4 Jan 2024 17:13:18 +0100 Subject: [PATCH] Add .as_ref() to suggestion to remove .to_string() --- .../src/methods/unnecessary_to_owned.rs | 19 ++++++++++++--- tests/ui/unnecessary_to_owned_on_split.fixed | 18 +++++++++++++- tests/ui/unnecessary_to_owned_on_split.rs | 18 +++++++++++++- tests/ui/unnecessary_to_owned_on_split.stderr | 24 ++++++++++++------- 4 files changed, 65 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs index 5f69cf6cd7ad..12106d44e365 100644 --- a/clippy_lints/src/methods/unnecessary_to_owned.rs +++ b/clippy_lints/src/methods/unnecessary_to_owned.rs @@ -3,13 +3,13 @@ use super::unnecessary_iter_cloned::{self, is_into_iter}; use clippy_config::msrvs::{self, Msrv}; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_opt; -use clippy_utils::ty::{get_iterator_item_ty, implements_trait, is_copy, peel_mid_ty_refs}; +use clippy_utils::ty::{get_iterator_item_ty, implements_trait, is_copy, is_type_lang_item, peel_mid_ty_refs}; 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::{BorrowKind, Expr, ExprKind, ItemKind, LangItem, Node}; use rustc_hir_typeck::{FnCtxt, Inherited}; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::LateContext; @@ -246,6 +246,19 @@ fn check_split_call_arg(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: Symb && let Some(receiver_snippet) = snippet_opt(cx, receiver.span) && let Some(arg_snippet) = snippet_opt(cx, argument_expr.span) { + // We may end-up here because of an expression like `x.to_string().split(…)` where the type of `x` + // implements `AsRef` but does not implement `Deref`. In this case, we have to + // add `.as_ref()` to the suggestion. + let as_ref = if is_type_lang_item(cx, cx.typeck_results().expr_ty(expr), LangItem::String) + && let Some(deref_trait_id) = cx.tcx.get_diagnostic_item(sym::Deref) + && cx.get_associated_type(cx.typeck_results().expr_ty(receiver), deref_trait_id, "Target") + != Some(cx.tcx.types.str_) + { + ".as_ref()" + } else { + "" + }; + // 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. @@ -255,7 +268,7 @@ fn check_split_call_arg(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: Symb parent.span, &format!("unnecessary use of `{method_name}`"), "use", - format!("{receiver_snippet}.split({arg_snippet})"), + format!("{receiver_snippet}{as_ref}.split({arg_snippet})"), Applicability::MaybeIncorrect, ); return true; diff --git a/tests/ui/unnecessary_to_owned_on_split.fixed b/tests/ui/unnecessary_to_owned_on_split.fixed index 53dc3c43e2f8..f87c898f9b79 100644 --- a/tests/ui/unnecessary_to_owned_on_split.fixed +++ b/tests/ui/unnecessary_to_owned_on_split.fixed @@ -1,4 +1,18 @@ -#[allow(clippy::single_char_pattern)] +#![allow(clippy::single_char_pattern)] + +struct Issue12068; + +impl AsRef for Issue12068 { + fn as_ref(&self) -> &str { + "" + } +} + +impl ToString for Issue12068 { + fn to_string(&self) -> String { + String::new() + } +} fn main() { let _ = "a".split('a').next().unwrap(); @@ -9,6 +23,8 @@ fn main() { //~^ ERROR: unnecessary use of `to_owned` let _ = "a".split("a").next().unwrap(); //~^ ERROR: unnecessary use of `to_owned` + let _ = Issue12068.as_ref().split('a').next().unwrap(); + //~^ ERROR: unnecessary use of `to_string` let _ = [1].split(|x| *x == 2).next().unwrap(); //~^ ERROR: unnecessary use of `to_vec` diff --git a/tests/ui/unnecessary_to_owned_on_split.rs b/tests/ui/unnecessary_to_owned_on_split.rs index 62400e7eee17..db5719e58809 100644 --- a/tests/ui/unnecessary_to_owned_on_split.rs +++ b/tests/ui/unnecessary_to_owned_on_split.rs @@ -1,4 +1,18 @@ -#[allow(clippy::single_char_pattern)] +#![allow(clippy::single_char_pattern)] + +struct Issue12068; + +impl AsRef for Issue12068 { + fn as_ref(&self) -> &str { + "" + } +} + +impl ToString for Issue12068 { + fn to_string(&self) -> String { + String::new() + } +} fn main() { let _ = "a".to_string().split('a').next().unwrap(); @@ -9,6 +23,8 @@ fn main() { //~^ ERROR: unnecessary use of `to_owned` let _ = "a".to_owned().split("a").next().unwrap(); //~^ ERROR: unnecessary use of `to_owned` + let _ = Issue12068.to_string().split('a').next().unwrap(); + //~^ ERROR: unnecessary use of `to_string` let _ = [1].to_vec().split(|x| *x == 2).next().unwrap(); //~^ ERROR: unnecessary use of `to_vec` diff --git a/tests/ui/unnecessary_to_owned_on_split.stderr b/tests/ui/unnecessary_to_owned_on_split.stderr index cfb3766d15ed..4cfaeed3384a 100644 --- a/tests/ui/unnecessary_to_owned_on_split.stderr +++ b/tests/ui/unnecessary_to_owned_on_split.stderr @@ -1,5 +1,5 @@ error: unnecessary use of `to_string` - --> $DIR/unnecessary_to_owned_on_split.rs:4:13 + --> $DIR/unnecessary_to_owned_on_split.rs:18:13 | LL | let _ = "a".to_string().split('a').next().unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `"a".split('a')` @@ -8,46 +8,52 @@ LL | let _ = "a".to_string().split('a').next().unwrap(); = 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 + --> $DIR/unnecessary_to_owned_on_split.rs:20: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 + --> $DIR/unnecessary_to_owned_on_split.rs:22: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 + --> $DIR/unnecessary_to_owned_on_split.rs:24:13 | LL | let _ = "a".to_owned().split("a").next().unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `"a".split("a")` +error: unnecessary use of `to_string` + --> $DIR/unnecessary_to_owned_on_split.rs:26:13 + | +LL | let _ = Issue12068.to_string().split('a').next().unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Issue12068.as_ref().split('a')` + error: unnecessary use of `to_vec` - --> $DIR/unnecessary_to_owned_on_split.rs:13:13 + --> $DIR/unnecessary_to_owned_on_split.rs:29: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 + --> $DIR/unnecessary_to_owned_on_split.rs:31: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 + --> $DIR/unnecessary_to_owned_on_split.rs:33: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 + --> $DIR/unnecessary_to_owned_on_split.rs:35: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 +error: aborting due to 9 previous errors