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::{FxHashMap, FxHashSet};
8 use rustc_errors::Applicability;
10 use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
11 use rustc_lint::{LateContext, LateLintPass, LintContext};
12 use rustc_middle::hir::map::Map;
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;
19 declare_clippy_lint! {
21 /// The lint checks for slice bindings in patterns that are only used to
22 /// access individual slice values.
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.
30 /// This lint currently only checks for immutable access inside `if let`
35 /// let slice: Option<&[u32]> = Some(&[1, 2, 3]);
37 /// if let Some(slice) = slice {
38 /// println!("{}", slice[0]);
43 /// let slice: Option<&[u32]> = Some(&[1, 2, 3]);
45 /// if let Some(&[first, ..]) = slice {
46 /// println!("{}", first);
49 #[clippy::version = "1.58.0"]
50 pub INDEX_REFUTABLE_SLICE,
52 "avoid indexing on slices which could be destructed"
55 #[derive(Copy, Clone)]
56 pub struct IndexRefutableSlice {
57 max_suggested_slice: u64,
58 msrv: Option<RustcVersion>,
61 impl IndexRefutableSlice {
62 pub fn new(max_suggested_slice_pattern_length: u64, msrv: Option<RustcVersion>) -> Self {
64 max_suggested_slice: max_suggested_slice_pattern_length,
70 impl_lint_pass!(IndexRefutableSlice => [INDEX_REFUTABLE_SLICE]);
72 impl LateLintPass<'_> for IndexRefutableSlice {
73 fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
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);
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();
85 for slice in filtered_slices.values() {
86 lint_slice(cx, slice);
92 extract_msrv_attr!(LateContext);
95 fn find_slice_values(cx: &LateContext<'_>, pat: &hir::Pat<'_>) -> FxHashMap<hir::HirId, SliceLintInformation> {
96 let mut removed_pat: FxHashSet<hir::HirId> = FxHashSet::default();
97 let mut slices: FxHashMap<hir::HirId, SliceLintInformation> = FxHashMap::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 {
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) {
110 if sub_pat.is_some() {
111 removed_pat.insert(value_hir_id);
112 slices.remove(&value_hir_id);
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));
123 let slice_info = slices
125 .or_insert_with(|| SliceLintInformation::new(ident, needs_ref));
126 slice_info.pattern_spans.push(pat.span);
134 fn lint_slice(cx: &LateContext<'_>, slice: &SliceLintInformation) {
135 let used_indices = slice
138 .map(|(index, _)| *index)
139 .collect::<FxHashSet<_>>();
141 let value_name = |index| format!("{}_{}", slice.ident.name, index);
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)
147 if used_indices.contains(&index) {
148 format!("{}{}", opt_ref, value_name(index))
153 .collect::<Vec<_>>();
154 let pat_sugg = format!("[{}, ..]", pat_sugg_idents.join(", "));
158 INDEX_REFUTABLE_SLICE,
160 "this binding can be a slice pattern to avoid indexing",
162 diag.multipart_suggestion(
163 "try using a slice pattern here",
167 .map(|span| (*span, pat_sugg.clone()))
169 Applicability::MaybeIncorrect,
172 diag.multipart_suggestion(
173 "and replace the index expressions here",
177 .map(|(index, span)| (*span, value_name(*index)))
179 Applicability::MaybeIncorrect,
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.
191 struct SliceLintInformation {
194 pattern_spans: Vec<Span>,
195 index_use: Vec<(u64, Span)>,
198 impl SliceLintInformation {
199 fn new(ident: Ident, needs_ref: bool) -> Self {
203 pattern_spans: Vec::new(),
204 index_use: Vec::new(),
209 fn filter_lintable_slices<'a, 'tcx>(
210 cx: &'a LateContext<'tcx>,
211 slice_lint_info: FxHashMap<hir::HirId, SliceLintInformation>,
212 max_suggested_slice: u64,
213 scope: &'tcx hir::Expr<'tcx>,
214 ) -> FxHashMap<hir::HirId, SliceLintInformation> {
215 let mut visitor = SliceIndexLintingVisitor {
221 intravisit::walk_expr(&mut visitor, scope);
223 visitor.slice_lint_info
226 struct SliceIndexLintingVisitor<'a, 'tcx> {
227 cx: &'a LateContext<'tcx>,
228 slice_lint_info: FxHashMap<hir::HirId, SliceLintInformation>,
229 max_suggested_slice: u64,
232 impl<'a, 'tcx> Visitor<'tcx> for SliceIndexLintingVisitor<'a, 'tcx> {
233 type Map = Map<'tcx>;
235 fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
236 NestedVisitorMap::OnlyBodies(self.cx.tcx.hir())
239 fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
240 if let Some(local_id) = path_to_local(expr) {
243 ref mut slice_lint_info,
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);
251 let map = cx.tcx.hir();
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;
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;
266 use_info.index_use.push((index_value, map.span(parent_expr.hir_id)));
271 // The slice was used for something other than indexing
272 self.slice_lint_info.remove(&local_id);
274 intravisit::walk_expr(self, expr);