redundant_box lint

Co-Authored-by: Félix Fischer <felix91gr@gmail.com>
Co-Authored-by: Hugo McNally <hugo.mcnally@gmail.com>

Author: 3 people

CHANGELOG.md

@@ -6201,6 +6201,7 @@ Released 2018-09-13
 [`redundant_as_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_as_str
 [`redundant_async_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_async_block
 [`redundant_at_rest_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_at_rest_pattern
+[`redundant_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_box
 [`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone
 [`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
 [`redundant_closure_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_call

clippy_lints/src/box_ref.rs

@@ -0,0 +1 @@
+

clippy_lints/src/declared_lints.rs

@@ -631,6 +631,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT_INFO,
     crate::read_zero_byte_vec::READ_ZERO_BYTE_VEC_INFO,
     crate::redundant_async_block::REDUNDANT_ASYNC_BLOCK_INFO,
+    crate::redundant_box::REDUNDANT_BOX_INFO,
     crate::redundant_clone::REDUNDANT_CLONE_INFO,
     crate::redundant_closure_call::REDUNDANT_CLOSURE_CALL_INFO,
     crate::redundant_else::REDUNDANT_ELSE_INFO,

clippy_lints/src/lib.rs

@@ -310,6 +310,7 @@ mod raw_strings;
 mod rc_clone_in_vec_init;
 mod read_zero_byte_vec;
 mod redundant_async_block;
+mod redundant_box;
 mod redundant_clone;
 mod redundant_closure_call;
 mod redundant_else;
@@ -946,5 +947,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap));
     store.register_late_pass(move |_| Box::new(redundant_test_prefix::RedundantTestPrefix));
     store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf)));
+    store.register_late_pass(|_| Box::new(redundant_box::RedundantBox));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_lints/src/redundant_box.rs

@@ -0,0 +1,78 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet;
+use clippy_utils::ty::approx_ty_size;
+use rustc_errors::Applicability;
+use rustc_hir::{AmbigArg, Expr, ExprKind, GenericArg, Path, PathSegment, QPath, Ty, TyKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::declare_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// ### Why is this bad?
+    ///
+    /// ### Example
+    /// ```no_run
+    /// // example code where clippy issues a warning
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// // example code which does not raise clippy warning
+    /// ```
+    #[clippy::version = "1.88.0"]
+    pub REDUNDANT_BOX,
+    nursery,
+    "default lint description"
+}
+
+// TODO Rename lint as we are not just checking references anymore
+declare_lint_pass!(RedundantBox => [REDUNDANT_BOX]);
+
+// TODO could we do everything with only check_ty() xor check_expr()?
+impl LateLintPass<'_> for RedundantBox {
+    fn check_ty<'tcx>(&mut self, cx: &LateContext<'tcx>, hir_ty: &Ty<'tcx, AmbigArg>) {
+        let ty = clippy_utils::ty::ty_from_hir_ty(cx, hir_ty.as_unambig_ty());
+        if let Some(boxed_ty) = ty.boxed_ty()
+            && is_thin_type(cx, boxed_ty)
+        // Extract the contained type for the lint suggestion span
+        // TODO is there a simpler way to do this?:
+            && let TyKind::Path(QPath::Resolved(_, Path { segments, .. })) = hir_ty.kind
+            && let [PathSegment { args: Some(args), .. }] = segments
+            && let [GenericArg::Type(ty)] = args.args
+        {
+            span_lint_and_sugg_(cx, hir_ty.span, ty.span);
+        }
+    }
+
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
+        let ty = cx.typeck_results().expr_ty(expr);
+        if let Some(boxed_ty) = ty.boxed_ty()
+            && is_thin_type(cx, boxed_ty)
+            && let ExprKind::Call(_, &[Expr { span, .. }]) = expr.kind
+        {
+            span_lint_and_sugg_(cx, expr.span, span);
+        }
+    }
+}
+
+fn is_thin_type<'tcx>(cx: &LateContext<'tcx>, ty: rustc_middle::ty::Ty<'tcx>) -> bool {
+    //TODO: usize's width will be the host's so lints may be misleading when the intended
+    // target is a different architecture. Can/should we do someting about it? Maybe make it
+    // configurable?
+    ty.is_sized(cx.tcx, cx.typing_env()) && {
+        let size = approx_ty_size(cx, ty);
+        0 < size && size <= size_of::<usize>() as u64
+    }
+}
+
+fn span_lint_and_sugg_(cx: &LateContext<'_>, from_span: rustc_span::Span, to_span: rustc_span::Span) {
+    span_lint_and_sugg(
+        cx,
+        REDUNDANT_BOX,
+        from_span,
+        "TODO: lint msg",
+        "Remove Box",
+        format!("{}", snippet(cx, to_span, "<default>")),
+        Applicability::MachineApplicable,
+    );
+}

