use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::visitors::LocalUsedVisitor;
-use clippy_utils::{higher, is_lang_ctor, path_to_local, peel_ref_operators, SpanlessEq};
+use clippy_utils::{higher, is_lang_ctor, is_unit_expr, path_to_local, peel_ref_operators, SpanlessEq};
use if_chain::if_chain;
use rustc_hir::LangItem::OptionNone;
-use rustc_hir::{Expr, ExprKind, Guard, HirId, Pat, PatKind, StmtKind};
+use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, MatchSource, Pat, PatKind, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{MultiSpan, Span};
impl<'tcx> LateLintPass<'tcx> for CollapsibleMatch {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
- if let Some(higher::IfLet {
- let_pat,
- if_then,
- if_else,
- ..
- }) = higher::IfLet::hir(expr)
- {
- check_arm(cx, if_then, None, let_pat, if_else);
-
- check_if_let(cx, if_then, let_pat);
- }
-
- if let ExprKind::Match(_expr, arms, _source) = expr.kind {
- if let Some(wild_arm) = arms.iter().rfind(|arm| is_wild_like(cx, &arm.pat.kind, &arm.guard)) {
- for arm in arms {
- check_arm(cx, arm.body, arm.guard.as_ref(), arm.pat, Some(wild_arm.body));
+ match IfLetOrMatch::parse(cx, expr) {
+ Some(IfLetOrMatch::Match(_, arms, _)) => {
+ if let Some(els_arm) = arms.iter().rfind(|arm| arm_is_wild_like(cx, arm)) {
+ for arm in arms {
+ check_arm(cx, true, arm.pat, arm.body, arm.guard.as_ref(), Some(els_arm.body));
+ }
}
}
-
- if let Some(first_arm) = arms.get(0) {
- check_if_let(cx, &first_arm.body, &first_arm.pat);
+ Some(IfLetOrMatch::IfLet(_, pat, body, els)) => {
+ check_arm(cx, false, pat, body, None, els);
}
+ None => {}
}
}
}
fn check_arm<'tcx>(
cx: &LateContext<'tcx>,
- outer_block: &'tcx Expr<'tcx>,
- outer_guard: Option<&Guard<'tcx>>,
+ outer_is_match: bool,
outer_pat: &'tcx Pat<'tcx>,
- wild_outer_block: Option<&'tcx Expr<'tcx>>,
+ outer_then_body: &'tcx Expr<'tcx>,
+ outer_guard: Option<&'tcx Guard<'tcx>>,
+ outer_else_body: Option<&'tcx Expr<'tcx>>
) {
- let expr = strip_singleton_blocks(outer_block);
+ let inner_expr = strip_singleton_blocks(outer_then_body);
if_chain! {
- if let ExprKind::Match(expr_in, arms_inner, _) = expr.kind;
- // the outer arm pattern and the inner match
- if expr_in.span.ctxt() == outer_pat.span.ctxt();
- // there must be no more than two arms in the inner match for this lint
- if arms_inner.len() == 2;
- // no if guards on the inner match
- if arms_inner.iter().all(|arm| arm.guard.is_none());
+ if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr);
+ if let Some((inner_scrutinee, inner_then_pat, inner_else_body)) = match inner {
+ IfLetOrMatch::IfLet(scrutinee, pat, _, els) => Some((scrutinee, pat, els)),
+ IfLetOrMatch::Match(scrutinee, arms, ..) => if_chain! {
+ // if there are more than two arms, collapsing would be non-trivial
+ if arms.len() == 2 && arms.iter().all(|a| a.guard.is_none());
+ // one of the arms must be "wild-like"
+ if let Some(wild_idx) = arms.iter().rposition(|a| arm_is_wild_like(cx, a));
+ then {
+ let (then, els) = (&arms[1 - wild_idx], &arms[wild_idx]);
+ Some((scrutinee, then.pat, Some(els.body)))
+ } else {
+ None
+ }
+ },
+ };
+ if outer_pat.span.ctxt() == inner_scrutinee.span.ctxt();
// match expression must be a local binding
// match <local> { .. }
- if let Some(binding_id) = path_to_local(peel_ref_operators(cx, expr_in));
- // one of the branches must be "wild-like"
- if let Some(wild_inner_arm_idx) = arms_inner.iter().rposition(|arm_inner| is_wild_like(cx, &arm_inner.pat.kind, &arm_inner.guard));
- let (wild_inner_arm, non_wild_inner_arm) =
- (&arms_inner[wild_inner_arm_idx], &arms_inner[1 - wild_inner_arm_idx]);
- if !pat_contains_or(non_wild_inner_arm.pat);
+ if let Some(binding_id) = path_to_local(peel_ref_operators(cx, inner_scrutinee));
+ if !pat_contains_or(inner_then_pat);
// the binding must come from the pattern of the containing match arm
// ..<local>.. => match <local> { .. }
if let Some(binding_span) = find_pat_binding(outer_pat, binding_id);
- // the "wild-like" branches must be equal
- if wild_outer_block.map(|el| SpanlessEq::new(cx).eq_expr(wild_inner_arm.body, el)).unwrap_or(true);
+ // the "else" branches must be equal
+ if match (outer_else_body, inner_else_body) {
+ (None, None) => true,
+ (None, Some(e)) | (Some(e), None) => is_unit_expr(e),
+ (Some(a), Some(b)) => SpanlessEq::new(cx).eq_expr(a, b),
+ };
// the binding must not be used in the if guard
let mut used_visitor = LocalUsedVisitor::new(cx, binding_id);
- if match outer_guard {
- None => true,
- Some(Guard::If(expr) | Guard::IfLet(_, expr)) => !used_visitor.check_expr(expr),
+ if outer_guard.map_or(true, |(Guard::If(e) | Guard::IfLet(_, e))| !used_visitor.check_expr(e));
+ // ...or anywhere in the inner expression
+ if match inner {
+ IfLetOrMatch::IfLet(_, _, body, els) => {
+ !used_visitor.check_expr(body) && els.map_or(true, |e| !used_visitor.check_expr(e))
+ },
+ IfLetOrMatch::Match(_, arms, ..) => !arms.iter().any(|arm| used_visitor.check_arm(arm)),
};
- // ...or anywhere in the inner match
- if !arms_inner.iter().any(|arm| used_visitor.check_arm(arm));
then {
- span_lint_and_then(
- cx,
- COLLAPSIBLE_MATCH,
- expr.span,
- "unnecessary nested match",
- |diag| {
- let mut help_span = MultiSpan::from_spans(vec![binding_span, non_wild_inner_arm.pat.span]);
- help_span.push_span_label(binding_span, "replace this binding".into());
- help_span.push_span_label(non_wild_inner_arm.pat.span, "with this pattern".into());
- diag.span_help(help_span, "the outer pattern can be modified to include the inner pattern");
- },
+ let msg = format!(
+ "this `{}` can be collapsed into the outer `{}`",
+ if matches!(inner, IfLetOrMatch::Match(..)) { "match" } else { "if let" },
+ if outer_is_match { "match" } else { "if let" },
);
- }
- }
-}
-
-fn check_if_let<'tcx>(cx: &LateContext<'tcx>, outer_expr: &'tcx Expr<'tcx>, outer_pat: &'tcx Pat<'tcx>) {
- let block_inner = strip_singleton_blocks(outer_expr);
- if_chain! {
- if let Some(higher::IfLet { if_then: inner_if_then, let_expr: inner_let_expr, let_pat: inner_let_pat, .. }) = higher::IfLet::hir(block_inner);
- if let Some(binding_id) = path_to_local(peel_ref_operators(cx, inner_let_expr));
- if let Some(binding_span) = find_pat_binding(outer_pat, binding_id);
- let mut used_visitor = LocalUsedVisitor::new(cx, binding_id);
- if !used_visitor.check_expr(inner_if_then);
- then {
span_lint_and_then(
cx,
COLLAPSIBLE_MATCH,
- block_inner.span,
- "unnecessary nested `if let` or `match`",
+ inner_expr.span,
+ &msg,
|diag| {
- let mut help_span = MultiSpan::from_spans(vec![binding_span, inner_let_pat.span]);
+ let mut help_span = MultiSpan::from_spans(vec![binding_span, inner_then_pat.span]);
help_span.push_span_label(binding_span, "replace this binding".into());
- help_span.push_span_label(inner_let_pat.span, "with this pattern".into());
+ help_span.push_span_label(inner_then_pat.span, "with this pattern".into());
diag.span_help(help_span, "the outer pattern can be modified to include the inner pattern");
},
);
expr
}
-/// A "wild-like" pattern is wild ("_") or `None`.
-/// For this lint to apply, both the outer and inner patterns
-/// must have "wild-like" branches that can be combined.
-fn is_wild_like(cx: &LateContext<'_>, pat_kind: &PatKind<'_>, arm_guard: &Option<Guard<'_>>) -> bool {
- if arm_guard.is_some() {
+enum IfLetOrMatch<'hir> {
+ Match(&'hir Expr<'hir>, &'hir [Arm<'hir>], MatchSource),
+ /// scrutinee, pattern, then block, else block
+ IfLet(&'hir Expr<'hir>, &'hir Pat<'hir>, &'hir Expr<'hir>, Option<&'hir Expr<'hir>>),
+}
+
+impl<'hir> IfLetOrMatch<'hir> {
+ fn parse(cx: &LateContext<'_>, expr: &Expr<'hir>) -> Option<Self> {
+ match expr.kind {
+ ExprKind::Match(expr, arms, source) => Some(Self::Match(expr, arms, source)),
+ _ => higher::IfLet::hir(cx, expr).map(|higher::IfLet { let_expr, let_pat, if_then, if_else }| {
+ Self::IfLet(let_expr, let_pat, if_then, if_else)
+ })
+ }
+ }
+}
+
+/// A "wild-like" arm has a wild (`_`) or `None` pattern and no guard. Such arms can be "collapsed"
+/// into a single wild arm without any significant loss in semantics or readability.
+fn arm_is_wild_like(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {
+ if arm.guard.is_some() {
return false;
}
- match pat_kind {
+ match arm.pat.kind {
PatKind::Binding(..) | PatKind::Wild => true,
PatKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone),
_ => false,
if_then,
if_else: Some(if_else),
..
- }) = higher::IfLet::hir(expr)
+ }) = higher::IfLet::hir(cx, expr)
{
op_visit.visit_expr(let_expr);
if op_visit.mutex_lock_called {
impl<'tcx> LateLintPass<'tcx> for OkIfLet {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if_chain! { //begin checking variables
- if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(expr);
+ if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(cx, expr);
if let ExprKind::MethodCall(_, ok_span, ref result_types, _) = let_expr.kind; //check is expr.ok() has type Result<T,E>.ok(, _)
if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = let_pat.kind; //get operation
if method_chain_args(let_expr, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized;
if_chain! {
if let Some(inner_expr) = inner_expr;
- if let Some(higher::IfLet { let_pat, let_expr, if_else: None, .. }) = higher::IfLet::hir(inner_expr);
+ if let Some(higher::IfLet { let_pat, let_expr, if_else: None, .. }) = higher::IfLet::hir(cx, inner_expr);
// Ensure match_expr in `if let` statement is the same as the pat from the for-loop
if let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind;
if path_to_local_id(let_expr, pat_hir_id);
let_expr,
if_else: Some(if_else),
..
- }) = higher::IfLet::hir(inner)
+ }) = higher::IfLet::hir(cx, inner)
{
if is_simple_break_expr(if_else) {
could_be_while_let(cx, expr, let_pat, let_expr);
let_expr,
if_then,
if_else: Some(if_else),
- }) = higher::IfLet::hir(expr)
+ }) = higher::IfLet::hir(cx, expr)
{
manage_lint(cx, expr, (&let_pat.kind, if_then), (&PatKind::Wild, if_else), let_expr);
}
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type, peel_mid_ty_refs};
use clippy_utils::visitors::LocalUsedVisitor;
use clippy_utils::{
- get_parent_expr, in_macro, is_expn_of, is_lang_ctor, is_lint_allowed, is_refutable, is_wild, meets_msrv, msrvs,
- path_to_local, path_to_local_id, peel_hir_pat_refs, peel_n_hir_expr_refs, recurse_or_patterns, remove_blocks,
- strip_pat_refs,
+ get_parent_expr, in_macro, is_expn_of, is_lang_ctor, is_lint_allowed, is_refutable, is_unit_expr, is_wild,
+ meets_msrv, msrvs, path_to_local, path_to_local_id, peel_hir_pat_refs, peel_n_hir_expr_refs, recurse_or_patterns,
+ remove_blocks, strip_pat_refs,
};
use clippy_utils::{paths, search_same, SpanlessEq, SpanlessHash};
use core::array;
if let ExprKind::Match(ref ex, ref arms, _) = expr.kind {
check_match_ref_pats(cx, ex, arms.iter().map(|el| el.pat), expr);
}
- if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(expr) {
+ if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(cx, expr) {
check_match_ref_pats(cx, let_expr, once(let_pat), expr);
}
}
let_expr,
if_then,
if_else: Some(if_else),
- }) = higher::IfLet::hir(expr)
+ }) = higher::IfLet::hir(cx, expr)
{
return find_matches_sugg(
cx,
.collect()
}
-fn is_unit_expr(expr: &Expr<'_>) -> bool {
- match expr.kind {
- ExprKind::Tup(v) if v.is_empty() => true,
- ExprKind::Block(b, _) if b.stmts.is_empty() && b.expr.is_none() => true,
- _ => false,
- }
-}
-
// Checks if arm has the form `None => None`
fn is_none_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {
matches!(arm.pat.kind, PatKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone))
let_pat,
let_expr,
..
- }) = higher::IfLet::ast(cx, expr)
+ }) = higher::IfLet::hir(cx, expr)
{
find_sugg_for_if_let(cx, expr, let_pat, let_expr, "if", if_else.is_some())
}
fn detect_option_if_let_else<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<OptionIfLetElseOccurence> {
if_chain! {
if !in_macro(expr.span); // Don't lint macros, because it behaves weirdly
- if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: Some(if_else) }) = higher::IfLet::hir(expr);
+ if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: Some(if_else) }) = higher::IfLet::hir(cx, expr);
if !is_else_clause(cx.tcx, expr);
if !is_result_ok(cx, let_expr); // Don't lint on Result::ok because a different lint does it already
if let PatKind::TupleStruct(struct_qpath, [inner_pat], _) = &let_pat.kind;
fn check_if_let_some_and_early_return_none(cx: &LateContext<'_>, expr: &Expr<'_>) {
if_chain! {
- if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: Some(if_else) }) = higher::IfLet::hir(expr);
+ if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: Some(if_else) }) = higher::IfLet::hir(cx, expr);
if Self::is_option(cx, let_expr);
if let PatKind::TupleStruct(ref path1, fields, None) = let_pat.kind;
}
impl<'hir> IfLet<'hir> {
- #[inline]
- pub fn ast(cx: &LateContext<'tcx>, expr: &Expr<'hir>) -> Option<Self> {
- let rslt = Self::hir(expr)?;
- Self::is_not_within_while_context(cx, expr)?;
- Some(rslt)
- }
-
- #[inline]
- pub const fn hir(expr: &Expr<'hir>) -> Option<Self> {
+ pub fn hir(cx: &LateContext<'_>, expr: &Expr<'hir>) -> Option<Self> {
if let ExprKind::If(
Expr {
kind: ExprKind::Let(let_pat, let_expr, _),
if_else,
) = expr.kind
{
+ let hir = cx.tcx.hir();
+ let mut iter = hir.parent_iter(expr.hir_id);
+ if let Some((_, Node::Block(Block { stmts: [], .. }))) = iter.next() {
+ if let Some((_, Node::Expr(Expr { kind: ExprKind::Loop(_, _, LoopSource::While, _), .. }))) = iter.next() {
+ // while loop desugar
+ return None;
+ }
+ }
return Some(Self {
let_pat,
let_expr,
}
None
}
-
- #[inline]
- fn is_not_within_while_context(cx: &LateContext<'tcx>, expr: &Expr<'hir>) -> Option<()> {
- let hir = cx.tcx.hir();
- let parent = hir.get_parent_node(expr.hir_id);
- let parent_parent = hir.get_parent_node(parent);
- let parent_parent_node = hir.get(parent_parent);
- if let Node::Expr(Expr {
- kind: ExprKind::Loop(_, _, LoopSource::While, _),
- ..
- }) = parent_parent_node
- {
- return None;
- }
- Some(())
- }
}
pub struct IfOrIfLet<'hir> {
}
}
+pub fn is_unit_expr(expr: &Expr<'_>) -> bool {
+ matches!(expr.kind, ExprKind::Block(Block { stmts: [], expr: None, .. }, _) | ExprKind::Tup([]))
+}
+
/// Checks if given pattern is a wildcard (`_`)
pub fn is_wild(pat: &Pat<'_>) -> bool {
matches!(pat.kind, PatKind::Wild)
}
fn negative_cases(res_opt: Result<Option<u32>, String>, res_res: Result<Result<u32, String>, String>) {
+ while let Some(x) = make() {
+ if let Some(1) = x {
+ todo!();
+ }
+ }
// no wild pattern in outer match
match res_opt {
Ok(val) => match val {
-error: unnecessary nested match
+error: this `match` can be collapsed into the outer `match`
--> $DIR/collapsible_match.rs:7:20
|
LL | Ok(val) => match val {
LL | Some(n) => foo(n),
| ^^^^^^^ with this pattern
-error: unnecessary nested match
+error: this `match` can be collapsed into the outer `match`
--> $DIR/collapsible_match.rs:16:20
|
LL | Ok(val) => match val {
LL | Some(n) => foo(n),
| ^^^^^^^ with this pattern
-error: unnecessary nested `if let` or `match`
+error: this `if let` can be collapsed into the outer `if let`
--> $DIR/collapsible_match.rs:25:9
|
LL | / if let Some(n) = val {
LL | if let Some(n) = val {
| ^^^^^^^ with this pattern
-error: unnecessary nested `if let` or `match`
+error: this `if let` can be collapsed into the outer `if let`
--> $DIR/collapsible_match.rs:32:9
|
LL | / if let Some(n) = val {
LL | if let Some(n) = val {
| ^^^^^^^ with this pattern
-error: unnecessary nested match
+error: this `match` can be collapsed into the outer `if let`
--> $DIR/collapsible_match.rs:43:9
|
LL | / match val {
LL | Some(n) => foo(n),
| ^^^^^^^ with this pattern
-error: unnecessary nested `if let` or `match`
+error: this `if let` can be collapsed into the outer `match`
--> $DIR/collapsible_match.rs:52:13
|
LL | / if let Some(n) = val {
LL | if let Some(n) = val {
| ^^^^^^^ with this pattern
-error: unnecessary nested match
+error: this `match` can be collapsed into the outer `if let`
--> $DIR/collapsible_match.rs:61:9
|
LL | / match val {
LL | Some(n) => foo(n),
| ^^^^^^^ with this pattern
-error: unnecessary nested `if let` or `match`
+error: this `if let` can be collapsed into the outer `match`
--> $DIR/collapsible_match.rs:72:13
|
LL | / if let Some(n) = val {
LL | if let Some(n) = val {
| ^^^^^^^ with this pattern
-error: unnecessary nested match
+error: this `match` can be collapsed into the outer `match`
--> $DIR/collapsible_match.rs:83:20
|
LL | Ok(val) => match val {
LL | Some(n) => foo(n),
| ^^^^^^^ with this pattern
-error: unnecessary nested match
+error: this `match` can be collapsed into the outer `match`
--> $DIR/collapsible_match.rs:92:22
|
LL | Some(val) => match val {
-error: unnecessary nested match
+error: this `match` can be collapsed into the outer `match`
--> $DIR/collapsible_match2.rs:13:34
|
LL | Ok(val) if make() => match val {
LL | Some(n) => foo(n),
| ^^^^^^^ with this pattern
-error: unnecessary nested match
+error: this `match` can be collapsed into the outer `match`
--> $DIR/collapsible_match2.rs:20:24
|
LL | Ok(val) => match val {
LL | Some(n) => foo(n),
| ^^^^^^^ with this pattern
-error: unnecessary nested match
+error: this `match` can be collapsed into the outer `match`
--> $DIR/collapsible_match2.rs:34:29
|
LL | $pat => match $e {
| replace this binding
= note: this error originates in the macro `mac` (in Nightly builds, run with -Z macro-backtrace for more info)
-error: unnecessary nested match
+error: this `match` can be collapsed into the outer `match`
--> $DIR/collapsible_match2.rs:51:20
|
LL | Some(s) => match *s {
LL | [n] => foo(n),
| ^^^ with this pattern
-error: unnecessary nested match
+error: this `match` can be collapsed into the outer `match`
--> $DIR/collapsible_match2.rs:60:24
|
LL | Some(ref s) => match &*s {
// run-rustfix
#![warn(clippy::if_let_some_result)]
+#![allow(dead_code)]
fn str_to_int(x: &str) -> i32 {
if let Ok(y) = x.parse() { y } else { 0 }
}
}
-fn main() {
- let _ = str_to_int("1");
- let _ = str_to_int_ok("2");
- let _ = strange_some_no_else("3");
+fn negative() {
+ while let Some(1) = "".parse().ok() {}
}
+
+fn main() {}
// run-rustfix
#![warn(clippy::if_let_some_result)]
+#![allow(dead_code)]
fn str_to_int(x: &str) -> i32 {
if let Some(y) = x.parse().ok() { y } else { 0 }
}
}
-fn main() {
- let _ = str_to_int("1");
- let _ = str_to_int_ok("2");
- let _ = strange_some_no_else("3");
+fn negative() {
+ while let Some(1) = "".parse().ok() {}
}
+
+fn main() {}
error: matching on `Some` with `ok()` is redundant
- --> $DIR/if_let_some_result.rs:6:5
+ --> $DIR/if_let_some_result.rs:7:5
|
LL | if let Some(y) = x.parse().ok() { y } else { 0 }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ~~~~~~~~~~~~~~~~~~~~~~~~
error: matching on `Some` with `ok()` is redundant
- --> $DIR/if_let_some_result.rs:16:9
+ --> $DIR/if_let_some_result.rs:17:9
|
LL | if let Some(y) = x . parse() . ok () {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
projects / rust.git / commitdiff