Move some cases from redundant_slicing to deref_by_slicing

This commit is contained in:
Jason Newcomb 2022-02-13 18:43:19 -05:00
parent 113ac6e15e
commit 7724d6773d
7 changed files with 81 additions and 52 deletions

View File

@ -6,7 +6,7 @@
use rustc_ast::util::parser::PREC_PREFIX; use rustc_ast::util::parser::PREC_PREFIX;
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, Lint};
use rustc_middle::ty::adjustment::{Adjust, AutoBorrow, AutoBorrowMutability}; use rustc_middle::ty::adjustment::{Adjust, AutoBorrow, AutoBorrowMutability};
use rustc_middle::ty::subst::GenericArg; use rustc_middle::ty::subst::GenericArg;
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
@ -44,7 +44,7 @@
declare_clippy_lint! { declare_clippy_lint! {
/// ### What it does /// ### What it does
/// Checks for slicing expression which are equivalent to dereferencing the /// Checks for slicing expressions which are equivalent to dereferencing the
/// value. /// value.
/// ///
/// ### Why is this bad? /// ### Why is this bad?
@ -68,6 +68,9 @@
declare_lint_pass!(RedundantSlicing => [REDUNDANT_SLICING, DEREF_BY_SLICING]); declare_lint_pass!(RedundantSlicing => [REDUNDANT_SLICING, DEREF_BY_SLICING]);
static REDUNDANT_SLICING_LINT: (&Lint, &str) = (REDUNDANT_SLICING, "redundant slicing of the whole range");
static DEREF_BY_SLICING_LINT: (&Lint, &str) = (DEREF_BY_SLICING, "slicing when dereferencing would work");
impl<'tcx> LateLintPass<'tcx> for RedundantSlicing { impl<'tcx> LateLintPass<'tcx> for RedundantSlicing {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if expr.span.from_expansion() { if expr.span.from_expansion() {
@ -89,7 +92,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
}); });
let mut app = Applicability::MachineApplicable; let mut app = Applicability::MachineApplicable;
let (lint, msg, help, sugg) = if expr_ty == indexed_ty { let ((lint, msg), help, sugg) = if expr_ty == indexed_ty {
if expr_ref_count > indexed_ref_count { if expr_ref_count > indexed_ref_count {
// Indexing takes self by reference and can't return a reference to that // Indexing takes self by reference and can't return a reference to that
// reference as it's a local variable. The only way this could happen is if // reference as it's a local variable. The only way this could happen is if
@ -100,9 +103,9 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
} }
let deref_count = indexed_ref_count - expr_ref_count; let deref_count = indexed_ref_count - expr_ref_count;
let (reborrow_str, help_str) = if mutability == Mutability::Mut { let (lint, reborrow_str, help_str) = if mutability == Mutability::Mut {
// The slice was used to reborrow the mutable reference. // The slice was used to reborrow the mutable reference.
("&mut *", "reborrow the original value instead") (DEREF_BY_SLICING_LINT, "&mut *", "reborrow the original value instead")
} else if matches!( } else if matches!(
parent_expr, parent_expr,
Some(Expr { Some(Expr {
@ -113,11 +116,11 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
matches!(a.kind, Adjust::Borrow(AutoBorrow::Ref(_, AutoBorrowMutability::Mut { .. }))) matches!(a.kind, Adjust::Borrow(AutoBorrow::Ref(_, AutoBorrowMutability::Mut { .. })))
}) { }) {
// The slice was used to make a temporary reference. // The slice was used to make a temporary reference.
("&*", "reborrow the original value instead") (DEREF_BY_SLICING_LINT, "&*", "reborrow the original value instead")
} else if deref_count != 0 { } else if deref_count != 0 {
("", "dereference the original value instead") (DEREF_BY_SLICING_LINT, "", "dereference the original value instead")
} else { } else {
("", "use the original value instead") (REDUNDANT_SLICING_LINT, "", "use the original value instead")
}; };
let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0; let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0;
@ -127,7 +130,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
format!("{}{}{}", reborrow_str, "*".repeat(deref_count), snip) format!("{}{}{}", reborrow_str, "*".repeat(deref_count), snip)
}; };
(REDUNDANT_SLICING, "redundant slicing of the whole range", help_str, sugg) (lint, help_str, sugg)
} else if let Some(target_id) = cx.tcx.lang_items().deref_target() { } else if let Some(target_id) = cx.tcx.lang_items().deref_target() {
if let Ok(deref_ty) = cx.tcx.try_normalize_erasing_regions( if let Ok(deref_ty) = cx.tcx.try_normalize_erasing_regions(
cx.param_env, cx.param_env,
@ -140,7 +143,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
} else { } else {
format!("&{}{}*{}", mutability.prefix_str(), "*".repeat(indexed_ref_count), snip) format!("&{}{}*{}", mutability.prefix_str(), "*".repeat(indexed_ref_count), snip)
}; };
(DEREF_BY_SLICING, "slicing when dereferencing would work", "dereference the original value instead", sugg) (DEREF_BY_SLICING_LINT, "dereference the original value instead", sugg)
} else { } else {
return; return;
} }

View File

@ -2,6 +2,8 @@
#![warn(clippy::deref_by_slicing)] #![warn(clippy::deref_by_slicing)]
use std::io::Read;
fn main() { fn main() {
let mut vec = vec![0]; let mut vec = vec![0];
let _ = &*vec; let _ = &*vec;
@ -9,8 +11,19 @@ fn main() {
let ref_vec = &mut vec; let ref_vec = &mut vec;
let _ = &**ref_vec; let _ = &**ref_vec;
let _ = &mut **ref_vec; let mut_slice = &mut **ref_vec;
let _ = &mut *mut_slice; // Err, re-borrows slice
let s = String::new(); let s = String::new();
let _ = &*s; let _ = &*s;
static S: &[u8] = &[0, 1, 2];
let _ = &mut &*S; // Err, re-borrows slice
let slice: &[u32] = &[0u32, 1u32];
let slice_ref = &slice;
let _ = *slice_ref; // Err, derefs slice
let bytes: &[u8] = &[];
let _ = (&*bytes).read_to_end(&mut vec![]).unwrap(); // Err, re-borrows slice
} }

View File

@ -2,6 +2,8 @@
#![warn(clippy::deref_by_slicing)] #![warn(clippy::deref_by_slicing)]
use std::io::Read;
fn main() { fn main() {
let mut vec = vec![0]; let mut vec = vec![0];
let _ = &vec[..]; let _ = &vec[..];
@ -9,8 +11,19 @@ fn main() {
let ref_vec = &mut vec; let ref_vec = &mut vec;
let _ = &ref_vec[..]; let _ = &ref_vec[..];
let _ = &mut ref_vec[..]; let mut_slice = &mut ref_vec[..];
let _ = &mut mut_slice[..]; // Err, re-borrows slice
let s = String::new(); let s = String::new();
let _ = &s[..]; let _ = &s[..];
static S: &[u8] = &[0, 1, 2];
let _ = &mut &S[..]; // Err, re-borrows slice
let slice: &[u32] = &[0u32, 1u32];
let slice_ref = &slice;
let _ = &slice_ref[..]; // Err, derefs slice
let bytes: &[u8] = &[];
let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap(); // Err, re-borrows slice
} }

View File

@ -1,5 +1,5 @@
error: slicing when dereferencing would work error: slicing when dereferencing would work
--> $DIR/deref_by_slicing.rs:7:13 --> $DIR/deref_by_slicing.rs:9:13
| |
LL | let _ = &vec[..]; LL | let _ = &vec[..];
| ^^^^^^^^ help: dereference the original value instead: `&*vec` | ^^^^^^^^ help: dereference the original value instead: `&*vec`
@ -7,28 +7,52 @@ LL | let _ = &vec[..];
= note: `-D clippy::deref-by-slicing` implied by `-D warnings` = note: `-D clippy::deref-by-slicing` implied by `-D warnings`
error: slicing when dereferencing would work error: slicing when dereferencing would work
--> $DIR/deref_by_slicing.rs:8:13 --> $DIR/deref_by_slicing.rs:10:13
| |
LL | let _ = &mut vec[..]; LL | let _ = &mut vec[..];
| ^^^^^^^^^^^^ help: dereference the original value instead: `&mut *vec` | ^^^^^^^^^^^^ help: dereference the original value instead: `&mut *vec`
error: slicing when dereferencing would work error: slicing when dereferencing would work
--> $DIR/deref_by_slicing.rs:11:13 --> $DIR/deref_by_slicing.rs:13:13
| |
LL | let _ = &ref_vec[..]; LL | let _ = &ref_vec[..];
| ^^^^^^^^^^^^ help: dereference the original value instead: `&**ref_vec` | ^^^^^^^^^^^^ help: dereference the original value instead: `&**ref_vec`
error: slicing when dereferencing would work error: slicing when dereferencing would work
--> $DIR/deref_by_slicing.rs:12:13 --> $DIR/deref_by_slicing.rs:14:21
| |
LL | let _ = &mut ref_vec[..]; LL | let mut_slice = &mut ref_vec[..];
| ^^^^^^^^^^^^^^^^ help: dereference the original value instead: `&mut **ref_vec` | ^^^^^^^^^^^^^^^^ help: dereference the original value instead: `&mut **ref_vec`
error: slicing when dereferencing would work error: slicing when dereferencing would work
--> $DIR/deref_by_slicing.rs:15:13 --> $DIR/deref_by_slicing.rs:15:13
| |
LL | let _ = &mut mut_slice[..]; // Err, re-borrows slice
| ^^^^^^^^^^^^^^^^^^ help: reborrow the original value instead: `&mut *mut_slice`
error: slicing when dereferencing would work
--> $DIR/deref_by_slicing.rs:18:13
|
LL | let _ = &s[..]; LL | let _ = &s[..];
| ^^^^^^ help: dereference the original value instead: `&*s` | ^^^^^^ help: dereference the original value instead: `&*s`
error: aborting due to 5 previous errors error: slicing when dereferencing would work
--> $DIR/deref_by_slicing.rs:21:18
|
LL | let _ = &mut &S[..]; // Err, re-borrows slice
| ^^^^^^ help: reborrow the original value instead: `&*S`
error: slicing when dereferencing would work
--> $DIR/deref_by_slicing.rs:25:13
|
LL | let _ = &slice_ref[..]; // Err, derefs slice
| ^^^^^^^^^^^^^^ help: dereference the original value instead: `*slice_ref`
error: slicing when dereferencing would work
--> $DIR/deref_by_slicing.rs:28:13
|
LL | let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap(); // Err, re-borrows slice
| ^^^^^^^^^^^^ help: reborrow the original value instead: `(&*bytes)`
error: aborting due to 9 previous errors

View File

@ -14,11 +14,11 @@ fn main() {
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 _ = &mut &S[..]; // Ok, re-borrows slice
let mut vec = vec![0]; let mut vec = vec![0];
let mut_slice = &mut vec[..]; // Ok, results in `&mut [_]` let mut_slice = &mut vec[..]; // Ok, results in `&mut [_]`
let _ = &mut *mut_slice; // Should reborrow instead of slice let _ = &mut mut_slice[..]; // Ok, re-borrows slice
let ref_vec = &vec; let ref_vec = &vec;
let _ = &ref_vec[..]; // Ok, results in `&[_]` let _ = &ref_vec[..]; // Ok, results in `&[_]`
@ -38,9 +38,9 @@ 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 = &slice;
let _ = *slice_ref; // Deref instead of slice let _ = &slice_ref[..]; // Ok, derefs slice
// Issue #7972 // Issue #7972
let bytes: &[u8] = &[]; let bytes: &[u8] = &[];
let _ = (&*bytes).read_to_end(&mut vec![]).unwrap(); let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap(); // Ok, re-borrows slice
} }

View File

@ -14,11 +14,11 @@ fn main() {
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 _ = &mut &S[..]; // Ok, re-borrows slice
let mut vec = vec![0]; let mut vec = vec![0];
let mut_slice = &mut vec[..]; // Ok, results in `&mut [_]` let mut_slice = &mut vec[..]; // Ok, results in `&mut [_]`
let _ = &mut mut_slice[..]; // Should reborrow instead of slice let _ = &mut mut_slice[..]; // Ok, re-borrows slice
let ref_vec = &vec; let ref_vec = &vec;
let _ = &ref_vec[..]; // Ok, results in `&[_]` let _ = &ref_vec[..]; // Ok, results in `&[_]`
@ -38,9 +38,9 @@ macro_rules! m2 {
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 = &slice;
let _ = &slice_ref[..]; // Deref instead of slice let _ = &slice_ref[..]; // Ok, derefs slice
// Issue #7972 // Issue #7972
let bytes: &[u8] = &[]; let bytes: &[u8] = &[];
let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap(); let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap(); // Ok, re-borrows slice
} }

View File

@ -12,35 +12,11 @@ error: redundant slicing of the whole range
LL | let _ = &(&*v)[..]; // Outer borrow is redundant LL | let _ = &(&*v)[..]; // Outer borrow is redundant
| ^^^^^^^^^^ help: use the original value instead: `(&*v)` | ^^^^^^^^^^ help: use the original value instead: `(&*v)`
error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:17: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:21: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 error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:31:13 --> $DIR/redundant_slicing.rs:31:13
| |
LL | let _ = &m!(slice)[..]; LL | let _ = &m!(slice)[..];
| ^^^^^^^^^^^^^^ help: use the original value instead: `slice` | ^^^^^^^^^^^^^^ help: use the original value instead: `slice`
error: redundant slicing of the whole range error: aborting due to 3 previous errors
--> $DIR/redundant_slicing.rs:41:13
|
LL | let _ = &slice_ref[..]; // Deref instead of slice
| ^^^^^^^^^^^^^^ help: dereference the original value instead: `*slice_ref`
error: redundant slicing of the whole range
--> $DIR/redundant_slicing.rs:45:13
|
LL | let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap();
| ^^^^^^^^^^^^ help: reborrow the original value instead: `(&*bytes)`
error: aborting due to 7 previous errors