diff --git a/clippy_lints/src/redundant_slicing.rs b/clippy_lints/src/redundant_slicing.rs index 0de4541b641..25a9072ef6e 100644 --- a/clippy_lints/src/redundant_slicing.rs +++ b/clippy_lints/src/redundant_slicing.rs @@ -6,7 +6,7 @@ use rustc_ast::util::parser::PREC_PREFIX; use rustc_errors::Applicability; 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::subst::GenericArg; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -44,7 +44,7 @@ declare_clippy_lint! { /// ### What it does - /// Checks for slicing expression which are equivalent to dereferencing the + /// Checks for slicing expressions which are equivalent to dereferencing the /// value. /// /// ### Why is this bad? @@ -68,6 +68,9 @@ 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 { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { 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 (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 { // 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 @@ -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 (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. - ("&mut *", "reborrow the original value instead") + (DEREF_BY_SLICING_LINT, "&mut *", "reborrow the original value instead") } else if matches!( parent_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 { .. }))) }) { // 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 { - ("", "dereference the original value instead") + (DEREF_BY_SLICING_LINT, "", "dereference the original value instead") } 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; @@ -127,7 +130,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { 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() { if let Ok(deref_ty) = cx.tcx.try_normalize_erasing_regions( cx.param_env, @@ -140,7 +143,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { } else { 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 { return; } diff --git a/tests/ui/deref_by_slicing.fixed b/tests/ui/deref_by_slicing.fixed index edb19de0700..b26276218b7 100644 --- a/tests/ui/deref_by_slicing.fixed +++ b/tests/ui/deref_by_slicing.fixed @@ -2,6 +2,8 @@ #![warn(clippy::deref_by_slicing)] +use std::io::Read; + fn main() { let mut vec = vec![0]; let _ = &*vec; @@ -9,8 +11,19 @@ fn main() { let ref_vec = &mut 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; + + 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 } diff --git a/tests/ui/deref_by_slicing.rs b/tests/ui/deref_by_slicing.rs index 6d489a44e76..6aa1408ba17 100644 --- a/tests/ui/deref_by_slicing.rs +++ b/tests/ui/deref_by_slicing.rs @@ -2,6 +2,8 @@ #![warn(clippy::deref_by_slicing)] +use std::io::Read; + fn main() { let mut vec = vec![0]; let _ = &vec[..]; @@ -9,8 +11,19 @@ fn main() { let ref_vec = &mut 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[..]; + + 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 } diff --git a/tests/ui/deref_by_slicing.stderr b/tests/ui/deref_by_slicing.stderr index 89e4ca718b5..ffd76de378d 100644 --- a/tests/ui/deref_by_slicing.stderr +++ b/tests/ui/deref_by_slicing.stderr @@ -1,5 +1,5 @@ error: slicing when dereferencing would work - --> $DIR/deref_by_slicing.rs:7:13 + --> $DIR/deref_by_slicing.rs:9:13 | LL | let _ = &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` error: slicing when dereferencing would work - --> $DIR/deref_by_slicing.rs:8:13 + --> $DIR/deref_by_slicing.rs:10:13 | LL | let _ = &mut vec[..]; | ^^^^^^^^^^^^ help: dereference the original value instead: `&mut *vec` error: slicing when dereferencing would work - --> $DIR/deref_by_slicing.rs:11:13 + --> $DIR/deref_by_slicing.rs:13:13 | LL | let _ = &ref_vec[..]; | ^^^^^^^^^^^^ help: dereference the original value instead: `&**ref_vec` 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[..]; - | ^^^^^^^^^^^^^^^^ help: dereference the original value instead: `&mut **ref_vec` +LL | let mut_slice = &mut ref_vec[..]; + | ^^^^^^^^^^^^^^^^ help: dereference the original value instead: `&mut **ref_vec` error: slicing when dereferencing would work --> $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[..]; | ^^^^^^ 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 diff --git a/tests/ui/redundant_slicing.fixed b/tests/ui/redundant_slicing.fixed index da3d953375a..8dd8d309237 100644 --- a/tests/ui/redundant_slicing.fixed +++ b/tests/ui/redundant_slicing.fixed @@ -14,11 +14,11 @@ fn main() { let _ = (&*v); // Outer borrow is redundant 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_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[..]; // Ok, results in `&[_]` @@ -38,9 +38,9 @@ fn main() { let _ = m2!(slice); // Don't lint in a macro let slice_ref = &slice; - let _ = *slice_ref; // Deref instead of slice + let _ = &slice_ref[..]; // Ok, derefs slice // Issue #7972 let bytes: &[u8] = &[]; - let _ = (&*bytes).read_to_end(&mut vec![]).unwrap(); + let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap(); // Ok, re-borrows slice } diff --git a/tests/ui/redundant_slicing.rs b/tests/ui/redundant_slicing.rs index 125b1d4591c..51c16dd8d65 100644 --- a/tests/ui/redundant_slicing.rs +++ b/tests/ui/redundant_slicing.rs @@ -14,11 +14,11 @@ fn main() { let _ = &(&*v)[..]; // Outer borrow is redundant 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_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[..]; // Ok, results in `&[_]` @@ -38,9 +38,9 @@ macro_rules! m2 { let _ = m2!(slice); // Don't lint in a macro let slice_ref = &slice; - let _ = &slice_ref[..]; // Deref instead of slice + let _ = &slice_ref[..]; // Ok, derefs slice // Issue #7972 let bytes: &[u8] = &[]; - let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap(); + let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap(); // Ok, re-borrows slice } diff --git a/tests/ui/redundant_slicing.stderr b/tests/ui/redundant_slicing.stderr index b1c1c28894a..82367143c07 100644 --- a/tests/ui/redundant_slicing.stderr +++ b/tests/ui/redundant_slicing.stderr @@ -12,35 +12,11 @@ error: redundant slicing of the whole range 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: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 --> $DIR/redundant_slicing.rs:31:13 | LL | let _ = &m!(slice)[..]; | ^^^^^^^^^^^^^^ help: use the original value instead: `slice` -error: redundant slicing of the whole range - --> $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 +error: aborting due to 3 previous errors