Improve redundant_slicing lint

* Lint when slicing triggers auto-deref
* Lint when slicing returns the same type as dereferencing
This commit is contained in:
Jason Newcomb 2022-01-03 15:25:00 -05:00
parent e2dc8ca080
commit faeeef3b9c
4 changed files with 134 additions and 31 deletions

View File

@ -1,11 +1,12 @@
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::get_parent_expr; use clippy_utils::get_parent_expr;
use clippy_utils::source::snippet_with_context; 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 if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::{BorrowKind, Expr, ExprKind, LangItem, Mutability}; use rustc_hir::{BorrowKind, Expr, ExprKind, LangItem, Mutability};
use rustc_lint::{LateContext, LateLintPass}; use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::subst::GenericArg;
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
declare_clippy_lint! { declare_clippy_lint! {
@ -53,25 +54,55 @@ impl<'tcx> LateLintPass<'tcx> for RedundantSlicing {
if addressee.span.ctxt() == ctxt; if addressee.span.ctxt() == ctxt;
if let ExprKind::Index(indexed, range) = addressee.kind; if let ExprKind::Index(indexed, range) = addressee.kind;
if is_type_lang_item(cx, cx.typeck_results().expr_ty_adjusted(range), LangItem::RangeFull); 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 { 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 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 { let (help, sugg) = if expr_ty == indexed_ty {
// The slice was used to reborrow the mutable reference. if expr_ref_count > indexed_ref_count {
("&mut *", "reborrow the original value instead") return;
} else if matches!( }
get_parent_expr(cx, expr),
Some(Expr { let (reborrow_str, help_str) = if mutability == Mutability::Mut {
kind: ExprKind::AddrOf(BorrowKind::Ref, 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),
// The slice was used to make a temporary reference. Some(Expr {
("&*", "reborrow the original value instead") 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 { } else {
("", "use the original value instead") return;
}; };
span_lint_and_sugg( span_lint_and_sugg(
@ -79,8 +110,8 @@ impl<'tcx> LateLintPass<'tcx> for RedundantSlicing {
REDUNDANT_SLICING, REDUNDANT_SLICING,
expr.span, expr.span,
"redundant slicing of the whole range", "redundant slicing of the whole range",
help_str, help,
format!("{}{}", reborrow_str, snip), sugg,
app, app,
); );
} }

View File

@ -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
}

View File

@ -1,21 +1,26 @@
// run-rustfix
#![allow(unused)] #![allow(unused)]
#![warn(clippy::redundant_slicing)] #![warn(clippy::redundant_slicing)]
fn main() { fn main() {
let slice: &[u32] = &[0]; let slice: &[u32] = &[0];
let _ = &slice[..]; let _ = &slice[..]; // Redundant slice
let v = vec![0]; let v = vec![0];
let _ = &v[..]; // Changes the type let _ = &v[..]; // Deref instead of slice
let _ = &(&v[..])[..]; // Outer borrow is redundant let _ = &(&*v)[..]; // Outer borrow is redundant
static S: &[u8] = &[0, 1, 2]; static S: &[u8] = &[0, 1, 2];
let err = &mut &S[..]; // Should reborrow instead of slice let err = &mut &S[..]; // Should reborrow instead of slice
let mut vec = vec![0]; 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 _ = &mut mut_slice[..]; // Should reborrow instead of slice
let ref_vec = &vec;
let _ = &ref_vec[..]; // Deref instead of slice
macro_rules! m { macro_rules! m {
($e:expr) => { ($e:expr) => {
$e $e
@ -29,4 +34,7 @@ fn main() {
}; };
} }
let _ = m2!(slice); // Don't lint in a macro let _ = m2!(slice); // Don't lint in a macro
let slice_ref = &slice;
let _ = &slice_ref[..]; // Deref instead of slice
} }

View File

@ -1,34 +1,58 @@
error: redundant slicing of the whole range 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` | ^^^^^^^^^^ help: use the original value instead: `slice`
| |
= note: `-D clippy::redundant-slicing` implied by `-D warnings` = note: `-D clippy::redundant-slicing` implied by `-D warnings`
error: redundant slicing of the whole range 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 LL | let _ = &v[..]; // Deref instead of slice
| ^^^^^^^^^^^^^ help: use the original value instead: `(&v[..])` | ^^^^^^ help: dereference the original value instead: `&*v`
error: redundant slicing of the whole range 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 LL | let err = &mut &S[..]; // Should reborrow instead of slice
| ^^^^^^ help: reborrow the original value instead: `&*S` | ^^^^^^ help: reborrow the original value instead: `&*S`
error: redundant slicing of the whole range 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 LL | let _ = &mut mut_slice[..]; // Should reborrow instead of slice
| ^^^^^^^^^^^^^^^^^^ help: reborrow the original value instead: `&mut *mut_slice` | ^^^^^^^^^^^^^^^^^^ help: reborrow the original value instead: `&mut *mut_slice`
error: redundant slicing of the whole range 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)[..]; LL | let _ = &m!(slice)[..];
| ^^^^^^^^^^^^^^ help: use the original value instead: `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