From 53c421a5793ab695cfdbc9941f51c59494f1c95c Mon Sep 17 00:00:00 2001 From: Krishna Sundarram Date: Tue, 8 Feb 2022 21:41:39 +0000 Subject: [PATCH 1/5] WIP Manual Slice --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_restriction.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/manual_slice.rs | 76 ++++++++++++++++++++ clippy_utils/src/msrvs.rs | 1 + tests/ui/manual_slice.rs | 7 ++ 7 files changed, 89 insertions(+) create mode 100644 clippy_lints/src/manual_slice.rs create mode 100644 tests/ui/manual_slice.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index c9adf77c0d63..ee467e353c4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3254,6 +3254,7 @@ Released 2018-09-13 [`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or [`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains [`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic +[`manual_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice [`manual_split_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once [`manual_str_repeat`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat [`manual_strip`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_strip diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index a80320a578f0..90a975be82e3 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -231,6 +231,7 @@ store.register_lints(&[ manual_map::MANUAL_MAP, manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE, manual_ok_or::MANUAL_OK_OR, + manual_slice::MANUAL_SLICE, manual_strip::MANUAL_STRIP, manual_unwrap_or::MANUAL_UNWRAP_OR, map_clone::MAP_CLONE, diff --git a/clippy_lints/src/lib.register_restriction.rs b/clippy_lints/src/lib.register_restriction.rs index 5a89fdb05a99..74b1c87d462a 100644 --- a/clippy_lints/src/lib.register_restriction.rs +++ b/clippy_lints/src/lib.register_restriction.rs @@ -26,6 +26,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve LintId::of(integer_division::INTEGER_DIVISION), LintId::of(let_underscore::LET_UNDERSCORE_MUST_USE), LintId::of(literal_representation::DECIMAL_LITERAL_REPRESENTATION), + LintId::of(manual_slice::MANUAL_SLICE), LintId::of(map_err_ignore::MAP_ERR_IGNORE), LintId::of(matches::REST_PAT_IN_FULLY_BOUND_STRUCTS), LintId::of(matches::WILDCARD_ENUM_MATCH_ARM), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 0b07726519ec..9bac13ab2de3 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -267,6 +267,7 @@ mod manual_bits; mod manual_map; mod manual_non_exhaustive; mod manual_ok_or; +mod manual_slice; mod manual_strip; mod manual_unwrap_or; mod map_clone; @@ -862,6 +863,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || Box::new(borrow_as_ptr::BorrowAsPtr::new(msrv))); store.register_late_pass(move || Box::new(manual_bits::ManualBits::new(msrv))); store.register_late_pass(|| Box::new(default_union_representation::DefaultUnionRepresentation)); + store.register_early_pass(move || Box::new(manual_slice::ManualSlice::new(msrv))); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/manual_slice.rs b/clippy_lints/src/manual_slice.rs new file mode 100644 index 000000000000..a157b239930f --- /dev/null +++ b/clippy_lints/src/manual_slice.rs @@ -0,0 +1,76 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::{meets_msrv, msrvs}; +use rustc_ast::ast::*; +use rustc_errors::Applicability; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_semver::RustcVersion; +use rustc_session::{declare_tool_lint, impl_lint_pass}; + +declare_clippy_lint! { + /// ### What it does + /// Finds arrays or vectors being converted to slices of the same length. + /// ### Why is this bad? + /// The methods `as_slice()` or `as_mut_slice()` could be used instead. + /// ### Example + /// ```rust + /// let mut arr: [u32; 1] = [1]; + /// let slice = &arr[..]; + /// let mutable_slice = &mut arr[..]; + /// ``` + /// Use instead: + /// ```rust + /// let mut arr: [u32; 1] = [1]; + /// let slice = arr.as_slice(); + /// let mutable_slice = arr.as_mut_slice(); + /// ``` + #[clippy::version = "1.60.0"] + pub MANUAL_SLICE, + restriction, + "default lint description" +} + +#[derive(Clone)] +pub struct ManualSlice { + msrv: Option, +} + +impl ManualSlice { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} + +impl_lint_pass!(ManualSlice => [MANUAL_SLICE]); + +impl EarlyLintPass for ManualSlice { + fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { + if !meets_msrv(self.msrv.as_ref(), &msrvs::NON_EXHAUSTIVE) { + return; + } + + if_chain! { + if let ExprKind::AddrOf(BorrowKind::Ref, mutability, inner) = &expr.kind; + if let ExprKind::Index(object, index) = &inner.kind; + if let ExprKind::Path(_, ref ident) = object.kind; + if let ExprKind::Range(None, None, _) = index.kind; + then { + let suggestion = match mutability { + Mutability::Not => "to_slice()", + Mutability::Mut => "to_mut_slice()", + }; + span_lint_and_sugg( + cx, + MANUAL_SLICE, + expr.span, + "converting to a slice of the same length", + "use", + format!("{:?}.{}", ident, suggestion), + Applicability::MachineApplicable + ); + } + } + } + + extract_msrv_attr!(EarlyContext); +} diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index a5b409ad96bb..337bb4e023b0 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -12,6 +12,7 @@ macro_rules! msrv_aliases { // names may refer to stabilized feature flags or library items msrv_aliases! { + 1,58,0 { MANUAL_SLICE } 1,53,0 { OR_PATTERNS, MANUAL_BITS } 1,52,0 { STR_SPLIT_ONCE } 1,51,0 { BORROW_AS_PTR } diff --git a/tests/ui/manual_slice.rs b/tests/ui/manual_slice.rs new file mode 100644 index 000000000000..4611e46109ac --- /dev/null +++ b/tests/ui/manual_slice.rs @@ -0,0 +1,7 @@ +#![warn(clippy::manual_slice)] + +fn main() { + let mut arr: [u32; 1] = [1]; + let slice = &arr[..]; + let mutable_slice = &mut arr[..]; +} From b5ea8ae45e8165cb89d078725415ce14b44069db Mon Sep 17 00:00:00 2001 From: Krishna Sundarram Date: Tue, 8 Feb 2022 22:37:39 +0000 Subject: [PATCH 2/5] Convert to LateLintPass --- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/manual_slice.rs | 30 ++++++++++++++++-------------- tests/ui/manual_slice.rs | 28 +++++++++++++++++++++++++--- tests/ui/manual_slice.stderr | 28 ++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 18 deletions(-) create mode 100644 tests/ui/manual_slice.stderr diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 9bac13ab2de3..ed814018f959 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -863,7 +863,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || Box::new(borrow_as_ptr::BorrowAsPtr::new(msrv))); store.register_late_pass(move || Box::new(manual_bits::ManualBits::new(msrv))); store.register_late_pass(|| Box::new(default_union_representation::DefaultUnionRepresentation)); - store.register_early_pass(move || Box::new(manual_slice::ManualSlice::new(msrv))); + store.register_late_pass(move || Box::new(manual_slice::ManualSlice::new(msrv))); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/manual_slice.rs b/clippy_lints/src/manual_slice.rs index a157b239930f..07aa569a55f8 100644 --- a/clippy_lints/src/manual_slice.rs +++ b/clippy_lints/src/manual_slice.rs @@ -1,8 +1,9 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::{meets_msrv, msrvs}; -use rustc_ast::ast::*; +use clippy_utils::{ + diagnostics::span_lint_and_sugg, meets_msrv, msrvs, source::snippet_with_context, ty::is_type_lang_item, +}; use rustc_errors::Applicability; -use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_hir::*; +use rustc_lint::{LateContext, LateLintPass}; use rustc_semver::RustcVersion; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -43,19 +44,20 @@ impl ManualSlice { impl_lint_pass!(ManualSlice => [MANUAL_SLICE]); -impl EarlyLintPass for ManualSlice { - fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { +impl<'tcx> LateLintPass<'tcx> for ManualSlice { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { if !meets_msrv(self.msrv.as_ref(), &msrvs::NON_EXHAUSTIVE) { return; } - + let ctxt = expr.span.ctxt(); if_chain! { if let ExprKind::AddrOf(BorrowKind::Ref, mutability, inner) = &expr.kind; - if let ExprKind::Index(object, index) = &inner.kind; - if let ExprKind::Path(_, ref ident) = object.kind; - if let ExprKind::Range(None, None, _) = index.kind; + if let ExprKind::Index(object, range) = &inner.kind; + if is_type_lang_item(cx, cx.typeck_results().expr_ty_adjusted(range), lang_items::LangItem::RangeFull); then { - let suggestion = match mutability { + let mut app = Applicability::MachineApplicable; + let snip = snippet_with_context(cx, object.span, ctxt, "..", &mut app).0; + let suggested_method = match mutability { Mutability::Not => "to_slice()", Mutability::Mut => "to_mut_slice()", }; @@ -65,12 +67,12 @@ impl EarlyLintPass for ManualSlice { expr.span, "converting to a slice of the same length", "use", - format!("{:?}.{}", ident, suggestion), - Applicability::MachineApplicable + format!("{}.{}", snip, suggested_method), + app ); } } } - extract_msrv_attr!(EarlyContext); + extract_msrv_attr!(LateContext); } diff --git a/tests/ui/manual_slice.rs b/tests/ui/manual_slice.rs index 4611e46109ac..59aa6ac480f7 100644 --- a/tests/ui/manual_slice.rs +++ b/tests/ui/manual_slice.rs @@ -1,7 +1,29 @@ #![warn(clippy::manual_slice)] fn main() { - let mut arr: [u32; 1] = [1]; - let slice = &arr[..]; - let mutable_slice = &mut arr[..]; + let mut arr: [u32; 3] = [1, 2, 3]; + + let arr_slice = &arr[..]; + let mutable_arr_slice = &mut arr[..]; + + let mut vec = vec![1, 2, 3]; + + let vec_slice = &vec[..]; + let mutable_vec_slice = &mut vec[..]; + + // Will not fire on any of these + + let partial_slice_1 = &arr[1..]; + let partial_slice_2 = &arr[..3]; + let partial_slice_3 = &arr[1..3]; + let partial_mut_slice_1 = &mut arr[1..]; + let partial_mut_slice_2 = &mut arr[..3]; + let partial_mut_slice_3 = &mut arr[1..3]; + + let partial_slice_1 = &vec[1..]; + let partial_slice_2 = &vec[..3]; + let partial_slice_3 = &vec[1..3]; + let partial_mut_slice_1 = &mut vec[1..]; + let partial_mut_slice_2 = &mut vec[..3]; + let partial_mut_slice_3 = &mut vec[1..3]; } diff --git a/tests/ui/manual_slice.stderr b/tests/ui/manual_slice.stderr new file mode 100644 index 000000000000..4f632d56c22f --- /dev/null +++ b/tests/ui/manual_slice.stderr @@ -0,0 +1,28 @@ +error: converting to a slice of the same length + --> $DIR/manual_slice.rs:6:21 + | +LL | let arr_slice = &arr[..]; + | ^^^^^^^^ help: use: `arr.to_slice()` + | + = note: `-D clippy::manual-slice` implied by `-D warnings` + +error: converting to a slice of the same length + --> $DIR/manual_slice.rs:7:29 + | +LL | let mutable_arr_slice = &mut arr[..]; + | ^^^^^^^^^^^^ help: use: `arr.to_mut_slice()` + +error: converting to a slice of the same length + --> $DIR/manual_slice.rs:11:21 + | +LL | let vec_slice = &vec[..]; + | ^^^^^^^^ help: use: `vec.to_slice()` + +error: converting to a slice of the same length + --> $DIR/manual_slice.rs:12:29 + | +LL | let mutable_vec_slice = &mut vec[..]; + | ^^^^^^^^^^^^ help: use: `vec.to_mut_slice()` + +error: aborting due to 4 previous errors + From 8123b1dc9f35fa334f1208fdc9bf7397b2803601 Mon Sep 17 00:00:00 2001 From: Krishna Sundarram Date: Tue, 8 Feb 2022 22:56:35 +0000 Subject: [PATCH 3/5] Expand example --- clippy_lints/src/manual_slice.rs | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/manual_slice.rs b/clippy_lints/src/manual_slice.rs index 07aa569a55f8..3696879a8849 100644 --- a/clippy_lints/src/manual_slice.rs +++ b/clippy_lints/src/manual_slice.rs @@ -2,7 +2,7 @@ use clippy_utils::{ diagnostics::span_lint_and_sugg, meets_msrv, msrvs, source::snippet_with_context, ty::is_type_lang_item, }; use rustc_errors::Applicability; -use rustc_hir::*; +use rustc_hir::{lang_items, BorrowKind, Expr, ExprKind, Mutability}; use rustc_lint::{LateContext, LateLintPass}; use rustc_semver::RustcVersion; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -14,15 +14,23 @@ declare_clippy_lint! { /// The methods `as_slice()` or `as_mut_slice()` could be used instead. /// ### Example /// ```rust - /// let mut arr: [u32; 1] = [1]; - /// let slice = &arr[..]; - /// let mutable_slice = &mut arr[..]; + /// let mut arr: [u32; 3] = [1, 2, 3]; + /// let arr_slice = &arr[..]; + /// let mutable_arr_slice = &mut arr[..]; + /// + /// let mut vec = vec![1, 2, 3]; + /// let vec_slice = &vec[..]; + /// let mutable_vec_slice = &mut vec[..]; /// ``` /// Use instead: /// ```rust - /// let mut arr: [u32; 1] = [1]; - /// let slice = arr.as_slice(); - /// let mutable_slice = arr.as_mut_slice(); + /// let mut arr: [u32; 3] = [1, 2, 3]; + /// let arr_slice = arr.as_slice(); + /// let mutable_arr_slice = arr.as_mut_slice(); + /// + /// let mut vec = vec![1, 2, 3]; + /// let vec_slice = vec.as_slice(); + /// let mutable_vec_slice = vec.as_mut_slice(); /// ``` #[clippy::version = "1.60.0"] pub MANUAL_SLICE, From 9d80f3b08dca277a265e0eadf607b35a1f180531 Mon Sep 17 00:00:00 2001 From: Krishna Sundarram Date: Tue, 8 Feb 2022 23:00:44 +0000 Subject: [PATCH 4/5] Add description string --- clippy_lints/src/manual_slice.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/manual_slice.rs b/clippy_lints/src/manual_slice.rs index 3696879a8849..0891df7de3c8 100644 --- a/clippy_lints/src/manual_slice.rs +++ b/clippy_lints/src/manual_slice.rs @@ -35,7 +35,7 @@ declare_clippy_lint! { #[clippy::version = "1.60.0"] pub MANUAL_SLICE, restriction, - "default lint description" + "Suggest use of array and Vec .as_slice() and .as_mut_slice() methods" } #[derive(Clone)] From e1db1453afdfecc9124eae18ea643f7a0adc9af0 Mon Sep 17 00:00:00 2001 From: Krishna Sundarram Date: Wed, 9 Feb 2022 07:35:08 +0000 Subject: [PATCH 5/5] Add other vec initialisation methods --- tests/ui/manual_slice.rs | 8 ++++++-- tests/ui/manual_slice.stderr | 34 +++++++++++++++++++++++++++++----- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/tests/ui/manual_slice.rs b/tests/ui/manual_slice.rs index 59aa6ac480f7..ee7997eabbdd 100644 --- a/tests/ui/manual_slice.rs +++ b/tests/ui/manual_slice.rs @@ -2,15 +2,19 @@ fn main() { let mut arr: [u32; 3] = [1, 2, 3]; - let arr_slice = &arr[..]; let mutable_arr_slice = &mut arr[..]; let mut vec = vec![1, 2, 3]; - let vec_slice = &vec[..]; let mutable_vec_slice = &mut vec[..]; + let vec_slice = &vec![1, 2, 3][..]; + let mutable_vec_slice = &mut vec![1, 2, 3][..]; + + let vec_slice: &[i32] = &Vec::new()[..]; + let mutable_vec_slice: &mut [i32] = &mut Vec::new()[..]; + // Will not fire on any of these let partial_slice_1 = &arr[1..]; diff --git a/tests/ui/manual_slice.stderr b/tests/ui/manual_slice.stderr index 4f632d56c22f..b945bcf2e9ac 100644 --- a/tests/ui/manual_slice.stderr +++ b/tests/ui/manual_slice.stderr @@ -1,5 +1,5 @@ error: converting to a slice of the same length - --> $DIR/manual_slice.rs:6:21 + --> $DIR/manual_slice.rs:5:21 | LL | let arr_slice = &arr[..]; | ^^^^^^^^ help: use: `arr.to_slice()` @@ -7,22 +7,46 @@ LL | let arr_slice = &arr[..]; = note: `-D clippy::manual-slice` implied by `-D warnings` error: converting to a slice of the same length - --> $DIR/manual_slice.rs:7:29 + --> $DIR/manual_slice.rs:6:29 | LL | let mutable_arr_slice = &mut arr[..]; | ^^^^^^^^^^^^ help: use: `arr.to_mut_slice()` error: converting to a slice of the same length - --> $DIR/manual_slice.rs:11:21 + --> $DIR/manual_slice.rs:9:21 | LL | let vec_slice = &vec[..]; | ^^^^^^^^ help: use: `vec.to_slice()` error: converting to a slice of the same length - --> $DIR/manual_slice.rs:12:29 + --> $DIR/manual_slice.rs:10:29 | LL | let mutable_vec_slice = &mut vec[..]; | ^^^^^^^^^^^^ help: use: `vec.to_mut_slice()` -error: aborting due to 4 previous errors +error: converting to a slice of the same length + --> $DIR/manual_slice.rs:12:21 + | +LL | let vec_slice = &vec![1, 2, 3][..]; + | ^^^^^^^^^^^^^^^^^^ help: use: `vec![1, 2, 3].to_slice()` + +error: converting to a slice of the same length + --> $DIR/manual_slice.rs:13:29 + | +LL | let mutable_vec_slice = &mut vec![1, 2, 3][..]; + | ^^^^^^^^^^^^^^^^^^^^^^ help: use: `vec![1, 2, 3].to_mut_slice()` + +error: converting to a slice of the same length + --> $DIR/manual_slice.rs:15:29 + | +LL | let vec_slice: &[i32] = &Vec::new()[..]; + | ^^^^^^^^^^^^^^^ help: use: `Vec::new().to_slice()` + +error: converting to a slice of the same length + --> $DIR/manual_slice.rs:16:41 + | +LL | let mutable_vec_slice: &mut [i32] = &mut Vec::new()[..]; + | ^^^^^^^^^^^^^^^^^^^ help: use: `Vec::new().to_mut_slice()` + +error: aborting due to 8 previous errors