Skip to content

Commit

Permalink
Improve redundant_slicing lint
Browse files Browse the repository at this point in the history
* Lint when slicing triggers auto-deref
* Lint when slicing returns the same type as dereferencing
  • Loading branch information
Jarcho committed Feb 17, 2022
1 parent e2dc8ca commit faeeef3
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 31 deletions.
67 changes: 49 additions & 18 deletions clippy_lints/src/redundant_slicing.rs
@@ -1,11 +1,12 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::get_parent_expr;
use clippy_utils::source::snippet_with_context;
use clippy_utils::ty::is_type_lang_item;
use clippy_utils::ty::{is_type_lang_item, peel_mid_ty_refs};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{BorrowKind, Expr, ExprKind, LangItem, Mutability};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::subst::GenericArg;
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
Expand Down Expand Up @@ -53,34 +54,64 @@ impl<'tcx> LateLintPass<'tcx> for RedundantSlicing {
if addressee.span.ctxt() == ctxt;
if let ExprKind::Index(indexed, range) = addressee.kind;
if is_type_lang_item(cx, cx.typeck_results().expr_ty_adjusted(range), LangItem::RangeFull);
if cx.typeck_results().expr_ty(expr) == cx.typeck_results().expr_ty(indexed);
then {
let (expr_ty, expr_ref_count) = peel_mid_ty_refs(cx.typeck_results().expr_ty(expr));
let (indexed_ty, indexed_ref_count) = peel_mid_ty_refs(cx.typeck_results().expr_ty(indexed));
let mut app = Applicability::MachineApplicable;
let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0;

let (reborrow_str, help_str) = if mutability == Mutability::Mut {
// The slice was used to reborrow the mutable reference.
("&mut *", "reborrow the original value instead")
} else if matches!(
get_parent_expr(cx, expr),
Some(Expr {
kind: ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _),
..
})
) {
// The slice was used to make a temporary reference.
("&*", "reborrow the original value instead")
let (help, sugg) = if expr_ty == indexed_ty {
if expr_ref_count > indexed_ref_count {
return;
}

let (reborrow_str, help_str) = if mutability == Mutability::Mut {
// The slice was used to reborrow the mutable reference.
("&mut *", "reborrow the original value instead")
} else if matches!(
get_parent_expr(cx, expr),
Some(Expr {
kind: ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _),
..
})
) {
// The slice was used to make a temporary reference.
("&*", "reborrow the original value instead")
} else if expr_ref_count != indexed_ref_count {
("", "dereference the original value instead")
} else {
("", "use the original value instead")
};

let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0;
(help_str, format!("{}{}{}", reborrow_str, "*".repeat(indexed_ref_count - expr_ref_count), snip))
} else if let Some(target_id) = cx.tcx.lang_items().deref_target() {
if let Ok(deref_ty) = cx.tcx.try_normalize_erasing_regions(
cx.param_env,
cx.tcx.mk_projection(target_id, cx.tcx.mk_substs([GenericArg::from(indexed_ty)].into_iter())),
) {
if deref_ty == expr_ty {
let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0;
(
"dereference the original value instead",
format!("&{}{}*{}", mutability.prefix_str(), "*".repeat(indexed_ref_count), snip),
)
} else {
return;
}
} else {
return;
}
} else {
("", "use the original value instead")
return;
};

span_lint_and_sugg(
cx,
REDUNDANT_SLICING,
expr.span,
"redundant slicing of the whole range",
help_str,
format!("{}{}", reborrow_str, snip),
help,
sugg,
app,
);
}
Expand Down
40 changes: 40 additions & 0 deletions tests/ui/redundant_slicing.fixed
@@ -0,0 +1,40 @@
// run-rustfix

#![allow(unused)]
#![warn(clippy::redundant_slicing)]

