Deprecate string_to_string

Author: smoelius

clippy_lints/src/declared_lints.rs

@@ -683,7 +683,6 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::strings::STRING_FROM_UTF8_AS_BYTES_INFO,
     crate::strings::STRING_LIT_AS_BYTES_INFO,
     crate::strings::STRING_SLICE_INFO,
-    crate::strings::STRING_TO_STRING_INFO,
     crate::strings::STR_TO_STRING_INFO,
     crate::strings::TRIM_SPLIT_WHITESPACE_INFO,
     crate::strlen_on_c_strings::STRLEN_ON_C_STRINGS_INFO,

clippy_lints/src/deprecated_lints.rs

@@ -34,6 +34,8 @@ declare_with_version! { DEPRECATED(DEPRECATED_VERSION) = [
     ("clippy::replace_consts", "`min_value` and `max_value` are now deprecated"),
     #[clippy::version = "pre 1.29.0"]
     ("clippy::should_assert_eq", "`assert!(a == b)` can now print the values the same way `assert_eq!(a, b) can"),
+    #[clippy::version = "1.87.0"]
+    ("clippy::string_to_string", "`clippy:implicit_clone` and `clippy:map_clone` cover those cases"),
     #[clippy::version = "pre 1.29.0"]
     ("clippy::unsafe_vector_initialization", "the suggested alternative could be substantially slower"),
     #[clippy::version = "pre 1.29.0"]

clippy_lints/src/lib.rs

@@ -782,7 +782,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_early_pass(|| Box::new(asm_syntax::InlineAsmX86IntelSyntax));
     store.register_late_pass(|_| Box::new(empty_drop::EmptyDrop));
     store.register_late_pass(|_| Box::new(strings::StrToString));
-    store.register_late_pass(|_| Box::new(strings::StringToString));
     store.register_late_pass(|_| Box::new(zero_sized_map_values::ZeroSizedMapValues));
     store.register_late_pass(|_| Box::<vec_init_then_push::VecInitThenPush>::default());
     store.register_late_pass(|_| Box::new(redundant_slicing::RedundantSlicing));

clippy_lints/src/strings.rs

@@ -13,8 +13,6 @@ use rustc_middle::ty;
 use rustc_session::declare_lint_pass;
 use rustc_span::source_map::Spanned;
 
-use std::ops::ControlFlow;
-
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for string appends of the form `x = x + y` (without
@@ -411,113 +409,6 @@ impl<'tcx> LateLintPass<'tcx> for StrToString {
     }
 }
 
-declare_clippy_lint! {
-    /// ### What it does
-    /// This lint checks for `.to_string()` method calls on values of type `String`.
-    ///
-    /// ### Why restrict this?
-    /// The `to_string` method is also used on other types to convert them to a string.
-    /// When called on a `String` it only clones the `String`, which can be more specifically
-    /// expressed with `.clone()`.
-    ///
-    /// ### Example
-    /// ```no_run
-    /// let msg = String::from("Hello World");
-    /// let _ = msg.to_string();
-    /// ```
-    /// Use instead:
-    /// ```no_run
-    /// let msg = String::from("Hello World");
-    /// let _ = msg.clone();
-    /// ```
-    #[clippy::version = "pre 1.29.0"]
-    pub STRING_TO_STRING,
-    restriction,
-    "using `to_string()` on a `String`, which should be `clone()`"
-}
-
-declare_lint_pass!(StringToString => [STRING_TO_STRING]);
-
-fn is_parent_map_like(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<rustc_span::Span> {
-    if let Some(parent_expr) = get_parent_expr(cx, expr)
-        && let ExprKind::MethodCall(name, _, _, parent_span) = parent_expr.kind
-        && name.ident.name == sym::map
-        && let Some(caller_def_id) = cx.typeck_results().type_dependent_def_id(parent_expr.hir_id)
-        && (clippy_utils::is_diag_item_method(cx, caller_def_id, sym::Result)
-            || clippy_utils::is_diag_item_method(cx, caller_def_id, sym::Option)
-            || clippy_utils::is_diag_trait_item(cx, caller_def_id, sym::Iterator))
-    {
-        Some(parent_span)
-    } else {
-        None
-    }
-}
-
-fn is_called_from_map_like(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<rustc_span::Span> {
-    // Look for a closure as parent of `expr`, discarding simple blocks
-    let parent_closure = cx
-        .tcx
-        .hir_parent_iter(expr.hir_id)
-        .try_fold(expr.hir_id, |child_hir_id, (_, node)| match node {
-            // Check that the child expression is the only expression in the block
-            Node::Block(block) if block.stmts.is_empty() && block.expr.map(|e| e.hir_id) == Some(child_hir_id) => {
-                ControlFlow::Continue(block.hir_id)
-            },
-            Node::Expr(expr) if matches!(expr.kind, ExprKind::Block(..)) => ControlFlow::Continue(expr.hir_id),
-            Node::Expr(expr) if matches!(expr.kind, ExprKind::Closure(_)) => ControlFlow::Break(Some(expr)),
-            _ => ControlFlow::Break(None),
-        })
-        .break_value()?;
-    is_parent_map_like(cx, parent_closure?)
-}
-
-fn suggest_cloned_string_to_string(cx: &LateContext<'_>, span: rustc_span::Span) {
-    span_lint_and_sugg(
-        cx,
-        STRING_TO_STRING,
-        span,
-        "`to_string()` called on a `String`",
-        "try",
-        "cloned()".to_string(),
-        Applicability::MachineApplicable,
-    );
-}
-
-impl<'tcx> LateLintPass<'tcx> for StringToString {
-    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) {
-        if expr.span.from_expansion() {
-            return;
-        }
-
-        match &expr.kind {
-            ExprKind::MethodCall(path, self_arg, [], _) => {
-                if path.ident.name == sym::to_string
-                    && let ty = cx.typeck_results().expr_ty(self_arg)
-                    && is_type_lang_item(cx, ty.peel_refs(), LangItem::String)
-                    && let Some(parent_span) = is_called_from_map_like(cx, expr)
-                {
-                    suggest_cloned_string_to_string(cx, parent_span);
-                }
-            },
-            ExprKind::Path(QPath::TypeRelative(ty, segment)) => {
-                if segment.ident.name == sym::to_string
-                    && let rustc_hir::TyKind::Path(QPath::Resolved(_, path)) = ty.peel_refs().kind
-                    && let rustc_hir::def::Res::Def(_, def_id) = path.res
-                    && cx
-                        .tcx
-                        .lang_items()
-                        .get(LangItem::String)
-                        .is_some_and(|lang_id| lang_id == def_id)
-                    && let Some(parent_span) = is_parent_map_like(cx, expr)
-                {
-                    suggest_cloned_string_to_string(cx, parent_span);
-                }
-            },
-            _ => {},
-        }
-    }
-}
-
 declare_clippy_lint! {
     /// ### What it does
     /// Warns about calling `str::trim` (or variants) before `str::split_whitespace`.

tests/ui/deprecated.rs

@@ -12,6 +12,7 @@
 #![warn(clippy::regex_macro)] //~ ERROR: lint `clippy::regex_macro`
 #![warn(clippy::replace_consts)] //~ ERROR: lint `clippy::replace_consts`
 #![warn(clippy::should_assert_eq)] //~ ERROR: lint `clippy::should_assert_eq`
+#![warn(clippy::string_to_string)] //~ ERROR: lint `clippy::string_to_string`
 #![warn(clippy::unsafe_vector_initialization)] //~ ERROR: lint `clippy::unsafe_vector_initialization`
 #![warn(clippy::unstable_as_mut_slice)] //~ ERROR: lint `clippy::unstable_as_mut_slice`
 #![warn(clippy::unstable_as_slice)] //~ ERROR: lint `clippy::unstable_as_slice`

tests/ui/deprecated.stderr

@@ -61,35 +61,41 @@ error: lint `clippy::should_assert_eq` has been removed: `assert!(a == b)` can n
 LL | #![warn(clippy::should_assert_eq)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: lint `clippy::unsafe_vector_initialization` has been removed: the suggested alternative could be substantially slower
+error: lint `clippy::string_to_string` has been removed: `clippy:implicit_clone` and `clippy:map_clone` cover those cases
   --> tests/ui/deprecated.rs:15:9
    |
+LL | #![warn(clippy::string_to_string)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: lint `clippy::unsafe_vector_initialization` has been removed: the suggested alternative could be substantially slower
+  --> tests/ui/deprecated.rs:16:9
+   |
 LL | #![warn(clippy::unsafe_vector_initialization)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: lint `clippy::unstable_as_mut_slice` has been removed: `Vec::as_mut_slice` is now stable
-  --> tests/ui/deprecated.rs:16:9
+  --> tests/ui/deprecated.rs:17:9
    |
 LL | #![warn(clippy::unstable_as_mut_slice)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: lint `clippy::unstable_as_slice` has been removed: `Vec::as_slice` is now stable
-  --> tests/ui/deprecated.rs:17:9
+  --> tests/ui/deprecated.rs:18:9
    |
 LL | #![warn(clippy::unstable_as_slice)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: lint `clippy::unused_collect` has been removed: `Iterator::collect` is now marked as `#[must_use]`
-  --> tests/ui/deprecated.rs:18:9
+  --> tests/ui/deprecated.rs:19:9
    |
 LL | #![warn(clippy::unused_collect)]
    |         ^^^^^^^^^^^^^^^^^^^^^^
 
 error: lint `clippy::wrong_pub_self_convention` has been removed: `clippy::wrong_self_convention` now covers this case via the `avoid-breaking-exported-api` config
-  --> tests/ui/deprecated.rs:19:9
+  --> tests/ui/deprecated.rs:20:9
    |
 LL | #![warn(clippy::wrong_pub_self_convention)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 15 previous errors
+error: aborting due to 16 previous errors