diff --git a/clippy_lints/src/matches/redundant_pattern_match.rs b/clippy_lints/src/matches/redundant_pattern_match.rs index b10c6c0ac593..df3d8a4227f8 100644 --- a/clippy_lints/src/matches/redundant_pattern_match.rs +++ b/clippy_lints/src/matches/redundant_pattern_match.rs @@ -4,12 +4,12 @@ use clippy_utils::res::{MaybeDef, MaybeTypeckRes}; use clippy_utils::sugg::{Sugg, make_unop}; use clippy_utils::ty::needs_ordered_drop; use clippy_utils::visitors::{any_temporaries_need_ordered_drop, for_each_expr_without_closures}; -use clippy_utils::{higher, is_expn_of, sym}; +use clippy_utils::{get_parent_expr, higher, is_expn_of, sym}; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; use rustc_hir::LangItem::{self, OptionNone, OptionSome, PollPending, PollReady, ResultErr, ResultOk}; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::{Arm, Expr, ExprKind, Node, Pat, PatExpr, PatExprKind, PatKind, QPath, UnOp}; +use rustc_hir::{Arm, BinOpKind, Expr, ExprKind, Node, Pat, PatExpr, PatExprKind, PatKind, QPath, UnOp}; use rustc_lint::LateContext; use rustc_middle::ty::{self, GenericArgKind, Ty}; use rustc_span::{Span, Symbol, kw}; @@ -312,6 +312,9 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op if let Some(guard) = maybe_guard { let guard = Sugg::hir_with_context(cx, guard, ctxt, "..", &mut app); let _ = write!(sugg, " && {}", guard.maybe_paren()); + if guard_sugg_needs_parens(cx, expr) { + sugg = format!("({sugg})"); + } } diag.span_suggestion_verbose(expr_span, format!("consider using `{good_method}`"), sugg, app); @@ -320,6 +323,20 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op } } +/// Whether the appended `&& guard` makes the suggestion bind looser than `expr`'s +/// parent, so the whole `recv.method() && guard` must be wrapped in parentheses. +fn guard_sugg_needs_parens(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + let Some(parent) = get_parent_expr(cx, expr) else { + return false; + }; + match parent.kind { + ExprKind::Binary(op, ..) => !matches!(op.node, BinOpKind::And | BinOpKind::Or), + ExprKind::Unary(..) | ExprKind::AddrOf(..) | ExprKind::Cast(..) => true, + ExprKind::MethodCall(_, receiver, ..) => receiver.hir_id == expr.hir_id, + _ => false, + } +} + fn found_good_method<'tcx>( cx: &LateContext<'_>, arms: &'tcx [Arm<'tcx>; 2], diff --git a/tests/ui/redundant_pattern_matching_option.fixed b/tests/ui/redundant_pattern_matching_option.fixed index dbebc4e9793a..42a86ab0541f 100644 --- a/tests/ui/redundant_pattern_matching_option.fixed +++ b/tests/ui/redundant_pattern_matching_option.fixed @@ -28,6 +28,29 @@ fn issue_11174_edge_cases(boolean: bool, boolean2: bool, maybe_some: Option) { + let _ = !(opt.is_none() && a); + //~^ redundant_pattern_matching + let _ = &(opt.is_none() && a); + //~^ redundant_pattern_matching + let _ = (opt.is_none() && a) as u8; + //~^ redundant_pattern_matching + let _ = (opt.is_none() && a) == b; + //~^ redundant_pattern_matching + let _ = (opt.is_none() && a).then_some(1); + //~^ redundant_pattern_matching + let _ = opt.is_none() && a; + //~^ redundant_pattern_matching + if opt.is_none() && a {} + //~^ redundant_pattern_matching + let _ = opt.is_none() && a && b; + //~^ redundant_pattern_matching + let _ = opt.is_none() && a || b; + //~^ redundant_pattern_matching + let _ = b.then_some(opt.is_none() && a); + //~^ redundant_pattern_matching +} + fn main() { if None::<()>.is_none() {} //~^ redundant_pattern_matching diff --git a/tests/ui/redundant_pattern_matching_option.rs b/tests/ui/redundant_pattern_matching_option.rs index b3bb74428a54..2ac793f9b723 100644 --- a/tests/ui/redundant_pattern_matching_option.rs +++ b/tests/ui/redundant_pattern_matching_option.rs @@ -28,6 +28,29 @@ fn issue_11174_edge_cases(boolean: bool, boolean2: bool, maybe_some: Option) { + let _ = !matches!(opt, None if a); + //~^ redundant_pattern_matching + let _ = &matches!(opt, None if a); + //~^ redundant_pattern_matching + let _ = matches!(opt, None if a) as u8; + //~^ redundant_pattern_matching + let _ = matches!(opt, None if a) == b; + //~^ redundant_pattern_matching + let _ = matches!(opt, None if a).then_some(1); + //~^ redundant_pattern_matching + let _ = matches!(opt, None if a); + //~^ redundant_pattern_matching + if matches!(opt, None if a) {} + //~^ redundant_pattern_matching + let _ = matches!(opt, None if a) && b; + //~^ redundant_pattern_matching + let _ = matches!(opt, None if a) || b; + //~^ redundant_pattern_matching + let _ = b.then_some(matches!(opt, None if a)); + //~^ redundant_pattern_matching +} + fn main() { if let None = None::<()> {} //~^ redundant_pattern_matching diff --git a/tests/ui/redundant_pattern_matching_option.stderr b/tests/ui/redundant_pattern_matching_option.stderr index 8f02838feb35..218a02fe1b3a 100644 --- a/tests/ui/redundant_pattern_matching_option.stderr +++ b/tests/ui/redundant_pattern_matching_option.stderr @@ -25,7 +25,127 @@ LL + let _ = maybe_some.is_none() && (boolean || boolean2); // guard needs p | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:32:12 + --> tests/ui/redundant_pattern_matching_option.rs:32:14 + | +LL | let _ = !matches!(opt, None if a); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `is_none()` + | +LL - let _ = !matches!(opt, None if a); +LL + let _ = !(opt.is_none() && a); + | + +error: redundant pattern matching + --> tests/ui/redundant_pattern_matching_option.rs:34:14 + | +LL | let _ = &matches!(opt, None if a); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `is_none()` + | +LL - let _ = &matches!(opt, None if a); +LL + let _ = &(opt.is_none() && a); + | + +error: redundant pattern matching + --> tests/ui/redundant_pattern_matching_option.rs:36:13 + | +LL | let _ = matches!(opt, None if a) as u8; + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `is_none()` + | +LL - let _ = matches!(opt, None if a) as u8; +LL + let _ = (opt.is_none() && a) as u8; + | + +error: redundant pattern matching + --> tests/ui/redundant_pattern_matching_option.rs:38:13 + | +LL | let _ = matches!(opt, None if a) == b; + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `is_none()` + | +LL - let _ = matches!(opt, None if a) == b; +LL + let _ = (opt.is_none() && a) == b; + | + +error: redundant pattern matching + --> tests/ui/redundant_pattern_matching_option.rs:40:13 + | +LL | let _ = matches!(opt, None if a).then_some(1); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `is_none()` + | +LL - let _ = matches!(opt, None if a).then_some(1); +LL + let _ = (opt.is_none() && a).then_some(1); + | + +error: redundant pattern matching + --> tests/ui/redundant_pattern_matching_option.rs:42:13 + | +LL | let _ = matches!(opt, None if a); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `is_none()` + | +LL - let _ = matches!(opt, None if a); +LL + let _ = opt.is_none() && a; + | + +error: redundant pattern matching + --> tests/ui/redundant_pattern_matching_option.rs:44:8 + | +LL | if matches!(opt, None if a) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `is_none()` + | +LL - if matches!(opt, None if a) {} +LL + if opt.is_none() && a {} + | + +error: redundant pattern matching + --> tests/ui/redundant_pattern_matching_option.rs:46:13 + | +LL | let _ = matches!(opt, None if a) && b; + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `is_none()` + | +LL - let _ = matches!(opt, None if a) && b; +LL + let _ = opt.is_none() && a && b; + | + +error: redundant pattern matching + --> tests/ui/redundant_pattern_matching_option.rs:48:13 + | +LL | let _ = matches!(opt, None if a) || b; + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `is_none()` + | +LL - let _ = matches!(opt, None if a) || b; +LL + let _ = opt.is_none() && a || b; + | + +error: redundant pattern matching + --> tests/ui/redundant_pattern_matching_option.rs:50:25 + | +LL | let _ = b.then_some(matches!(opt, None if a)); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `is_none()` + | +LL - let _ = b.then_some(matches!(opt, None if a)); +LL + let _ = b.then_some(opt.is_none() && a); + | + +error: redundant pattern matching + --> tests/ui/redundant_pattern_matching_option.rs:55:12 | LL | if let None = None::<()> {} | ^^^^ @@ -37,7 +157,7 @@ LL + if None::<()>.is_none() {} | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:35:12 + --> tests/ui/redundant_pattern_matching_option.rs:58:12 | LL | if let Some(_) = Some(42) {} | ^^^^^^^ @@ -49,7 +169,7 @@ LL + if Some(42).is_some() {} | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:38:12 + --> tests/ui/redundant_pattern_matching_option.rs:61:12 | LL | if let Some(_) = Some(42) { | ^^^^^^^ @@ -61,7 +181,7 @@ LL + if Some(42).is_some() { | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:45:15 + --> tests/ui/redundant_pattern_matching_option.rs:68:15 | LL | while let Some(_) = Some(42) {} | ^^^^^^^ @@ -73,7 +193,7 @@ LL + while Some(42).is_some() {} | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:48:15 + --> tests/ui/redundant_pattern_matching_option.rs:71:15 | LL | while let None = Some(42) {} | ^^^^ @@ -85,7 +205,7 @@ LL + while Some(42).is_none() {} | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:51:15 + --> tests/ui/redundant_pattern_matching_option.rs:74:15 | LL | while let None = None::<()> {} | ^^^^ @@ -97,7 +217,7 @@ LL + while None::<()>.is_none() {} | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:55:15 + --> tests/ui/redundant_pattern_matching_option.rs:78:15 | LL | while let Some(_) = v.pop() { | ^^^^^^^ @@ -109,7 +229,7 @@ LL + while v.pop().is_some() { | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:64:5 + --> tests/ui/redundant_pattern_matching_option.rs:87:5 | LL | / match Some(42) { LL | | @@ -129,7 +249,7 @@ LL + Some(42).is_some(); | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:70:5 + --> tests/ui/redundant_pattern_matching_option.rs:93:5 | LL | / match None::<()> { LL | | @@ -149,7 +269,7 @@ LL + None::<()>.is_none(); | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:76:13 + --> tests/ui/redundant_pattern_matching_option.rs:99:13 | LL | let _ = match None::<()> { | _____________^ @@ -170,7 +290,7 @@ LL + let _ = None::<()>.is_none(); | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:83:20 + --> tests/ui/redundant_pattern_matching_option.rs:106:20 | LL | let _ = if let Some(_) = opt { true } else { false }; | ^^^^^^^ @@ -182,7 +302,7 @@ LL + let _ = if opt.is_some() { true } else { false }; | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:90:20 + --> tests/ui/redundant_pattern_matching_option.rs:113:20 | LL | let _ = if let Some(_) = gen_opt() { | ^^^^^^^ @@ -194,7 +314,7 @@ LL + let _ = if gen_opt().is_some() { | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:93:19 + --> tests/ui/redundant_pattern_matching_option.rs:116:19 | LL | } else if let None = gen_opt() { | ^^^^ @@ -206,7 +326,7 @@ LL + } else if gen_opt().is_none() { | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:100:12 + --> tests/ui/redundant_pattern_matching_option.rs:123:12 | LL | if let Some(..) = gen_opt() {} | ^^^^^^^^ @@ -218,7 +338,7 @@ LL + if gen_opt().is_some() {} | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:116:12 + --> tests/ui/redundant_pattern_matching_option.rs:139:12 | LL | if let Some(_) = Some(42) {} | ^^^^^^^ @@ -230,7 +350,7 @@ LL + if Some(42).is_some() {} | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:119:12 + --> tests/ui/redundant_pattern_matching_option.rs:142:12 | LL | if let None = None::<()> {} | ^^^^ @@ -242,7 +362,7 @@ LL + if None::<()>.is_none() {} | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:122:15 + --> tests/ui/redundant_pattern_matching_option.rs:145:15 | LL | while let Some(_) = Some(42) {} | ^^^^^^^ @@ -254,7 +374,7 @@ LL + while Some(42).is_some() {} | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:125:15 + --> tests/ui/redundant_pattern_matching_option.rs:148:15 | LL | while let None = None::<()> {} | ^^^^ @@ -266,7 +386,7 @@ LL + while None::<()>.is_none() {} | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:128:5 + --> tests/ui/redundant_pattern_matching_option.rs:151:5 | LL | / match Some(42) { LL | | @@ -286,7 +406,7 @@ LL + Some(42).is_some(); | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:134:5 + --> tests/ui/redundant_pattern_matching_option.rs:157:5 | LL | / match None::<()> { LL | | @@ -306,7 +426,7 @@ LL + None::<()>.is_none(); | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:143:12 + --> tests/ui/redundant_pattern_matching_option.rs:166:12 | LL | if let None = *(&None::<()>) {} | ^^^^ @@ -318,7 +438,7 @@ LL + if (&None::<()>).is_none() {} | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:145:12 + --> tests/ui/redundant_pattern_matching_option.rs:168:12 | LL | if let None = *&None::<()> {} | ^^^^ @@ -330,7 +450,7 @@ LL + if (&None::<()>).is_none() {} | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:152:5 + --> tests/ui/redundant_pattern_matching_option.rs:175:5 | LL | / match x { LL | | @@ -350,7 +470,7 @@ LL + x.is_some(); | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:158:5 + --> tests/ui/redundant_pattern_matching_option.rs:181:5 | LL | / match x { LL | | @@ -370,7 +490,7 @@ LL + x.is_none(); | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:164:5 + --> tests/ui/redundant_pattern_matching_option.rs:187:5 | LL | / match x { LL | | @@ -390,7 +510,7 @@ LL + x.is_none(); | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:170:5 + --> tests/ui/redundant_pattern_matching_option.rs:193:5 | LL | / match x { LL | | @@ -410,7 +530,7 @@ LL + x.is_some(); | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:186:13 + --> tests/ui/redundant_pattern_matching_option.rs:209:13 | LL | let _ = matches!(x, Some(_)); | ^^^^^^^^^^^^^^^^^^^^ @@ -422,7 +542,7 @@ LL + let _ = x.is_some(); | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:189:13 + --> tests/ui/redundant_pattern_matching_option.rs:212:13 | LL | let _ = matches!(x, None); | ^^^^^^^^^^^^^^^^^ @@ -434,7 +554,7 @@ LL + let _ = x.is_none(); | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:200:17 + --> tests/ui/redundant_pattern_matching_option.rs:223:17 | LL | let _ = matches!(*p, None); | ^^^^^^^^^^^^^^^^^^ @@ -446,7 +566,7 @@ LL + let _ = (*p).is_none(); | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:208:16 + --> tests/ui/redundant_pattern_matching_option.rs:231:16 | LL | if let Some(_) = x? { | ^^^^^^^ @@ -458,7 +578,7 @@ LL + if x?.is_some() { | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:228:16 + --> tests/ui/redundant_pattern_matching_option.rs:251:16 | LL | if let Some(_) = x.await { | ^^^^^^^ @@ -470,7 +590,7 @@ LL + if x.await.is_some() { | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:241:12 + --> tests/ui/redundant_pattern_matching_option.rs:264:12 | LL | if let Some(_) = (x! {}) {}; | ^^^^^^^ @@ -482,7 +602,7 @@ LL + if x! {}.is_some() {}; | error: redundant pattern matching - --> tests/ui/redundant_pattern_matching_option.rs:243:15 + --> tests/ui/redundant_pattern_matching_option.rs:266:15 | LL | while let Some(_) = (x! {}) {} | ^^^^^^^ @@ -493,5 +613,5 @@ LL - while let Some(_) = (x! {}) {} LL + while x! {}.is_some() {} | -error: aborting due to 35 previous errors +error: aborting due to 45 previous errors