From 837bc9906552af5da429bef4b37be588b8d2f49d Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Thu, 31 Dec 2020 11:10:13 -0500 Subject: [PATCH 1/5] Initial implementation of redundant_slicing lint --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 5 +++ clippy_lints/src/redundant_slicing.rs | 56 +++++++++++++++++++++++++++ tests/ui/redundant_slicing.rs | 11 ++++++ tests/ui/redundant_slicing.stderr | 16 ++++++++ 5 files changed, 89 insertions(+) create mode 100644 clippy_lints/src/redundant_slicing.rs create mode 100644 tests/ui/redundant_slicing.rs create mode 100644 tests/ui/redundant_slicing.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index b0e9ad55b4f..85f6929f924 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2162,6 +2162,7 @@ Released 2018-09-13 [`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern [`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching [`redundant_pub_crate`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pub_crate +[`redundant_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing [`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes [`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref [`ref_option_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_option_ref diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 4f9ebb4af3d..70fdfd22caa 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -301,6 +301,7 @@ mod redundant_closure_call; mod redundant_else; mod redundant_field_names; mod redundant_pub_crate; +mod redundant_slicing; mod redundant_static_lifetimes; mod ref_option_ref; mod reference; @@ -849,6 +850,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &redundant_else::REDUNDANT_ELSE, &redundant_field_names::REDUNDANT_FIELD_NAMES, &redundant_pub_crate::REDUNDANT_PUB_CRATE, + &redundant_slicing::REDUNDANT_SLICING, &redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES, &ref_option_ref::REF_OPTION_REF, &reference::DEREF_ADDROF, @@ -1229,6 +1231,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box vec_init_then_push::VecInitThenPush::default()); store.register_late_pass(move || box types::PtrAsPtr::new(msrv)); store.register_late_pass(|| box case_sensitive_file_extension_comparisons::CaseSensitiveFileExtensionComparisons); + store.register_late_pass(|| box redundant_slicing::RedundantSlicing); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), @@ -1591,6 +1594,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&redundant_clone::REDUNDANT_CLONE), LintId::of(&redundant_closure_call::REDUNDANT_CLOSURE_CALL), LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES), + LintId::of(&redundant_slicing::REDUNDANT_SLICING), LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES), LintId::of(&reference::DEREF_ADDROF), LintId::of(&reference::REF_IN_DEREF), @@ -1835,6 +1839,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&ptr_offset_with_cast::PTR_OFFSET_WITH_CAST), LintId::of(&ranges::RANGE_ZIP_WITH_LEN), LintId::of(&redundant_closure_call::REDUNDANT_CLOSURE_CALL), + LintId::of(&redundant_slicing::REDUNDANT_SLICING), LintId::of(&reference::DEREF_ADDROF), LintId::of(&reference::REF_IN_DEREF), LintId::of(&repeat_once::REPEAT_ONCE), diff --git a/clippy_lints/src/redundant_slicing.rs b/clippy_lints/src/redundant_slicing.rs new file mode 100644 index 00000000000..686298b6943 --- /dev/null +++ b/clippy_lints/src/redundant_slicing.rs @@ -0,0 +1,56 @@ +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, LangItem}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::TyS; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +use crate::utils::{is_type_lang_item, snippet_with_applicability, span_lint_and_sugg}; + +declare_clippy_lint! { + /// **What it does:** Checks for redundant slicing expressions which use the full range, and + /// do not change the type. + /// + /// **Why is this bad?** It unnecessarily adds complexity to the expression. + /// + /// **Known problems:** If the type being sliced has an implementation of `Index` + /// that actually changes anything then it can't be removed. However, this would be surprising + /// to people reading the code and should have a note with it. + /// + /// **Example:** + /// + /// ```ignore + /// fn get_slice(x: &[u32]) -> &[u32] { + /// &x[..] + /// } + /// ``` + /// Use instead: + /// ```ignore + /// fn get_slice(x: &[u32]) -> &[u32] { + /// x + /// } + /// ``` + pub REDUNDANT_SLICING, + complexity, + "redundant slicing of the whole range of a type" +} + +declare_lint_pass!(RedundantSlicing => [REDUNDANT_SLICING]); + +impl LateLintPass<'_> for RedundantSlicing { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if_chain! { + if let ExprKind::AddrOf(_, _, addressee) = expr.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 TyS::same_type(cx.typeck_results().expr_ty(expr), cx.typeck_results().expr_ty(indexed)); + then { + let mut app = Applicability::MachineApplicable; + let hint = snippet_with_applicability(cx, indexed.span, "..", &mut app).into_owned(); + + span_lint_and_sugg(cx, REDUNDANT_SLICING, expr.span, "redundant slicing of the whole range", + "use the original slice instead", hint, app); + } + } + } +} diff --git a/tests/ui/redundant_slicing.rs b/tests/ui/redundant_slicing.rs new file mode 100644 index 00000000000..922b8b4ce57 --- /dev/null +++ b/tests/ui/redundant_slicing.rs @@ -0,0 +1,11 @@ +#![allow(unused)] +#![warn(clippy::redundant_slicing)] + +fn main() { + let x: &[u32] = &[0]; + let err = &x[..]; + + let v = vec![0]; + let ok = &v[..]; + let err = &(&v[..])[..]; +} diff --git a/tests/ui/redundant_slicing.stderr b/tests/ui/redundant_slicing.stderr new file mode 100644 index 00000000000..9efd6484ad0 --- /dev/null +++ b/tests/ui/redundant_slicing.stderr @@ -0,0 +1,16 @@ +error: redundant slicing of the whole range + --> $DIR/redundant_slicing.rs:6:15 + | +LL | let err = &x[..]; + | ^^^^^^ help: use the original slice instead: `x` + | + = note: `-D clippy::redundant-slicing` implied by `-D warnings` + +error: redundant slicing of the whole range + --> $DIR/redundant_slicing.rs:10:15 + | +LL | let err = &(&v[..])[..]; + | ^^^^^^^^^^^^^ help: use the original slice instead: `(&v[..])` + +error: aborting due to 2 previous errors + From 2a41d40807ccf01268c29ba64ac9ea0efe1b26f7 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Thu, 31 Dec 2020 11:42:12 -0500 Subject: [PATCH 2/5] fix new lint error --- clippy_lints/src/map_unit_fn.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/map_unit_fn.rs b/clippy_lints/src/map_unit_fn.rs index e50d11a4d71..01126e86199 100644 --- a/clippy_lints/src/map_unit_fn.rs +++ b/clippy_lints/src/map_unit_fn.rs @@ -131,7 +131,7 @@ fn reduce_unit_expression<'a>(cx: &LateContext<'_>, expr: &'a hir::Expr<'_>) -> Some(expr.span) }, hir::ExprKind::Block(ref block, _) => { - match (&block.stmts[..], block.expr.as_ref()) { + match (block.stmts, block.expr.as_ref()) { (&[], Some(inner_expr)) => { // If block only contains an expression, // reduce `{ X }` to `X` From 27c0d6c14bde219bcaf72b4bcb6426f6f8650f6e Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Thu, 31 Dec 2020 12:07:24 -0500 Subject: [PATCH 3/5] don't lint external macro expansions --- clippy_lints/src/redundant_slicing.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/redundant_slicing.rs b/clippy_lints/src/redundant_slicing.rs index 686298b6943..5140b30a473 100644 --- a/clippy_lints/src/redundant_slicing.rs +++ b/clippy_lints/src/redundant_slicing.rs @@ -2,7 +2,7 @@ use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, LangItem}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::TyS; +use rustc_middle::{lint::in_external_macro, ty::TyS}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use crate::utils::{is_type_lang_item, snippet_with_applicability, span_lint_and_sugg}; @@ -39,6 +39,10 @@ declare_lint_pass!(RedundantSlicing => [REDUNDANT_SLICING]); impl LateLintPass<'_> for RedundantSlicing { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if in_external_macro(cx.sess, expr.span) { + return; + } + if_chain! { if let ExprKind::AddrOf(_, _, addressee) = expr.kind; if let ExprKind::Index(indexed, range) = addressee.kind; From bf028b3f4a440bc8a390837da3a3f940177de1e6 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Thu, 31 Dec 2020 14:40:07 -0500 Subject: [PATCH 4/5] fix copy-paste error --- clippy_lints/src/redundant_slicing.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/redundant_slicing.rs b/clippy_lints/src/redundant_slicing.rs index 5140b30a473..4478f559266 100644 --- a/clippy_lints/src/redundant_slicing.rs +++ b/clippy_lints/src/redundant_slicing.rs @@ -1,7 +1,7 @@ use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, LangItem}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::{lint::in_external_macro, ty::TyS}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -39,7 +39,7 @@ declare_lint_pass!(RedundantSlicing => [REDUNDANT_SLICING]); impl LateLintPass<'_> for RedundantSlicing { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if in_external_macro(cx.sess, expr.span) { + if in_external_macro(cx.sess(), expr.span) { return; } From 9146a77032e7d8c96ccae177c60ddb04b6159329 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Thu, 14 Jan 2021 21:38:57 +0000 Subject: [PATCH 5/5] Update clippy_lints/src/redundant_slicing.rs Co-authored-by: Philipp Krones --- clippy_lints/src/redundant_slicing.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/redundant_slicing.rs b/clippy_lints/src/redundant_slicing.rs index 4478f559266..e5ced13514f 100644 --- a/clippy_lints/src/redundant_slicing.rs +++ b/clippy_lints/src/redundant_slicing.rs @@ -52,8 +52,15 @@ impl LateLintPass<'_> for RedundantSlicing { let mut app = Applicability::MachineApplicable; let hint = snippet_with_applicability(cx, indexed.span, "..", &mut app).into_owned(); - span_lint_and_sugg(cx, REDUNDANT_SLICING, expr.span, "redundant slicing of the whole range", - "use the original slice instead", hint, app); + span_lint_and_sugg( + cx, + REDUNDANT_SLICING, + expr.span, + "redundant slicing of the whole range", + "use the original slice instead", + hint, + app, + ); } } }