Skip to content

New redundant boxed types lint #14919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/box_ref.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}
73 changes: 73 additions & 0 deletions clippy_lints/src/redundant_box.rs
Original file line number Diff line number Diff line change
@@ -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, "<default>")),
Applicability::MachineApplicable,
);
}
89 changes: 89 additions & 0 deletions tests/ui/redundant_box.fixed
Original file line number Diff line number Diff line change
@@ -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 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) -> &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);
// }
89 changes: 89 additions & 0 deletions tests/ui/redundant_box.rs
Original file line number Diff line number Diff line change
@@ -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 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) -> 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);
// }
65 changes: 65 additions & 0 deletions tests/ui/redundant_box.stderr
Original file line number Diff line number Diff line change
@@ -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