tests/ui/redundant_box.fixed

@@ -0,0 +1,89 @@
+#![warn(clippy::redundant_box)]
+
+fn main() {}
+
+fn string_slice_is_ok_to_box() {
+    let the_slice = "";
+    let the_ok = Box::new(the_slice);
+}
+
+fn ref_to_string_slice_is_redundant_to_box() {
+    let the_slice = "";
+    let redundant = &the_slice;
+    //~^ redundant_box
+}
+
+fn vec_slice_is_ok_to_box() {
+    let the_vec = Vec::<u8>::new();
+    let the_slice = &the_vec[0..3];
+    let the_ok = Box::new(the_slice);
+}
+
+fn wide_int_is_ok_to_box() {
+    let the_wide_int = 1u128;
+    let the_ok = Box::new(the_wide_int);
+}
+
+fn wide_int_ref_is_redundant_to_box() {
+    let the_wide_int = 1u128;
+    let redundant = &the_wide_int;
+    //~^ redundant_box
+}
+
+// Tests for the cases listed on https://github.com/rust-lang/rust-clippy/issues/2394
+// Box<&T>
+//TODO: Maybe these could go away as they are caught by `clippy::redundant_allocation``
+fn generic_boxed_thin_ref_is_redundant_to_box() {
+    #[allow(clippy::redundant_allocation)]
+    fn fun<T>(t: &T) -> &T {
+        //~^ redundant_box
+        t
+        //~^ redundant_box
+    }
+
+    let thin = 1u32;
+    let redundant = &thin;
+    //~^ redundant_box
+}
+
+fn generic_boxed_wide_ref_is_ok_to_box() {
+    fn fun<T>(ok_to_box: T) -> Box<T> {
+        Box::new(ok_to_box)
+    }
+
+    let wide = "";
+    let ok_boxed_wide_ref = fun(wide);
+}
+
+// Box::new(SomeT) where sizeof::<T>() <= sizeof::<usize>()
+// unless there are Box::into_raw calls within the function
+fn smaller_than_usize_is_redundant_to_box() {
+    let redundant = 1u16;
+    //~^ redundant_box
+}
+
+fn usize_ref_is_redundant_to_box() {
+    let the_val = 1usize;
+    let redundant = &the_val;
+    //~^ redundant_box
+}
+
+fn reference_to_a_literal_is_redundant_to_box() {
+    let a = 1u32;
+    let redundant = &a;
+    //~^ redundant_box
+}
+
+fn type_annotations_of_a_boxed_int_is_redundant_to_box() {
+    let a = 1u32;
+    let redundant = &a;
+    //~^ redundant_box
+    let redundant: &u32 = redundant;
+    //~^ redundant_box
+}
+
+// TODO: Investigate how to implement this:
+// fn smaller_than_usize_is_ok_to_box_if_using_into_raw() {
+//     let boxed = Box::new(1u8);
+//     let ptr = Box::into_raw(boxed);
+// }

tests/ui/redundant_box.rs

