From 9075c9e8e7724313644ec1859c4c2a97db4b227f Mon Sep 17 00:00:00 2001 From: xFrednet Date: Mon, 13 Sep 2021 15:14:24 +0200 Subject: [PATCH] Update documentation and message for `avoidable_slice_indexing` Co-authored-by: camsteffen --- clippy_lints/src/avoidable_slice_indexing.rs | 23 ++++++----- .../if_let_slice_destruction.stderr | 40 +++++++++---------- .../slice_indexing_in_macro.stderr | 4 +- 3 files changed, 34 insertions(+), 33 deletions(-) diff --git a/clippy_lints/src/avoidable_slice_indexing.rs b/clippy_lints/src/avoidable_slice_indexing.rs index 69e27d76004c..08a693fb3cce 100644 --- a/clippy_lints/src/avoidable_slice_indexing.rs +++ b/clippy_lints/src/avoidable_slice_indexing.rs @@ -19,14 +19,16 @@ use std::convert::TryInto; declare_clippy_lint! { /// ### What it does - /// The lint checks for slices that come from value deconstruction and - /// are only used to access individual slice values. + /// The lint checks for slice bindings in patterns that are only used to + /// access individual slice values. /// /// ### Why is this bad? - /// Accessing slice values using indices can lead to panics. + /// Accessing slice values using indices can lead to panics. Using a refutable + /// pattern and binding to the individual values can avoid these. /// /// ### Limitations - /// This lint currently only checks for immutable access. + /// This lint currently only checks for immutable access inside `if let` + /// patterns. /// /// ### Example /// ```rust @@ -64,7 +66,7 @@ impl AvoidableSliceIndexing { } } -impl_lint_pass!(AvoidableSliceIndexing => [AVOIDABLE_SLICE_INDEXING]); +impl_lint_pass!(AvoidableSliceIndexing => [AVOIDABLE_SLICE_INDEXING]); impl LateLintPass<'_> for AvoidableSliceIndexing { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { @@ -157,10 +159,10 @@ fn lint_slice(cx: &LateContext<'_>, slice: &SliceLintInformation) { cx, AVOIDABLE_SLICE_INDEXING, slice.ident.span, - "this slice can be deconstructured to avoid indexing", + "this binding can be a slice pattern to avoid indexing", |diag| { diag.multipart_suggestion( - "try destructing the slice with a pattern here", + "try using a slice pattern here", slice .pattern_spans .iter() @@ -180,10 +182,9 @@ fn lint_slice(cx: &LateContext<'_>, slice: &SliceLintInformation) { ); // I thought about adding a note to the lint message to inform the user that these - // refactorings will remove the expressions that determine which access should be - // used. However, I decided against this, as `filter_lintable_slices` will only - // return slices where all access indices are known at compile time. They will - // therefore not have any side effects when they are removed. + // refactorings will remove the index expression. However, I decided against this, + // as `filter_lintable_slices` will only return slices where all access indices are + // known at compile time. The removal should therefore not have any side effects. }, ); } diff --git a/tests/ui/avoidable_slice_indexing/if_let_slice_destruction.stderr b/tests/ui/avoidable_slice_indexing/if_let_slice_destruction.stderr index b1d80cf8772a..73a5d5979567 100644 --- a/tests/ui/avoidable_slice_indexing/if_let_slice_destruction.stderr +++ b/tests/ui/avoidable_slice_indexing/if_let_slice_destruction.stderr @@ -1,4 +1,4 @@ -error: this slice can be deconstructured to avoid indexing +error: this binding can be a slice pattern to avoid indexing --> $DIR/if_let_slice_destruction.rs:13:17 | LL | if let Some(slice) = slice { @@ -9,7 +9,7 @@ note: the lint level is defined here | LL | #![deny(clippy::avoidable_slice_indexing)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: try destructing the slice with a pattern here +help: try using a slice pattern here | LL | if let Some([slice_0, ..]) = slice { | ~~~~~~~~~~~~~ @@ -18,13 +18,13 @@ help: and replacing the index expressions here LL | println!("{}", slice_0); | ~~~~~~~ -error: this slice can be deconstructured to avoid indexing +error: this binding can be a slice pattern to avoid indexing --> $DIR/if_let_slice_destruction.rs:19:17 | LL | if let Some(slice) = slice { | ^^^^^ | -help: try destructing the slice with a pattern here +help: try using a slice pattern here | LL | if let Some([ref slice_0, ..]) = slice { | ~~~~~~~~~~~~~~~~~ @@ -33,13 +33,13 @@ help: and replacing the index expressions here LL | println!("{}", slice_0); | ~~~~~~~ -error: this slice can be deconstructured to avoid indexing +error: this binding can be a slice pattern to avoid indexing --> $DIR/if_let_slice_destruction.rs:25:17 | LL | if let Some(slice) = slice { | ^^^^^ | -help: try destructing the slice with a pattern here +help: try using a slice pattern here | LL | if let Some([ref slice_0, _, ref slice_2, ..]) = slice { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -49,13 +49,13 @@ LL ~ println!("{}", slice_2); LL ~ println!("{}", slice_0); | -error: this slice can be deconstructured to avoid indexing +error: this binding can be a slice pattern to avoid indexing --> $DIR/if_let_slice_destruction.rs:32:26 | LL | if let SomeEnum::One(slice) | SomeEnum::Three(slice) = slice_wrapped { | ^^^^^ | -help: try destructing the slice with a pattern here +help: try using a slice pattern here | LL | if let SomeEnum::One([ref slice_0, ..]) | SomeEnum::Three([ref slice_0, ..]) = slice_wrapped { | ~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~ @@ -64,13 +64,13 @@ help: and replacing the index expressions here LL | println!("{}", slice_0); | ~~~~~~~ -error: this slice can be deconstructured to avoid indexing +error: this binding can be a slice pattern to avoid indexing --> $DIR/if_let_slice_destruction.rs:39:29 | LL | if let (SomeEnum::Three(a), Some(b)) = (a_wrapped, b_wrapped) { | ^ | -help: try destructing the slice with a pattern here +help: try using a slice pattern here | LL | if let (SomeEnum::Three([_, _, ref a_2, ..]), Some(b)) = (a_wrapped, b_wrapped) { | ~~~~~~~~~~~~~~~~~~~ @@ -79,13 +79,13 @@ help: and replacing the index expressions here LL | println!("{} -> {}", a_2, b[1]); | ~~~ -error: this slice can be deconstructured to avoid indexing +error: this binding can be a slice pattern to avoid indexing --> $DIR/if_let_slice_destruction.rs:39:38 | LL | if let (SomeEnum::Three(a), Some(b)) = (a_wrapped, b_wrapped) { | ^ | -help: try destructing the slice with a pattern here +help: try using a slice pattern here | LL | if let (SomeEnum::Three(a), Some([_, ref b_1, ..])) = (a_wrapped, b_wrapped) { | ~~~~~~~~~~~~~~~~ @@ -94,13 +94,13 @@ help: and replacing the index expressions here LL | println!("{} -> {}", a[2], b_1); | ~~~ -error: this slice can be deconstructured to avoid indexing +error: this binding can be a slice pattern to avoid indexing --> $DIR/if_let_slice_destruction.rs:46:21 | LL | if let Some(ref slice) = slice { | ^^^^^ | -help: try destructing the slice with a pattern here +help: try using a slice pattern here | LL | if let Some([_, ref slice_1, ..]) = slice { | ~~~~~~~~~~~~~~~~~~~~ @@ -109,13 +109,13 @@ help: and replacing the index expressions here LL | println!("{:?}", slice_1); | ~~~~~~~ -error: this slice can be deconstructured to avoid indexing +error: this binding can be a slice pattern to avoid indexing --> $DIR/if_let_slice_destruction.rs:54:17 | LL | if let Some(slice) = &slice { | ^^^^^ | -help: try destructing the slice with a pattern here +help: try using a slice pattern here | LL | if let Some([ref slice_0, ..]) = &slice { | ~~~~~~~~~~~~~~~~~ @@ -124,13 +124,13 @@ help: and replacing the index expressions here LL | println!("{:?}", slice_0); | ~~~~~~~ -error: this slice can be deconstructured to avoid indexing +error: this binding can be a slice pattern to avoid indexing --> $DIR/if_let_slice_destruction.rs:123:17 | LL | if let Some(slice) = wrap.inner { | ^^^^^ | -help: try destructing the slice with a pattern here +help: try using a slice pattern here | LL | if let Some([ref slice_0, ..]) = wrap.inner { | ~~~~~~~~~~~~~~~~~ @@ -139,13 +139,13 @@ help: and replacing the index expressions here LL | println!("This is awesome! {}", slice_0); | ~~~~~~~ -error: this slice can be deconstructured to avoid indexing +error: this binding can be a slice pattern to avoid indexing --> $DIR/if_let_slice_destruction.rs:130:17 | LL | if let Some(slice) = wrap.inner { | ^^^^^ | -help: try destructing the slice with a pattern here +help: try using a slice pattern here | LL | if let Some([ref slice_0, ..]) = wrap.inner { | ~~~~~~~~~~~~~~~~~ diff --git a/tests/ui/avoidable_slice_indexing/slice_indexing_in_macro.stderr b/tests/ui/avoidable_slice_indexing/slice_indexing_in_macro.stderr index 8b80c9c5d5bc..42db2306ab13 100644 --- a/tests/ui/avoidable_slice_indexing/slice_indexing_in_macro.stderr +++ b/tests/ui/avoidable_slice_indexing/slice_indexing_in_macro.stderr @@ -1,4 +1,4 @@ -error: this slice can be deconstructured to avoid indexing +error: this binding can be a slice pattern to avoid indexing --> $DIR/slice_indexing_in_macro.rs:23:21 | LL | if let Some(slice) = slice; @@ -9,7 +9,7 @@ note: the lint level is defined here | LL | #![deny(clippy::avoidable_slice_indexing)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: try destructing the slice with a pattern here +help: try using a slice pattern here | LL | if let Some([slice_0, ..]) = slice; | ~~~~~~~~~~~~~