Add lint clones_into_boxed_slices

Author: aedank0

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

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()"));
+                }
+            }
+        }
+    }
+}

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,

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::<replace_box::ReplaceBox>::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);

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,

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<str> = Box::from(s);
+    //~^ clones_into_boxed_slices
+    let _: Box<str> = Box::from(s);
+    //~^ clones_into_boxed_slices
+    let ref_s = &s;
+    let _: Box<str> = Box::from(*ref_s);
+    //~^ clones_into_boxed_slices
+    let boxed_s: Box<str> = Box::from(s);
+    let _: Box<str> = boxed_s.clone();
+    //~^ clones_into_boxed_slices
+    let rc_s: Rc<str> = Rc::from(s);
+    let _: Box<str> = Box::from(&*rc_s);
+    //~^ clones_into_boxed_slices
+    let _: Box<str> = Box::from(s);
+    //~^ clones_into_boxed_slices
+    let _: Box<str> = Box::from(&s[..2]);
+    //~^ clones_into_boxed_slices
+    let _: Box<str> = Box::from(s);
+    //~^ clones_into_boxed_slices
+    let string = String::from(s);
+    let _: Box<str> = Box::from(string.as_str());
+    //~^ clones_into_boxed_slices
+    let _: Box<str> = Box::from(string.as_str());
+    //~^ clones_into_boxed_slices
+    let _: Box<str> = Box::from(string.as_str());
+    //~^ clones_into_boxed_slices
+
+    let c_str = c"test";
+    let _: Box<CStr> = Box::from(c_str);
+    //~^ clones_into_boxed_slices
+    let c_string = CString::from(c_str);
+    let _: Box<CStr> = Box::from(c_string.as_c_str());
+    //~^ clones_into_boxed_slices
+    let _: Box<CStr> = Box::from(c_string.as_c_str());
+    //~^ clones_into_boxed_slices
+    let _: Box<CStr> = Box::from(c_str);
+    //~^ clones_into_boxed_slices
+
+    let os_str = OsStr::new("test");
+    let _: Box<OsStr> = Box::from(os_str);
+    //~^ clones_into_boxed_slices
+    let _: Box<OsStr> = Box::from(os_str);
+    //~^ clones_into_boxed_slices
+    let os_string = OsString::from(os_str);
+    let _: Box<OsStr> = Box::from(os_string.as_os_str());
+    //~^ clones_into_boxed_slices
+
+    let path = Path::new("./");
+    let _: Box<Path> = Box::from(path);
+    //~^ clones_into_boxed_slices
+    let _: Box<Path> = Box::from(path);
+    //~^ clones_into_boxed_slices
+    let path_buf = PathBuf::from("./");
+    let _: Box<Path> = Box::from(path_buf.as_path());
+    //~^ clones_into_boxed_slices
+    let _: Box<Path> = 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<OsStr> = OsString::from(to_os_str).into_boxed_os_str();
+    let to_path = String::from("./");
+    let _: Box<Path> = 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<str> = Dummy {}.to_string().into_boxed_str();
+
+    // Do lint when only inner comes from macro
+    let _: Box<str> = Box::from(create_str!("te", "st"));
+    //~^ clones_into_boxed_slices
+
+    // Don't lint when only part is in macro
+    let _: Box<str> = to_string!("test").into_boxed_str();
+
+    // Don't lint here but do lint in the macro def
+    let _: Box<str> = in_macro!("test");
+}