Skip to content

Commit

Permalink
Update documentation and message for avoidable_slice_indexing
Browse files Browse the repository at this point in the history
Co-authored-by: camsteffen <cam.steffen94@gmail.com>
  • Loading branch information
xFrednet and camsteffen committed Sep 13, 2021
1 parent 79406a4 commit 9075c9e
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 33 deletions.
23 changes: 12 additions & 11 deletions clippy_lints/src/avoidable_slice_indexing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<'_>) {
Expand Down Expand Up @@ -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()
Expand All @@ -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.
},
);
}
Expand Down
40 changes: 20 additions & 20 deletions tests/ui/avoidable_slice_indexing/if_let_slice_destruction.stderr
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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 {
| ~~~~~~~~~~~~~
Expand All @@ -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 {
| ~~~~~~~~~~~~~~~~~
Expand All @@ -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 {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand All @@ -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 {
| ~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~
Expand All @@ -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) {
| ~~~~~~~~~~~~~~~~~~~
Expand All @@ -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) {
| ~~~~~~~~~~~~~~~~
Expand All @@ -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 {
| ~~~~~~~~~~~~~~~~~~~~
Expand All @@ -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 {
| ~~~~~~~~~~~~~~~~~
Expand All @@ -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 {
| ~~~~~~~~~~~~~~~~~
Expand All @@ -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 {
| ~~~~~~~~~~~~~~~~~
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
| ~~~~~~~~~~~~~
Expand Down

0 comments on commit 9075c9e

Please sign in to comment.