From 3af383b4eda8961250521facbd33afca20fe414a Mon Sep 17 00:00:00 2001 From: Aedan Kearns Date: Fri, 14 Nov 2025 19:34:18 -0500 Subject: [PATCH 1/4] Add lint `clones_into_boxed_slices` --- CHANGELOG.md | 1 + clippy_lints/src/clones_into_boxed_slices.rs | 204 +++++++++++++++++++ clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_utils/src/sym.rs | 5 + tests/ui/clones_into_boxed_slices.fixed | 126 ++++++++++++ tests/ui/clones_into_boxed_slices.rs | 126 ++++++++++++ tests/ui/clones_into_boxed_slices.stderr | 178 ++++++++++++++++ tests/ui/crashes/ice-3969.rs | 2 +- tests/ui/unnecessary_box_returns.rs | 2 +- 10 files changed, 645 insertions(+), 2 deletions(-) create mode 100644 clippy_lints/src/clones_into_boxed_slices.rs create mode 100644 tests/ui/clones_into_boxed_slices.fixed create mode 100644 tests/ui/clones_into_boxed_slices.rs create mode 100644 tests/ui/clones_into_boxed_slices.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 78b81b5b74d6..8ecd779a8a8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6258,6 +6258,7 @@ Released 2018-09-13 [`clone_on_ref_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_ref_ptr [`cloned_instead_of_copied`]: https://rust-lang.github.io/rust-clippy/master/index.html#cloned_instead_of_copied [`cloned_ref_to_slice_refs`]: https://rust-lang.github.io/rust-clippy/master/index.html#cloned_ref_to_slice_refs +[`clones_into_boxed_slices`]: https://rust-lang.github.io/rust-clippy/master/index.html#clones_into_boxed_slices [`cmp_nan`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_nan [`cmp_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_null [`cmp_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_owned diff --git a/clippy_lints/src/clones_into_boxed_slices.rs b/clippy_lints/src/clones_into_boxed_slices.rs new file mode 100644 index 000000000000..f60c37a4b46b --- /dev/null +++ b/clippy_lints/src/clones_into_boxed_slices.rs @@ -0,0 +1,204 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::res::MaybeDef; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::sugg::Sugg; +use clippy_utils::sym; +use rustc_ast::{BorrowKind, UnOp}; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, LangItem, QPath}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{self, Ty}; +use rustc_session::declare_lint_pass; +use rustc_span::Span; + +declare_clippy_lint! { + /// ### What it does + /// Checks for clones that are immediately converted into boxed slices instead of using `Box::from(...)`. + /// + /// ### Why is this bad? + /// Using `Box::from(...)` is more concise and avoids creating an unnecessary temporary object. + /// + /// ### Example + /// ```no_run + /// "example".to_string().to_boxed_str() + /// ``` + /// Use instead: + /// ```no_run + /// Box::from("example") + /// ``` + #[clippy::version = "1.93.0"] + pub CLONES_INTO_BOXED_SLICES, + perf, + "Cloning then converting into boxed slice instead of using Box::from" +} +declare_lint_pass!(ClonesIntoBoxedSlices => [CLONES_INTO_BOXED_SLICES]); + +fn count_refs(mut expr_ty: Ty<'_>) -> i64 { + let mut count = 0; + while let ty::Ref(_, inner, _) = expr_ty.kind() { + expr_ty = *inner; + count += 1; + } + count +} + +// Shows the lint with a suggestion using the given parts +// Assures that the inner argument is correctly ref'd/deref'd in the suggestion based on needs_ref +fn show_lint( + cx: &LateContext<'_>, + full_span: Span, + mut inner: &Expr<'_>, + needs_ref: bool, + sugg_prefix: Option<&str>, + placeholder: &str, + sugg_suffix: Option<&str>, +) { + let mut applicability = Applicability::MachineApplicable; + + while let ExprKind::AddrOf(BorrowKind::Ref, _, expr) | ExprKind::Unary(UnOp::Deref, expr) = inner.kind { + inner = expr; + } + + let mut sugg = Sugg::hir_with_context(cx, inner, full_span.ctxt(), placeholder, &mut applicability); + + let inner_ty = cx.typeck_results().expr_ty(inner); + let mut ref_count = count_refs(inner_ty); + if needs_ref { + if ty_is_slice_like(cx, inner_ty.peel_refs()) { + ref_count -= 1; + } else { + // Inner argument is in some kind of Rc-like object, so it should be addr_deref'd to get a reference + // to the underlying slice + sugg = sugg.addr_deref(); + } + } + while ref_count > 0 { + sugg = sugg.deref(); + ref_count -= 1; + } + while ref_count < 0 { + sugg = sugg.addr(); + ref_count += 1; + } + + span_lint_and_sugg( + cx, + CLONES_INTO_BOXED_SLICES, + full_span, + "clone into boxed slice", + "use", + format!( + "Box::from({}{}{})", + sugg_prefix.unwrap_or_default(), + sugg, + sugg_suffix.unwrap_or_default() + ), + applicability, + ); +} + +// Is the given type a slice, path, or one of the str types +fn ty_is_slice_like(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + ty.is_slice() + || ty.is_str() + || ty.is_diag_item(cx, sym::cstr_type) + || ty.is_diag_item(cx, sym::Path) + || ty.is_diag_item(cx, sym::OsStr) +} + +// Checks if an expression is one of the into_boxed_... methods preceded by a clone-like function +// Then shows the lint with a suggestion that depends on the types of the inner argument and the +// resulting Box +impl<'tcx> LateLintPass<'tcx> for ClonesIntoBoxedSlices { + fn check_expr(&mut self, cx: &LateContext<'tcx>, second_method: &'tcx Expr<'_>) { + // Is the second method into_boxed_...? + if let ExprKind::MethodCall(second_method_path, first_method, _, _) = second_method.kind + && second_method.span.eq_ctxt(first_method.span) + && [ + sym::into_boxed_str, + sym::into_boxed_slice, + sym::into_boxed_path, + sym::into_boxed_c_str, + sym::into_boxed_os_str, + ] + .contains(&second_method_path.ident.name) + { + let arg = match first_method.kind { + // Is the first method clone-like? + ExprKind::MethodCall(first_method_path, left, _, _) + if [ + sym::to_owned, + sym::clone, + sym::to_string, + sym::to_path_buf, + sym::to_os_string, + sym::to_vec, + ] + .contains(&first_method_path.ident.name) => + { + Some(left) + }, + // Also check for from(...) constructor + ExprKind::Call( + Expr { + hir_id: _, + kind: ExprKind::Path(QPath::TypeRelative(call_out_ty, call_path)), + span: _, + }, + args, + ) if call_path.ident.name == sym::from && cx.typeck_results().expr_ty(&args[0]).is_ref() => { + Some(&args[0]) + }, + _ => None, + }; + + if let Some(arg) = arg { + let full_span = second_method.span.to(first_method.span); + let arg_ty = cx.typeck_results().expr_ty(arg); + let inner_ty = arg_ty.peel_refs(); + if ty_is_slice_like(cx, inner_ty) { + if second_method_path.ident.name == sym::into_boxed_path && !inner_ty.is_diag_item(cx, sym::Path) { + // PathBuf's from(...) can convert from other str types, + // so Path::new(...) must be used to assure resulting Box is the correct type + show_lint(cx, full_span, arg, true, Some("Path::new("), "...", Some(")")); + } else if let ExprKind::Unary(UnOp::Deref, deref_inner) = arg.kind + && cx + .typeck_results() + .expr_ty(deref_inner) + .is_lang_item(cx, LangItem::OwnedBox) + { + // Special case when inner argument is already in a Box: just use Box::clone + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + CLONES_INTO_BOXED_SLICES, + full_span, + "clone into boxed slice", + "use", + format!( + "{}.clone()", + snippet_with_applicability(cx, deref_inner.span, "...", &mut applicability) + ), + applicability, + ); + } else { + // Inner type is a ref to a slice, so it can be directly used in the suggestion + show_lint(cx, full_span, arg, true, None, "...", None); + } + // For all the following the inner type is owned, so they have to be converted to a + // reference first for the suggestion + } else if inner_ty.is_lang_item(cx, LangItem::String) { + show_lint(cx, full_span, arg, false, None, "(...)", Some(".as_str()")); + } else if inner_ty.is_diag_item(cx, sym::cstring_type) { + show_lint(cx, full_span, arg, false, None, "(...)", Some(".as_c_str()")); + } else if inner_ty.is_diag_item(cx, sym::PathBuf) { + show_lint(cx, full_span, arg, false, None, "(...)", Some(".as_path()")); + } else if inner_ty.is_diag_item(cx, sym::Vec) { + show_lint(cx, full_span, arg, false, Some("&"), "(...)", Some("[..]")); + } else if inner_ty.is_diag_item(cx, sym::OsString) { + show_lint(cx, full_span, arg, false, None, "(...)", Some(".as_os_str()")); + } + } + } + } +} diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 4a350dca2993..d7c2fe0920c4 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -78,6 +78,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::cfg_not_test::CFG_NOT_TEST_INFO, crate::checked_conversions::CHECKED_CONVERSIONS_INFO, crate::cloned_ref_to_slice_refs::CLONED_REF_TO_SLICE_REFS_INFO, + crate::clones_into_boxed_slices::CLONES_INTO_BOXED_SLICES_INFO, crate::coerce_container_to_any::COERCE_CONTAINER_TO_ANY_INFO, crate::cognitive_complexity::COGNITIVE_COMPLEXITY_INFO, crate::collapsible_if::COLLAPSIBLE_ELSE_IF_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 230d83dacc95..c3c6f30579d7 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -87,6 +87,7 @@ mod casts; mod cfg_not_test; mod checked_conversions; mod cloned_ref_to_slice_refs; +mod clones_into_boxed_slices; mod coerce_container_to_any; mod cognitive_complexity; mod collapsible_if; @@ -848,6 +849,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co Box::new(|_| Box::new(toplevel_ref_arg::ToplevelRefArg)), Box::new(|_| Box::new(volatile_composites::VolatileComposites)), Box::new(|_| Box::::default()), + Box::new(|_| Box::new(clones_into_boxed_slices::ClonesIntoBoxedSlices)), // add late passes here, used by `cargo dev new_lint` ]; store.late_passes.extend(late_lints); diff --git a/clippy_utils/src/sym.rs b/clippy_utils/src/sym.rs index 1d1537dd0e91..f676bdce94b9 100644 --- a/clippy_utils/src/sym.rs +++ b/clippy_utils/src/sym.rs @@ -185,6 +185,11 @@ generate! { inspect, int_roundings, into, + into_boxed_c_str, + into_boxed_os_str, + into_boxed_path, + into_boxed_slice, + into_boxed_str, into_bytes, into_ok, into_owned, diff --git a/tests/ui/clones_into_boxed_slices.fixed b/tests/ui/clones_into_boxed_slices.fixed new file mode 100644 index 000000000000..14ef6ae2d153 --- /dev/null +++ b/tests/ui/clones_into_boxed_slices.fixed @@ -0,0 +1,126 @@ +#![warn(clippy::clones_into_boxed_slices)] + +use std::borrow::ToOwned; +use std::ffi::{CStr, CString, OsStr, OsString}; +use std::fmt::{Display, Formatter}; +use std::path::{Path, PathBuf}; +use std::rc::Rc; + +struct Dummy {} +impl Display for Dummy { + fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { + write!(f, "implements display") + } +} + +macro_rules! create_str { + ($a:expr, $b:expr) => { + concat!($a, $b, "!") + }; +} + +macro_rules! to_string { + ($s:expr) => { + $s.to_string() + }; +} + +macro_rules! in_macro { + ($s:expr) => { + Box::from("test") + //~^ clones_into_boxed_slices + }; +} + +fn main() { + let s = "test"; + let _: Box = Box::from(s); + //~^ clones_into_boxed_slices + let _: Box = Box::from(s); + //~^ clones_into_boxed_slices + let ref_s = &s; + let _: Box = Box::from(*ref_s); + //~^ clones_into_boxed_slices + let boxed_s: Box = Box::from(s); + let _: Box = boxed_s.clone(); + //~^ clones_into_boxed_slices + let rc_s: Rc = Rc::from(s); + let _: Box = Box::from(&*rc_s); + //~^ clones_into_boxed_slices + let _: Box = Box::from(s); + //~^ clones_into_boxed_slices + let _: Box = Box::from(&s[..2]); + //~^ clones_into_boxed_slices + let _: Box = Box::from(s); + //~^ clones_into_boxed_slices + let string = String::from(s); + let _: Box = Box::from(string.as_str()); + //~^ clones_into_boxed_slices + let _: Box = Box::from(string.as_str()); + //~^ clones_into_boxed_slices + let _: Box = Box::from(string.as_str()); + //~^ clones_into_boxed_slices + + let c_str = c"test"; + let _: Box = Box::from(c_str); + //~^ clones_into_boxed_slices + let c_string = CString::from(c_str); + let _: Box = Box::from(c_string.as_c_str()); + //~^ clones_into_boxed_slices + let _: Box = Box::from(c_string.as_c_str()); + //~^ clones_into_boxed_slices + let _: Box = Box::from(c_str); + //~^ clones_into_boxed_slices + + let os_str = OsStr::new("test"); + let _: Box = Box::from(os_str); + //~^ clones_into_boxed_slices + let _: Box = Box::from(os_str); + //~^ clones_into_boxed_slices + let os_string = OsString::from(os_str); + let _: Box = Box::from(os_string.as_os_str()); + //~^ clones_into_boxed_slices + + let path = Path::new("./"); + let _: Box = Box::from(path); + //~^ clones_into_boxed_slices + let _: Box = Box::from(path); + //~^ clones_into_boxed_slices + let path_buf = PathBuf::from("./"); + let _: Box = Box::from(path_buf.as_path()); + //~^ clones_into_boxed_slices + let _: Box = Box::from(Path::new("./")); + //~^ clones_into_boxed_slices + + //Conversions that are necessary and don't clone; don't lint + let to_os_str = String::from("os_str"); + let _: Box = OsString::from(to_os_str).into_boxed_os_str(); + let to_path = String::from("./"); + let _: Box = PathBuf::from(to_path).into_boxed_path(); + + let test_vec = vec![0u32, 16u32]; + let _: Box<[u32]> = Box::from(&test_vec[..]); + //~^ clones_into_boxed_slices + let slice: &[u32] = &test_vec; + let _: Box<[u32]> = Box::from(slice); + //~^ clones_into_boxed_slices + let _: Box<[u32]> = Box::from(slice); + //~^ clones_into_boxed_slices + let _: Box<[u32]> = Box::from(slice); + //~^ clones_into_boxed_slices + + let _: Box<[u32]> = test_vec.into_boxed_slice(); + + //Shouldn't lint because to_string is necessary + let _: Box = Dummy {}.to_string().into_boxed_str(); + + // Do lint when only inner comes from macro + let _: Box = Box::from(create_str!("te", "st")); + //~^ clones_into_boxed_slices + + // Don't lint when only part is in macro + let _: Box = to_string!("test").into_boxed_str(); + + // Don't lint here but do lint in the macro def + let _: Box = in_macro!("test"); +} diff --git a/tests/ui/clones_into_boxed_slices.rs b/tests/ui/clones_into_boxed_slices.rs new file mode 100644 index 000000000000..81eb9f4937ae --- /dev/null +++ b/tests/ui/clones_into_boxed_slices.rs @@ -0,0 +1,126 @@ +#![warn(clippy::clones_into_boxed_slices)] + +use std::borrow::ToOwned; +use std::ffi::{CStr, CString, OsStr, OsString}; +use std::fmt::{Display, Formatter}; +use std::path::{Path, PathBuf}; +use std::rc::Rc; + +struct Dummy {} +impl Display for Dummy { + fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { + write!(f, "implements display") + } +} + +macro_rules! create_str { + ($a:expr, $b:expr) => { + concat!($a, $b, "!") + }; +} + +macro_rules! to_string { + ($s:expr) => { + $s.to_string() + }; +} + +macro_rules! in_macro { + ($s:expr) => { + $s.to_string().into_boxed_str() + //~^ clones_into_boxed_slices + }; +} + +fn main() { + let s = "test"; + let _: Box = s.to_string().into_boxed_str(); + //~^ clones_into_boxed_slices + let _: Box = (&s).to_string().into_boxed_str(); + //~^ clones_into_boxed_slices + let ref_s = &s; + let _: Box = ref_s.to_string().into_boxed_str(); + //~^ clones_into_boxed_slices + let boxed_s: Box = Box::from(s); + let _: Box = (*boxed_s).to_owned().into_boxed_str(); + //~^ clones_into_boxed_slices + let rc_s: Rc = Rc::from(s); + let _: Box = (*rc_s).to_owned().into_boxed_str(); + //~^ clones_into_boxed_slices + let _: Box = s.to_owned().into_boxed_str(); + //~^ clones_into_boxed_slices + let _: Box = s[..2].to_owned().into_boxed_str(); + //~^ clones_into_boxed_slices + let _: Box = String::from(s).into_boxed_str(); + //~^ clones_into_boxed_slices + let string = String::from(s); + let _: Box = String::from(&string).into_boxed_str(); + //~^ clones_into_boxed_slices + let _: Box = string.clone().into_boxed_str(); + //~^ clones_into_boxed_slices + let _: Box = string.to_owned().into_boxed_str(); + //~^ clones_into_boxed_slices + + let c_str = c"test"; + let _: Box = c_str.to_owned().into_boxed_c_str(); + //~^ clones_into_boxed_slices + let c_string = CString::from(c_str); + let _: Box = c_string.clone().into_boxed_c_str(); + //~^ clones_into_boxed_slices + let _: Box = c_string.to_owned().into_boxed_c_str(); + //~^ clones_into_boxed_slices + let _: Box = CString::from(c_str).into_boxed_c_str(); + //~^ clones_into_boxed_slices + + let os_str = OsStr::new("test"); + let _: Box = os_str.to_owned().into_boxed_os_str(); + //~^ clones_into_boxed_slices + let _: Box = os_str.to_os_string().into_boxed_os_str(); + //~^ clones_into_boxed_slices + let os_string = OsString::from(os_str); + let _: Box = os_string.clone().into_boxed_os_str(); + //~^ clones_into_boxed_slices + + let path = Path::new("./"); + let _: Box = path.to_owned().into_boxed_path(); + //~^ clones_into_boxed_slices + let _: Box = path.to_path_buf().into_boxed_path(); + //~^ clones_into_boxed_slices + let path_buf = PathBuf::from("./"); + let _: Box = path_buf.clone().into_boxed_path(); + //~^ clones_into_boxed_slices + let _: Box = PathBuf::from("./").into_boxed_path(); + //~^ clones_into_boxed_slices + + //Conversions that are necessary and don't clone; don't lint + let to_os_str = String::from("os_str"); + let _: Box = OsString::from(to_os_str).into_boxed_os_str(); + let to_path = String::from("./"); + let _: Box = PathBuf::from(to_path).into_boxed_path(); + + let test_vec = vec![0u32, 16u32]; + let _: Box<[u32]> = test_vec.clone().into_boxed_slice(); + //~^ clones_into_boxed_slices + let slice: &[u32] = &test_vec; + let _: Box<[u32]> = Vec::from(slice).into_boxed_slice(); + //~^ clones_into_boxed_slices + let _: Box<[u32]> = slice.to_owned().into_boxed_slice(); + //~^ clones_into_boxed_slices + let _: Box<[u32]> = slice.to_vec().into_boxed_slice(); + //~^ clones_into_boxed_slices + + let _: Box<[u32]> = test_vec.into_boxed_slice(); + + //Shouldn't lint because to_string is necessary + let _: Box = Dummy {}.to_string().into_boxed_str(); + + // Do lint when only inner comes from macro + let _: Box = create_str!("te", "st").to_string().into_boxed_str(); + //~^ clones_into_boxed_slices + + // Don't lint when only part is in macro + let _: Box = to_string!("test").into_boxed_str(); + + // Don't lint here but do lint in the macro def + let _: Box = in_macro!("test"); +} diff --git a/tests/ui/clones_into_boxed_slices.stderr b/tests/ui/clones_into_boxed_slices.stderr new file mode 100644 index 000000000000..05cbcf7cc726 --- /dev/null +++ b/tests/ui/clones_into_boxed_slices.stderr @@ -0,0 +1,178 @@ +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:37:23 + | +LL | let _: Box = s.to_string().into_boxed_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(s)` + | + = note: `-D clippy::clones-into-boxed-slices` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::clones_into_boxed_slices)]` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:39:23 + | +LL | let _: Box = (&s).to_string().into_boxed_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(s)` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:42:23 + | +LL | let _: Box = ref_s.to_string().into_boxed_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(*ref_s)` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:45:23 + | +LL | let _: Box = (*boxed_s).to_owned().into_boxed_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `boxed_s.clone()` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:48:23 + | +LL | let _: Box = (*rc_s).to_owned().into_boxed_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(&*rc_s)` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:50:23 + | +LL | let _: Box = s.to_owned().into_boxed_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(s)` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:52:23 + | +LL | let _: Box = s[..2].to_owned().into_boxed_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(&s[..2])` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:54:23 + | +LL | let _: Box = String::from(s).into_boxed_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(s)` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:57:23 + | +LL | let _: Box = String::from(&string).into_boxed_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(string.as_str())` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:59:23 + | +LL | let _: Box = string.clone().into_boxed_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(string.as_str())` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:61:23 + | +LL | let _: Box = string.to_owned().into_boxed_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(string.as_str())` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:65:24 + | +LL | let _: Box = c_str.to_owned().into_boxed_c_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(c_str)` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:68:24 + | +LL | let _: Box = c_string.clone().into_boxed_c_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(c_string.as_c_str())` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:70:24 + | +LL | let _: Box = c_string.to_owned().into_boxed_c_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(c_string.as_c_str())` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:72:24 + | +LL | let _: Box = CString::from(c_str).into_boxed_c_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(c_str)` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:76:25 + | +LL | let _: Box = os_str.to_owned().into_boxed_os_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(os_str)` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:78:25 + | +LL | let _: Box = os_str.to_os_string().into_boxed_os_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(os_str)` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:81:25 + | +LL | let _: Box = os_string.clone().into_boxed_os_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(os_string.as_os_str())` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:85:24 + | +LL | let _: Box = path.to_owned().into_boxed_path(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(path)` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:87:24 + | +LL | let _: Box = path.to_path_buf().into_boxed_path(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(path)` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:90:24 + | +LL | let _: Box = path_buf.clone().into_boxed_path(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(path_buf.as_path())` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:92:24 + | +LL | let _: Box = PathBuf::from("./").into_boxed_path(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(Path::new("./"))` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:102:25 + | +LL | let _: Box<[u32]> = test_vec.clone().into_boxed_slice(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(&test_vec[..])` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:105:25 + | +LL | let _: Box<[u32]> = Vec::from(slice).into_boxed_slice(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(slice)` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:107:25 + | +LL | let _: Box<[u32]> = slice.to_owned().into_boxed_slice(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(slice)` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:109:25 + | +LL | let _: Box<[u32]> = slice.to_vec().into_boxed_slice(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(slice)` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:118:23 + | +LL | let _: Box = create_str!("te", "st").to_string().into_boxed_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(create_str!("te", "st"))` + +error: clone into boxed slice + --> tests/ui/clones_into_boxed_slices.rs:30:9 + | +LL | $s.to_string().into_boxed_str() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from("test")` +... +LL | let _: Box = in_macro!("test"); + | ----------------- in this macro invocation + | + = note: this error originates in the macro `in_macro` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 28 previous errors + diff --git a/tests/ui/crashes/ice-3969.rs b/tests/ui/crashes/ice-3969.rs index fa3ca6f1d538..56ca37f58ac7 100644 --- a/tests/ui/crashes/ice-3969.rs +++ b/tests/ui/crashes/ice-3969.rs @@ -6,7 +6,7 @@ // Check that tautalogically false bounds are accepted, and are used // in type inference. #![feature(trivial_bounds)] -#![allow(unused)] +#![allow(unused, clippy::clones_into_boxed_slices)] trait A {} impl A for i32 {} diff --git a/tests/ui/unnecessary_box_returns.rs b/tests/ui/unnecessary_box_returns.rs index a7ab7e90be06..1db98cfa5232 100644 --- a/tests/ui/unnecessary_box_returns.rs +++ b/tests/ui/unnecessary_box_returns.rs @@ -45,7 +45,7 @@ pub fn bxed_foo() -> Box { // don't lint: str is unsized fn bxed_str() -> Box { - "Hello, world!".to_string().into_boxed_str() + Box::from("Hello, world!") } // don't lint: function contains the word, "box" From 372f713af59d2fa369a43bebd20ebd21b2b82228 Mon Sep 17 00:00:00 2001 From: Aedan Kearns Date: Fri, 14 Nov 2025 20:54:50 -0500 Subject: [PATCH 2/4] Fix examples in lint docs --- clippy_lints/src/clones_into_boxed_slices.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/clones_into_boxed_slices.rs b/clippy_lints/src/clones_into_boxed_slices.rs index f60c37a4b46b..86cc6d4f7bc6 100644 --- a/clippy_lints/src/clones_into_boxed_slices.rs +++ b/clippy_lints/src/clones_into_boxed_slices.rs @@ -20,11 +20,11 @@ declare_clippy_lint! { /// /// ### Example /// ```no_run - /// "example".to_string().to_boxed_str() + /// let boxed: Box = "example".to_string().into_boxed_str(); /// ``` /// Use instead: /// ```no_run - /// Box::from("example") + /// let boxed: Box = Box::from("example"); /// ``` #[clippy::version = "1.93.0"] pub CLONES_INTO_BOXED_SLICES, From 2fe11a4333b5f88938080a31ba27cf8e2d6441f9 Mon Sep 17 00:00:00 2001 From: aedank0 <76075721+aedank0@users.noreply.github.com> Date: Wed, 19 Nov 2025 17:12:24 -0500 Subject: [PATCH 3/4] Apply Suggestions Co-authored-by: Samuel Tardieu --- clippy_lints/src/clones_into_boxed_slices.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/clones_into_boxed_slices.rs b/clippy_lints/src/clones_into_boxed_slices.rs index 86cc6d4f7bc6..d838d30b259d 100644 --- a/clippy_lints/src/clones_into_boxed_slices.rs +++ b/clippy_lints/src/clones_into_boxed_slices.rs @@ -72,13 +72,11 @@ fn show_lint( sugg = sugg.addr_deref(); } } - while ref_count > 0 { + for _ in 0..ref_count { sugg = sugg.deref(); - ref_count -= 1; } - while ref_count < 0 { + for _ in 0..-ref_count { sugg = sugg.addr(); - ref_count += 1; } span_lint_and_sugg( @@ -99,11 +97,7 @@ fn show_lint( // Is the given type a slice, path, or one of the str types fn ty_is_slice_like(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { - ty.is_slice() - || ty.is_str() - || ty.is_diag_item(cx, sym::cstr_type) - || ty.is_diag_item(cx, sym::Path) - || ty.is_diag_item(cx, sym::OsStr) + ty.is_slice() || ty.is_str() || matches!(ty.opt_diag_name(cx.tcx), Some(sym::cstr_type | sym::Path | sym::OsStr)) } // Checks if an expression is one of the into_boxed_... methods preceded by a clone-like function From de839181c6e332e82d929d82c39d2f2cbba20ddc Mon Sep 17 00:00:00 2001 From: Aedan Kearns Date: Mon, 24 Nov 2025 19:46:46 -0500 Subject: [PATCH 4/4] Move to methods and implement other suggestions --- clippy_lints/src/clones_into_boxed_slices.rs | 198 ------------------ clippy_lints/src/declared_lints.rs | 2 +- clippy_lints/src/lib.rs | 2 - .../src/methods/clones_into_boxed_slices.rs | 144 +++++++++++++ clippy_lints/src/methods/mod.rs | 64 +++++- clippy_utils/src/msrvs.rs | 1 + tests/ui/clones_into_boxed_slices.fixed | 47 ++++- tests/ui/clones_into_boxed_slices.rs | 43 +++- tests/ui/clones_into_boxed_slices.stderr | 69 +++--- 9 files changed, 317 insertions(+), 253 deletions(-) delete mode 100644 clippy_lints/src/clones_into_boxed_slices.rs create mode 100644 clippy_lints/src/methods/clones_into_boxed_slices.rs diff --git a/clippy_lints/src/clones_into_boxed_slices.rs b/clippy_lints/src/clones_into_boxed_slices.rs deleted file mode 100644 index d838d30b259d..000000000000 --- a/clippy_lints/src/clones_into_boxed_slices.rs +++ /dev/null @@ -1,198 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::res::MaybeDef; -use clippy_utils::source::snippet_with_applicability; -use clippy_utils::sugg::Sugg; -use clippy_utils::sym; -use rustc_ast::{BorrowKind, UnOp}; -use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, LangItem, QPath}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::{self, Ty}; -use rustc_session::declare_lint_pass; -use rustc_span::Span; - -declare_clippy_lint! { - /// ### What it does - /// Checks for clones that are immediately converted into boxed slices instead of using `Box::from(...)`. - /// - /// ### Why is this bad? - /// Using `Box::from(...)` is more concise and avoids creating an unnecessary temporary object. - /// - /// ### Example - /// ```no_run - /// let boxed: Box = "example".to_string().into_boxed_str(); - /// ``` - /// Use instead: - /// ```no_run - /// let boxed: Box = Box::from("example"); - /// ``` - #[clippy::version = "1.93.0"] - pub CLONES_INTO_BOXED_SLICES, - perf, - "Cloning then converting into boxed slice instead of using Box::from" -} -declare_lint_pass!(ClonesIntoBoxedSlices => [CLONES_INTO_BOXED_SLICES]); - -fn count_refs(mut expr_ty: Ty<'_>) -> i64 { - let mut count = 0; - while let ty::Ref(_, inner, _) = expr_ty.kind() { - expr_ty = *inner; - count += 1; - } - count -} - -// Shows the lint with a suggestion using the given parts -// Assures that the inner argument is correctly ref'd/deref'd in the suggestion based on needs_ref -fn show_lint( - cx: &LateContext<'_>, - full_span: Span, - mut inner: &Expr<'_>, - needs_ref: bool, - sugg_prefix: Option<&str>, - placeholder: &str, - sugg_suffix: Option<&str>, -) { - let mut applicability = Applicability::MachineApplicable; - - while let ExprKind::AddrOf(BorrowKind::Ref, _, expr) | ExprKind::Unary(UnOp::Deref, expr) = inner.kind { - inner = expr; - } - - let mut sugg = Sugg::hir_with_context(cx, inner, full_span.ctxt(), placeholder, &mut applicability); - - let inner_ty = cx.typeck_results().expr_ty(inner); - let mut ref_count = count_refs(inner_ty); - if needs_ref { - if ty_is_slice_like(cx, inner_ty.peel_refs()) { - ref_count -= 1; - } else { - // Inner argument is in some kind of Rc-like object, so it should be addr_deref'd to get a reference - // to the underlying slice - sugg = sugg.addr_deref(); - } - } - for _ in 0..ref_count { - sugg = sugg.deref(); - } - for _ in 0..-ref_count { - sugg = sugg.addr(); - } - - span_lint_and_sugg( - cx, - CLONES_INTO_BOXED_SLICES, - full_span, - "clone into boxed slice", - "use", - format!( - "Box::from({}{}{})", - sugg_prefix.unwrap_or_default(), - sugg, - sugg_suffix.unwrap_or_default() - ), - applicability, - ); -} - -// Is the given type a slice, path, or one of the str types -fn ty_is_slice_like(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { - ty.is_slice() || ty.is_str() || matches!(ty.opt_diag_name(cx.tcx), Some(sym::cstr_type | sym::Path | sym::OsStr)) -} - -// Checks if an expression is one of the into_boxed_... methods preceded by a clone-like function -// Then shows the lint with a suggestion that depends on the types of the inner argument and the -// resulting Box -impl<'tcx> LateLintPass<'tcx> for ClonesIntoBoxedSlices { - fn check_expr(&mut self, cx: &LateContext<'tcx>, second_method: &'tcx Expr<'_>) { - // Is the second method into_boxed_...? - if let ExprKind::MethodCall(second_method_path, first_method, _, _) = second_method.kind - && second_method.span.eq_ctxt(first_method.span) - && [ - sym::into_boxed_str, - sym::into_boxed_slice, - sym::into_boxed_path, - sym::into_boxed_c_str, - sym::into_boxed_os_str, - ] - .contains(&second_method_path.ident.name) - { - let arg = match first_method.kind { - // Is the first method clone-like? - ExprKind::MethodCall(first_method_path, left, _, _) - if [ - sym::to_owned, - sym::clone, - sym::to_string, - sym::to_path_buf, - sym::to_os_string, - sym::to_vec, - ] - .contains(&first_method_path.ident.name) => - { - Some(left) - }, - // Also check for from(...) constructor - ExprKind::Call( - Expr { - hir_id: _, - kind: ExprKind::Path(QPath::TypeRelative(call_out_ty, call_path)), - span: _, - }, - args, - ) if call_path.ident.name == sym::from && cx.typeck_results().expr_ty(&args[0]).is_ref() => { - Some(&args[0]) - }, - _ => None, - }; - - if let Some(arg) = arg { - let full_span = second_method.span.to(first_method.span); - let arg_ty = cx.typeck_results().expr_ty(arg); - let inner_ty = arg_ty.peel_refs(); - if ty_is_slice_like(cx, inner_ty) { - if second_method_path.ident.name == sym::into_boxed_path && !inner_ty.is_diag_item(cx, sym::Path) { - // PathBuf's from(...) can convert from other str types, - // so Path::new(...) must be used to assure resulting Box is the correct type - show_lint(cx, full_span, arg, true, Some("Path::new("), "...", Some(")")); - } else if let ExprKind::Unary(UnOp::Deref, deref_inner) = arg.kind - && cx - .typeck_results() - .expr_ty(deref_inner) - .is_lang_item(cx, LangItem::OwnedBox) - { - // Special case when inner argument is already in a Box: just use Box::clone - let mut applicability = Applicability::MachineApplicable; - span_lint_and_sugg( - cx, - CLONES_INTO_BOXED_SLICES, - full_span, - "clone into boxed slice", - "use", - format!( - "{}.clone()", - snippet_with_applicability(cx, deref_inner.span, "...", &mut applicability) - ), - applicability, - ); - } else { - // Inner type is a ref to a slice, so it can be directly used in the suggestion - show_lint(cx, full_span, arg, true, None, "...", None); - } - // For all the following the inner type is owned, so they have to be converted to a - // reference first for the suggestion - } else if inner_ty.is_lang_item(cx, LangItem::String) { - show_lint(cx, full_span, arg, false, None, "(...)", Some(".as_str()")); - } else if inner_ty.is_diag_item(cx, sym::cstring_type) { - show_lint(cx, full_span, arg, false, None, "(...)", Some(".as_c_str()")); - } else if inner_ty.is_diag_item(cx, sym::PathBuf) { - show_lint(cx, full_span, arg, false, None, "(...)", Some(".as_path()")); - } else if inner_ty.is_diag_item(cx, sym::Vec) { - show_lint(cx, full_span, arg, false, Some("&"), "(...)", Some("[..]")); - } else if inner_ty.is_diag_item(cx, sym::OsString) { - show_lint(cx, full_span, arg, false, None, "(...)", Some(".as_os_str()")); - } - } - } - } -} diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index d7c2fe0920c4..0d0acd1464e7 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -78,7 +78,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::cfg_not_test::CFG_NOT_TEST_INFO, crate::checked_conversions::CHECKED_CONVERSIONS_INFO, crate::cloned_ref_to_slice_refs::CLONED_REF_TO_SLICE_REFS_INFO, - crate::clones_into_boxed_slices::CLONES_INTO_BOXED_SLICES_INFO, crate::coerce_container_to_any::COERCE_CONTAINER_TO_ANY_INFO, crate::cognitive_complexity::COGNITIVE_COMPLEXITY_INFO, crate::collapsible_if::COLLAPSIBLE_ELSE_IF_INFO, @@ -356,6 +355,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::methods::CHARS_NEXT_CMP_INFO, crate::methods::CLEAR_WITH_DRAIN_INFO, crate::methods::CLONED_INSTEAD_OF_COPIED_INFO, + crate::methods::CLONES_INTO_BOXED_SLICES_INFO, crate::methods::CLONE_ON_COPY_INFO, crate::methods::CLONE_ON_REF_PTR_INFO, crate::methods::COLLAPSIBLE_STR_REPLACE_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index c3c6f30579d7..230d83dacc95 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -87,7 +87,6 @@ mod casts; mod cfg_not_test; mod checked_conversions; mod cloned_ref_to_slice_refs; -mod clones_into_boxed_slices; mod coerce_container_to_any; mod cognitive_complexity; mod collapsible_if; @@ -849,7 +848,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co Box::new(|_| Box::new(toplevel_ref_arg::ToplevelRefArg)), Box::new(|_| Box::new(volatile_composites::VolatileComposites)), Box::new(|_| Box::::default()), - Box::new(|_| Box::new(clones_into_boxed_slices::ClonesIntoBoxedSlices)), // add late passes here, used by `cargo dev new_lint` ]; store.late_passes.extend(late_lints); diff --git a/clippy_lints/src/methods/clones_into_boxed_slices.rs b/clippy_lints/src/methods/clones_into_boxed_slices.rs new file mode 100644 index 000000000000..af904ec06eae --- /dev/null +++ b/clippy_lints/src/methods/clones_into_boxed_slices.rs @@ -0,0 +1,144 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::res::MaybeDef; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::sugg::Sugg; +use clippy_utils::sym; +use clippy_utils::ty::peel_and_count_ty_refs; +use rustc_ast::{BorrowKind, UnOp}; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, LangItem}; +use rustc_lint::LateContext; +use rustc_middle::ty::Ty; +use rustc_span::{Span, Symbol}; + +use super::CLONES_INTO_BOXED_SLICES; + +// Shows the lint with a suggestion using the given parts +// Assures that the inner argument is correctly ref'd/deref'd in the suggestion based on needs_ref +fn show_lint( + cx: &LateContext<'_>, + full_span: Span, + mut inner: &Expr<'_>, + needs_ref: bool, + suggestion: (Option<&str>, &str, Option<&str>), + degrade_app_to: Option, +) { + let mut applicability = degrade_app_to.unwrap_or(Applicability::MachineApplicable); + + while let ExprKind::AddrOf(BorrowKind::Ref, _, expr) | ExprKind::Unary(UnOp::Deref, expr) = inner.kind { + inner = expr; + } + + let mut sugg = Sugg::hir_with_context(cx, inner, full_span.ctxt(), suggestion.1, &mut applicability); + + let inner_ty = cx.typeck_results().expr_ty(inner); + let (inner_ty_peeled, ref_count, _) = peel_and_count_ty_refs(inner_ty); + let mut ref_count = ref_count as i128; + if needs_ref { + if ty_is_slice_like(cx, inner_ty_peeled) { + ref_count -= 1; + } else { + // Inner argument is in some kind of Rc-like object, so it should be addr_deref'd to get a reference + // to the underlying slice + sugg = sugg.addr_deref(); + } + } + for _ in 0..ref_count { + sugg = sugg.deref(); + } + for _ in 0..-ref_count { + sugg = sugg.addr(); + } + + span_lint_and_sugg( + cx, + CLONES_INTO_BOXED_SLICES, + full_span, + "clone into boxed slice", + "use", + format!( + "Box::from({}{}{})", + suggestion.0.unwrap_or_default(), + sugg, + suggestion.2.unwrap_or_default() + ), + applicability, + ); +} + +// Is the given type a slice, path, or one of the str types +fn ty_is_slice_like(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + ty.is_slice() || ty.is_str() || matches!(ty.opt_diag_name(&cx.tcx), Some(sym::cstr_type | sym::Path | sym::OsStr)) +} + +// Shows the lint with a suggestion that depends on the types of the inner argument and the +// resulting Box +pub(super) fn check( + cx: &LateContext<'_>, + first_method_ty: Ty<'_>, + second_method_name: Symbol, + full_span: Span, + arg: &Expr<'_>, +) { + let first_ty_diag_name = first_method_ty.opt_diag_name(cx); + if (second_method_name == sym::into_boxed_c_str && first_ty_diag_name != Some(sym::cstring_type)) + || (second_method_name == sym::into_boxed_os_str && first_ty_diag_name != Some(sym::OsString)) + || (second_method_name == sym::into_boxed_path && first_ty_diag_name != Some(sym::PathBuf)) + || (second_method_name == sym::into_boxed_str && !first_method_ty.is_lang_item(cx, LangItem::String)) + || (second_method_name == sym::into_boxed_slice && first_ty_diag_name != Some(sym::Vec)) + { + return; + } + + let arg_ty = cx.typeck_results().expr_ty(arg); + let inner_ty = arg_ty.peel_refs(); + if ty_is_slice_like(cx, inner_ty) { + if second_method_name == sym::into_boxed_path && !inner_ty.is_diag_item(cx, sym::Path) { + // PathBuf's from(...) can convert from other str types, + // so Path::new(...) must be used to assure resulting Box is the correct type + show_lint( + cx, + full_span, + arg, + true, + (Some("Path::new("), "...", Some(")")), + Some(Applicability::MaybeIncorrect), + ); + } else if let ExprKind::Unary(UnOp::Deref, deref_inner) = arg.kind + && cx + .typeck_results() + .expr_ty(deref_inner) + .is_lang_item(cx, LangItem::OwnedBox) + { + // Special case when inner argument is already in a Box: just use Box::clone + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + CLONES_INTO_BOXED_SLICES, + full_span, + "clone into boxed slice", + "use", + format!( + "{}.clone()", + snippet_with_applicability(cx, deref_inner.span, "...", &mut applicability) + ), + applicability, + ); + } else { + // Inner type is a ref to a slice, so it can be directly used in the suggestion + show_lint(cx, full_span, arg, true, (None, "...", None), None); + } + // For all the following the inner type is owned, so they have to be converted to a + // reference first for the suggestion + } else if inner_ty.is_lang_item(cx, LangItem::String) { + show_lint(cx, full_span, arg, false, (None, "(...)", Some(".as_str()")), None); + } else if inner_ty.is_diag_item(cx, sym::cstring_type) { + show_lint(cx, full_span, arg, false, (None, "(...)", Some(".as_c_str()")), None); + } else if inner_ty.is_diag_item(cx, sym::PathBuf) { + show_lint(cx, full_span, arg, false, (None, "(...)", Some(".as_path()")), None); + } else if inner_ty.is_diag_item(cx, sym::Vec) { + show_lint(cx, full_span, arg, false, (Some("&"), "(...)", Some("[..]")), None); + } else if inner_ty.is_diag_item(cx, sym::OsString) { + show_lint(cx, full_span, arg, false, (None, "(...)", Some(".as_os_str()")), None); + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index c22b0a548e3d..2b2c5cc3f6ee 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -13,6 +13,7 @@ mod clear_with_drain; mod clone_on_copy; mod clone_on_ref_ptr; mod cloned_instead_of_copied; +mod clones_into_boxed_slices; mod collapsible_str_replace; mod double_ended_iterator_last; mod drain_collect; @@ -156,7 +157,7 @@ use clippy_utils::res::{MaybeDef, MaybeTypeckRes}; use clippy_utils::{contains_return, iter_input_pats, peel_blocks, sym}; pub use path_ends_with_ext::DEFAULT_ALLOWED_DOTFILES; use rustc_data_structures::fx::FxHashSet; -use rustc_hir::{self as hir, Expr, ExprKind, Node, Stmt, StmtKind, TraitItem, TraitItemKind}; +use rustc_hir::{self as hir, Expr, ExprKind, Node, QPath, Stmt, StmtKind, TraitItem, TraitItemKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::ty::TraitRef; use rustc_session::impl_lint_pass; @@ -4714,6 +4715,27 @@ declare_clippy_lint! { "filtering `std::io::Lines` with `filter_map()`, `flat_map()`, or `flatten()` might cause an infinite loop" } +declare_clippy_lint! { + /// ### What it does + /// Checks for clones that are immediately converted into boxed slices instead of using `Box::from(...)`. + /// + /// ### Why is this bad? + /// Using `Box::from(...)` is more concise and avoids creating an unnecessary temporary object. + /// + /// ### Example + /// ```no_run + /// let boxed: Box = "example".to_string().into_boxed_str(); + /// ``` + /// Use instead: + /// ```no_run + /// let boxed: Box = Box::from("example"); + /// ``` + #[clippy::version = "1.93.0"] + pub CLONES_INTO_BOXED_SLICES, + perf, + "Cloning then converting into boxed slice instead of using Box::from" +} + #[expect(clippy::struct_excessive_bools)] pub struct Methods { avoid_breaking_exported_api: bool, @@ -4897,6 +4919,7 @@ impl_lint_pass!(Methods => [ REDUNDANT_ITER_CLONED, UNNECESSARY_OPTION_MAP_OR_ELSE, LINES_FILTER_MAP_OK, + CLONES_INTO_BOXED_SLICES, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -5279,6 +5302,45 @@ impl Methods { (sym::hash, [arg]) => { unit_hash::check(cx, expr, recv, arg); }, + ( + second_name @ (sym::into_boxed_c_str + | sym::into_boxed_os_str + | sym::into_boxed_path + | sym::into_boxed_slice + | sym::into_boxed_str), + _, + ) if self.msrv.meets(cx, msrvs::CLONES_INTO_BOXED_SLICES) => { + let first_ty = cx.typeck_results().expr_ty(recv).peel_refs(); + let full_span = expr.span.to(recv.span); + if let Some(( + sym::clone + | sym::to_os_string + | sym::to_owned + | sym::to_path_buf + | sym::to_string + | sym::to_vec, + left, + _, + _, + _, + )) = method_call(recv) + { + clones_into_boxed_slices::check(cx, first_ty, second_name, full_span, left); + } else if let ExprKind::Call( + Expr { + hir_id: _, + kind: ExprKind::Path(QPath::TypeRelative(_, call_path)), + span: _, + }, + args, + ) = recv.kind + && call_path.ident.name == sym::from + //&& cx.ty_based_def(recv).opt_parent(cx).is_diag_item(cx, sym::From) + && cx.typeck_results().expr_ty(&args[0]).is_ref() + { + clones_into_boxed_slices::check(cx, first_ty, second_name, full_span, &args[0]); + } + }, (sym::is_empty, []) => { match method_call(recv) { Some((prev_method @ (sym::as_bytes | sym::bytes), prev_recv, [], _, _)) => { diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 86d17a8231d5..ba6fd64f5b6c 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -81,6 +81,7 @@ msrv_aliases! { 1,16,0 { STR_REPEAT, RESULT_UNWRAP_OR_DEFAULT } 1,15,0 { MAYBE_BOUND_IN_WHERE } 1,13,0 { QUESTION_MARK_OPERATOR } + 1,7,0 { CLONES_INTO_BOXED_SLICES } } /// `#[clippy::msrv]` attributes are rarely used outside of Clippy's test suite, as a basic diff --git a/tests/ui/clones_into_boxed_slices.fixed b/tests/ui/clones_into_boxed_slices.fixed index 14ef6ae2d153..a060c1ba0b20 100644 --- a/tests/ui/clones_into_boxed_slices.fixed +++ b/tests/ui/clones_into_boxed_slices.fixed @@ -6,13 +6,34 @@ use std::fmt::{Display, Formatter}; use std::path::{Path, PathBuf}; use std::rc::Rc; +#[derive(Clone)] struct Dummy {} +impl Dummy { + fn from(_s: &str) -> Self { + Self {} + } + fn into_boxed_str(self) -> Box { + Box::from("dummy") + } + fn into_boxed_c_str(self) -> Box { + Box::from(c"dummy") + } +} impl Display for Dummy { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { write!(f, "implements display") } } +trait SameName { + fn into_boxed_c_str(self) -> Box; +} +impl SameName for Vec { + fn into_boxed_c_str(self) -> Box { + Box::from(c"u32 vec") + } +} + macro_rules! create_str { ($a:expr, $b:expr) => { concat!($a, $b, "!") @@ -25,10 +46,10 @@ macro_rules! to_string { }; } +// Don't lint in macros macro_rules! in_macro { ($s:expr) => { - Box::from("test") - //~^ clones_into_boxed_slices + $s.to_string().into_boxed_str() }; } @@ -114,13 +135,23 @@ fn main() { //Shouldn't lint because to_string is necessary let _: Box = Dummy {}.to_string().into_boxed_str(); - // Do lint when only inner comes from macro - let _: Box = Box::from(create_str!("te", "st")); + // Don't lint macros + let _: Box = create_str!("te", "st").to_string().into_boxed_str(); + let _: Box = to_string!("test").into_boxed_str(); + let _: Box = in_macro!("test"); + + let _: Box = { s.to_string() }.into_boxed_str(); + let _: Box = Box::from({ s.to_string() }.as_str()); //~^ clones_into_boxed_slices - // Don't lint when only part is in macro - let _: Box = to_string!("test").into_boxed_str(); + let _: Box = Box::from({ s }); + //~^ clones_into_boxed_slices - // Don't lint here but do lint in the macro def - let _: Box = in_macro!("test"); + let _: Box = Dummy {}.clone().into_boxed_str(); + let _: Box = Dummy::from("test").into_boxed_str(); + let _: Box = Dummy::from("test").into_boxed_c_str(); + + let test_vec = vec![42u32]; + let _: Box = test_vec.clone().into_boxed_c_str(); + let _: Box = Vec::from(&[43u32]).into_boxed_c_str(); } diff --git a/tests/ui/clones_into_boxed_slices.rs b/tests/ui/clones_into_boxed_slices.rs index 81eb9f4937ae..cb70f633d16a 100644 --- a/tests/ui/clones_into_boxed_slices.rs +++ b/tests/ui/clones_into_boxed_slices.rs @@ -6,13 +6,34 @@ use std::fmt::{Display, Formatter}; use std::path::{Path, PathBuf}; use std::rc::Rc; +#[derive(Clone)] struct Dummy {} +impl Dummy { + fn from(_s: &str) -> Self { + Self {} + } + fn into_boxed_str(self) -> Box { + Box::from("dummy") + } + fn into_boxed_c_str(self) -> Box { + Box::from(c"dummy") + } +} impl Display for Dummy { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { write!(f, "implements display") } } +trait SameName { + fn into_boxed_c_str(self) -> Box; +} +impl SameName for Vec { + fn into_boxed_c_str(self) -> Box { + Box::from(c"u32 vec") + } +} + macro_rules! create_str { ($a:expr, $b:expr) => { concat!($a, $b, "!") @@ -25,10 +46,10 @@ macro_rules! to_string { }; } +// Don't lint in macros macro_rules! in_macro { ($s:expr) => { $s.to_string().into_boxed_str() - //~^ clones_into_boxed_slices }; } @@ -114,13 +135,23 @@ fn main() { //Shouldn't lint because to_string is necessary let _: Box = Dummy {}.to_string().into_boxed_str(); - // Do lint when only inner comes from macro + // Don't lint macros let _: Box = create_str!("te", "st").to_string().into_boxed_str(); + let _: Box = to_string!("test").into_boxed_str(); + let _: Box = in_macro!("test"); + + let _: Box = { s.to_string() }.into_boxed_str(); + let _: Box = String::from(&{ s.to_string() }).into_boxed_str(); //~^ clones_into_boxed_slices - // Don't lint when only part is in macro - let _: Box = to_string!("test").into_boxed_str(); + let _: Box = (&{ s }).to_string().into_boxed_str(); + //~^ clones_into_boxed_slices - // Don't lint here but do lint in the macro def - let _: Box = in_macro!("test"); + let _: Box = Dummy {}.clone().into_boxed_str(); + let _: Box = Dummy::from("test").into_boxed_str(); + let _: Box = Dummy::from("test").into_boxed_c_str(); + + let test_vec = vec![42u32]; + let _: Box = test_vec.clone().into_boxed_c_str(); + let _: Box = Vec::from(&[43u32]).into_boxed_c_str(); } diff --git a/tests/ui/clones_into_boxed_slices.stderr b/tests/ui/clones_into_boxed_slices.stderr index 05cbcf7cc726..cdea0b3acaa5 100644 --- a/tests/ui/clones_into_boxed_slices.stderr +++ b/tests/ui/clones_into_boxed_slices.stderr @@ -1,5 +1,5 @@ error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:37:23 + --> tests/ui/clones_into_boxed_slices.rs:58:23 | LL | let _: Box = s.to_string().into_boxed_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(s)` @@ -8,171 +8,166 @@ LL | let _: Box = s.to_string().into_boxed_str(); = help: to override `-D warnings` add `#[allow(clippy::clones_into_boxed_slices)]` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:39:23 + --> tests/ui/clones_into_boxed_slices.rs:60:23 | LL | let _: Box = (&s).to_string().into_boxed_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(s)` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:42:23 + --> tests/ui/clones_into_boxed_slices.rs:63:23 | LL | let _: Box = ref_s.to_string().into_boxed_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(*ref_s)` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:45:23 + --> tests/ui/clones_into_boxed_slices.rs:66:23 | LL | let _: Box = (*boxed_s).to_owned().into_boxed_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `boxed_s.clone()` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:48:23 + --> tests/ui/clones_into_boxed_slices.rs:69:23 | LL | let _: Box = (*rc_s).to_owned().into_boxed_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(&*rc_s)` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:50:23 + --> tests/ui/clones_into_boxed_slices.rs:71:23 | LL | let _: Box = s.to_owned().into_boxed_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(s)` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:52:23 + --> tests/ui/clones_into_boxed_slices.rs:73:23 | LL | let _: Box = s[..2].to_owned().into_boxed_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(&s[..2])` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:54:23 + --> tests/ui/clones_into_boxed_slices.rs:75:23 | LL | let _: Box = String::from(s).into_boxed_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(s)` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:57:23 + --> tests/ui/clones_into_boxed_slices.rs:78:23 | LL | let _: Box = String::from(&string).into_boxed_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(string.as_str())` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:59:23 + --> tests/ui/clones_into_boxed_slices.rs:80:23 | LL | let _: Box = string.clone().into_boxed_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(string.as_str())` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:61:23 + --> tests/ui/clones_into_boxed_slices.rs:82:23 | LL | let _: Box = string.to_owned().into_boxed_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(string.as_str())` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:65:24 + --> tests/ui/clones_into_boxed_slices.rs:86:24 | LL | let _: Box = c_str.to_owned().into_boxed_c_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(c_str)` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:68:24 + --> tests/ui/clones_into_boxed_slices.rs:89:24 | LL | let _: Box = c_string.clone().into_boxed_c_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(c_string.as_c_str())` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:70:24 + --> tests/ui/clones_into_boxed_slices.rs:91:24 | LL | let _: Box = c_string.to_owned().into_boxed_c_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(c_string.as_c_str())` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:72:24 + --> tests/ui/clones_into_boxed_slices.rs:93:24 | LL | let _: Box = CString::from(c_str).into_boxed_c_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(c_str)` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:76:25 + --> tests/ui/clones_into_boxed_slices.rs:97:25 | LL | let _: Box = os_str.to_owned().into_boxed_os_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(os_str)` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:78:25 + --> tests/ui/clones_into_boxed_slices.rs:99:25 | LL | let _: Box = os_str.to_os_string().into_boxed_os_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(os_str)` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:81:25 + --> tests/ui/clones_into_boxed_slices.rs:102:25 | LL | let _: Box = os_string.clone().into_boxed_os_str(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(os_string.as_os_str())` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:85:24 + --> tests/ui/clones_into_boxed_slices.rs:106:24 | LL | let _: Box = path.to_owned().into_boxed_path(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(path)` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:87:24 + --> tests/ui/clones_into_boxed_slices.rs:108:24 | LL | let _: Box = path.to_path_buf().into_boxed_path(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(path)` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:90:24 + --> tests/ui/clones_into_boxed_slices.rs:111:24 | LL | let _: Box = path_buf.clone().into_boxed_path(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(path_buf.as_path())` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:92:24 + --> tests/ui/clones_into_boxed_slices.rs:113:24 | LL | let _: Box = PathBuf::from("./").into_boxed_path(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(Path::new("./"))` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:102:25 + --> tests/ui/clones_into_boxed_slices.rs:123:25 | LL | let _: Box<[u32]> = test_vec.clone().into_boxed_slice(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(&test_vec[..])` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:105:25 + --> tests/ui/clones_into_boxed_slices.rs:126:25 | LL | let _: Box<[u32]> = Vec::from(slice).into_boxed_slice(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(slice)` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:107:25 + --> tests/ui/clones_into_boxed_slices.rs:128:25 | LL | let _: Box<[u32]> = slice.to_owned().into_boxed_slice(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(slice)` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:109:25 + --> tests/ui/clones_into_boxed_slices.rs:130:25 | LL | let _: Box<[u32]> = slice.to_vec().into_boxed_slice(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(slice)` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:118:23 + --> tests/ui/clones_into_boxed_slices.rs:144:23 | -LL | let _: Box = create_str!("te", "st").to_string().into_boxed_str(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from(create_str!("te", "st"))` +LL | let _: Box = String::from(&{ s.to_string() }).into_boxed_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from({ s.to_string() }.as_str())` error: clone into boxed slice - --> tests/ui/clones_into_boxed_slices.rs:30:9 - | -LL | $s.to_string().into_boxed_str() - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from("test")` -... -LL | let _: Box = in_macro!("test"); - | ----------------- in this macro invocation + --> tests/ui/clones_into_boxed_slices.rs:147:23 | - = note: this error originates in the macro `in_macro` (in Nightly builds, run with -Z macro-backtrace for more info) +LL | let _: Box = (&{ s }).to_string().into_boxed_str(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Box::from({ s })` error: aborting due to 28 previous errors