diff options
| -rw-r--r-- | clippy_lints/src/len_zero.rs | 30 | ||||
| -rw-r--r-- | clippy_utils/src/higher.rs | 92 | ||||
| -rw-r--r-- | tests/ui/comparison_to_empty.fixed | 12 | ||||
| -rw-r--r-- | tests/ui/comparison_to_empty.rs | 12 | ||||
| -rw-r--r-- | tests/ui/comparison_to_empty.stderr | 40 |
5 files changed, 176 insertions, 10 deletions
diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index e64d46bd6e3..a707921ce7e 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -7,11 +7,10 @@ use rustc_ast::ast::LitKind; use rustc_errors::Applicability; use rustc_hir::def::Res; use rustc_hir::def_id::{DefId, DefIdSet}; -use rustc_hir::lang_items::LangItem; use rustc_hir::{ AssocItemKind, BinOpKind, Expr, ExprKind, FnRetTy, GenericArg, GenericBound, ImplItem, ImplItemKind, - ImplicitSelfKind, Item, ItemKind, Mutability, Node, PathSegment, PrimTy, QPath, TraitItemRef, TyKind, - TypeBindingKind, + ImplicitSelfKind, Item, ItemKind, LangItem, Mutability, Node, PatKind, PathSegment, PrimTy, QPath, TraitItemRef, + TyKind, TypeBindingKind, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, AssocKind, FnSig, Ty}; @@ -168,6 +167,31 @@ impl<'tcx> LateLintPass<'tcx> for LenZero { return; } + if let ExprKind::Let(lt) = expr.kind + && has_is_empty(cx, lt.init) + && match lt.pat.kind { + PatKind::Slice([], _, []) => true, + PatKind::Lit(lit) if is_empty_string(lit) => true, + _ => false, + } + { + let mut applicability = Applicability::MachineApplicable; + + let lit1 = peel_ref_operators(cx, lt.init); + let lit_str = + Sugg::hir_with_context(cx, lit1, lt.span.ctxt(), "_", &mut applicability).maybe_par(); + + span_lint_and_sugg( + cx, + COMPARISON_TO_EMPTY, + lt.span, + "comparison to empty slice using `if let`", + "using `is_empty` is clearer and more explicit", + format!("{lit_str}.is_empty()"), + applicability, + ); + } + if let ExprKind::Binary(Spanned { node: cmp, .. }, left, right) = expr.kind { // expr.span might contains parenthesis, see issue #10529 let actual_span = left.span.with_hi(right.span.hi()); diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index 802adbd4d2d..312e6ea40c5 100644 --- a/clippy_utils/src/higher.rs +++ b/clippy_utils/src/higher.rs @@ -5,6 +5,7 @@ use crate::consts::{constant_simple, Constant}; use crate::ty::is_type_diagnostic_item; use crate::{is_expn_of, match_def_path, paths}; +use hir::BinOpKind; use if_chain::if_chain; use rustc_ast::ast; use rustc_hir as hir; @@ -137,6 +138,97 @@ impl<'hir> IfLet<'hir> { } } +/// A `let` chain, like `if true && let Some(true) = x {}` +#[derive(Debug)] +pub struct LetChain<'hir> { + pub conds: Vec<IfOrIfLetInChain<'hir>>, + pub if_then: &'hir Expr<'hir>, + pub if_else: Option<&'hir Expr<'hir>>, +} + +impl<'hir> LetChain<'hir> { + pub fn hir(expr: &Expr<'hir>) -> Option<Self> { + if let ExprKind::If(cond, if_then, if_else) = expr.kind { + let mut conds = vec![]; + let mut cursor = cond; + while let ExprKind::Binary(binop, lhs, rhs) = cursor.kind + && let BinOpKind::And = binop.node + { + cursor = lhs; + conds.push(IfOrIfLetInChain::hir(rhs)?); + } + + // The final lhs cannot be `&&` + conds.push(IfOrIfLetInChain::hir(cursor)?); + + return Some(Self { + conds, + if_then, + if_else, + }); + } + + None + } +} + +/// An `if let` or `if` expression in a let chain. +#[derive(Debug)] +pub enum IfOrIfLetInChain<'hir> { + If(IfInChain<'hir>), + IfLet(IfLetInChain<'hir>), +} + +impl<'hir> IfOrIfLetInChain<'hir> { + pub fn hir(expr: &Expr<'hir>) -> Option<Self> { + match expr.kind { + ExprKind::DropTemps(cond) => Some(IfInChain { cond }.into()), + ExprKind::Let(hir::Let { + pat: let_pat, + init: let_expr, + span: let_span, + .. + }) => Some( + IfLetInChain { + let_pat, + let_expr, + let_span: *let_span, + } + .into(), + ), + _ => None, + } + } +} + +impl<'hir> From<IfInChain<'hir>> for IfOrIfLetInChain<'hir> { + fn from(value: IfInChain<'hir>) -> Self { + Self::If(value) + } +} + +impl<'hir> From<IfLetInChain<'hir>> for IfOrIfLetInChain<'hir> { + fn from(value: IfLetInChain<'hir>) -> Self { + Self::IfLet(value) + } +} + +/// An `if` expression in a let chain. +#[derive(Debug)] +pub struct IfInChain<'hir> { + pub cond: &'hir Expr<'hir>, +} + +/// An `if let` expression in a let chain. +#[derive(Debug)] +pub struct IfLetInChain<'hir> { + pub let_span: Span, + /// `if let` pattern + pub let_pat: &'hir Pat<'hir>, + /// `if let` scrutinee + pub let_expr: &'hir Expr<'hir>, +} + /// An `if let` or `match` expression. Useful for lints that trigger on one or the other. #[derive(Debug)] pub enum IfLetOrMatch<'hir> { diff --git a/tests/ui/comparison_to_empty.fixed b/tests/ui/comparison_to_empty.fixed index c92dd509ebb..af219eed0b8 100644 --- a/tests/ui/comparison_to_empty.fixed +++ b/tests/ui/comparison_to_empty.fixed @@ -1,7 +1,8 @@ //@run-rustfix #![warn(clippy::comparison_to_empty)] -#![allow(clippy::useless_vec)] +#![allow(clippy::borrow_deref_ref, clippy::needless_if, clippy::useless_vec)] +#![feature(let_chains)] fn main() { // Disallow comparisons to empty @@ -12,6 +13,11 @@ fn main() { let v = vec![0]; let _ = v.is_empty(); let _ = !v.is_empty(); + if (*v).is_empty() {} + let s = [0].as_slice(); + if s.is_empty() {} + if s.is_empty() {} + if s.is_empty() && s.is_empty() {} // Allow comparisons to non-empty let s = String::new(); @@ -21,4 +27,8 @@ fn main() { let v = vec![0]; let _ = v == [0]; let _ = v != [0]; + if let [0] = &*v {} + let s = [0].as_slice(); + if let [0] = s {} + if let [0] = &*s && s == [0] {} } diff --git a/tests/ui/comparison_to_empty.rs b/tests/ui/comparison_to_empty.rs index b3489714380..21e65184d50 100644 --- a/tests/ui/comparison_to_empty.rs +++ b/tests/ui/comparison_to_empty.rs @@ -1,7 +1,8 @@ //@run-rustfix #![warn(clippy::comparison_to_empty)] -#![allow(clippy::useless_vec)] +#![allow(clippy::borrow_deref_ref, clippy::needless_if, clippy::useless_vec)] +#![feature(let_chains)] fn main() { // Disallow comparisons to empty @@ -12,6 +13,11 @@ fn main() { let v = vec![0]; let _ = v == []; let _ = v != []; + if let [] = &*v {} + let s = [0].as_slice(); + if let [] = s {} + if let [] = &*s {} + if let [] = &*s && s == [] {} // Allow comparisons to non-empty let s = String::new(); @@ -21,4 +27,8 @@ fn main() { let v = vec![0]; let _ = v == [0]; let _ = v != [0]; + if let [0] = &*v {} + let s = [0].as_slice(); + if let [0] = s {} + if let [0] = &*s && s == [0] {} } diff --git a/tests/ui/comparison_to_empty.stderr b/tests/ui/comparison_to_empty.stderr index cc09b17eb89..f29782ed80d 100644 --- a/tests/ui/comparison_to_empty.stderr +++ b/tests/ui/comparison_to_empty.stderr @@ -1,5 +1,5 @@ error: comparison to empty slice - --> $DIR/comparison_to_empty.rs:9:13 + --> $DIR/comparison_to_empty.rs:10:13 | LL | let _ = s == ""; | ^^^^^^^ help: using `is_empty` is clearer and more explicit: `s.is_empty()` @@ -7,22 +7,52 @@ LL | let _ = s == ""; = note: `-D clippy::comparison-to-empty` implied by `-D warnings` error: comparison to empty slice - --> $DIR/comparison_to_empty.rs:10:13 + --> $DIR/comparison_to_empty.rs:11:13 | LL | let _ = s != ""; | ^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!s.is_empty()` error: comparison to empty slice - --> $DIR/comparison_to_empty.rs:13:13 + --> $DIR/comparison_to_empty.rs:14:13 | LL | let _ = v == []; | ^^^^^^^ help: using `is_empty` is clearer and more explicit: `v.is_empty()` error: comparison to empty slice - --> $DIR/comparison_to_empty.rs:14:13 + --> $DIR/comparison_to_empty.rs:15:13 | LL | let _ = v != []; | ^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!v.is_empty()` -error: aborting due to 4 previous errors +error: comparison to empty slice using `if let` + --> $DIR/comparison_to_empty.rs:16:8 + | +LL | if let [] = &*v {} + | ^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `(*v).is_empty()` + +error: comparison to empty slice using `if let` + --> $DIR/comparison_to_empty.rs:18:8 + | +LL | if let [] = s {} + | ^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `s.is_empty()` + +error: comparison to empty slice using `if let` + --> $DIR/comparison_to_empty.rs:19:8 + | +LL | if let [] = &*s {} + | ^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `s.is_empty()` + +error: comparison to empty slice using `if let` + --> $DIR/comparison_to_empty.rs:20:8 + | +LL | if let [] = &*s && s == [] {} + | ^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `s.is_empty()` + +error: comparison to empty slice + --> $DIR/comparison_to_empty.rs:20:24 + | +LL | if let [] = &*s && s == [] {} + | ^^^^^^^ help: using `is_empty` is clearer and more explicit: `s.is_empty()` + +error: aborting due to 9 previous errors |