add guard to suggestion instead of not linting

This commit is contained in:
y21 2023-07-17 21:18:11 +02:00
parent c26801ee92
commit e7fd44f213
4 changed files with 118 additions and 55 deletions

View file

@ -3,18 +3,20 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::source::{snippet, walk_span_to_context};
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{is_type_diagnostic_item, needs_ordered_drop};
use clippy_utils::visitors::any_temporaries_need_ordered_drop;
use clippy_utils::visitors::{any_temporaries_need_ordered_drop, for_each_expr};
use clippy_utils::{higher, is_expn_of, is_trait_method};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::LangItem::{self, OptionNone, OptionSome, PollPending, PollReady, ResultErr, ResultOk};
use rustc_hir::{Arm, Expr, ExprKind, Node, Pat, PatKind, QPath, UnOp};
use rustc_hir::{Arm, Expr, ExprKind, Guard, Node, Pat, PatKind, QPath, UnOp};
use rustc_lint::LateContext;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_middle::ty::{self, Ty};
use rustc_span::{sym, Symbol};
use std::fmt::Write;
use std::ops::ControlFlow;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) {
@ -199,36 +201,61 @@ fn find_sugg_for_if_let<'tcx>(
}
pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op: &Expr<'_>, arms: &[Arm<'_>]) {
if let [arm1, arm2] = arms
&& arm1.guard.is_none()
&& arm2.guard.is_none()
{
let node_pair = (&arm1.pat.kind, &arm2.pat.kind);
if arms.len() == 2 {
let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind);
if let Some(good_method) = found_good_method(cx, arms, node_pair) {
if let Some((good_method, maybe_guard)) = found_good_method(cx, arms, node_pair) {
let span = is_expn_of(expr.span, "matches").unwrap_or(expr.span.to(op.span));
let result_expr = match &op.kind {
ExprKind::AddrOf(_, _, borrowed) => borrowed,
_ => op,
};
let mut sugg = format!("{}.{good_method}", snippet(cx, result_expr.span, "_"));
if let Some(guard) = maybe_guard {
let Guard::If(guard) = *guard else { return }; // `...is_none() && let ...` is a syntax error
// wow, the HIR for match guards in `PAT if let PAT = expr && expr => ...` is annoying!
// `guard` here is `Guard::If` with the let expression somewhere deep in the tree of exprs,
// counter to the intuition that it should be `Guard::IfLet`, so we need another check
// to see that there aren't any let chains anywhere in the guard, as that would break
// if we suggest `t.is_none() && (let X = y && z)` for:
// `match t { None if let X = y && z => true, _ => false }`
let has_nested_let_chain = for_each_expr(guard, |expr| {
if matches!(expr.kind, ExprKind::Let(..)) {
ControlFlow::Break(())
} else {
ControlFlow::Continue(())
}
})
.is_some();
if has_nested_let_chain {
return;
}
let guard = Sugg::hir(cx, guard, "..");
let _ = write!(sugg, " && {}", guard.maybe_par());
}
span_lint_and_sugg(
cx,
REDUNDANT_PATTERN_MATCHING,
span,
&format!("redundant pattern matching, consider using `{good_method}`"),
"try",
format!("{}.{good_method}", snippet(cx, result_expr.span, "_")),
sugg,
Applicability::MachineApplicable,
);
}
}
}
fn found_good_method<'a>(
fn found_good_method<'tcx>(
cx: &LateContext<'_>,
arms: &[Arm<'_>],
arms: &'tcx [Arm<'tcx>],
node: (&PatKind<'_>, &PatKind<'_>),
) -> Option<&'a str> {
) -> Option<(&'static str, Option<&'tcx Guard<'tcx>>)> {
match node {
(
PatKind::TupleStruct(ref path_left, patterns_left, _),
@ -314,7 +341,11 @@ fn get_ident(path: &QPath<'_>) -> Option<rustc_span::symbol::Ident> {
}
}
fn get_good_method<'a>(cx: &LateContext<'_>, arms: &[Arm<'_>], path_left: &QPath<'_>) -> Option<&'a str> {
fn get_good_method<'tcx>(
cx: &LateContext<'_>,
arms: &'tcx [Arm<'tcx>],
path_left: &QPath<'_>,
) -> Option<(&'static str, Option<&'tcx Guard<'tcx>>)> {
if let Some(name) = get_ident(path_left) {
return match name.as_str() {
"Ok" => {
@ -380,16 +411,16 @@ fn is_pat_variant(cx: &LateContext<'_>, pat: &Pat<'_>, path: &QPath<'_>, expecte
}
#[expect(clippy::too_many_arguments)]
fn find_good_method_for_match<'a>(
fn find_good_method_for_match<'a, 'tcx>(
cx: &LateContext<'_>,
arms: &[Arm<'_>],
arms: &'tcx [Arm<'tcx>],
path_left: &QPath<'_>,
path_right: &QPath<'_>,
expected_item_left: Item,
expected_item_right: Item,
should_be_left: &'a str,
should_be_right: &'a str,
) -> Option<&'a str> {
) -> Option<(&'a str, Option<&'tcx Guard<'tcx>>)> {
let first_pat = arms[0].pat;
let second_pat = arms[1].pat;
@ -407,22 +438,22 @@ fn find_good_method_for_match<'a>(
match body_node_pair {
(ExprKind::Lit(lit_left), ExprKind::Lit(lit_right)) => match (&lit_left.node, &lit_right.node) {
(LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
(LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
(LitKind::Bool(true), LitKind::Bool(false)) => Some((should_be_left, arms[0].guard.as_ref())),
(LitKind::Bool(false), LitKind::Bool(true)) => Some((should_be_right, arms[1].guard.as_ref())),
_ => None,
},
_ => None,
}
}
fn find_good_method_for_matches_macro<'a>(
fn find_good_method_for_matches_macro<'a, 'tcx>(
cx: &LateContext<'_>,
arms: &[Arm<'_>],
arms: &'tcx [Arm<'tcx>],
path_left: &QPath<'_>,
expected_item_left: Item,
should_be_left: &'a str,
should_be_right: &'a str,
) -> Option<&'a str> {
) -> Option<(&'a str, Option<&'tcx Guard<'tcx>>)> {
let first_pat = arms[0].pat;
let body_node_pair = if is_pat_variant(cx, first_pat, path_left, expected_item_left) {
@ -433,8 +464,8 @@ fn find_good_method_for_matches_macro<'a>(
match body_node_pair {
(ExprKind::Lit(lit_left), ExprKind::Lit(lit_right)) => match (&lit_left.node, &lit_right.node) {
(LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
(LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
(LitKind::Bool(true), LitKind::Bool(false)) => Some((should_be_left, arms[0].guard.as_ref())),
(LitKind::Bool(false), LitKind::Bool(true)) => Some((should_be_right, arms[1].guard.as_ref())),
_ => None,
},
_ => None,

View file

@ -10,9 +10,19 @@
clippy::equatable_if_let,
clippy::if_same_then_else
)]
#![feature(let_chains, if_let_guard)]
fn issue_11174<T>(boolean: bool, maybe_some: Option<T>) -> bool {
matches!(maybe_some, None if !boolean)
maybe_some.is_none() && (!boolean)
}
fn issue_11174_edge_cases<T>(boolean: bool, boolean2: bool, maybe_some: Option<T>) {
let _ = maybe_some.is_none() && (boolean || boolean2); // guard needs parentheses
let _ = match maybe_some { // can't use `matches!` here
// because `expr` metavars in macros don't allow let exprs
None if let Some(x) = Some(0) && x > 5 => true,
_ => false
};
}
fn main() {

View file

@ -10,11 +10,21 @@
clippy::equatable_if_let,
clippy::if_same_then_else
)]
#![feature(let_chains, if_let_guard)]
fn issue_11174<T>(boolean: bool, maybe_some: Option<T>) -> bool {
matches!(maybe_some, None if !boolean)
}
fn issue_11174_edge_cases<T>(boolean: bool, boolean2: bool, maybe_some: Option<T>) {
let _ = matches!(maybe_some, None if boolean || boolean2); // guard needs parentheses
let _ = match maybe_some { // can't use `matches!` here
// because `expr` metavars in macros don't allow let exprs
None if let Some(x) = Some(0) && x > 5 => true,
_ => false
};
}
fn main() {
if let None = None::<()> {}

View file

@ -1,49 +1,61 @@
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:19:12
--> $DIR/redundant_pattern_matching_option.rs:16:5
|
LL | if let None = None::<()> {}
| -------^^^^------------- help: try: `if None::<()>.is_none()`
LL | matches!(maybe_some, None if !boolean)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `maybe_some.is_none() && (!boolean)`
|
= note: `-D clippy::redundant-pattern-matching` implied by `-D warnings`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:20:13
|
LL | let _ = matches!(maybe_some, None if boolean || boolean2); // guard needs parentheses
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `maybe_some.is_none() && (boolean || boolean2)`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:29:12
|
LL | if let None = None::<()> {}
| -------^^^^------------- help: try: `if None::<()>.is_none()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching_option.rs:21:12
--> $DIR/redundant_pattern_matching_option.rs:31:12
|
LL | if let Some(_) = Some(42) {}
| -------^^^^^^^----------- help: try: `if Some(42).is_some()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching_option.rs:23:12
--> $DIR/redundant_pattern_matching_option.rs:33:12
|
LL | if let Some(_) = Some(42) {
| -------^^^^^^^----------- help: try: `if Some(42).is_some()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching_option.rs:29:15
--> $DIR/redundant_pattern_matching_option.rs:39:15
|
LL | while let Some(_) = Some(42) {}
| ----------^^^^^^^----------- help: try: `while Some(42).is_some()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:31:15
--> $DIR/redundant_pattern_matching_option.rs:41:15
|
LL | while let None = Some(42) {}
| ----------^^^^----------- help: try: `while Some(42).is_none()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:33:15
--> $DIR/redundant_pattern_matching_option.rs:43:15
|
LL | while let None = None::<()> {}
| ----------^^^^------------- help: try: `while None::<()>.is_none()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching_option.rs:36:15
--> $DIR/redundant_pattern_matching_option.rs:46:15
|
LL | while let Some(_) = v.pop() {
| ----------^^^^^^^---------- help: try: `while v.pop().is_some()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching_option.rs:44:5
--> $DIR/redundant_pattern_matching_option.rs:54:5
|
LL | / match Some(42) {
LL | | Some(_) => true,
@ -52,7 +64,7 @@ LL | | };
| |_____^ help: try: `Some(42).is_some()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:49:5
--> $DIR/redundant_pattern_matching_option.rs:59:5
|
LL | / match None::<()> {
LL | | Some(_) => false,
@ -61,7 +73,7 @@ LL | | };
| |_____^ help: try: `None::<()>.is_none()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:54:13
--> $DIR/redundant_pattern_matching_option.rs:64:13
|
LL | let _ = match None::<()> {
| _____________^
@ -71,55 +83,55 @@ LL | | };
| |_____^ help: try: `None::<()>.is_none()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching_option.rs:60:20
--> $DIR/redundant_pattern_matching_option.rs:70:20
|
LL | let _ = if let Some(_) = opt { true } else { false };
| -------^^^^^^^------ help: try: `if opt.is_some()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching_option.rs:66:20
--> $DIR/redundant_pattern_matching_option.rs:76:20
|
LL | let _ = if let Some(_) = gen_opt() {
| -------^^^^^^^------------ help: try: `if gen_opt().is_some()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:68:19
--> $DIR/redundant_pattern_matching_option.rs:78:19
|
LL | } else if let None = gen_opt() {
| -------^^^^------------ help: try: `if gen_opt().is_none()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching_option.rs:74:12
--> $DIR/redundant_pattern_matching_option.rs:84:12
|
LL | if let Some(..) = gen_opt() {}
| -------^^^^^^^^------------ help: try: `if gen_opt().is_some()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching_option.rs:89:12
--> $DIR/redundant_pattern_matching_option.rs:99:12
|
LL | if let Some(_) = Some(42) {}
| -------^^^^^^^----------- help: try: `if Some(42).is_some()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:91:12
--> $DIR/redundant_pattern_matching_option.rs:101:12
|
LL | if let None = None::<()> {}
| -------^^^^------------- help: try: `if None::<()>.is_none()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching_option.rs:93:15
--> $DIR/redundant_pattern_matching_option.rs:103:15
|
LL | while let Some(_) = Some(42) {}
| ----------^^^^^^^----------- help: try: `while Some(42).is_some()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:95:15
--> $DIR/redundant_pattern_matching_option.rs:105:15
|
LL | while let None = None::<()> {}
| ----------^^^^------------- help: try: `while None::<()>.is_none()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching_option.rs:97:5
--> $DIR/redundant_pattern_matching_option.rs:107:5
|
LL | / match Some(42) {
LL | | Some(_) => true,
@ -128,7 +140,7 @@ LL | | };
| |_____^ help: try: `Some(42).is_some()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:102:5
--> $DIR/redundant_pattern_matching_option.rs:112:5
|
LL | / match None::<()> {
LL | | Some(_) => false,
@ -137,19 +149,19 @@ LL | | };
| |_____^ help: try: `None::<()>.is_none()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:110:12
--> $DIR/redundant_pattern_matching_option.rs:120:12
|
LL | if let None = *(&None::<()>) {}
| -------^^^^----------------- help: try: `if (&None::<()>).is_none()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:111:12
--> $DIR/redundant_pattern_matching_option.rs:121:12
|
LL | if let None = *&None::<()> {}
| -------^^^^--------------- help: try: `if (&None::<()>).is_none()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching_option.rs:117:5
--> $DIR/redundant_pattern_matching_option.rs:127:5
|
LL | / match x {
LL | | Some(_) => true,
@ -158,7 +170,7 @@ LL | | };
| |_____^ help: try: `x.is_some()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:122:5
--> $DIR/redundant_pattern_matching_option.rs:132:5
|
LL | / match x {
LL | | None => true,
@ -167,7 +179,7 @@ LL | | };
| |_____^ help: try: `x.is_none()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:127:5
--> $DIR/redundant_pattern_matching_option.rs:137:5
|
LL | / match x {
LL | | Some(_) => false,
@ -176,7 +188,7 @@ LL | | };
| |_____^ help: try: `x.is_none()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching_option.rs:132:5
--> $DIR/redundant_pattern_matching_option.rs:142:5
|
LL | / match x {
LL | | None => false,
@ -185,16 +197,16 @@ LL | | };
| |_____^ help: try: `x.is_some()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching_option.rs:147:13
--> $DIR/redundant_pattern_matching_option.rs:157:13
|
LL | let _ = matches!(x, Some(_));
| ^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_some()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching_option.rs:149:13
--> $DIR/redundant_pattern_matching_option.rs:159:13
|
LL | let _ = matches!(x, None);
| ^^^^^^^^^^^^^^^^^ help: try: `x.is_none()`
error: aborting due to 28 previous errors
error: aborting due to 30 previous errors