mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-10 15:14:29 +00:00
New index_refutable_slice lint
* Finding pattern slices for `avoidable_slice_indexing` * `avoidable_slice_indexing` analysing slice usage * Add configuration to `avoidable_slice_indexing` * Emitting `avoidable_slice_indexing` with suggestions * Dogfooding and fixing bugs * Add ui-toml test for `avoidable_slice_indexing` * Correctly suggest `ref` keywords for `avoidable_slice_indexing` * Test and document `mut` for `avoid_slice_indexing` * Handle macros with `avoidable_slice_indexing` lint * Ignore slices with sub patterns in `avoidable_slice_indexing` * Update lint description for `avoidable_slice_indexing` * Move `avoidable_slice_indexing` to nursery * Added more tests for `avoidable_slice_indexing` * Update documentation and message for `avoidable_slice_indexing` * Teach `avoidable_slice_indexing` about `HirId`s and `Visitors` * Rename lint to `index_refutable_slice` and connected config
This commit is contained in:
xFrednet
parent
3bfe98d372
commit
e444cbe5d6
18 changed files with 729 additions and 5 deletions
|
|
@ -2904,6 +2904,7 @@ Released 2018-09-13
|
|||
[`imprecise_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#imprecise_flops
|
||||
[`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping
|
||||
[`inconsistent_struct_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_struct_constructor
|
||||
[`index_refutable_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#index_refutable_slice
|
||||
[`indexing_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing
|
||||
[`ineffective_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_bit_mask
|
||||
[`inefficient_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_to_string
|
||||
|
|
|
|||
276
clippy_lints/src/index_refutable_slice.rs
Normal file
276
clippy_lints/src/index_refutable_slice.rs
Normal file
|
|
@ -0,0 +1,276 @@
|
|||
use clippy_utils::consts::{constant, Constant};
|
||||
use clippy_utils::diagnostics::span_lint_and_then;
|
||||
use clippy_utils::higher::IfLet;
|
||||
use clippy_utils::ty::is_copy;
|
||||
use clippy_utils::{is_expn_of, is_lint_allowed, meets_msrv, msrvs, path_to_local};
|
||||
use if_chain::if_chain;
|
||||
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir as hir;
|
||||
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
|
||||
use rustc_lint::{LateContext, LateLintPass, LintContext};
|
||||
use rustc_middle::hir::map::Map;
|
||||
use rustc_middle::ty;
|
||||
use rustc_semver::RustcVersion;
|
||||
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||
use rustc_span::{symbol::Ident, Span};
|
||||
use std::convert::TryInto;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// 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. Using refutable
|
||||
/// patterns can avoid these. Binding to individual values also improves the
|
||||
/// readability as they can be named.
|
||||
///
|
||||
/// ### Limitations
|
||||
/// This lint currently only checks for immutable access inside `if let`
|
||||
/// patterns.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```rust
|
||||
/// let slice: Option<&[u32]> = Some(&[1, 2, 3]);
|
||||
///
|
||||
/// if let Some(slice) = slice {
|
||||
/// println!("{}", slice[0]);
|
||||
/// }
|
||||
/// ```
|
||||
/// Use instead:
|
||||
/// ```rust
|
||||
/// let slice: Option<&[u32]> = Some(&[1, 2, 3]);
|
||||
///
|
||||
/// if let Some(&[first, ..]) = slice {
|
||||
/// println!("{}", first);
|
||||
/// }
|
||||
/// ```
|
||||
#[clippy::version = "1.58.0"]
|
||||
pub INDEX_REFUTABLE_SLICE,
|
||||
nursery,
|
||||
"avoid indexing on slices which could be destructed"
|
||||
}
|
||||
|
||||
#[derive(Copy, Clone)]
|
||||
pub struct IndexRefutableSlice {
|
||||
max_suggested_slice: u64,
|
||||
msrv: Option<RustcVersion>,
|
||||
}
|
||||
|
||||
impl IndexRefutableSlice {
|
||||
pub fn new(max_suggested_slice_pattern_length: u64, msrv: Option<RustcVersion>) -> Self {
|
||||
Self {
|
||||
max_suggested_slice: max_suggested_slice_pattern_length,
|
||||
msrv,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl_lint_pass!(IndexRefutableSlice => [INDEX_REFUTABLE_SLICE]);
|
||||
|
||||
impl LateLintPass<'_> for IndexRefutableSlice {
|
||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
|
||||
if_chain! {
|
||||
if !expr.span.from_expansion() || is_expn_of(expr.span, "if_chain").is_some();
|
||||
if let Some(IfLet {let_pat, if_then, ..}) = IfLet::hir(cx, expr);
|
||||
if !is_lint_allowed(cx, INDEX_REFUTABLE_SLICE, expr.hir_id);
|
||||
if meets_msrv(self.msrv.as_ref(), &msrvs::SLICE_PATTERNS);
|
||||
|
||||
let found_slices = find_slice_values(cx, let_pat);
|
||||
if !found_slices.is_empty();
|
||||
let filtered_slices = filter_lintable_slices(cx, found_slices, self.max_suggested_slice, if_then);
|
||||
if !filtered_slices.is_empty();
|
||||
then {
|
||||
for slice in filtered_slices.values() {
|
||||
lint_slice(cx, slice);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
extract_msrv_attr!(LateContext);
|
||||
}
|
||||
|
||||
fn find_slice_values(cx: &LateContext<'_>, pat: &hir::Pat<'_>) -> FxHashMap<hir::HirId, SliceLintInformation> {
|
||||
let mut removed_pat: FxHashSet<hir::HirId> = FxHashSet::default();
|
||||
let mut slices: FxHashMap<hir::HirId, SliceLintInformation> = FxHashMap::default();
|
||||
pat.walk_always(|pat| {
|
||||
if let hir::PatKind::Binding(binding, value_hir_id, 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(&value_hir_id) {
|
||||
return;
|
||||
}
|
||||
if sub_pat.is_some() {
|
||||
removed_pat.insert(value_hir_id);
|
||||
slices.remove(&value_hir_id);
|
||||
return;
|
||||
}
|
||||
|
||||
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 copied.
|
||||
// This will need to be adjusted if the lint want to support multable access in the future
|
||||
let src_is_ref = bound_ty.is_ref() && binding != hir::BindingAnnotation::Ref;
|
||||
let needs_ref = !(src_is_ref || is_copy(cx, inner_ty));
|
||||
|
||||
let slice_info = slices
|
||||
.entry(value_hir_id)
|
||||
.or_insert_with(|| SliceLintInformation::new(ident, needs_ref));
|
||||
slice_info.pattern_spans.push(pat.span);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
slices
|
||||
}
|
||||
|
||||
fn lint_slice(cx: &LateContext<'_>, slice: &SliceLintInformation) {
|
||||
let used_indices = slice
|
||||
.index_use
|
||||
.iter()
|
||||
.map(|(index, _)| *index)
|
||||
.collect::<FxHashSet<_>>();
|
||||
|
||||
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 { "" };
|
||||
let pat_sugg_idents = (0..=*max_index)
|
||||
.map(|index| {
|
||||
if used_indices.contains(&index) {
|
||||
format!("{}{}", opt_ref, value_name(index))
|
||||
} else {
|
||||
"_".to_string()
|
||||
}
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let pat_sugg = format!("[{}, ..]", pat_sugg_idents.join(", "));
|
||||
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
INDEX_REFUTABLE_SLICE,
|
||||
slice.ident.span,
|
||||
"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(),
|
||||
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.
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct SliceLintInformation {
|
||||
ident: Ident,
|
||||
needs_ref: bool,
|
||||
pattern_spans: Vec<Span>,
|
||||
index_use: Vec<(u64, Span)>,
|
||||
}
|
||||
|
||||
impl SliceLintInformation {
|
||||
fn new(ident: Ident, needs_ref: bool) -> Self {
|
||||
Self {
|
||||
ident,
|
||||
needs_ref,
|
||||
pattern_spans: Vec::new(),
|
||||
index_use: Vec::new(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn filter_lintable_slices<'a, 'tcx>(
|
||||
cx: &'a LateContext<'tcx>,
|
||||
slice_lint_info: FxHashMap<hir::HirId, SliceLintInformation>,
|
||||
max_suggested_slice: u64,
|
||||
scope: &'tcx hir::Expr<'tcx>,
|
||||
) -> FxHashMap<hir::HirId, SliceLintInformation> {
|
||||
let mut visitor = SliceIndexLintingVisitor {
|
||||
cx,
|
||||
slice_lint_info,
|
||||
max_suggested_slice,
|
||||
};
|
||||
|
||||
intravisit::walk_expr(&mut visitor, scope);
|
||||
|
||||
visitor.slice_lint_info
|
||||
}
|
||||
|
||||
struct SliceIndexLintingVisitor<'a, 'tcx> {
|
||||
cx: &'a LateContext<'tcx>,
|
||||
slice_lint_info: FxHashMap<hir::HirId, SliceLintInformation>,
|
||||
max_suggested_slice: u64,
|
||||
}
|
||||
|
||||
impl<'a, 'tcx> Visitor<'tcx> for SliceIndexLintingVisitor<'a, 'tcx> {
|
||||
type Map = Map<'tcx>;
|
||||
|
||||
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
|
||||
NestedVisitorMap::OnlyBodies(self.cx.tcx.hir())
|
||||
}
|
||||
|
||||
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
|
||||
if let Some(local_id) = path_to_local(expr) {
|
||||
let Self {
|
||||
cx,
|
||||
ref mut slice_lint_info,
|
||||
max_suggested_slice,
|
||||
} = *self;
|
||||
|
||||
if_chain! {
|
||||
// Check if this is even a local we're interested in
|
||||
if let Some(use_info) = slice_lint_info.get_mut(&local_id);
|
||||
|
||||
let map = cx.tcx.hir();
|
||||
|
||||
// Checking for slice indexing
|
||||
let parent_id = map.get_parent_node(expr.hir_id);
|
||||
if let Some(hir::Node::Expr(parent_expr)) = map.find(parent_id);
|
||||
if let hir::ExprKind::Index(_, index_expr) = parent_expr.kind;
|
||||
if let Some((Constant::Int(index_value), _)) = constant(cx, cx.typeck_results(), index_expr);
|
||||
if let Ok(index_value) = index_value.try_into();
|
||||
if index_value < max_suggested_slice;
|
||||
|
||||
// Make sure that this slice index is read only
|
||||
let maybe_addrof_id = map.get_parent_node(parent_id);
|
||||
if let Some(hir::Node::Expr(maybe_addrof_expr)) = map.find(maybe_addrof_id);
|
||||
if let hir::ExprKind::AddrOf(_kind, hir::Mutability::Not, _inner_expr) = maybe_addrof_expr.kind;
|
||||
then {
|
||||
use_info.index_use.push((index_value, map.span(parent_expr.hir_id)));
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// The slice was used for something other than indexing
|
||||
self.slice_lint_info.remove(&local_id);
|
||||
}
|
||||
intravisit::walk_expr(self, expr);
|
||||
}
|
||||
}
|
||||
|
|
@ -168,6 +168,7 @@ store.register_lints(&[
|
|||
implicit_return::IMPLICIT_RETURN,
|
||||
implicit_saturating_sub::IMPLICIT_SATURATING_SUB,
|
||||
inconsistent_struct_constructor::INCONSISTENT_STRUCT_CONSTRUCTOR,
|
||||
index_refutable_slice::INDEX_REFUTABLE_SLICE,
|
||||
indexing_slicing::INDEXING_SLICING,
|
||||
indexing_slicing::OUT_OF_BOUNDS_INDEXING,
|
||||
infinite_iter::INFINITE_ITER,
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
|
|||
LintId::of(floating_point_arithmetic::IMPRECISE_FLOPS),
|
||||
LintId::of(floating_point_arithmetic::SUBOPTIMAL_FLOPS),
|
||||
LintId::of(future_not_send::FUTURE_NOT_SEND),
|
||||
LintId::of(index_refutable_slice::INDEX_REFUTABLE_SLICE),
|
||||
LintId::of(let_if_seq::USELESS_LET_IF_SEQ),
|
||||
LintId::of(missing_const_for_fn::MISSING_CONST_FOR_FN),
|
||||
LintId::of(mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL),
|
||||
|
|
|
|||
|
|
@ -238,6 +238,7 @@ mod implicit_hasher;
|
|||
mod implicit_return;
|
||||
mod implicit_saturating_sub;
|
||||
mod inconsistent_struct_constructor;
|
||||
mod index_refutable_slice;
|
||||
mod indexing_slicing;
|
||||
mod infinite_iter;
|
||||
mod inherent_impl;
|
||||
|
|
@ -580,6 +581,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
|
||||
store.register_late_pass(|| Box::new(size_of_in_element_count::SizeOfInElementCount));
|
||||
store.register_late_pass(|| Box::new(same_name_method::SameNameMethod));
|
||||
let max_suggested_slice_pattern_length = conf.max_suggested_slice_pattern_length;
|
||||
store.register_late_pass(move || {
|
||||
Box::new(index_refutable_slice::IndexRefutableSlice::new(
|
||||
max_suggested_slice_pattern_length,
|
||||
msrv,
|
||||
))
|
||||
});
|
||||
store.register_late_pass(|| Box::new(map_clone::MapClone));
|
||||
store.register_late_pass(|| Box::new(map_err_ignore::MapErrIgnore));
|
||||
store.register_late_pass(|| Box::new(shadow::Shadow::default()));
|
||||
|
|
|
|||
|
|
@ -231,8 +231,8 @@ impl<'a, 'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> {
|
|||
} else {
|
||||
// find `unwrap[_err]()` calls:
|
||||
if_chain! {
|
||||
if let ExprKind::MethodCall(method_name, _, args, _) = expr.kind;
|
||||
if let Some(id) = path_to_local(&args[0]);
|
||||
if let ExprKind::MethodCall(method_name, _, [self_arg, ..], _) = expr.kind;
|
||||
if let Some(id) = path_to_local(self_arg);
|
||||
if [sym::unwrap, sym::expect, sym!(unwrap_err)].contains(&method_name.ident.name);
|
||||
let call_to_unwrap = [sym::unwrap, sym::expect].contains(&method_name.ident.name);
|
||||
if let Some(unwrappable) = self.unwrappables.iter()
|
||||
|
|
|
|||
|
|
@ -148,7 +148,7 @@ define_Conf! {
|
|||
///
|
||||
/// Suppress lints whenever the suggested change would cause breakage for other crates.
|
||||
(avoid_breaking_exported_api: bool = true),
|
||||
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR.
|
||||
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE.
|
||||
///
|
||||
/// The minimum rust version that the project supports
|
||||
(msrv: Option<String> = None),
|
||||
|
|
@ -296,6 +296,12 @@ define_Conf! {
|
|||
///
|
||||
/// Whether to apply the raw pointer heuristic to determine if a type is `Send`.
|
||||
(enable_raw_pointer_heuristic_for_send: bool = true),
|
||||
/// Lint: INDEX_REFUTABLE_SLICE.
|
||||
///
|
||||
/// When Clippy suggests using a slice pattern, this is the maximum number of elements allowed in
|
||||
/// the slice pattern that is suggested. If more elements would be necessary, the lint is suppressed.
|
||||
/// For example, `[_, _, _, e, ..]` is a slice pattern with 4 elements.
|
||||
(max_suggested_slice_pattern_length: u64 = 3),
|
||||
}
|
||||
|
||||
/// Search for the configuration file.
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@ msrv_aliases! {
|
|||
1,46,0 { CONST_IF_MATCH }
|
||||
1,45,0 { STR_STRIP_PREFIX }
|
||||
1,43,0 { LOG2_10, LOG10_2 }
|
||||
1,42,0 { MATCHES_MACRO }
|
||||
1,42,0 { MATCHES_MACRO, SLICE_PATTERNS }
|
||||
1,41,0 { RE_REBALANCING_COHERENCE, RESULT_MAP_OR_ELSE }
|
||||
1,40,0 { MEM_TAKE, NON_EXHAUSTIVE, OPTION_AS_DEREF }
|
||||
1,38,0 { POINTER_CAST }
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ use rustc_trait_selection::traits::query::normalize::AtExt;
|
|||
|
||||
use crate::{match_def_path, must_use_attr};
|
||||
|
||||
// Checks if the given type implements copy.
|
||||
pub fn is_copy<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
|
||||
ty.is_copy_modulo_regions(cx.tcx.at(DUMMY_SP), cx.param_env)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1 @@
|
|||
max-suggested-slice-pattern-length = 8
|
||||
|
|
@ -0,0 +1,23 @@
|
|||
#![deny(clippy::index_refutable_slice)]
|
||||
|
||||
fn below_limit() {
|
||||
let slice: Option<&[u32]> = Some(&[1, 2, 3]);
|
||||
if let Some(slice) = slice {
|
||||
// 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();
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
error: this binding can be a slice pattern to avoid indexing
|
||||
--> $DIR/index_refutable_slice.rs:5:17
|
||||
|
|
||||
LL | if let Some(slice) = slice {
|
||||
| ^^^^^
|
||||
|
|
||||
note: the lint level is defined here
|
||||
--> $DIR/index_refutable_slice.rs:1:9
|
||||
|
|
||||
LL | #![deny(clippy::index_refutable_slice)]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
help: try using a slice pattern here
|
||||
|
|
||||
LL | if let Some([_, _, _, _, _, _, _, slice_7, ..]) = slice {
|
||||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
help: and replace the index expressions here
|
||||
|
|
||||
LL | println!("{}", slice_7);
|
||||
| ~~~~~~~
|
||||
|
||||
error: aborting due to previous error
|
||||
|
||||
|
|
@ -59,10 +59,20 @@ fn manual_strip_msrv() {
|
|||
}
|
||||
}
|
||||
|
||||
fn check_index_refutable_slice() {
|
||||
// This shouldn't trigger `clippy::index_refutable_slice` as the suggestion
|
||||
// would only be valid from 1.42.0 onward
|
||||
let slice: Option<&[u32]> = Some(&[1]);
|
||||
if let Some(slice) = slice {
|
||||
println!("{}", slice[0]);
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {
|
||||
option_as_ref_deref();
|
||||
match_like_matches();
|
||||
match_same_arms();
|
||||
match_same_arms2();
|
||||
manual_strip_msrv();
|
||||
check_index_refutable_slice();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `disallowed-types`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `standard-macro-braces`, `enforced-import-renames`, `allowed-scripts`, `enable-raw-pointer-heuristic-for-send`, `third-party` at line 5 column 1
|
||||
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `disallowed-types`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `standard-macro-braces`, `enforced-import-renames`, `allowed-scripts`, `enable-raw-pointer-heuristic-for-send`, `max-suggested-slice-pattern-length`, `third-party` at line 5 column 1
|
||||
|
||||
error: aborting due to previous error
|
||||
|
||||
|
|
|
|||
166
tests/ui/index_refutable_slice/if_let_slice_binding.rs
Normal file
166
tests/ui/index_refutable_slice/if_let_slice_binding.rs
Normal file
|
|
@ -0,0 +1,166 @@
|
|||
#![deny(clippy::index_refutable_slice)]
|
||||
|
||||
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) = slice {
|
||||
println!("{}", slice[0]);
|
||||
}
|
||||
|
||||
// Try with copy
|
||||
let slice: Option<[u32; 3]> = Some([1, 2, 3]);
|
||||
if let Some(slice) = slice {
|
||||
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) = slice {
|
||||
println!("{}", slice[2]);
|
||||
println!("{}", slice[0]);
|
||||
}
|
||||
|
||||
// Multiple bindings
|
||||
let slice_wrapped: SomeEnum<[u32; 3]> = SomeEnum::One([5, 6, 7]);
|
||||
if let SomeEnum::One(slice) | SomeEnum::Three(slice) = slice_wrapped {
|
||||
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), Some(b)) = (a_wrapped, b_wrapped) {
|
||||
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) = slice {
|
||||
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) = &slice {
|
||||
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) = wrap.inner {
|
||||
if wrap.is_awesome {
|
||||
println!("This is awesome! {}", slice[0]);
|
||||
}
|
||||
}
|
||||
|
||||
// Test 2: function access
|
||||
if let Some(slice) = wrap.inner {
|
||||
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() {}
|
||||
158
tests/ui/index_refutable_slice/if_let_slice_binding.stderr
Normal file
158
tests/ui/index_refutable_slice/if_let_slice_binding.stderr
Normal file
|
|
@ -0,0 +1,158 @@
|
|||
error: this binding can be a slice pattern to avoid indexing
|
||||
--> $DIR/if_let_slice_binding.rs:13:17
|
||||
|
|
||||
LL | if let Some(slice) = slice {
|
||||
| ^^^^^
|
||||
|
|
||||
note: the lint level is defined here
|
||||
--> $DIR/if_let_slice_binding.rs:1:9
|
||||
|
|
||||
LL | #![deny(clippy::index_refutable_slice)]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
help: try using a slice pattern here
|
||||
|
|
||||
LL | if let Some([slice_0, ..]) = slice {
|
||||
| ~~~~~~~~~~~~~
|
||||
help: and replace the index expressions here
|
||||
|
|
||||
LL | println!("{}", slice_0);
|
||||
| ~~~~~~~
|
||||
|
||||
error: this binding can be a slice pattern to avoid indexing
|
||||
--> $DIR/if_let_slice_binding.rs:19:17
|
||||
|
|
||||
LL | if let Some(slice) = slice {
|
||||
| ^^^^^
|
||||
|
|
||||
help: try using a slice pattern here
|
||||
|
|
||||
LL | if let Some([slice_0, ..]) = slice {
|
||||
| ~~~~~~~~~~~~~
|
||||
help: and replace the index expressions here
|
||||
|
|
||||
LL | println!("{}", slice_0);
|
||||
| ~~~~~~~
|
||||
|
||||
error: this binding can be a slice pattern to avoid indexing
|
||||
--> $DIR/if_let_slice_binding.rs:25: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
|
||||
|
|
||||
LL ~ println!("{}", slice_2);
|
||||
LL ~ println!("{}", slice_0);
|
||||
|
|
||||
|
||||
error: this binding can be a slice pattern to avoid indexing
|
||||
--> $DIR/if_let_slice_binding.rs:32:26
|
||||
|
|
||||
LL | if let SomeEnum::One(slice) | SomeEnum::Three(slice) = slice_wrapped {
|
||||
| ^^^^^
|
||||
|
|
||||
help: try using a slice pattern here
|
||||
|
|
||||
LL | if let SomeEnum::One([slice_0, ..]) | SomeEnum::Three([slice_0, ..]) = slice_wrapped {
|
||||
| ~~~~~~~~~~~~~ ~~~~~~~~~~~~~
|
||||
help: and replace the index expressions here
|
||||
|
|
||||
LL | println!("{}", slice_0);
|
||||
| ~~~~~~~
|
||||
|
||||
error: this binding can be a slice pattern to avoid indexing
|
||||
--> $DIR/if_let_slice_binding.rs:39:29
|
||||
|
|
||||
LL | if let (SomeEnum::Three(a), Some(b)) = (a_wrapped, b_wrapped) {
|
||||
| ^
|
||||
|
|
||||
help: try using a slice pattern here
|
||||
|
|
||||
LL | if let (SomeEnum::Three([_, _, a_2, ..]), Some(b)) = (a_wrapped, b_wrapped) {
|
||||
| ~~~~~~~~~~~~~~~
|
||||
help: and replace the index expressions here
|
||||
|
|
||||
LL | println!("{} -> {}", a_2, b[1]);
|
||||
| ~~~
|
||||
|
||||
error: this binding can be a slice pattern to avoid indexing
|
||||
--> $DIR/if_let_slice_binding.rs:39:38
|
||||
|
|
||||
LL | if let (SomeEnum::Three(a), Some(b)) = (a_wrapped, b_wrapped) {
|
||||
| ^
|
||||
|
|
||||
help: try using a slice pattern here
|
||||
|
|
||||
LL | if let (SomeEnum::Three(a), Some([_, b_1, ..])) = (a_wrapped, b_wrapped) {
|
||||
| ~~~~~~~~~~~~
|
||||
help: and replace the index expressions here
|
||||
|
|
||||
LL | println!("{} -> {}", a[2], b_1);
|
||||
| ~~~
|
||||
|
||||
error: this binding can be a slice pattern to avoid indexing
|
||||
--> $DIR/if_let_slice_binding.rs:46:21
|
||||
|
|
||||
LL | if let Some(ref slice) = slice {
|
||||
| ^^^^^
|
||||
|
|
||||
help: try using a slice pattern here
|
||||
|
|
||||
LL | if let Some([_, ref slice_1, ..]) = slice {
|
||||
| ~~~~~~~~~~~~~~~~~~~~
|
||||
help: and replace the index expressions here
|
||||
|
|
||||
LL | println!("{:?}", slice_1);
|
||||
| ~~~~~~~
|
||||
|
||||
error: this binding can be a slice pattern to avoid indexing
|
||||
--> $DIR/if_let_slice_binding.rs:54:17
|
||||
|
|
||||
LL | if let Some(slice) = &slice {
|
||||
| ^^^^^
|
||||
|
|
||||
help: try using a slice pattern here
|
||||
|
|
||||
LL | if let Some([slice_0, ..]) = &slice {
|
||||
| ~~~~~~~~~~~~~
|
||||
help: and replace the index expressions here
|
||||
|
|
||||
LL | println!("{:?}", slice_0);
|
||||
| ~~~~~~~
|
||||
|
||||
error: this binding can be a slice pattern to avoid indexing
|
||||
--> $DIR/if_let_slice_binding.rs:123:17
|
||||
|
|
||||
LL | if let Some(slice) = wrap.inner {
|
||||
| ^^^^^
|
||||
|
|
||||
help: try using a slice pattern here
|
||||
|
|
||||
LL | if let Some([slice_0, ..]) = wrap.inner {
|
||||
| ~~~~~~~~~~~~~
|
||||
help: and replace the index expressions here
|
||||
|
|
||||
LL | println!("This is awesome! {}", slice_0);
|
||||
| ~~~~~~~
|
||||
|
||||
error: this binding can be a slice pattern to avoid indexing
|
||||
--> $DIR/if_let_slice_binding.rs:130:17
|
||||
|
|
||||
LL | if let Some(slice) = wrap.inner {
|
||||
| ^^^^^
|
||||
|
|
||||
help: try using a slice pattern here
|
||||
|
|
||||
LL | if let Some([slice_0, ..]) = wrap.inner {
|
||||
| ~~~~~~~~~~~~~
|
||||
help: and replace the index expressions here
|
||||
|
|
||||
LL | println!("This is super awesome! {}", slice_0);
|
||||
| ~~~~~~~
|
||||
|
||||
error: aborting due to 10 previous errors
|
||||
|
||||
28
tests/ui/index_refutable_slice/slice_indexing_in_macro.rs
Normal file
28
tests/ui/index_refutable_slice/slice_indexing_in_macro.rs
Normal file
|
|
@ -0,0 +1,28 @@
|
|||
#![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) = slice;
|
||||
then {
|
||||
println!("{}", slice[0]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
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;
|
||||
| ^^^^^
|
||||
|
|
||||
note: the lint level is defined here
|
||||
--> $DIR/slice_indexing_in_macro.rs:1:9
|
||||
|
|
||||
LL | #![deny(clippy::index_refutable_slice)]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
help: try using a slice pattern here
|
||||
|
|
||||
LL | if let Some([slice_0, ..]) = slice;
|
||||
| ~~~~~~~~~~~~~
|
||||
help: and replace the index expressions here
|
||||
|
|
||||
LL | println!("{}", slice_0);
|
||||
| ~~~~~~~
|
||||
|
||||
error: aborting due to previous error
|
||||
|
||||
Loading…
Reference in a new issue