diff --git a/CHANGELOG.md b/CHANGELOG.md index c9adf77c0..ef7139d63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3105,6 +3105,7 @@ Released 2018-09-13 [`deprecated_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_cfg_attr [`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver [`deref_addrof`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_addrof +[`deref_by_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_by_slicing [`derivable_impls`]: https://rust-lang.github.io/rust-clippy/master/index.html#derivable_impls [`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq [`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index a80320a57..2451a5aab 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -420,6 +420,7 @@ store.register_lints(&[ redundant_else::REDUNDANT_ELSE, redundant_field_names::REDUNDANT_FIELD_NAMES, redundant_pub_crate::REDUNDANT_PUB_CRATE, + redundant_slicing::DEREF_BY_SLICING, redundant_slicing::REDUNDANT_SLICING, redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES, ref_option_ref::REF_OPTION_REF, diff --git a/clippy_lints/src/lib.register_restriction.rs b/clippy_lints/src/lib.register_restriction.rs index 5a89fdb05..f89f35b88 100644 --- a/clippy_lints/src/lib.register_restriction.rs +++ b/clippy_lints/src/lib.register_restriction.rs @@ -51,6 +51,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve LintId::of(panic_unimplemented::UNIMPLEMENTED), LintId::of(panic_unimplemented::UNREACHABLE), LintId::of(pattern_type_mismatch::PATTERN_TYPE_MISMATCH), + LintId::of(redundant_slicing::DEREF_BY_SLICING), LintId::of(same_name_method::SAME_NAME_METHOD), LintId::of(shadow::SHADOW_REUSE), LintId::of(shadow::SHADOW_SAME), diff --git a/clippy_lints/src/redundant_slicing.rs b/clippy_lints/src/redundant_slicing.rs index 1cc0990b2..0de4541b6 100644 --- a/clippy_lints/src/redundant_slicing.rs +++ b/clippy_lints/src/redundant_slicing.rs @@ -42,7 +42,31 @@ declare_clippy_lint! { "redundant slicing of the whole range of a type" } -declare_lint_pass!(RedundantSlicing => [REDUNDANT_SLICING]); +declare_clippy_lint! { + /// ### What it does + /// Checks for slicing expression which are equivalent to dereferencing the + /// value. + /// + /// ### Why is this bad? + /// Some people may prefer to dereference rather than slice. + /// + /// ### Example + /// ```rust + /// let vec = vec![1, 2, 3]; + /// let slice = &vec[..]; + /// ``` + /// Use instead: + /// ```rust + /// let vec = vec![1, 2, 3]; + /// let slice = &*vec; + /// ``` + #[clippy::version = "1.60.0"] + pub DEREF_BY_SLICING, + restriction, + "slicing instead of dereferencing" +} + +declare_lint_pass!(RedundantSlicing => [REDUNDANT_SLICING, DEREF_BY_SLICING]); impl<'tcx> LateLintPass<'tcx> for RedundantSlicing { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { @@ -65,7 +89,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantSlicing { }); let mut app = Applicability::MachineApplicable; - let (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 @@ -103,7 +127,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantSlicing { format!("{}{}{}", reborrow_str, "*".repeat(deref_count), snip) }; - (help_str, sugg) + (REDUNDANT_SLICING, "redundant slicing of the whole range", 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, @@ -116,7 +140,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantSlicing { } else { format!("&{}{}*{}", mutability.prefix_str(), "*".repeat(indexed_ref_count), snip) }; - ("dereference the original value instead", sugg) + (DEREF_BY_SLICING, "slicing when dereferencing would work", "dereference the original value instead", sugg) } else { return; } @@ -129,9 +153,9 @@ impl<'tcx> LateLintPass<'tcx> for RedundantSlicing { span_lint_and_sugg( cx, - REDUNDANT_SLICING, + lint, expr.span, - "redundant slicing of the whole range", + msg, help, sugg, app, diff --git a/tests/ui/deref_by_slicing.fixed b/tests/ui/deref_by_slicing.fixed new file mode 100644 index 000000000..edb19de07 --- /dev/null +++ b/tests/ui/deref_by_slicing.fixed @@ -0,0 +1,16 @@ +// run-rustfix + +#![warn(clippy::deref_by_slicing)] + +fn main() { + let mut vec = vec![0]; + let _ = &*vec; + let _ = &mut *vec; + + let ref_vec = &mut vec; + let _ = &**ref_vec; + let _ = &mut **ref_vec; + + let s = String::new(); + let _ = &*s; +} diff --git a/tests/ui/deref_by_slicing.rs b/tests/ui/deref_by_slicing.rs new file mode 100644 index 000000000..6d489a44e --- /dev/null +++ b/tests/ui/deref_by_slicing.rs @@ -0,0 +1,16 @@ +// run-rustfix + +#![warn(clippy::deref_by_slicing)] + +fn main() { + let mut vec = vec![0]; + let _ = &vec[..]; + let _ = &mut vec[..]; + + let ref_vec = &mut vec; + let _ = &ref_vec[..]; + let _ = &mut ref_vec[..]; + + let s = String::new(); + let _ = &s[..]; +} diff --git a/tests/ui/deref_by_slicing.stderr b/tests/ui/deref_by_slicing.stderr new file mode 100644 index 000000000..89e4ca718 --- /dev/null +++ b/tests/ui/deref_by_slicing.stderr @@ -0,0 +1,34 @@ +error: slicing when dereferencing would work + --> $DIR/deref_by_slicing.rs:7:13 + | +LL | let _ = &vec[..]; + | ^^^^^^^^ help: dereference the original value instead: `&*vec` + | + = note: `-D clippy::deref-by-slicing` implied by `-D warnings` + +error: slicing when dereferencing would work + --> $DIR/deref_by_slicing.rs:8: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 + | +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 + | +LL | let _ = &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 _ = &s[..]; + | ^^^^^^ help: dereference the original value instead: `&*s` + +error: aborting due to 5 previous errors + diff --git a/tests/ui/redundant_slicing.fixed b/tests/ui/redundant_slicing.fixed index 98b2adec4..da3d95337 100644 --- a/tests/ui/redundant_slicing.fixed +++ b/tests/ui/redundant_slicing.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused)] +#![allow(unused, clippy::deref_by_slicing)] #![warn(clippy::redundant_slicing)] use std::io::Read; @@ -10,18 +10,18 @@ fn main() { let _ = slice; // Redundant slice let v = vec![0]; - let _ = &*v; // Deref instead of slice + let _ = &v[..]; // Ok, results in `&[_]` 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_slice = &mut vec[..]; // Ok, results in `&mut [_]` let _ = &mut *mut_slice; // Should reborrow instead of slice let ref_vec = &vec; - let _ = &**ref_vec; // Deref instead of slice + let _ = &ref_vec[..]; // Ok, results in `&[_]` macro_rules! m { ($e:expr) => { diff --git a/tests/ui/redundant_slicing.rs b/tests/ui/redundant_slicing.rs index b84a304ec..125b1d459 100644 --- a/tests/ui/redundant_slicing.rs +++ b/tests/ui/redundant_slicing.rs @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused)] +#![allow(unused, clippy::deref_by_slicing)] #![warn(clippy::redundant_slicing)] use std::io::Read; @@ -10,18 +10,18 @@ fn main() { let _ = &slice[..]; // Redundant slice let v = vec![0]; - let _ = &v[..]; // Deref instead of slice + let _ = &v[..]; // Ok, results in `&[_]` 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_slice = &mut vec[..]; // Ok, results in `&mut [_]` let _ = &mut mut_slice[..]; // Should reborrow instead of slice let ref_vec = &vec; - let _ = &ref_vec[..]; // Deref instead of slice + let _ = &ref_vec[..]; // Ok, results in `&[_]` macro_rules! m { ($e:expr) => { diff --git a/tests/ui/redundant_slicing.stderr b/tests/ui/redundant_slicing.stderr index bce351117..b1c1c2889 100644 --- a/tests/ui/redundant_slicing.stderr +++ b/tests/ui/redundant_slicing.stderr @@ -6,12 +6,6 @@ LL | let _ = &slice[..]; // Redundant slice | = note: `-D clippy::redundant-slicing` implied by `-D warnings` -error: redundant slicing of the whole range - --> $DIR/redundant_slicing.rs:13:13 - | -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:14:13 | @@ -24,24 +18,12 @@ error: redundant slicing of the whole range 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:20: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: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:24: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:31:13 | @@ -60,5 +42,5 @@ error: redundant slicing of the whole range LL | let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap(); | ^^^^^^^^^^^^ help: reborrow the original value instead: `(&*bytes)` -error: aborting due to 10 previous errors +error: aborting due to 7 previous errors