From 19f4708a071e362b43ba185267456321e849dc27 Mon Sep 17 00:00:00 2001 From: Jacherr Date: Mon, 11 Mar 2024 22:15:19 +0000 Subject: [PATCH 1/2] new lint: `unnecessary_indexing` do not lint after mutable reference is taken check path to local on conditional receiver, check mutable borrow after ref, do not lint on mutable auto borrow fix autoborrow/mutability checks remove unneded `extra_locals` inline if ststements; check locality earlier remove unnecessary impl on IndexCheckResult check for borrows in if block and change inner `Some` based on this --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/manual_strip.rs | 4 +- clippy_lints/src/unnecessary_indexing.rs | 220 ++++++++++++++++++ .../if_let_slice_binding.fixed | 1 + .../if_let_slice_binding.rs | 1 + .../if_let_slice_binding.stderr | 20 +- tests/ui/unnecessary_indexing.rs | 131 +++++++++++ tests/ui/unnecessary_indexing.stderr | 160 +++++++++++++ 10 files changed, 529 insertions(+), 12 deletions(-) create mode 100644 clippy_lints/src/unnecessary_indexing.rs create mode 100644 tests/ui/unnecessary_indexing.rs create mode 100644 tests/ui/unnecessary_indexing.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index b87d026fda19..f9b359a81656 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6160,6 +6160,7 @@ Released 2018-09-13 [`unnecessary_first_then_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_first_then_check [`unnecessary_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fold [`unnecessary_get_then_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_get_then_check +[`unnecessary_indexing`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_indexing [`unnecessary_join`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_join [`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations [`unnecessary_literal_bound`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_literal_bound diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 6d6b415f8cdd..dd441b4e7dd3 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -754,6 +754,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::unit_types::UNIT_CMP_INFO, crate::unnecessary_box_returns::UNNECESSARY_BOX_RETURNS_INFO, crate::unnecessary_literal_bound::UNNECESSARY_LITERAL_BOUND_INFO, + crate::unnecessary_indexing::UNNECESSARY_INDEXING_INFO, crate::unnecessary_map_on_constructor::UNNECESSARY_MAP_ON_CONSTRUCTOR_INFO, crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO, crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 7888119567b9..f4d52eaf544d 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -368,6 +368,7 @@ mod uninit_vec; mod unit_return_expecting_ord; mod unit_types; mod unnecessary_box_returns; +mod unnecessary_indexing; mod unnecessary_literal_bound; mod unnecessary_map_on_constructor; mod unnecessary_owned_empty_strings; @@ -974,5 +975,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(move |_| Box::new(arbitrary_source_item_ordering::ArbitrarySourceItemOrdering::new(conf))); store.register_late_pass(|_| Box::new(unneeded_struct_pattern::UnneededStructPattern)); store.register_late_pass(|_| Box::new(unnecessary_semicolon::UnnecessarySemicolon)); + store.register_late_pass(|_| Box::new(unnecessary_indexing::UnnecessaryIndexing)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/manual_strip.rs b/clippy_lints/src/manual_strip.rs index d69384a2cb70..0f1e04e08fd6 100644 --- a/clippy_lints/src/manual_strip.rs +++ b/clippy_lints/src/manual_strip.rs @@ -96,7 +96,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualStrip { } let strippings = find_stripping(cx, strip_kind, target_res, pattern, then); - if !strippings.is_empty() { + if let Some(first_stripping) = strippings.first() { let kind_word = match strip_kind { StripKind::Prefix => "prefix", StripKind::Suffix => "suffix", @@ -106,7 +106,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualStrip { span_lint_and_then( cx, MANUAL_STRIP, - strippings[0], + *first_stripping, format!("stripping a {kind_word} manually"), |diag| { diag.span_note(test_span, format!("the {kind_word} was tested here")); diff --git a/clippy_lints/src/unnecessary_indexing.rs b/clippy_lints/src/unnecessary_indexing.rs new file mode 100644 index 000000000000..b0ce552b6b0b --- /dev/null +++ b/clippy_lints/src/unnecessary_indexing.rs @@ -0,0 +1,220 @@ +use std::ops::ControlFlow; + +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::snippet; +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::visitors::for_each_expr; +use clippy_utils::{path_to_local, path_to_local_id}; +use rustc_ast::{LitKind, Mutability}; +use rustc_errors::Applicability; +use rustc_hir::{Block, Expr, ExprKind, HirId, LetStmt, Node, UnOp}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::adjustment::{Adjust, AutoBorrow, AutoBorrowMutability}; +use rustc_session::declare_lint_pass; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// Checks for the use of `seq.is_empty()` in an if-conditional where `seq` is a slice, array, or Vec, + /// and in which the first element of the sequence is indexed. + /// + /// ### Why is this bad? + /// This code is unnecessarily complicated and can instead be simplified to the use of an + /// if..let expression which accesses the first element of the sequence. + /// + /// ### Example + /// ```no_run + /// let a: &[i32] = &[1]; + /// if !a.is_empty() { + /// let b = a[0]; + /// } + /// ``` + /// Use instead: + /// ```no_run + /// let a: &[i32] = &[1]; + /// if let Some(b) = a.first() { + /// + /// } + /// ``` + #[clippy::version = "1.78.0"] + pub UNNECESSARY_INDEXING, + complexity, + "unnecessary use of `seq.is_empty()` in a conditional when if..let is more appropriate" +} + +declare_lint_pass!(UnnecessaryIndexing => [UNNECESSARY_INDEXING]); + +impl<'tcx> LateLintPass<'tcx> for UnnecessaryIndexing { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'_ Expr<'tcx>) { + if let Some(if_expr) = clippy_utils::higher::If::hir(expr) + // check for negation + && let ExprKind::Unary(UnOp::Not, unary_inner) = if_expr.cond.kind + // check for call of is_empty + && let ExprKind::MethodCall(method, conditional_receiver, _, _) = unary_inner.kind + && method.ident.as_str() == "is_empty" + && let expr_ty = cx.typeck_results().expr_ty(conditional_receiver) + && let peeled = expr_ty.peel_refs() + && (peeled.is_slice() || peeled.is_array() || is_type_diagnostic_item(cx, peeled, sym::Vec)) + && let ExprKind::Block(block, _) = if_expr.then.kind + // do not lint if conditional receiver is mutable reference + && expr_ty.ref_mutability() != Some(Mutability::Mut) + && let Some(con_path) = path_to_local(conditional_receiver) + && let Some(r) = process_indexing(cx, block, con_path) + && let Some(receiver) = r.index_receiver + { + span_lint_and_then( + cx, + UNNECESSARY_INDEXING, + expr.span, + "condition can be simplified with if..let syntax", + |diag| { + if let Some(first_local) = r.first_local + && first_local.pat.simple_ident().is_some() + { + diag.span_suggestion( + if_expr.cond.span, + "consider using if..let syntax (variable may need to be dereferenced)", + format!( + "let Some({}{}) = {}.first()", + // if we arent borrowing anything then we can pass a reference here for correctness + if r.extra_exprs_borrow.is_empty() { "&" } else { "" }, + snippet(cx, first_local.pat.span, ".."), + snippet(cx, receiver.span, "..") + ), + Applicability::Unspecified, + ); + diag.span_suggestion(first_local.span, "remove this line", "", Applicability::Unspecified); + if !r.extra_exprs_borrow.is_empty() { + let mut index_accesses = r + .extra_exprs_borrow + .iter() + .map(|x| (x.span, snippet(cx, first_local.pat.span, "..").to_string())) + .collect::>(); + + index_accesses.extend( + r.extra_exprs_copy + .iter() + .map(|x| (x.span, snippet(cx, first_local.pat.span, "..").to_string())), + ); + + diag.multipart_suggestion( + "set this indexing to be the `Some` variant (may need dereferencing)", + index_accesses, + Applicability::Unspecified, + ); + } else if !r.extra_exprs_copy.is_empty() { + let index_accesses = r + .extra_exprs_copy + .iter() + .map(|x| (x.span, snippet(cx, first_local.pat.span, "..").to_string())) + .collect::>(); + + diag.multipart_suggestion( + "set this indexing to be the `Some` variant", + index_accesses, + Applicability::Unspecified, + ); + } + } else if r.extra_exprs_borrow.is_empty() { + let mut index_accesses = vec![( + if_expr.cond.span, + format!("let Some(&element) = {}.first()", snippet(cx, receiver.span, "..")), + )]; + index_accesses.extend(r.extra_exprs_copy.iter().map(|x| (x.span, "element".to_owned()))); + + diag.multipart_suggestion( + "consider using if..let syntax", + index_accesses, + Applicability::Unspecified, + ); + } else { + let mut index_accesses = vec![( + if_expr.cond.span, + format!("let Some(element) = {}.first()", snippet(cx, receiver.span, "..")), + )]; + index_accesses.extend(r.extra_exprs_borrow.iter().map(|x| (x.span, "element".to_owned()))); + index_accesses.extend(r.extra_exprs_copy.iter().map(|x| (x.span, "element".to_owned()))); + + diag.multipart_suggestion( + "consider using if..let syntax (variable may need to be dereferenced)", + index_accesses, + Applicability::Unspecified, + ); + } + }, + ); + } + } +} + +struct IndexCheckResult<'a> { + // the receiver for the index operation, only Some in the event the indexing is via a direct primitive + index_receiver: Option<&'a Expr<'a>>, + // first local in the block - used as pattern for `Some(pat)` + first_local: Option<&'a LetStmt<'a>>, + // any other index expressions to replace with `pat` (or "element" if no local exists) + extra_exprs_borrow: Vec<&'a Expr<'a>>, + // copied extra index expressions: if we only have these and no borrows we can provide a correct suggestion of `let + // Some(&a) = ...` + extra_exprs_copy: Vec<&'a Expr<'a>>, +} + +/// Checks the block for any indexing of the conditional receiver. Returns `None` if the block +/// contains code that invalidates the lint, e.g., the receiver is accessed via a mutable reference. +fn process_indexing<'a>( + cx: &LateContext<'a>, + block: &'a Block<'a>, + conditional_receiver_hid: HirId, +) -> Option> { + let mut index_receiver: Option<&Expr<'_>> = None; + let mut first_local: Option<&LetStmt<'_>> = None; + let mut extra_exprs_borrow: Vec<&Expr<'_>> = vec![]; + let mut extra_exprs_copy: Vec<&Expr<'_>> = vec![]; + + // if res == Some(()), then mutation occurred + // & therefore we should not lint on this + let res = for_each_expr(cx, block.stmts, |x| { + let adjustments = cx.typeck_results().expr_adjustments(x); + if let ExprKind::Index(receiver, index, _) = x.kind + && let ExprKind::Lit(lit) = index.kind + && let LitKind::Int(val, _) = lit.node + && path_to_local_id(receiver, conditional_receiver_hid) + && val.0 == 0 + { + index_receiver = Some(receiver); + if let Node::LetStmt(local) = cx.tcx.parent_hir_node(x.hir_id) { + if first_local.is_none() { + first_local = Some(local); + return ControlFlow::Continue::<()>(()); + }; + } + + if let Node::Expr(x) = cx.tcx.parent_hir_node(x.hir_id) + && let ExprKind::AddrOf(_, _, _) = x.kind + { + extra_exprs_borrow.push(x); + } else { + extra_exprs_copy.push(x); + }; + } else if adjustments.iter().any(|adjustment| { + matches!( + adjustment.kind, + Adjust::Borrow(AutoBorrow::Ref(_, AutoBorrowMutability::Mut { .. })) + ) + }) { + // do not lint on mutable auto borrows (https://github.com/rust-lang/rust-clippy/pull/12464#discussion_r1600352696) + return ControlFlow::Break(()); + } else if let ExprKind::AddrOf(_, Mutability::Mut, _) = x.kind { + return ControlFlow::Break(()); + }; + + ControlFlow::Continue::<()>(()) + }); + + res.is_none().then_some(IndexCheckResult { + index_receiver, + first_local, + extra_exprs_borrow, + extra_exprs_copy, + }) +} diff --git a/tests/ui/index_refutable_slice/if_let_slice_binding.fixed b/tests/ui/index_refutable_slice/if_let_slice_binding.fixed index ea8e56e18b08..243619f608a9 100644 --- a/tests/ui/index_refutable_slice/if_let_slice_binding.fixed +++ b/tests/ui/index_refutable_slice/if_let_slice_binding.fixed @@ -1,5 +1,6 @@ #![deny(clippy::index_refutable_slice)] #![allow(clippy::uninlined_format_args, clippy::needless_lifetimes)] +#![allow(clippy::unnecessary_indexing)] enum SomeEnum { One(T), diff --git a/tests/ui/index_refutable_slice/if_let_slice_binding.rs b/tests/ui/index_refutable_slice/if_let_slice_binding.rs index 1c1d1c4cbe46..c4085ec98b43 100644 --- a/tests/ui/index_refutable_slice/if_let_slice_binding.rs +++ b/tests/ui/index_refutable_slice/if_let_slice_binding.rs @@ -1,5 +1,6 @@ #![deny(clippy::index_refutable_slice)] #![allow(clippy::uninlined_format_args, clippy::needless_lifetimes)] +#![allow(clippy::unnecessary_indexing)] enum SomeEnum { One(T), diff --git a/tests/ui/index_refutable_slice/if_let_slice_binding.stderr b/tests/ui/index_refutable_slice/if_let_slice_binding.stderr index 14ee2e54cab1..07c253d25f35 100644 --- a/tests/ui/index_refutable_slice/if_let_slice_binding.stderr +++ b/tests/ui/index_refutable_slice/if_let_slice_binding.stderr @@ -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:14:17 + --> tests/ui/index_refutable_slice/if_let_slice_binding.rs:15:17 | LL | if let Some(slice) = slice { | ^^^^^ @@ -17,7 +17,7 @@ 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:21:17 + --> tests/ui/index_refutable_slice/if_let_slice_binding.rs:22:17 | LL | if let Some(slice) = slice { | ^^^^^ @@ -30,7 +30,7 @@ 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:28:17 + --> tests/ui/index_refutable_slice/if_let_slice_binding.rs:29:17 | LL | if let Some(slice) = slice { | ^^^^^ @@ -44,7 +44,7 @@ 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:36:26 + --> tests/ui/index_refutable_slice/if_let_slice_binding.rs:37:26 | LL | if let SomeEnum::One(slice) | SomeEnum::Three(slice) = slice_wrapped { | ^^^^^ @@ -57,7 +57,7 @@ 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:44:29 + --> tests/ui/index_refutable_slice/if_let_slice_binding.rs:45:29 | LL | if let (SomeEnum::Three(a), Some(b)) = (a_wrapped, b_wrapped) { | ^ @@ -71,7 +71,7 @@ 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:44:38 + --> tests/ui/index_refutable_slice/if_let_slice_binding.rs:45:38 | LL | if let (SomeEnum::Three(a), Some(b)) = (a_wrapped, b_wrapped) { | ^ @@ -85,7 +85,7 @@ 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:53:21 + --> tests/ui/index_refutable_slice/if_let_slice_binding.rs:54:21 | LL | if let Some(ref slice) = slice { | ^^^^^ @@ -98,7 +98,7 @@ 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:62:17 + --> tests/ui/index_refutable_slice/if_let_slice_binding.rs:63:17 | LL | if let Some(slice) = &slice { | ^^^^^ @@ -111,7 +111,7 @@ 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:132:17 + --> tests/ui/index_refutable_slice/if_let_slice_binding.rs:133:17 | LL | if let Some(slice) = wrap.inner { | ^^^^^ @@ -125,7 +125,7 @@ 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:140:17 + --> tests/ui/index_refutable_slice/if_let_slice_binding.rs:141:17 | LL | if let Some(slice) = wrap.inner { | ^^^^^ diff --git a/tests/ui/unnecessary_indexing.rs b/tests/ui/unnecessary_indexing.rs new file mode 100644 index 000000000000..31d2b2d0433b --- /dev/null +++ b/tests/ui/unnecessary_indexing.rs @@ -0,0 +1,131 @@ +//@no-rustfix +#![allow(unused)] +#![allow(dropping_copy_types)] +#![allow(dropping_references)] +#![warn(clippy::unnecessary_indexing)] + +fn c(x: i32) -> i32 { + println!("{x}"); + 10 +} + +struct Struct; +impl Struct { + pub fn a(x: i32) -> i32 { + println!("{x}"); + 10 + } +} + +fn main() { + // lint on vecs with a call + let a: Vec = vec![1]; + if !a.is_empty() { + let b = c(a[0]); + } + + // lint on vecs with a method call + let a: Vec = vec![1]; + if !a.is_empty() { + let b = Struct::a(a[0]); + } + + // lint on arrays with a call + let a: &[i32] = &[1]; + if !a.is_empty() { + let b = c(a[0]); + } + + // lint on arrays with a method call + let a: &[i32] = &[1]; + if !a.is_empty() { + let b = Struct::a(a[0]); + } + + // lint on vecs with a local access + let a: Vec = vec![1]; + if !a.is_empty() { + let b = a[0]; + } + + // lint on arrays with a local access + let a: &[i32] = &[1]; + if !a.is_empty() { + let b = a[0]; + } + + // lint when access is not first line + let a: &[i32] = &[1]; + if !a.is_empty() { + dbg!(a); + let b = a[0]; + } + + // lint on multiple accesses/locals + let a: &[i32] = &[1]; + if !a.is_empty() { + dbg!(a); + let b = &a[0]; + let c = a[0]; + drop(a[0]); + } + + // lint on multiple accesses + let a: &[i32] = &[1]; + if !a.is_empty() { + dbg!(a); + drop(a[0]); + drop(a[0]); + } + + // dont lint when not accessing [0] + let a: &[i32] = &[1, 2]; + if !a.is_empty() { + let b = a[1]; + } + + // dont lint when access is dynamic + const T: usize = 0; + + let a: &[i32] = &[1]; + if !a.is_empty() { + let b = a[T]; + } + + // dont lint without unary + let a: &[i32] = &[1]; + if a.is_empty() { + let b = a[0]; + } + + // dont lint if we have mutable reference + let mut a: &[i32] = &[1]; + if !a.is_empty() { + drop(&mut a); + let b = a[0]; + } + + // dont lint if we have a mutable reference, even if the mutable reference occurs after what we are + // linting against + let mut a: &[i32] = &[1]; + if !a.is_empty() { + let b = a[0]; + drop(&mut a); + } + + // dont lint on mutable auto borrow + let mut a = vec![1, 2, 3]; + if !a.is_empty() { + a.push(1); + let b = a[0]; + b; + } + + // do not lint if conditional receiver is mutable reference + let a = &mut vec![1, 2, 3]; + if !a.is_empty() { + let b = a[0]; + a; + b; + } +} diff --git a/tests/ui/unnecessary_indexing.stderr b/tests/ui/unnecessary_indexing.stderr new file mode 100644 index 000000000000..15714a5c25e2 --- /dev/null +++ b/tests/ui/unnecessary_indexing.stderr @@ -0,0 +1,160 @@ +error: condition can be simplified with if..let syntax + --> tests/ui/unnecessary_indexing.rs:23:5 + | +LL | / if !a.is_empty() { +LL | | let b = c(a[0]); +LL | | } + | |_____^ + | + = note: `-D clippy::unnecessary-indexing` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unnecessary_indexing)]` +help: consider using if..let syntax + | +LL ~ if let Some(&element) = a.first() { +LL ~ let b = c(element); + | + +error: condition can be simplified with if..let syntax + --> tests/ui/unnecessary_indexing.rs:29:5 + | +LL | / if !a.is_empty() { +LL | | let b = Struct::a(a[0]); +LL | | } + | |_____^ + | +help: consider using if..let syntax + | +LL ~ if let Some(&element) = a.first() { +LL ~ let b = Struct::a(element); + | + +error: condition can be simplified with if..let syntax + --> tests/ui/unnecessary_indexing.rs:35:5 + | +LL | / if !a.is_empty() { +LL | | let b = c(a[0]); +LL | | } + | |_____^ + | +help: consider using if..let syntax + | +LL ~ if let Some(&element) = a.first() { +LL ~ let b = c(element); + | + +error: condition can be simplified with if..let syntax + --> tests/ui/unnecessary_indexing.rs:41:5 + | +LL | / if !a.is_empty() { +LL | | let b = Struct::a(a[0]); +LL | | } + | |_____^ + | +help: consider using if..let syntax + | +LL ~ if let Some(&element) = a.first() { +LL ~ let b = Struct::a(element); + | + +error: condition can be simplified with if..let syntax + --> tests/ui/unnecessary_indexing.rs:47:5 + | +LL | / if !a.is_empty() { +LL | | let b = a[0]; +LL | | } + | |_____^ + | +help: consider using if..let syntax (variable may need to be dereferenced) + | +LL | if let Some(&b) = a.first() { + | ~~~~~~~~~~~~~~~~~~~~~~~~ +help: remove this line + | +LL - let b = a[0]; +LL + + | + +error: condition can be simplified with if..let syntax + --> tests/ui/unnecessary_indexing.rs:53:5 + | +LL | / if !a.is_empty() { +LL | | let b = a[0]; +LL | | } + | |_____^ + | +help: consider using if..let syntax (variable may need to be dereferenced) + | +LL | if let Some(&b) = a.first() { + | ~~~~~~~~~~~~~~~~~~~~~~~~ +help: remove this line + | +LL - let b = a[0]; +LL + + | + +error: condition can be simplified with if..let syntax + --> tests/ui/unnecessary_indexing.rs:59:5 + | +LL | / if !a.is_empty() { +LL | | dbg!(a); +LL | | let b = a[0]; +LL | | } + | |_____^ + | +help: consider using if..let syntax (variable may need to be dereferenced) + | +LL | if let Some(&b) = a.first() { + | ~~~~~~~~~~~~~~~~~~~~~~~~ +help: remove this line + | +LL - let b = a[0]; +LL + + | + +error: condition can be simplified with if..let syntax + --> tests/ui/unnecessary_indexing.rs:66:5 + | +LL | / if !a.is_empty() { +LL | | dbg!(a); +LL | | let b = &a[0]; +LL | | let c = a[0]; +LL | | drop(a[0]); +LL | | } + | |_____^ + | +help: consider using if..let syntax (variable may need to be dereferenced) + | +LL | if let Some(c) = a.first() { + | ~~~~~~~~~~~~~~~~~~~~~~~ +help: remove this line + | +LL - let c = a[0]; +LL + + | +help: set this indexing to be the `Some` variant (may need dereferencing) + | +LL ~ let b = c; +LL | let c = a[0]; +LL ~ drop(c); + | + +error: condition can be simplified with if..let syntax + --> tests/ui/unnecessary_indexing.rs:75:5 + | +LL | / if !a.is_empty() { +LL | | dbg!(a); +LL | | drop(a[0]); +LL | | drop(a[0]); +LL | | } + | |_____^ + | +help: consider using if..let syntax + | +LL ~ if let Some(&element) = a.first() { +LL | dbg!(a); +LL ~ drop(element); +LL ~ drop(element); + | + +error: aborting due to 9 previous errors + From d7202a11d73043c9061e7c87552e5312ac58aedb Mon Sep 17 00:00:00 2001 From: J-ZhengLi Date: Wed, 22 Jan 2025 10:38:06 +0800 Subject: [PATCH 2/2] adjust applicability to `MaybeIncorrect` & don't lint with expanded if condition & lint messages etc. --- clippy_lints/src/declared_lints.rs | 2 +- clippy_lints/src/unnecessary_indexing.rs | 110 ++++++---------- tests/ui/unnecessary_indexing.fixed | 157 +++++++++++++++++++++++ tests/ui/unnecessary_indexing.rs | 28 +++- tests/ui/unnecessary_indexing.stderr | 131 +++++++++++-------- 5 files changed, 300 insertions(+), 128 deletions(-) create mode 100644 tests/ui/unnecessary_indexing.fixed diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index dd441b4e7dd3..77c492e78cf8 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -753,8 +753,8 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::unit_types::UNIT_ARG_INFO, crate::unit_types::UNIT_CMP_INFO, crate::unnecessary_box_returns::UNNECESSARY_BOX_RETURNS_INFO, - crate::unnecessary_literal_bound::UNNECESSARY_LITERAL_BOUND_INFO, crate::unnecessary_indexing::UNNECESSARY_INDEXING_INFO, + crate::unnecessary_literal_bound::UNNECESSARY_LITERAL_BOUND_INFO, crate::unnecessary_map_on_constructor::UNNECESSARY_MAP_ON_CONSTRUCTOR_INFO, crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO, crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO, diff --git a/clippy_lints/src/unnecessary_indexing.rs b/clippy_lints/src/unnecessary_indexing.rs index b0ce552b6b0b..110f72b8a0bc 100644 --- a/clippy_lints/src/unnecessary_indexing.rs +++ b/clippy_lints/src/unnecessary_indexing.rs @@ -11,7 +11,7 @@ use rustc_hir::{Block, Expr, ExprKind, HirId, LetStmt, Node, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::adjustment::{Adjust, AutoBorrow, AutoBorrowMutability}; use rustc_session::declare_lint_pass; -use rustc_span::sym; +use rustc_span::{Span, sym}; declare_clippy_lint! { /// ### What it does @@ -36,7 +36,7 @@ declare_clippy_lint! { /// /// } /// ``` - #[clippy::version = "1.78.0"] + #[clippy::version = "1.86.0"] pub UNNECESSARY_INDEXING, complexity, "unnecessary use of `seq.is_empty()` in a conditional when if..let is more appropriate" @@ -47,6 +47,7 @@ declare_lint_pass!(UnnecessaryIndexing => [UNNECESSARY_INDEXING]); impl<'tcx> LateLintPass<'tcx> for UnnecessaryIndexing { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'_ Expr<'tcx>) { if let Some(if_expr) = clippy_utils::higher::If::hir(expr) + && !if_expr.cond.span.from_expansion() // check for negation && let ExprKind::Unary(UnOp::Not, unary_inner) = if_expr.cond.kind // check for call of is_empty @@ -60,87 +61,56 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryIndexing { && expr_ty.ref_mutability() != Some(Mutability::Mut) && let Some(con_path) = path_to_local(conditional_receiver) && let Some(r) = process_indexing(cx, block, con_path) - && let Some(receiver) = r.index_receiver + && let Some(receiver_span) = r.index_receiver_span { + let receiver = snippet(cx, receiver_span, ".."); + let mut suggestions: Vec<(Span, String)> = vec![]; + let mut message = "consider using `if..let` syntax instead of indexing".to_string(); span_lint_and_then( cx, UNNECESSARY_INDEXING, expr.span, - "condition can be simplified with if..let syntax", + "condition can be simplified with `if..let` syntax", |diag| { if let Some(first_local) = r.first_local - && first_local.pat.simple_ident().is_some() + && let Some(name) = first_local.pat.simple_ident().map(|ident| ident.name) { - diag.span_suggestion( + suggestions.push(( if_expr.cond.span, - "consider using if..let syntax (variable may need to be dereferenced)", format!( - "let Some({}{}) = {}.first()", + "let Some({}{name}) = {receiver}.first()", // if we arent borrowing anything then we can pass a reference here for correctness if r.extra_exprs_borrow.is_empty() { "&" } else { "" }, - snippet(cx, first_local.pat.span, ".."), - snippet(cx, receiver.span, "..") ), - Applicability::Unspecified, - ); - diag.span_suggestion(first_local.span, "remove this line", "", Applicability::Unspecified); - if !r.extra_exprs_borrow.is_empty() { - let mut index_accesses = r - .extra_exprs_borrow - .iter() - .map(|x| (x.span, snippet(cx, first_local.pat.span, "..").to_string())) - .collect::>(); + )); + suggestions.push((first_local.span, String::new())); - index_accesses.extend( - r.extra_exprs_copy + if !r.extra_exprs_borrow.is_empty() { + suggestions.extend( + r.extra_exprs_borrow .iter() - .map(|x| (x.span, snippet(cx, first_local.pat.span, "..").to_string())), + .chain(r.extra_exprs_copy.iter()) + .map(|x| (x.span, name.to_string())), ); - diag.multipart_suggestion( - "set this indexing to be the `Some` variant (may need dereferencing)", - index_accesses, - Applicability::Unspecified, - ); + message.push_str(", and replacing indexing expression(s) with the value in `Some` variant"); } else if !r.extra_exprs_copy.is_empty() { - let index_accesses = r - .extra_exprs_copy - .iter() - .map(|x| (x.span, snippet(cx, first_local.pat.span, "..").to_string())) - .collect::>(); - - diag.multipart_suggestion( - "set this indexing to be the `Some` variant", - index_accesses, - Applicability::Unspecified, - ); + suggestions.extend(r.extra_exprs_copy.iter().map(|x| (x.span, name.to_string()))); } } else if r.extra_exprs_borrow.is_empty() { - let mut index_accesses = vec![( - if_expr.cond.span, - format!("let Some(&element) = {}.first()", snippet(cx, receiver.span, "..")), - )]; - index_accesses.extend(r.extra_exprs_copy.iter().map(|x| (x.span, "element".to_owned()))); - - diag.multipart_suggestion( - "consider using if..let syntax", - index_accesses, - Applicability::Unspecified, - ); + suggestions.push((if_expr.cond.span, format!("let Some(&element) = {receiver}.first()"))); + suggestions.extend(r.extra_exprs_copy.iter().map(|x| (x.span, "element".to_owned()))); } else { - let mut index_accesses = vec![( - if_expr.cond.span, - format!("let Some(element) = {}.first()", snippet(cx, receiver.span, "..")), - )]; - index_accesses.extend(r.extra_exprs_borrow.iter().map(|x| (x.span, "element".to_owned()))); - index_accesses.extend(r.extra_exprs_copy.iter().map(|x| (x.span, "element".to_owned()))); - - diag.multipart_suggestion( - "consider using if..let syntax (variable may need to be dereferenced)", - index_accesses, - Applicability::Unspecified, + suggestions.push((if_expr.cond.span, format!("let Some(element) = {receiver}.first()"))); + suggestions.extend( + r.extra_exprs_borrow + .iter() + .chain(r.extra_exprs_copy.iter()) + .map(|x| (x.span, "element".to_owned())), ); } + + diag.multipart_suggestion(message, suggestions, Applicability::MaybeIncorrect); }, ); } @@ -148,8 +118,8 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryIndexing { } struct IndexCheckResult<'a> { - // the receiver for the index operation, only Some in the event the indexing is via a direct primitive - index_receiver: Option<&'a Expr<'a>>, + // span of the receiver for the index operation, only Some in the event the indexing is via a direct primitive + index_receiver_span: Option, // first local in the block - used as pattern for `Some(pat)` first_local: Option<&'a LetStmt<'a>>, // any other index expressions to replace with `pat` (or "element" if no local exists) @@ -166,14 +136,14 @@ fn process_indexing<'a>( block: &'a Block<'a>, conditional_receiver_hid: HirId, ) -> Option> { - let mut index_receiver: Option<&Expr<'_>> = None; + let mut index_receiver_span: Option = None; let mut first_local: Option<&LetStmt<'_>> = None; let mut extra_exprs_borrow: Vec<&Expr<'_>> = vec![]; let mut extra_exprs_copy: Vec<&Expr<'_>> = vec![]; // if res == Some(()), then mutation occurred // & therefore we should not lint on this - let res = for_each_expr(cx, block.stmts, |x| { + let res = for_each_expr(cx, block, |x| { let adjustments = cx.typeck_results().expr_adjustments(x); if let ExprKind::Index(receiver, index, _) = x.kind && let ExprKind::Lit(lit) = index.kind @@ -181,12 +151,12 @@ fn process_indexing<'a>( && path_to_local_id(receiver, conditional_receiver_hid) && val.0 == 0 { - index_receiver = Some(receiver); + index_receiver_span = Some(receiver.span); if let Node::LetStmt(local) = cx.tcx.parent_hir_node(x.hir_id) { if first_local.is_none() { first_local = Some(local); return ControlFlow::Continue::<()>(()); - }; + } } if let Node::Expr(x) = cx.tcx.parent_hir_node(x.hir_id) @@ -195,24 +165,24 @@ fn process_indexing<'a>( extra_exprs_borrow.push(x); } else { extra_exprs_copy.push(x); - }; + } } else if adjustments.iter().any(|adjustment| { matches!( adjustment.kind, - Adjust::Borrow(AutoBorrow::Ref(_, AutoBorrowMutability::Mut { .. })) + Adjust::Borrow(AutoBorrow::Ref(AutoBorrowMutability::Mut { .. })) ) }) { // do not lint on mutable auto borrows (https://github.com/rust-lang/rust-clippy/pull/12464#discussion_r1600352696) return ControlFlow::Break(()); } else if let ExprKind::AddrOf(_, Mutability::Mut, _) = x.kind { return ControlFlow::Break(()); - }; + } ControlFlow::Continue::<()>(()) }); res.is_none().then_some(IndexCheckResult { - index_receiver, + index_receiver_span, first_local, extra_exprs_borrow, extra_exprs_copy, diff --git a/tests/ui/unnecessary_indexing.fixed b/tests/ui/unnecessary_indexing.fixed new file mode 100644 index 000000000000..58d136ebd987 --- /dev/null +++ b/tests/ui/unnecessary_indexing.fixed @@ -0,0 +1,157 @@ +#![allow(unused)] +#![allow(dropping_copy_types)] +#![allow(dropping_references)] +#![warn(clippy::unnecessary_indexing)] + +macro_rules! not_empty { + ($seq:ident) => { + !$seq.is_empty() + }; +} + +fn c(x: i32) -> i32 { + println!("{x}"); + 10 +} + +struct Struct; +impl Struct { + pub fn a(x: i32) -> i32 { + println!("{x}"); + 10 + } +} + +fn main() { + // lint on vecs with a call + let a: Vec = vec![1]; + if let Some(&element) = a.first() { + //~^ ERROR: condition can be simplified with `if..let` syntax + let b = c(element); + } + + // lint on vecs with a method call + let a: Vec = vec![1]; + if let Some(&element) = a.first() { + //~^ ERROR: condition can be simplified with `if..let` syntax + let b = Struct::a(element); + } + + // lint on arrays with a call + let a: &[i32] = &[1]; + if let Some(&element) = a.first() { + //~^ ERROR: condition can be simplified with `if..let` syntax + let b = c(element); + } + + // lint on arrays with a method call + let a: &[i32] = &[1]; + if let Some(&element) = a.first() { + //~^ ERROR: condition can be simplified with `if..let` syntax + let b = Struct::a(element); + } + + // lint on vecs with a local access + let a: Vec = vec![1]; + if let Some(&b) = a.first() { + //~^ ERROR: condition can be simplified with `if..let` syntax + + } + + // lint on arrays with a local access + let a: &[i32] = &[1]; + if let Some(&b) = a.first() { + //~^ ERROR: condition can be simplified with `if..let` syntax + + } + + // lint when access is not first line + let a: &[i32] = &[1]; + if let Some(&b) = a.first() { + //~^ ERROR: condition can be simplified with `if..let` syntax + dbg!(a); + + } + + // lint on multiple accesses/locals + let a: &[i32] = &[1]; + if let Some(c) = a.first() { + //~^ ERROR: condition can be simplified with `if..let` syntax + dbg!(a); + let b = c; + + drop(c); + } + + // lint on multiple accesses + let a: &[i32] = &[1]; + if let Some(&element) = a.first() { + //~^ ERROR: condition can be simplified with `if..let` syntax + dbg!(a); + drop(element); + drop(element); + } + + let _first = if let Some(&element) = a.first() { + //~^ ERROR: condition can be simplified with `if..let` syntax + element + } else { + 1 + }; + + // don't lint when the condition is from expansion + if not_empty!(a) { + let b = a[0]; + } + + // dont lint when not accessing [0] + let a: &[i32] = &[1, 2]; + if !a.is_empty() { + let b = a[1]; + } + + // dont lint when access is dynamic + const T: usize = 0; + + let a: &[i32] = &[1]; + if !a.is_empty() { + let b = a[T]; + } + + // dont lint without unary + let a: &[i32] = &[1]; + if a.is_empty() { + let b = a[0]; + } + + // dont lint if we have mutable reference + let mut a: &[i32] = &[1]; + if !a.is_empty() { + drop(&mut a); + let b = a[0]; + } + + // dont lint if we have a mutable reference, even if the mutable reference occurs after what we are + // linting against + let mut a: &[i32] = &[1]; + if !a.is_empty() { + let b = a[0]; + drop(&mut a); + } + + // dont lint on mutable auto borrow + let mut a = vec![1, 2, 3]; + if !a.is_empty() { + a.push(1); + let b = a[0]; + b; + } + + // do not lint if conditional receiver is mutable reference + let a = &mut vec![1, 2, 3]; + if !a.is_empty() { + let b = a[0]; + a; + b; + } +} diff --git a/tests/ui/unnecessary_indexing.rs b/tests/ui/unnecessary_indexing.rs index 31d2b2d0433b..b4cf56f9d964 100644 --- a/tests/ui/unnecessary_indexing.rs +++ b/tests/ui/unnecessary_indexing.rs @@ -1,9 +1,14 @@ -//@no-rustfix #![allow(unused)] #![allow(dropping_copy_types)] #![allow(dropping_references)] #![warn(clippy::unnecessary_indexing)] +macro_rules! not_empty { + ($seq:ident) => { + !$seq.is_empty() + }; +} + fn c(x: i32) -> i32 { println!("{x}"); 10 @@ -21,42 +26,49 @@ fn main() { // lint on vecs with a call let a: Vec = vec![1]; if !a.is_empty() { + //~^ ERROR: condition can be simplified with `if..let` syntax let b = c(a[0]); } // lint on vecs with a method call let a: Vec = vec![1]; if !a.is_empty() { + //~^ ERROR: condition can be simplified with `if..let` syntax let b = Struct::a(a[0]); } // lint on arrays with a call let a: &[i32] = &[1]; if !a.is_empty() { + //~^ ERROR: condition can be simplified with `if..let` syntax let b = c(a[0]); } // lint on arrays with a method call let a: &[i32] = &[1]; if !a.is_empty() { + //~^ ERROR: condition can be simplified with `if..let` syntax let b = Struct::a(a[0]); } // lint on vecs with a local access let a: Vec = vec![1]; if !a.is_empty() { + //~^ ERROR: condition can be simplified with `if..let` syntax let b = a[0]; } // lint on arrays with a local access let a: &[i32] = &[1]; if !a.is_empty() { + //~^ ERROR: condition can be simplified with `if..let` syntax let b = a[0]; } // lint when access is not first line let a: &[i32] = &[1]; if !a.is_empty() { + //~^ ERROR: condition can be simplified with `if..let` syntax dbg!(a); let b = a[0]; } @@ -64,6 +76,7 @@ fn main() { // lint on multiple accesses/locals let a: &[i32] = &[1]; if !a.is_empty() { + //~^ ERROR: condition can be simplified with `if..let` syntax dbg!(a); let b = &a[0]; let c = a[0]; @@ -73,11 +86,24 @@ fn main() { // lint on multiple accesses let a: &[i32] = &[1]; if !a.is_empty() { + //~^ ERROR: condition can be simplified with `if..let` syntax dbg!(a); drop(a[0]); drop(a[0]); } + let _first = if !a.is_empty() { + //~^ ERROR: condition can be simplified with `if..let` syntax + a[0] + } else { + 1 + }; + + // don't lint when the condition is from expansion + if not_empty!(a) { + let b = a[0]; + } + // dont lint when not accessing [0] let a: &[i32] = &[1, 2]; if !a.is_empty() { diff --git a/tests/ui/unnecessary_indexing.stderr b/tests/ui/unnecessary_indexing.stderr index 15714a5c25e2..aba4067a3ef2 100644 --- a/tests/ui/unnecessary_indexing.stderr +++ b/tests/ui/unnecessary_indexing.stderr @@ -1,120 +1,124 @@ -error: condition can be simplified with if..let syntax - --> tests/ui/unnecessary_indexing.rs:23:5 +error: condition can be simplified with `if..let` syntax + --> tests/ui/unnecessary_indexing.rs:28:5 | LL | / if !a.is_empty() { +LL | | LL | | let b = c(a[0]); LL | | } | |_____^ | = note: `-D clippy::unnecessary-indexing` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::unnecessary_indexing)]` -help: consider using if..let syntax +help: consider using `if..let` syntax instead of indexing | LL ~ if let Some(&element) = a.first() { +LL | LL ~ let b = c(element); | -error: condition can be simplified with if..let syntax - --> tests/ui/unnecessary_indexing.rs:29:5 +error: condition can be simplified with `if..let` syntax + --> tests/ui/unnecessary_indexing.rs:35:5 | LL | / if !a.is_empty() { +LL | | LL | | let b = Struct::a(a[0]); LL | | } | |_____^ | -help: consider using if..let syntax +help: consider using `if..let` syntax instead of indexing | LL ~ if let Some(&element) = a.first() { +LL | LL ~ let b = Struct::a(element); | -error: condition can be simplified with if..let syntax - --> tests/ui/unnecessary_indexing.rs:35:5 +error: condition can be simplified with `if..let` syntax + --> tests/ui/unnecessary_indexing.rs:42:5 | LL | / if !a.is_empty() { +LL | | LL | | let b = c(a[0]); LL | | } | |_____^ | -help: consider using if..let syntax +help: consider using `if..let` syntax instead of indexing | LL ~ if let Some(&element) = a.first() { +LL | LL ~ let b = c(element); | -error: condition can be simplified with if..let syntax - --> tests/ui/unnecessary_indexing.rs:41:5 +error: condition can be simplified with `if..let` syntax + --> tests/ui/unnecessary_indexing.rs:49:5 | LL | / if !a.is_empty() { +LL | | LL | | let b = Struct::a(a[0]); LL | | } | |_____^ | -help: consider using if..let syntax +help: consider using `if..let` syntax instead of indexing | LL ~ if let Some(&element) = a.first() { +LL | LL ~ let b = Struct::a(element); | -error: condition can be simplified with if..let syntax - --> tests/ui/unnecessary_indexing.rs:47:5 +error: condition can be simplified with `if..let` syntax + --> tests/ui/unnecessary_indexing.rs:56:5 | LL | / if !a.is_empty() { +LL | | LL | | let b = a[0]; LL | | } | |_____^ | -help: consider using if..let syntax (variable may need to be dereferenced) +help: consider using `if..let` syntax instead of indexing | -LL | if let Some(&b) = a.first() { - | ~~~~~~~~~~~~~~~~~~~~~~~~ -help: remove this line - | -LL - let b = a[0]; -LL + +LL ~ if let Some(&b) = a.first() { +LL | +LL ~ | -error: condition can be simplified with if..let syntax - --> tests/ui/unnecessary_indexing.rs:53:5 +error: condition can be simplified with `if..let` syntax + --> tests/ui/unnecessary_indexing.rs:63:5 | LL | / if !a.is_empty() { +LL | | LL | | let b = a[0]; LL | | } | |_____^ | -help: consider using if..let syntax (variable may need to be dereferenced) - | -LL | if let Some(&b) = a.first() { - | ~~~~~~~~~~~~~~~~~~~~~~~~ -help: remove this line +help: consider using `if..let` syntax instead of indexing | -LL - let b = a[0]; -LL + +LL ~ if let Some(&b) = a.first() { +LL | +LL ~ | -error: condition can be simplified with if..let syntax - --> tests/ui/unnecessary_indexing.rs:59:5 +error: condition can be simplified with `if..let` syntax + --> tests/ui/unnecessary_indexing.rs:70:5 | LL | / if !a.is_empty() { +LL | | LL | | dbg!(a); LL | | let b = a[0]; LL | | } | |_____^ | -help: consider using if..let syntax (variable may need to be dereferenced) +help: consider using `if..let` syntax instead of indexing | -LL | if let Some(&b) = a.first() { - | ~~~~~~~~~~~~~~~~~~~~~~~~ -help: remove this line - | -LL - let b = a[0]; -LL + +LL ~ if let Some(&b) = a.first() { +LL | +LL | dbg!(a); +LL ~ | -error: condition can be simplified with if..let syntax - --> tests/ui/unnecessary_indexing.rs:66:5 +error: condition can be simplified with `if..let` syntax + --> tests/ui/unnecessary_indexing.rs:78:5 | LL | / if !a.is_empty() { +LL | | LL | | dbg!(a); LL | | let b = &a[0]; LL | | let c = a[0]; @@ -122,39 +126,54 @@ LL | | drop(a[0]); LL | | } | |_____^ | -help: consider using if..let syntax (variable may need to be dereferenced) - | -LL | if let Some(c) = a.first() { - | ~~~~~~~~~~~~~~~~~~~~~~~ -help: remove this line - | -LL - let c = a[0]; -LL + - | -help: set this indexing to be the `Some` variant (may need dereferencing) +help: consider using `if..let` syntax instead of indexing, and replacing indexing expression(s) with the value in `Some` variant | +LL ~ if let Some(c) = a.first() { +LL | +LL | dbg!(a); LL ~ let b = c; -LL | let c = a[0]; +LL ~ LL ~ drop(c); | -error: condition can be simplified with if..let syntax - --> tests/ui/unnecessary_indexing.rs:75:5 +error: condition can be simplified with `if..let` syntax + --> tests/ui/unnecessary_indexing.rs:88:5 | LL | / if !a.is_empty() { +LL | | LL | | dbg!(a); LL | | drop(a[0]); LL | | drop(a[0]); LL | | } | |_____^ | -help: consider using if..let syntax +help: consider using `if..let` syntax instead of indexing | LL ~ if let Some(&element) = a.first() { +LL | LL | dbg!(a); LL ~ drop(element); LL ~ drop(element); | -error: aborting due to 9 previous errors +error: condition can be simplified with `if..let` syntax + --> tests/ui/unnecessary_indexing.rs:95:18 + | +LL | let _first = if !a.is_empty() { + | __________________^ +LL | | +LL | | a[0] +LL | | } else { +LL | | 1 +LL | | }; + | |_____^ + | +help: consider using `if..let` syntax instead of indexing + | +LL ~ let _first = if let Some(&element) = a.first() { +LL | +LL ~ element + | + +error: aborting due to 10 previous errors