src/tools/clippy/clippy_lints/src/index_refutable_slice.rs

   1 use clippy_utils::consts::{constant, Constant};
   2 use clippy_utils::diagnostics::span_lint_and_then;
   3 use clippy_utils::higher::IfLet;
   4 use clippy_utils::ty::is_copy;
   5 use clippy_utils::{is_expn_of, is_lint_allowed, meets_msrv, msrvs, path_to_local};
   6 use if_chain::if_chain;
   7 use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
   8 use rustc_errors::Applicability;
   9 use rustc_hir as hir;
  10 use rustc_hir::intravisit::{self, Visitor};
  11 use rustc_lint::{LateContext, LateLintPass};
  12 use rustc_middle::hir::nested_filter;
  13 use rustc_middle::ty;
  14 use rustc_semver::RustcVersion;
  15 use rustc_session::{declare_tool_lint, impl_lint_pass};
  16 use rustc_span::{symbol::Ident, Span};
  17 use std::convert::TryInto;
  18
  19 declare_clippy_lint! {
  20     /// ### What it does
  21     /// The lint checks for slice bindings in patterns that are only used to
  22     /// access individual slice values.
  23     ///
  24     /// ### Why is this bad?
  25     /// Accessing slice values using indices can lead to panics. Using refutable
  26     /// patterns can avoid these. Binding to individual values also improves the
  27     /// readability as they can be named.
  28     ///
  29     /// ### Limitations
  30     /// This lint currently only checks for immutable access inside `if let`
  31     /// patterns.
  32     ///
  33     /// ### Example
  34     /// ```rust
  35     /// let slice: Option<&[u32]> = Some(&[1, 2, 3]);
  36     ///
  37     /// if let Some(slice) = slice {
  38     ///     println!("{}", slice[0]);
  39     /// }
  40     /// ```
  41     /// Use instead:
  42     /// ```rust
  43     /// let slice: Option<&[u32]> = Some(&[1, 2, 3]);
  44     ///
  45     /// if let Some(&[first, ..]) = slice {
  46     ///     println!("{}", first);
  47     /// }
  48     /// ```
  49     #[clippy::version = "1.58.0"]
  50     pub INDEX_REFUTABLE_SLICE,
  51     nursery,
  52     "avoid indexing on slices which could be destructed"
  53 }
  54
  55 #[derive(Copy, Clone)]
  56 pub struct IndexRefutableSlice {
  57     max_suggested_slice: u64,
  58     msrv: Option<RustcVersion>,
  59 }
  60
  61 impl IndexRefutableSlice {
  62     pub fn new(max_suggested_slice_pattern_length: u64, msrv: Option<RustcVersion>) -> Self {
  63         Self {
  64             max_suggested_slice: max_suggested_slice_pattern_length,
  65             msrv,
  66         }
  67     }
  68 }
  69
  70 impl_lint_pass!(IndexRefutableSlice => [INDEX_REFUTABLE_SLICE]);
  71
  72 impl<'tcx> LateLintPass<'tcx> for IndexRefutableSlice {
  73     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
  74         if_chain! {
  75             if !expr.span.from_expansion() || is_expn_of(expr.span, "if_chain").is_some();
  76             if let Some(IfLet {let_pat, if_then, ..}) = IfLet::hir(cx, expr);
  77             if !is_lint_allowed(cx, INDEX_REFUTABLE_SLICE, expr.hir_id);
  78             if meets_msrv(self.msrv.as_ref(), &msrvs::SLICE_PATTERNS);
  79
  80             let found_slices = find_slice_values(cx, let_pat);
  81             if !found_slices.is_empty();
  82             let filtered_slices = filter_lintable_slices(cx, found_slices, self.max_suggested_slice, if_then);
  83             if !filtered_slices.is_empty();
  84             then {
  85                 for slice in filtered_slices.values() {
  86                     lint_slice(cx, slice);
  87                 }
  88             }
  89         }
  90     }
  91
  92     extract_msrv_attr!(LateContext);
  93 }
  94
  95 fn find_slice_values(cx: &LateContext<'_>, pat: &hir::Pat<'_>) -> FxIndexMap<hir::HirId, SliceLintInformation> {
  96     let mut removed_pat: FxHashSet<hir::HirId> = FxHashSet::default();
  97     let mut slices: FxIndexMap<hir::HirId, SliceLintInformation> = FxIndexMap::default();
  98     pat.walk_always(|pat| {
  99         if let hir::PatKind::Binding(binding, value_hir_id, ident, sub_pat) = pat.kind {
 100             // We'll just ignore mut and ref mut for simplicity sake right now
 101             if let hir::BindingAnnotation::Mutable | hir::BindingAnnotation::RefMut = binding {
 102                 return;
 103             }
 104
 105             // This block catches bindings with sub patterns. It would be hard to build a correct suggestion
 106             // for them and it's likely that the user knows what they are doing in such a case.
 107             if removed_pat.contains(&value_hir_id) {
 108                 return;
 109             }
 110             if sub_pat.is_some() {
 111                 removed_pat.insert(value_hir_id);
 112                 slices.remove(&value_hir_id);
 113                 return;
 114             }
 115
 116             let bound_ty = cx.typeck_results().node_type(pat.hir_id);
 117             if let ty::Slice(inner_ty) | ty::Array(inner_ty, _) = bound_ty.peel_refs().kind() {
 118                 // The values need to use the `ref` keyword if they can't be copied.
 119                 // This will need to be adjusted if the lint want to support multable access in the future
 120                 let src_is_ref = bound_ty.is_ref() && binding != hir::BindingAnnotation::Ref;
 121                 let needs_ref = !(src_is_ref || is_copy(cx, inner_ty));
 122
 123                 let slice_info = slices
 124                     .entry(value_hir_id)
 125                     .or_insert_with(|| SliceLintInformation::new(ident, needs_ref));
 126                 slice_info.pattern_spans.push(pat.span);
 127             }
 128         }
 129     });
 130
 131     slices
 132 }
 133
 134 fn lint_slice(cx: &LateContext<'_>, slice: &SliceLintInformation) {
 135     let used_indices = slice
 136         .index_use
 137         .iter()
 138         .map(|(index, _)| *index)
 139         .collect::<FxHashSet<_>>();
 140
 141     let value_name = |index| format!("{}_{}", slice.ident.name, index);
 142
 143     if let Some(max_index) = used_indices.iter().max() {
 144         let opt_ref = if slice.needs_ref { "ref " } else { "" };
 145         let pat_sugg_idents = (0..=*max_index)
 146             .map(|index| {
 147                 if used_indices.contains(&index) {
 148                     format!("{}{}", opt_ref, value_name(index))
 149                 } else {
 150                     "_".to_string()
 151                 }
 152             })
 153             .collect::<Vec<_>>();
 154         let pat_sugg = format!("[{}, ..]", pat_sugg_idents.join(", "));
 155
 156         span_lint_and_then(
 157             cx,
 158             INDEX_REFUTABLE_SLICE,
 159             slice.ident.span,
 160             "this binding can be a slice pattern to avoid indexing",
 161             |diag| {
 162                 diag.multipart_suggestion(
 163                     "try using a slice pattern here",
 164                     slice
 165                         .pattern_spans
 166                         .iter()
 167                         .map(|span| (*span, pat_sugg.clone()))
 168                         .collect(),
 169                     Applicability::MaybeIncorrect,
 170                 );
 171
 172                 diag.multipart_suggestion(
 173                     "and replace the index expressions here",
 174                     slice
 175                         .index_use
 176                         .iter()
 177                         .map(|(index, span)| (*span, value_name(*index)))
 178                         .collect(),
 179                     Applicability::MaybeIncorrect,
 180                 );
 181
 182                 // The lint message doesn't contain a warning about the removed index expression,
 183                 // since `filter_lintable_slices` will only return slices where all access indices
 184                 // are known at compile time. Therefore, they can be removed without side effects.
 185             },
 186         );
 187     }
 188 }
 189
 190 #[derive(Debug)]
 191 struct SliceLintInformation {
 192     ident: Ident,
 193     needs_ref: bool,
 194     pattern_spans: Vec<Span>,
 195     index_use: Vec<(u64, Span)>,
 196 }
 197
 198 impl SliceLintInformation {
 199     fn new(ident: Ident, needs_ref: bool) -> Self {
 200         Self {
 201             ident,
 202             needs_ref,
 203             pattern_spans: Vec::new(),
 204             index_use: Vec::new(),
 205         }
 206     }
 207 }
 208
 209 fn filter_lintable_slices<'a, 'tcx>(
 210     cx: &'a LateContext<'tcx>,
 211     slice_lint_info: FxIndexMap<hir::HirId, SliceLintInformation>,
 212     max_suggested_slice: u64,
 213     scope: &'tcx hir::Expr<'tcx>,
 214 ) -> FxIndexMap<hir::HirId, SliceLintInformation> {
 215     let mut visitor = SliceIndexLintingVisitor {
 216         cx,
 217         slice_lint_info,
 218         max_suggested_slice,
 219     };
 220
 221     intravisit::walk_expr(&mut visitor, scope);
 222
 223     visitor.slice_lint_info
 224 }
 225
 226 struct SliceIndexLintingVisitor<'a, 'tcx> {
 227     cx: &'a LateContext<'tcx>,
 228     slice_lint_info: FxIndexMap<hir::HirId, SliceLintInformation>,
 229     max_suggested_slice: u64,
 230 }
 231
 232 impl<'a, 'tcx> Visitor<'tcx> for SliceIndexLintingVisitor<'a, 'tcx> {
 233     type NestedFilter = nested_filter::OnlyBodies;
 234
 235     fn nested_visit_map(&mut self) -> Self::Map {
 236         self.cx.tcx.hir()
 237     }
 238
 239     fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
 240         if let Some(local_id) = path_to_local(expr) {
 241             let Self {
 242                 cx,
 243                 ref mut slice_lint_info,
 244                 max_suggested_slice,
 245             } = *self;
 246
 247             if_chain! {
 248                 // Check if this is even a local we're interested in
 249                 if let Some(use_info) = slice_lint_info.get_mut(&local_id);
 250
 251                 let map = cx.tcx.hir();
 252
 253                 // Checking for slice indexing
 254                 let parent_id = map.get_parent_node(expr.hir_id);
 255                 if let Some(hir::Node::Expr(parent_expr)) = map.find(parent_id);
 256                 if let hir::ExprKind::Index(_, index_expr) = parent_expr.kind;
 257                 if let Some((Constant::Int(index_value), _)) = constant(cx, cx.typeck_results(), index_expr);
 258                 if let Ok(index_value) = index_value.try_into();
 259                 if index_value < max_suggested_slice;
 260
 261                 // Make sure that this slice index is read only
 262                 let maybe_addrof_id = map.get_parent_node(parent_id);
 263                 if let Some(hir::Node::Expr(maybe_addrof_expr)) = map.find(maybe_addrof_id);
 264                 if let hir::ExprKind::AddrOf(_kind, hir::Mutability::Not, _inner_expr) = maybe_addrof_expr.kind;
 265                 then {
 266                     use_info.index_use.push((index_value, map.span(parent_expr.hir_id)));
 267                     return;
 268                 }
 269             }
 270
 271             // The slice was used for something other than indexing
 272             self.slice_lint_info.remove(&local_id);
 273         }
 274         intravisit::walk_expr(self, expr);
 275     }
 276 }