diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a98217f625a..4247bf614a90 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/clippy_lints/src/box_ref.rs b/clippy_lints/src/box_ref.rs new file mode 100644 index 000000000000..8b137891791f --- /dev/null +++ b/clippy_lints/src/box_ref.rs @@ -0,0 +1 @@ + diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 5fcb851dfebc..b5bf95223781 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs old mode 100644 new mode 100755 index 92eb3d7a7c59..a3692834e5a1 --- a/clippy_lints/src/lib.rs +++ b/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` } diff --git a/clippy_lints/src/redundant_box.rs b/clippy_lints/src/redundant_box.rs new file mode 100644 index 000000000000..6d9f6babcd8c --- /dev/null +++ b/clippy_lints/src/redundant_box.rs @@ -0,0 +1,73 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::qpath_generic_tys; +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, LintContext}; +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) + && let TyKind::Path(path) = hir_ty.kind + && let Some(boxed_ty) = qpath_generic_tys(&path).next() + { + emit_lint(cx, hir_ty.span, boxed_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 + { + emit_lint(cx, expr.span, span); + } + } +} + +fn is_thin_type<'tcx>(cx: &LateContext<'tcx>, ty: rustc_middle::ty::Ty<'tcx>) -> bool { + ty.is_sized(cx.tcx, cx.typing_env()) && { + let size = 8 * approx_ty_size(cx, ty); + 0 < size && size <= cx.sess().target.pointer_width as u64 + } +} + +fn emit_lint(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, "")), + Applicability::MachineApplicable, + ); +} diff --git a/tests/ui/redundant_box.fixed b/tests/ui/redundant_box.fixed new file mode 100644 index 000000000000..698bd750ea3a --- /dev/null +++ b/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::::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 some of 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 { + //~^ redundant_box + t + //~^ redundant_box + } + + let thin = 1u32; + let redundant = &thin; + //~^ redundant_box +} + +fn generic_boxed_wide_ref_is_ok_to_box() { + fn fun(ok_to_box: T) -> Box { + Box::new(ok_to_box) + } + + let wide = ""; + let ok_boxed_wide_ref = fun(wide); +} + +// Box::new(SomeT) where sizeof::() <= sizeof::() +// 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); +// } diff --git a/tests/ui/redundant_box.rs b/tests/ui/redundant_box.rs new file mode 100755 index 000000000000..3832c73d2de3 --- /dev/null +++ b/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::::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 some of 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) -> 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(ok_to_box: T) -> Box { + Box::new(ok_to_box) + } + + let wide = ""; + let ok_boxed_wide_ref = fun(wide); +} + +// Box::new(SomeT) where sizeof::() <= sizeof::() +// 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); +// } diff --git a/tests/ui/redundant_box.stderr b/tests/ui/redundant_box.stderr new file mode 100644 index 000000000000..20ce335e025e --- /dev/null +++ b/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) -> 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 +