user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix: fixed multipart_suggestion in index_refutable_slice uitest (#13727)

Browse source 
This should address #13099 for the derivable_impls test. As this
combines everything into a single multipart_suggestion, the feedback
message is a little less "targeted" than it was before, but now it
provides a complete`--fix`able suggestion - e.g.:

```
error: this binding can be a slice pattern to avoid indexing
  --> tests/ui-toml/max_suggested_slice_pattern_length/index_refutable_slice.rs:5:17
   |
LL |     if let Some(slice) = slice {
   |                 ^^^^^
   |
note: the lint level is defined here
  --> tests/ui-toml/max_suggested_slice_pattern_length/index_refutable_slice.rs:1:9
   |
LL | #![deny(clippy::index_refutable_slice)]
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: replace the binding and indexed access with a slice pattern
   |
LL ~     if let Some([_, _, _, _, _, _, _, slice_7, ..]) = slice {
LL |
LL |         // This would usually not be linted but is included now due to the
LL |         // index limit in the config file
LL ~         println!("{}", slice_7);
   |
```

changelog: [index_refutable_slice]: Fixed multipart_suggestions to
provide correct rustfix-able lint
This commit is contained in:
Timo 2024-12-10 19:03:59 +00:00 • committed by GitHub
commit 2a28347897
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 311 additions and 110 deletions
Download patch file Download diff file Expand all files Collapse all files

36
clippy_lints/src/index_refutable_slice.rs
View file

@ -135,7 +135,7 @@ fn lint_slice(cx: &LateContext<'_>, slice: &SliceLintInformation) {
.map(|(index, _)| *index)
.collect::<FxIndexSet<_>>();
let value_name = |index| format!("{}_{index}", slice.ident.name);
let value_name = |index| format!("{}_{}", slice.ident.name, index);
if let Some(max_index) = used_indices.iter().max() {
let opt_ref = if slice.needs_ref { "ref " } else { "" };
@ -150,6 +150,18 @@ fn lint_slice(cx: &LateContext<'_>, slice: &SliceLintInformation) {
.collect::<Vec<_>>();
let pat_sugg = format!("[{}, ..]", pat_sugg_idents.join(", "));
let mut suggestions = Vec::new();
// Add the binding pattern suggestion
if !slice.pattern_spans.is_empty() {
suggestions.extend(slice.pattern_spans.iter().map(|span| (*span, pat_sugg.clone())));
}
// Add the index replacement suggestions
if !slice.index_use.is_empty() {
suggestions.extend(slice.index_use.iter().map(|(index, span)| (*span, value_name(*index))));
}
span_lint_and_then(
cx,
INDEX_REFUTABLE_SLICE,
@ -157,28 +169,10 @@ fn lint_slice(cx: &LateContext<'_>, slice: &SliceLintInformation) {
"this binding can be a slice pattern to avoid indexing",
|diag| {
diag.multipart_suggestion(
"try using a slice pattern here",
slice
.pattern_spans
.iter()
.map(|span| (*span, pat_sugg.clone()))
.collect(),
"replace the binding and indexed access with a slice pattern",
suggestions,
Applicability::MaybeIncorrect,
);
diag.multipart_suggestion(
"and replace the index expressions here",
slice
.index_use
.iter()
.map(|(index, span)| (*span, value_name(*index)))
.collect(),
Applicability::MaybeIncorrect,
);
// The lint message doesn't contain a warning about the removed index expression,
// since `filter_lintable_slices` will only return slices where all access indices
// are known at compile time. Therefore, they can be removed without side effects.
},
);
}

24
tests/ui-toml/max_suggested_slice_pattern_length/index_refutable_slice.fixed Normal file
View file

@ -0,0 +1,24 @@
#![deny(clippy::index_refutable_slice)]
fn below_limit() {
let slice: Option<&[u32]> = Some(&[1, 2, 3]);
if let Some([_, _, _, _, _, _, _, slice_7, ..]) = slice {
//~^ ERROR: binding can be a slice pattern
// This would usually not be linted but is included now due to the
// index limit in the config file
println!("{}", slice_7);
}
}
fn above_limit() {
let slice: Option<&[u32]> = Some(&[1, 2, 3]);
if let Some(slice) = slice {
// This will not be linted as 8 is above the limit
println!("{}", slice[8]);
}
}
fn main() {
below_limit();
above_limit();
}

2
tests/ui-toml/max_suggested_slice_pattern_length/index_refutable_slice.rs
View file

@ -1,7 +1,5 @@
#![deny(clippy::index_refutable_slice)]
//@no-rustfix: need to change the suggestion to a multipart suggestion
fn below_limit() {
let slice: Option<&[u32]> = Some(&[1, 2, 3]);
if let Some(slice) = slice {

14
tests/ui-toml/max_suggested_slice_pattern_length/index_refutable_slice.stderr
View file

@ -1,5 +1,5 @@
error: this binding can be a slice pattern to avoid indexing
--> tests/ui-toml/max_suggested_slice_pattern_length/index_refutable_slice.rs:7:17
--> tests/ui-toml/max_suggested_slice_pattern_length/index_refutable_slice.rs:5:17
|
LL | if let Some(slice) = slice {
| ^^^^^
@ -9,14 +9,14 @@ note: the lint level is defined here
|
LL | #![deny(clippy::index_refutable_slice)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: try using a slice pattern here
help: replace the binding and indexed access with a slice pattern
|
LL | if let Some([_, _, _, _, _, _, _, slice_7, ..]) = slice {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
help: and replace the index expressions here
LL ~ if let Some([_, _, _, _, _, _, _, slice_7, ..]) = slice {
LL |
LL | // This would usually not be linted but is included now due to the
LL | // index limit in the config file
LL ~ println!("{}", slice_7);
|
LL | println!("{}", slice_7);
| ~~~~~~~
error: aborting due to 1 previous error

177
tests/ui/index_refutable_slice/if_let_slice_binding.fixed Normal file
View file

@ -0,0 +1,177 @@
#![deny(clippy::index_refutable_slice)]
#![allow(clippy::uninlined_format_args, clippy::needless_lifetimes)]
enum SomeEnum<T> {
One(T),
Two(T),
Three(T),
Four(T),
}
fn lintable_examples() {
// Try with reference
let slice: Option<&[u32]> = Some(&[1, 2, 3]);
if let Some([slice_0, ..]) = slice {
//~^ ERROR: this binding can be a slice pattern to avoid indexing
println!("{}", slice_0);
}
// Try with copy
let slice: Option<[u32; 3]> = Some([1, 2, 3]);
if let Some([slice_0, ..]) = slice {
//~^ ERROR: this binding can be a slice pattern to avoid indexing
println!("{}", slice_0);
}
// Try with long slice and small indices
let slice: Option<[u32; 9]> = Some([1, 2, 3, 4, 5, 6, 7, 8, 9]);
if let Some([slice_0, _, slice_2, ..]) = slice {
//~^ ERROR: this binding can be a slice pattern to avoid indexing
println!("{}", slice_2);
println!("{}", slice_0);
}
// Multiple bindings
let slice_wrapped: SomeEnum<[u32; 3]> = SomeEnum::One([5, 6, 7]);
if let SomeEnum::One([slice_0, ..]) | SomeEnum::Three([slice_0, ..]) = slice_wrapped {
//~^ ERROR: this binding can be a slice pattern to avoid indexing
println!("{}", slice_0);
}
// Two lintable slices in one if let
let a_wrapped: SomeEnum<[u32; 3]> = SomeEnum::One([9, 5, 1]);
let b_wrapped: Option<[u32; 2]> = Some([4, 6]);
if let (SomeEnum::Three([_, _, a_2, ..]), Some([_, b_1, ..])) = (a_wrapped, b_wrapped) {
//~^ ERROR: this binding can be a slice pattern to avoid indexing
//~| ERROR: this binding can be a slice pattern to avoid indexing
println!("{} -> {}", a_2, b_1);
}
// This requires the slice values to be borrowed as the slice values can only be
// borrowed and `String` doesn't implement copy
let slice: Option<[String; 2]> = Some([String::from("1"), String::from("2")]);
if let Some([_, ref slice_1, ..]) = slice {
//~^ ERROR: this binding can be a slice pattern to avoid indexing
println!("{:?}", slice_1);
}
println!("{:?}", slice);
// This should not suggest using the `ref` keyword as the scrutinee is already
// a reference
let slice: Option<[String; 2]> = Some([String::from("1"), String::from("2")]);
if let Some([slice_0, ..]) = &slice {
//~^ ERROR: this binding can be a slice pattern to avoid indexing
println!("{:?}", slice_0);
}
println!("{:?}", slice);
}
fn slice_index_above_limit() {
let slice: Option<&[u32]> = Some(&[1, 2, 3]);
if let Some(slice) = slice {
// Would cause a panic, IDK
println!("{}", slice[7]);
}
}
fn slice_is_used() {
let slice: Option<&[u32]> = Some(&[1, 2, 3]);
if let Some(slice) = slice {
println!("{:?}", slice.len());
}
let slice: Option<&[u32]> = Some(&[1, 2, 3]);
if let Some(slice) = slice {
println!("{:?}", slice.to_vec());
}
let opt: Option<[String; 2]> = Some([String::from("Hello"), String::from("world")]);
if let Some(slice) = opt {
if !slice.is_empty() {
println!("first: {}", slice[0]);
}
}
}
/// The slice is used by an external function and should therefore not be linted
fn check_slice_as_arg() {
fn is_interesting<T>(slice: &[T; 2]) -> bool {
!slice.is_empty()
}
let slice_wrapped: Option<[String; 2]> = Some([String::from("Hello"), String::from("world")]);
if let Some(slice) = &slice_wrapped {
if is_interesting(slice) {
println!("This is interesting {}", slice[0]);
}
}
println!("{:?}", slice_wrapped);
}
fn check_slice_in_struct() {
#[derive(Debug)]
struct Wrapper<'a> {
inner: Option<&'a [String]>,
is_awesome: bool,
}
impl<'a> Wrapper<'a> {
fn is_super_awesome(&self) -> bool {
self.is_awesome
}
}
let inner = &[String::from("New"), String::from("World")];
let wrap = Wrapper {
inner: Some(inner),
is_awesome: true,
};
// Test 1: Field access
if let Some([slice_0, ..]) = wrap.inner {
//~^ ERROR: this binding can be a slice pattern to avoid indexing
if wrap.is_awesome {
println!("This is awesome! {}", slice_0);
}
}
// Test 2: function access
if let Some([slice_0, ..]) = wrap.inner {
//~^ ERROR: this binding can be a slice pattern to avoid indexing
if wrap.is_super_awesome() {
println!("This is super awesome! {}", slice_0);
}
}
println!("Complete wrap: {:?}", wrap);
}
/// This would be a nice additional feature to have in the future, but adding it
/// now would make the PR too large. This is therefore only a test that we don't
/// lint cases we can't make a reasonable suggestion for
fn mutable_slice_index() {
// Mut access
let mut slice: Option<[String; 1]> = Some([String::from("Penguin")]);
if let Some(ref mut slice) = slice {
slice[0] = String::from("Mr. Penguin");
}
println!("Use after modification: {:?}", slice);
// Mut access on reference
let mut slice: Option<[String; 1]> = Some([String::from("Cat")]);
if let Some(slice) = &mut slice {
slice[0] = String::from("Lord Meow Meow");
}
println!("Use after modification: {:?}", slice);
}
/// The lint will ignore bindings with sub patterns as it would be hard
/// to build correct suggestions for these instances :)
fn binding_with_sub_pattern() {
let slice: Option<&[u32]> = Some(&[1, 2, 3]);
if let Some(slice @ [_, _, _]) = slice {
println!("{:?}", slice[2]);
}
}
fn main() {}

2
tests/ui/index_refutable_slice/if_let_slice_binding.rs
View file

@ -1,8 +1,6 @@
#![deny(clippy::index_refutable_slice)]
#![allow(clippy::uninlined_format_args, clippy::needless_lifetimes)]
//@no-rustfix: need to change the suggestion to a multipart suggestion
enum SomeEnum<T> {
One(T),
Two(T),

122
tests/ui/index_refutable_slice/if_let_slice_binding.stderr
View file

@ -1,5 +1,5 @@
error: this binding can be a slice pattern to avoid indexing
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:16:17
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:14:17
|
LL | if let Some(slice) = slice {
| ^^^^^
@ -9,150 +9,134 @@ note: the lint level is defined here
|
LL | #![deny(clippy::index_refutable_slice)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: try using a slice pattern here
help: replace the binding and indexed access with a slice pattern
|
LL | if let Some([slice_0, ..]) = slice {
| ~~~~~~~~~~~~~
help: and replace the index expressions here
LL ~ if let Some([slice_0, ..]) = slice {
LL |
LL ~ println!("{}", slice_0);
|
LL | println!("{}", slice_0);
| ~~~~~~~
error: this binding can be a slice pattern to avoid indexing
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:23:17
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:21:17
|
LL | if let Some(slice) = slice {
| ^^^^^
|
help: try using a slice pattern here
help: replace the binding and indexed access with a slice pattern
|
LL | if let Some([slice_0, ..]) = slice {
| ~~~~~~~~~~~~~
help: and replace the index expressions here
LL ~ if let Some([slice_0, ..]) = slice {
LL |
LL ~ println!("{}", slice_0);
|
LL | println!("{}", slice_0);
| ~~~~~~~
error: this binding can be a slice pattern to avoid indexing
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:30:17
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:28:17
|
LL | if let Some(slice) = slice {
| ^^^^^
|
help: try using a slice pattern here
|
LL | if let Some([slice_0, _, slice_2, ..]) = slice {
| ~~~~~~~~~~~~~~~~~~~~~~~~~
help: and replace the index expressions here
help: replace the binding and indexed access with a slice pattern
|
LL ~ if let Some([slice_0, _, slice_2, ..]) = slice {
LL |
LL ~ println!("{}", slice_2);
LL ~ println!("{}", slice_0);
|
error: this binding can be a slice pattern to avoid indexing
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:38:26
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:36:26
|
LL | if let SomeEnum::One(slice) | SomeEnum::Three(slice) = slice_wrapped {
| ^^^^^
|
help: try using a slice pattern here
help: replace the binding and indexed access with a slice pattern
|
LL | if let SomeEnum::One([slice_0, ..]) | SomeEnum::Three([slice_0, ..]) = slice_wrapped {
| ~~~~~~~~~~~~~ ~~~~~~~~~~~~~
help: and replace the index expressions here
LL ~ if let SomeEnum::One([slice_0, ..]) | SomeEnum::Three([slice_0, ..]) = slice_wrapped {
LL |
LL ~ println!("{}", slice_0);
|
LL | println!("{}", slice_0);
| ~~~~~~~
error: this binding can be a slice pattern to avoid indexing
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:46:29
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:44:29
|
LL | if let (SomeEnum::Three(a), Some(b)) = (a_wrapped, b_wrapped) {
| ^
|
help: try using a slice pattern here
help: replace the binding and indexed access with a slice pattern
|
LL | if let (SomeEnum::Three([_, _, a_2, ..]), Some(b)) = (a_wrapped, b_wrapped) {
| ~~~~~~~~~~~~~~~
help: and replace the index expressions here
LL ~ if let (SomeEnum::Three([_, _, a_2, ..]), Some(b)) = (a_wrapped, b_wrapped) {
LL |
LL |
LL ~ println!("{} -> {}", a_2, b[1]);
|
LL | println!("{} -> {}", a_2, b[1]);
| ~~~
error: this binding can be a slice pattern to avoid indexing
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:46:38
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:44:38
|
LL | if let (SomeEnum::Three(a), Some(b)) = (a_wrapped, b_wrapped) {
| ^
|
help: try using a slice pattern here
help: replace the binding and indexed access with a slice pattern
|
LL | if let (SomeEnum::Three(a), Some([_, b_1, ..])) = (a_wrapped, b_wrapped) {
| ~~~~~~~~~~~~
help: and replace the index expressions here
LL ~ if let (SomeEnum::Three(a), Some([_, b_1, ..])) = (a_wrapped, b_wrapped) {
LL |
LL |
LL ~ println!("{} -> {}", a[2], b_1);
|
LL | println!("{} -> {}", a[2], b_1);
| ~~~
error: this binding can be a slice pattern to avoid indexing
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:55:21
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:53:21
|
LL | if let Some(ref slice) = slice {
| ^^^^^
|
help: try using a slice pattern here
help: replace the binding and indexed access with a slice pattern
|
LL | if let Some([_, ref slice_1, ..]) = slice {
| ~~~~~~~~~~~~~~~~~~~~
help: and replace the index expressions here
LL ~ if let Some([_, ref slice_1, ..]) = slice {
LL |
LL ~ println!("{:?}", slice_1);
|
LL | println!("{:?}", slice_1);
| ~~~~~~~
error: this binding can be a slice pattern to avoid indexing
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:64:17
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:62:17
|
LL | if let Some(slice) = &slice {
| ^^^^^
|
help: try using a slice pattern here
help: replace the binding and indexed access with a slice pattern
|
LL | if let Some([slice_0, ..]) = &slice {
| ~~~~~~~~~~~~~
help: and replace the index expressions here
LL ~ if let Some([slice_0, ..]) = &slice {
LL |
LL ~ println!("{:?}", slice_0);
|
LL | println!("{:?}", slice_0);
| ~~~~~~~
error: this binding can be a slice pattern to avoid indexing
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:134:17
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:132:17
|
LL | if let Some(slice) = wrap.inner {
| ^^^^^
|
help: try using a slice pattern here
help: replace the binding and indexed access with a slice pattern
|
LL | if let Some([slice_0, ..]) = wrap.inner {
| ~~~~~~~~~~~~~
help: and replace the index expressions here
LL ~ if let Some([slice_0, ..]) = wrap.inner {
LL |
LL | if wrap.is_awesome {
LL ~ println!("This is awesome! {}", slice_0);
|
LL | println!("This is awesome! {}", slice_0);
| ~~~~~~~
error: this binding can be a slice pattern to avoid indexing
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:142:17
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:140:17
|
LL | if let Some(slice) = wrap.inner {
| ^^^^^
|
help: try using a slice pattern here
help: replace the binding and indexed access with a slice pattern
|
LL | if let Some([slice_0, ..]) = wrap.inner {
| ~~~~~~~~~~~~~
help: and replace the index expressions here
LL ~ if let Some([slice_0, ..]) = wrap.inner {
LL |
LL | if wrap.is_super_awesome() {
LL ~ println!("This is super awesome! {}", slice_0);
|
LL | println!("This is super awesome! {}", slice_0);
| ~~~~~~~
error: aborting due to 10 previous errors

29
tests/ui/index_refutable_slice/slice_indexing_in_macro.fixed Normal file
View file

@ -0,0 +1,29 @@
#![deny(clippy::index_refutable_slice)]
extern crate if_chain;
use if_chain::if_chain;
macro_rules! if_let_slice_macro {
() => {
// This would normally be linted
let slice: Option<&[u32]> = Some(&[1, 2, 3]);
if let Some(slice) = slice {
println!("{}", slice[0]);
}
};
}
fn main() {
// Don't lint this
if_let_slice_macro!();
// Do lint this
if_chain! {
let slice: Option<&[u32]> = Some(&[1, 2, 3]);
if let Some([slice_0, ..]) = slice;
//~^ ERROR: this binding can be a slice pattern to avoid indexing
then {
println!("{}", slice_0);
}
}
}

2
tests/ui/index_refutable_slice/slice_indexing_in_macro.rs
View file

@ -1,7 +1,5 @@
#![deny(clippy::index_refutable_slice)]
//@no-rustfix: need to change the suggestion to a multipart suggestion
extern crate if_chain;
use if_chain::if_chain;

13
tests/ui/index_refutable_slice/slice_indexing_in_macro.stderr
View file

@ -1,5 +1,5 @@
error: this binding can be a slice pattern to avoid indexing
--> tests/ui/index_refutable_slice/slice_indexing_in_macro.rs:25:21
--> tests/ui/index_refutable_slice/slice_indexing_in_macro.rs:23:21
|
LL | if let Some(slice) = slice;
| ^^^^^
@ -9,14 +9,13 @@ note: the lint level is defined here
|
LL | #![deny(clippy::index_refutable_slice)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: try using a slice pattern here
help: replace the binding and indexed access with a slice pattern
|
LL | if let Some([slice_0, ..]) = slice;
| ~~~~~~~~~~~~~
help: and replace the index expressions here
LL ~ if let Some([slice_0, ..]) = slice;
LL |
LL | then {
LL ~ println!("{}", slice_0);
|
LL | println!("{}", slice_0);
| ~~~~~~~
error: aborting due to 1 previous error