fn main() {
let slice: &[u32] = &[0];
let _ = slice; // Redundant slice

let v = vec![0];
let _ = &*v; // Deref instead of slice
let _ = (&*v); // Outer borrow is redundant

static S: &[u8] = &[0, 1, 2];
let err = &mut &*S; // Should reborrow instead of slice

let mut vec = vec![0];
let mut_slice = &mut *vec; // Deref instead of slice
let _ = &mut *mut_slice; // Should reborrow instead of slice

let ref_vec = &vec;
let _ = &**ref_vec; // Deref instead of slice

macro_rules! m {
($e:expr) => {
$e
};
}
let _ = slice;

macro_rules! m2 {
($e:expr) => {
&$e[..]
};
}
let _ = m2!(slice); // Don't lint in a macro

let slice_ref = &slice;
let _ = *slice_ref; // Deref instead of slice
}
16 changes: 12 additions & 4 deletions tests/ui/redundant_slicing.rs
@@ -1,21 +1,26 @@
// run-rustfix

#![allow(unused)]
#![warn(clippy::redundant_slicing)]

fn main() {
let slice: &[u32] = &[0];
let _ = &slice[..];
let _ = &slice[..]; // Redundant slice

let v = vec![0];
let _ = &v[..]; // Changes the type
let _ = &(&v[..])[..]; // Outer borrow is redundant
let _ = &v[..]; // Deref instead of slice
let _ = &(&*v)[..]; // Outer borrow is redundant

static S: &[u8] = &[0, 1, 2];
let err = &mut &S[..]; // Should reborrow instead of slice

let mut vec = vec![0];
let mut_slice = &mut *vec;
let mut_slice = &mut vec[..]; // Deref instead of slice
let _ = &mut mut_slice[..]; // Should reborrow instead of slice

let ref_vec = &vec;
let _ = &ref_vec[..]; // Deref instead of slice

macro_rules! m {
($e:expr) => {
$e
Expand All @@ -29,4 +34,7 @@ fn main() {
};
}
let _ = m2!(slice); // Don't lint in a macro

let slice_ref = &slice;
let _ = &slice_ref[..]; // Deref instead of slice
}
42 changes: 33 additions & 9 deletions tests/ui/redundant_slicing.stderr
@@ -1,34 +1,58 @@
error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:6:13
--> $DIR/redundant_slicing.rs:8:13
|
LL | let _ = &slice[..];
LL | let _ = &slice[..]; // Redundant slice
| ^^^^^^^^^^ help: use the original value instead: `slice`
|
= note: `-D clippy::redundant-slicing` implied by `-D warnings`

error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:10:13
--> $DIR/redundant_slicing.rs:11:13
|
LL | let _ = &(&v[..])[..]; // Outer borrow is redundant
| ^^^^^^^^^^^^^ help: use the original value instead: `(&v[..])`
LL | let _ = &v[..]; // Deref instead of slice
| ^^^^^^ help: dereference the original value instead: `&*v`

error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:13:20
--> $DIR/redundant_slicing.rs:12:13
|
LL | let _ = &(&*v)[..]; // Outer borrow is redundant
| ^^^^^^^^^^ help: use the original value instead: `(&*v)`

error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:15:20
|
LL | let err = &mut &S[..]; // Should reborrow instead of slice
| ^^^^^^ help: reborrow the original value instead: `&*S`

error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:17:13
--> $DIR/redundant_slicing.rs:18:21
|
LL | let mut_slice = &mut vec[..]; // Deref instead of slice
| ^^^^^^^^^^^^ help: dereference the original value instead: `&mut *vec`

error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:19:13
|
LL | let _ = &mut mut_slice[..]; // Should reborrow instead of slice
| ^^^^^^^^^^^^^^^^^^ help: reborrow the original value instead: `&mut *mut_slice`

error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:24:13
--> $DIR/redundant_slicing.rs:22:13
|
LL | let _ = &ref_vec[..]; // Deref instead of slice
| ^^^^^^^^^^^^ help: dereference the original value instead: `&**ref_vec`

error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:29:13
|
LL | let _ = &m!(slice)[..];
| ^^^^^^^^^^^^^^ help: use the original value instead: `slice`

error: aborting due to 5 previous errors
error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:39:13
|
LL | let _ = &slice_ref[..]; // Deref instead of slice
| ^^^^^^^^^^^^^^ help: dereference the original value instead: `*slice_ref`

error: aborting due to 9 previous errors

0 comments on commit faeeef3

Please sign in to comment.