Introduce hir::ExprKind::Let - Take 2

Author: c410-f3r

clippy_lints/src/assertions_on_constants.rs

@@ -1,5 +1,6 @@
 use clippy_utils::consts::{constant, Constant};
 use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::higher;
 use clippy_utils::source::snippet_opt;
 use clippy_utils::{is_direct_expn_of, is_expn_of, match_panic_call};
 use if_chain::if_chain;
@@ -116,8 +117,8 @@ enum AssertKind {
 /// where `message` is any expression and `c` is a constant bool.
 fn match_assert_with_message<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<AssertKind> {
     if_chain! {
-        if let ExprKind::If(cond, then, _) = expr.kind;
-        if let ExprKind::Unary(UnOp::Not, expr) = cond.kind;
+        if let Some(higher::If { cond, then, .. }) = higher::If::hir(expr);
+        if let ExprKind::Unary(UnOp::Not, ref expr) = cond.kind;
         // bind the first argument of the `assert!` macro
         if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.typeck_results(), expr);
         // block

clippy_lints/src/blocks_in_if_conditions.rs

@@ -1,4 +1,5 @@
 use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
+use clippy_utils::higher;
 use clippy_utils::source::snippet_block_with_applicability;
 use clippy_utils::ty::implements_trait;
 use clippy_utils::{differing_macro_contexts, get_parent_expr};
@@ -92,7 +93,7 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInIfConditions {
         if in_external_macro(cx.sess(), expr.span) {
             return;
         }
-        if let ExprKind::If(cond, _, _) = &expr.kind {
+        if let Some(higher::If { cond, .. }) = higher::If::hir(expr) {
             if let ExprKind::Block(block, _) = &cond.kind {
                 if block.rules == BlockCheckMode::DefaultBlock {
                     if block.stmts.is_empty() {

clippy_lints/src/collapsible_match.rs

@@ -1,9 +1,9 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::visitors::LocalUsedVisitor;
-use clippy_utils::{is_lang_ctor, path_to_local, peel_ref_operators, SpanlessEq};
+use clippy_utils::{higher, is_lang_ctor, path_to_local, peel_ref_operators, SpanlessEq};
 use if_chain::if_chain;
 use rustc_hir::LangItem::OptionNone;
-use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind, StmtKind};
+use rustc_hir::{Expr, ExprKind, Guard, HirId, Pat, PatKind, StmtKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::{MultiSpan, Span};
@@ -49,22 +49,44 @@ declare_lint_pass!(CollapsibleMatch => [COLLAPSIBLE_MATCH]);
 
 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| arm_is_wild_like(cx, arm)) {
+            if let Some(wild_arm) = arms.iter().rfind(|arm| is_wild_like(cx, &arm.pat.kind, &arm.guard)) {
                 for arm in arms {
-                    check_arm(arm, wild_arm, cx);
+                    check_arm(cx, arm.body, arm.guard.as_ref(), arm.pat, Some(wild_arm.body));
                 }
             }
+
+            if let Some(first_arm) = arms.get(0) {
+                check_if_let(cx, &first_arm.body, &first_arm.pat);
+            }
         }
     }
 }
 
-fn check_arm<'tcx>(arm: &Arm<'tcx>, wild_outer_arm: &Arm<'tcx>, cx: &LateContext<'tcx>) {
-    let expr = strip_singleton_blocks(arm.body);
+fn check_arm<'tcx>(
+    cx: &LateContext<'tcx>,
+    outer_block: &'tcx Expr<'tcx>,
+    outer_guard: Option<&Guard<'tcx>>,
+    outer_pat: &'tcx Pat<'tcx>,
+    wild_outer_block: Option<&'tcx Expr<'tcx>>,
+) {
+    let expr = strip_singleton_blocks(outer_block);
     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() == arm.pat.span.ctxt();
+        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
@@ -73,18 +95,18 @@ fn check_arm<'tcx>(arm: &Arm<'tcx>, wild_outer_arm: &Arm<'tcx>, cx: &LateContext
         // 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| arm_is_wild_like(cx, arm_inner));
+        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);
         // the binding must come from the pattern of the containing match arm
         // ..<local>.. => match <local> { .. }
-        if let Some(binding_span) = find_pat_binding(arm.pat, binding_id);
+        if let Some(binding_span) = find_pat_binding(outer_pat, binding_id);
         // the "wild-like" branches must be equal
-        if SpanlessEq::new(cx).eq_expr(wild_inner_arm.body, wild_outer_arm.body);
+        if wild_outer_block.map(|el| SpanlessEq::new(cx).eq_expr(wild_inner_arm.body, el)).unwrap_or(true);
         // the binding must not be used in the if guard
         let mut used_visitor = LocalUsedVisitor::new(cx, binding_id);
-        if match arm.guard {
+        if match outer_guard {
             None => true,
             Some(Guard::If(expr) | Guard::IfLet(_, expr)) => !used_visitor.check_expr(expr),
         };
@@ -107,6 +129,31 @@ fn check_arm<'tcx>(arm: &Arm<'tcx>, wild_outer_arm: &Arm<'tcx>, cx: &LateContext
     }
 }
 
+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`",
+                |diag| {
+                    let mut help_span = MultiSpan::from_spans(vec![binding_span, inner_let_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());
+                    diag.span_help(help_span, "the outer pattern can be modified to include the inner pattern");
+                },
+            );
+        }
+    }
+}
+
 fn strip_singleton_blocks<'hir>(mut expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> {
     while let ExprKind::Block(block, _) = expr.kind {
         match (block.stmts, block.expr) {
@@ -122,13 +169,13 @@ fn strip_singleton_blocks<'hir>(mut expr: &'hir Expr<'hir>) -> &'hir Expr<'hir>
 }
 
 /// A "wild-like" pattern is wild ("_") or `None`.
-/// For this lint to apply, both the outer and inner match expressions
+/// For this lint to apply, both the outer and inner patterns
 /// must have "wild-like" branches that can be combined.
-fn arm_is_wild_like(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {
-    if arm.guard.is_some() {
+fn is_wild_like(cx: &LateContext<'_>, pat_kind: &PatKind<'_>, arm_guard: &Option<Guard<'_>>) -> bool {
+    if arm_guard.is_some() {
         return false;
     }
-    match arm.pat.kind {
+    match pat_kind {
         PatKind::Binding(..) | PatKind::Wild => true,
         PatKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone),
         _ => false,

clippy_lints/src/copies.rs

@@ -316,9 +316,10 @@ fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> Option<
     let mut start_eq = usize::MAX;
     let mut end_eq = usize::MAX;
     let mut expr_eq = true;
-    for win in blocks.windows(2) {
-        let l_stmts = win[0].stmts;
-        let r_stmts = win[1].stmts;
+    let mut iter = blocks.windows(2);
+    while let Some(&[win0, win1]) = iter.next() {
+        let l_stmts = win0.stmts;
+        let r_stmts = win1.stmts;
 
         // `SpanlessEq` now keeps track of the locals and is therefore context sensitive clippy#6752.
         // The comparison therefore needs to be done in a way that builds the correct context.
@@ -335,22 +336,22 @@ fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> Option<
             it1.zip(it2)
                 .fold(0, |acc, (l, r)| if evaluator.eq_stmt(l, r) { acc + 1 } else { 0 })
         };
-        let block_expr_eq = both(&win[0].expr, &win[1].expr, |l, r| evaluator.eq_expr(l, r));
+        let block_expr_eq = both(&win0.expr, &win1.expr, |l, r| evaluator.eq_expr(l, r));
 
         // IF_SAME_THEN_ELSE
         if_chain! {
             if block_expr_eq;
             if l_stmts.len() == r_stmts.len();
             if l_stmts.len() == current_start_eq;
-            if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win[0].hir_id);
-            if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win[1].hir_id);
+            if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win0.hir_id);
+            if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win1.hir_id);
             then {
                 span_lint_and_note(
                     cx,
                     IF_SAME_THEN_ELSE,
-                    win[0].span,
+                    win0.span,
                     "this `if` has identical blocks",
-                    Some(win[1].span),
+                    Some(win1.span),
                     "same as this",
                 );
 

clippy_lints/src/dereference.rs

@@ -232,6 +232,7 @@ fn is_linted_explicit_deref_position(parent: Option<Node<'_>>, child_id: HirId,
         | ExprKind::If(..)
         | ExprKind::Loop(..)
         | ExprKind::Match(..)
+        | ExprKind::Let(..)
         | ExprKind::Closure(..)
         | ExprKind::Block(..)
         | ExprKind::Assign(..)

clippy_lints/src/entry.rs

@@ -1,10 +1,12 @@
+use clippy_utils::higher;
 use clippy_utils::{
     can_move_expr_to_closure_no_visit,
     diagnostics::span_lint_and_sugg,
     is_expr_final_block_expr, is_expr_used_or_unified, match_def_path, paths, peel_hir_expr_while,
     source::{reindent_multiline, snippet_indent, snippet_with_applicability, snippet_with_context},
     SpanlessEq,
 };
+use core::fmt::Write;
 use rustc_errors::Applicability;
 use rustc_hir::{
     intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor},
@@ -13,7 +15,6 @@ use rustc_hir::{
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::{Span, SyntaxContext, DUMMY_SP};
-use std::fmt::Write;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -62,10 +63,11 @@ declare_lint_pass!(HashMapPass => [MAP_ENTRY]);
 impl<'tcx> LateLintPass<'tcx> for HashMapPass {
     #[allow(clippy::too_many_lines)]
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        let (cond_expr, then_expr, else_expr) = match expr.kind {
-            ExprKind::If(c, t, e) => (c, t, e),
+        let (cond_expr, then_expr, else_expr) = match higher::If::hir(expr) {
+            Some(higher::If { cond, then, r#else }) => (cond, then, r#else),
             _ => return,
         };
+
         let (map_ty, contains_expr) = match try_parse_contains(cx, cond_expr) {
             Some(x) => x,
             None => return,

clippy_lints/src/eta_reduction.rs

@@ -100,7 +100,7 @@ fn check_closure<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
 
         if ex.span.ctxt() != expr.span.ctxt() {
             if decl.inputs.is_empty() {
-                if let Some(VecArgs::Vec(&[])) = higher::vec_macro(cx, ex) {
+                if let Some(VecArgs::Vec(&[])) = higher::VecArgs::hir(cx, ex) {
                     // replace `|| vec![]` with `Vec::new`
                     span_lint_and_sugg(
                         cx,

clippy_lints/src/floating_point_arithmetic.rs

@@ -3,6 +3,7 @@ use clippy_utils::consts::{
     Constant::{Int, F32, F64},
 };
 use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::higher;
 use clippy_utils::{eq_expr_value, get_parent_expr, numeric_literal, sugg};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
@@ -545,11 +546,11 @@ fn are_negated<'a>(cx: &LateContext<'_>, expr1: &'a Expr<'a>, expr2: &'a Expr<'a
 
 fn check_custom_abs(cx: &LateContext<'_>, expr: &Expr<'_>) {
     if_chain! {
-        if let ExprKind::If(cond, body, else_body) = expr.kind;
-        if let ExprKind::Block(block, _) = body.kind;
+        if let Some(higher::If { cond, then, r#else }) = higher::If::hir(expr);
+        if let ExprKind::Block(block, _) = then.kind;
         if block.stmts.is_empty();
         if let Some(if_body_expr) = block.expr;
-        if let Some(ExprKind::Block(else_block, _)) = else_body.map(|el| &el.kind);
+        if let Some(ExprKind::Block(else_block, _)) = r#else.map(|el| &el.kind);
         if else_block.stmts.is_empty();
         if let Some(else_body_expr) = else_block.expr;
         if let Some((if_expr_positive, body)) = are_negated(cx, if_body_expr, else_body_expr);

clippy_lints/src/if_let_mutex.rs

@@ -1,9 +1,10 @@
 use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::higher;
 use clippy_utils::ty::is_type_diagnostic_item;
 use clippy_utils::SpanlessEq;
 use if_chain::if_chain;
 use rustc_hir::intravisit::{self as visit, NestedVisitorMap, Visitor};
-use rustc_hir::{Expr, ExprKind, MatchSource};
+use rustc_hir::{Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::hir::map::Map;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -42,7 +43,7 @@ declare_clippy_lint! {
 declare_lint_pass!(IfLetMutex => [IF_LET_MUTEX]);
 
 impl<'tcx> LateLintPass<'tcx> for IfLetMutex {
-    fn check_expr(&mut self, cx: &LateContext<'tcx>, ex: &'tcx Expr<'tcx>) {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
         let mut arm_visit = ArmVisitor {
             mutex_lock_called: false,
             found_mutex: None,
@@ -53,25 +54,23 @@ impl<'tcx> LateLintPass<'tcx> for IfLetMutex {
             found_mutex: None,
             cx,
         };
-        if let ExprKind::Match(
-            op,
-            arms,
-            MatchSource::IfLetDesugar {
-                contains_else_clause: true,
-            },
-        ) = ex.kind
+        if let Some(higher::IfLet {
+            let_expr,
+            if_then,
+            if_else: Some(if_else),
+            ..
+        }) = higher::IfLet::hir(expr)
         {
-            op_visit.visit_expr(op);
+            op_visit.visit_expr(let_expr);
             if op_visit.mutex_lock_called {
-                for arm in arms {
-                    arm_visit.visit_arm(arm);
-                }
+                arm_visit.visit_expr(if_then);
+                arm_visit.visit_expr(if_else);
 
                 if arm_visit.mutex_lock_called && arm_visit.same_mutex(cx, op_visit.found_mutex.unwrap()) {
                     span_lint_and_help(
                         cx,
                         IF_LET_MUTEX,
-                        ex.span,
+                        expr.span,
                         "calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock",
                         None,
                         "move the lock call outside of the `if let ...` expression",

clippy_lints/src/if_let_some_result.rs

@@ -1,10 +1,11 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::higher;
 use clippy_utils::method_chain_args;
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::ty::is_type_diagnostic_item;
 use if_chain::if_chain;
 use rustc_errors::Applicability;
-use rustc_hir::{Expr, ExprKind, MatchSource, PatKind, QPath};
+use rustc_hir::{Expr, ExprKind, PatKind, QPath};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::sym;
@@ -44,17 +45,17 @@ declare_lint_pass!(OkIfLet => [IF_LET_SOME_RESULT]);
 impl<'tcx> LateLintPass<'tcx> for OkIfLet {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         if_chain! { //begin checking variables
-            if let ExprKind::Match(op, body, MatchSource::IfLetDesugar { .. }) = expr.kind; //test if expr is if let
-            if let ExprKind::MethodCall(_, ok_span, result_types, _) = op.kind; //check is expr.ok() has type Result<T,E>.ok(, _)
-            if let PatKind::TupleStruct(QPath::Resolved(_, x), y, _)  = body[0].pat.kind; //get operation
-            if method_chain_args(op, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized;
+            if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(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 is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&result_types[0]), sym::result_type);
             if rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_path(x, false)) == "Some";
 
             then {
                 let mut applicability = Applicability::MachineApplicable;
                 let some_expr_string = snippet_with_applicability(cx, y[0].span, "", &mut applicability);
-                let trimmed_ok = snippet_with_applicability(cx, op.span.until(ok_span), "", &mut applicability);
+                let trimmed_ok = snippet_with_applicability(cx, let_expr.span.until(ok_span), "", &mut applicability);
                 let sugg = format!(
                     "if let Ok({}) = {}",
                     some_expr_string,
@@ -63,7 +64,7 @@ impl<'tcx> LateLintPass<'tcx> for OkIfLet {
                 span_lint_and_sugg(
                     cx,
                     IF_LET_SOME_RESULT,
-                    expr.span.with_hi(op.span.hi()),
+                    expr.span.with_hi(let_expr.span.hi()),
                     "matching on `Some` with `ok()` is redundant",
                     &format!("consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string),
                     sugg,