@@ -0,0 +1,89 @@
+#![warn(clippy::redundant_box)]
+
+fn main() {}
+
+fn string_slice_is_ok_to_box() {
+    let the_slice = "";
+    let the_ok = Box::new(the_slice);
+}
+
+fn ref_to_string_slice_is_redundant_to_box() {
+    let the_slice = "";
+    let redundant = Box::new(&the_slice);
+    //~^ redundant_box
+}
+
+fn vec_slice_is_ok_to_box() {
+    let the_vec = Vec::<u8>::new();
+    let the_slice = &the_vec[0..3];
+    let the_ok = Box::new(the_slice);
+}
+
+fn wide_int_is_ok_to_box() {
+    let the_wide_int = 1u128;
+    let the_ok = Box::new(the_wide_int);
+}
+
+fn wide_int_ref_is_redundant_to_box() {
+    let the_wide_int = 1u128;
+    let redundant = Box::new(&the_wide_int);
+    //~^ redundant_box
+}
+
+// Tests for the cases listed on https://github.com/rust-lang/rust-clippy/issues/2394
+// Box<&T>
+//TODO: Maybe these could go away as they are caught by `clippy::redundant_allocation``
+fn generic_boxed_thin_ref_is_redundant_to_box() {
+    #[allow(clippy::redundant_allocation)]
+    fn fun<T>(t: &T) -> Box<&T> {
+        //~^ redundant_box
+        Box::new(t)
+        //~^ redundant_box
+    }
+
+    let thin = 1u32;
+    let redundant = fun(&thin);
+    //~^ redundant_box
+}
+
+fn generic_boxed_wide_ref_is_ok_to_box() {
+    fn fun<T>(ok_to_box: T) -> Box<T> {
+        Box::new(ok_to_box)
+    }
+
+    let wide = "";
+    let ok_boxed_wide_ref = fun(wide);
+}
+
+// Box::new(SomeT) where sizeof::<T>() <= sizeof::<usize>()
+// unless there are Box::into_raw calls within the function
+fn smaller_than_usize_is_redundant_to_box() {
+    let redundant = Box::new(1u16);
+    //~^ redundant_box
+}
+
+fn usize_ref_is_redundant_to_box() {
+    let the_val = 1usize;
+    let redundant = Box::new(&the_val);
+    //~^ redundant_box
+}
+
+fn reference_to_a_literal_is_redundant_to_box() {
+    let a = 1u32;
+    let redundant = Box::new(&a);
+    //~^ redundant_box
+}
+
+fn type_annotations_of_a_boxed_int_is_redundant_to_box() {
+    let a = 1u32;
+    let redundant = Box::new(&a);
+    //~^ redundant_box
+    let redundant: Box<&u32> = redundant;
+    //~^ redundant_box
+}
+
+// TODO: Investigate how to implement this:
+// fn smaller_than_usize_is_ok_to_box_if_using_into_raw() {
+//     let boxed = Box::new(1u8);
+//     let ptr = Box::into_raw(boxed);
+// }

tests/ui/redundant_box.stderr

@@ -0,0 +1,65 @@
+error: TODO: lint msg
+  --> tests/ui/redundant_box.rs:12:21
+   |
+LL |     let redundant = Box::new(&the_slice);
+   |                     ^^^^^^^^^^^^^^^^^^^^ help: Remove Box: `&the_slice`
+   |
+   = note: `-D clippy::redundant-box` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::redundant_box)]`
+
+error: TODO: lint msg
+  --> tests/ui/redundant_box.rs:29:21
+   |
+LL |     let redundant = Box::new(&the_wide_int);
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^ help: Remove Box: `&the_wide_int`
+
+error: TODO: lint msg
+  --> tests/ui/redundant_box.rs:38:25
+   |
+LL |     fn fun<T>(t: &T) -> Box<&T> {
+   |                         ^^^^^^^ help: Remove Box: `&T`
+
+error: TODO: lint msg
+  --> tests/ui/redundant_box.rs:40:9
+   |
+LL |         Box::new(t)
+   |         ^^^^^^^^^^^ help: Remove Box: `t`
+
+error: TODO: lint msg
+  --> tests/ui/redundant_box.rs:45:21
+   |
+LL |     let redundant = fun(&thin);
+   |                     ^^^^^^^^^^ help: Remove Box: `&thin`
+
+error: TODO: lint msg
+  --> tests/ui/redundant_box.rs:61:21
+   |
+LL |     let redundant = Box::new(1u16);
+   |                     ^^^^^^^^^^^^^^ help: Remove Box: `1u16`
+
+error: TODO: lint msg
+  --> tests/ui/redundant_box.rs:67:21
+   |
+LL |     let redundant = Box::new(&the_val);
+   |                     ^^^^^^^^^^^^^^^^^^ help: Remove Box: `&the_val`
+
+error: TODO: lint msg
+  --> tests/ui/redundant_box.rs:73:21
+   |
+LL |     let redundant = Box::new(&a);
+   |                     ^^^^^^^^^^^^ help: Remove Box: `&a`
+
+error: TODO: lint msg
+  --> tests/ui/redundant_box.rs:79:21
+   |
+LL |     let redundant = Box::new(&a);
+   |                     ^^^^^^^^^^^^ help: Remove Box: `&a`
+
+error: TODO: lint msg
+  --> tests/ui/redundant_box.rs:81:20
+   |
+LL |     let redundant: Box<&u32> = redundant;
+   |                    ^^^^^^^^^ help: Remove Box: `&u32`
+
+error: aborting due to 10 previous errors
+