Ignore slices with sub patterns in avoidable_slice_indexing

Author: xFrednet

clippy_lints/src/avoidable_slice_indexing.rs

@@ -87,32 +87,44 @@ impl LateLintPass<'_> for AvoidableSliceIndexing {
 }
 
 fn find_slice_values(cx: &LateContext<'_>, pat: &hir::Pat<'_>) -> Option<FxHashMap<hir::HirId, SliceLintInformation>> {
-    // TODO xFrednet 2021-08-31: Test with sub pattern stuff
-
+    let mut removed_pat: FxHashSet<Symbol> = FxHashSet::default();
     let mut ident_to_id: FxHashMap<Symbol, hir::HirId> = FxHashMap::default();
     let mut slices: FxHashMap<hir::HirId, SliceLintInformation> = FxHashMap::default();
-    pat.each_binding(|binding, hir_id, span, ident| {
-        // We'll just ignore mut and ref mut for simplicity sake right now
-        if let hir::BindingAnnotation::Mutable | hir::BindingAnnotation::RefMut = binding {
-            return;
-        }
+    pat.walk_always(|pat| {
+        if let hir::PatKind::Binding(binding, _, ident, sub_pat) = pat.kind {
+            // We'll just ignore mut and ref mut for simplicity sake right now
+            if let hir::BindingAnnotation::Mutable | hir::BindingAnnotation::RefMut = binding {
+                return;
+            }
+            
+            // This block catches bindings with sub patterns. It would be hard to build a correct suggestion
+            // for them and it's likely that the user knows what they are doing in such a case.
+            if removed_pat.contains(&ident.name) {
+                return;
+            }
+            if sub_pat.is_some() {
+                removed_pat.insert(ident.name);
+                ident_to_id.remove(&ident.name).map(|key| slices.remove(&key));
+                return;
+            }
 
-        let bound_ty = cx.typeck_results().node_type(hir_id);
-        if let ty::Slice(inner_ty) | ty::Array(inner_ty, _) = bound_ty.peel_refs().kind() {
-            // The values need to use the `ref` keyword if they can't be coppied.
-            // This will need to be adjusted if the lint want to support multable access in the future
-            let needs_ref = !bound_ty.is_ref()
-                || !cx
-                    .tcx
-                    .lang_items()
-                    .copy_trait()
-                    .map_or(false, |trait_id| implements_trait(cx, inner_ty, trait_id, &[]));
+            let bound_ty = cx.typeck_results().node_type(pat.hir_id);
+            if let ty::Slice(inner_ty) | ty::Array(inner_ty, _) = bound_ty.peel_refs().kind() {
+                // The values need to use the `ref` keyword if they can't be coppied.
+                // This will need to be adjusted if the lint want to support multable access in the future
+                let needs_ref = !bound_ty.is_ref()
+                    || !cx
+                        .tcx
+                        .lang_items()
+                        .copy_trait()
+                        .map_or(false, |trait_id| implements_trait(cx, inner_ty, trait_id, &[]));
 
-            let key = *ident_to_id.entry(ident.name).or_insert(hir_id);
-            let slice_info = slices
-                .entry(key)
-                .or_insert_with(|| SliceLintInformation::new(ident, needs_ref));
-            slice_info.pattern_spans.push(span);
+                let key = *ident_to_id.entry(ident.name).or_insert(pat.hir_id);
+                let slice_info = slices
+                    .entry(key)
+                    .or_insert_with(|| SliceLintInformation::new(ident, needs_ref));
+                slice_info.pattern_spans.push(pat.span);
+            }
         }
     });
 

tests/ui/avoidable_slice_indexing/if_let_slice_destruction.rs

@@ -97,10 +97,12 @@ fn mutable_slice_index() {
     println!("Use after modification: {:?}", slice);
 }
 
-fn complex_binding() {
+/// 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[8]);
+        println!("{:?}", slice[2]);
     }
 }
 

tests/ui/avoidable_slice_indexing/slice_indexing_in_macro.stderr

@@ -1,5 +1,5 @@
 error: this slice can be deconstructured to avoid indexing
-  --> $DIR/slice_indexing_in_macro.rs:24:21
+  --> $DIR/slice_indexing_in_macro.rs:23:21
    |
 LL |         if let Some(slice) = slice;
    |                     ^^^^^