Skip to content

Commit b97d4c0

Browse files
c410-f3rc410-f3r
committed
Introduce hir::ExprKind::Let - Take 2
1 parent 80bff87 commit b97d4c0

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+1043
-669
lines changed

‎clippy_lints/src/assertions_on_constants.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use clippy_utils::consts::{constant, Constant};
22
use clippy_utils::diagnostics::span_lint_and_help;
3+
use clippy_utils::higher;
34
use clippy_utils::source::snippet_opt;
45
use clippy_utils::{is_direct_expn_of, is_expn_of, match_panic_call};
56
use if_chain::if_chain;
@@ -116,8 +117,8 @@ enum AssertKind {
116117
/// where `message` is any expression and `c` is a constant bool.
117118
fn match_assert_with_message<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<AssertKind> {
118119
if_chain! {
119-
if let ExprKind::If(cond, then, _) = expr.kind;
120-
if let ExprKind::Unary(UnOp::Not, expr) = cond.kind;
120+
if let Some(higher::If { cond, then, .. }) = higher::If::hir(expr);
121+
if let ExprKind::Unary(UnOp::Not, ref expr) = cond.kind;
121122
// bind the first argument of the `assert!` macro
122123
if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.typeck_results(), expr);
123124
// block

‎clippy_lints/src/blocks_in_if_conditions.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
2+
use clippy_utils::higher;
23
use clippy_utils::source::snippet_block_with_applicability;
34
use clippy_utils::ty::implements_trait;
45
use clippy_utils::{differing_macro_contexts, get_parent_expr};
@@ -92,7 +93,7 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInIfConditions {
9293
if in_external_macro(cx.sess(), expr.span) {
9394
return;
9495
}
95-
if let ExprKind::If(cond, _, _) = &expr.kind {
96+
if let Some(higher::If { cond, .. }) = higher::If::hir(expr) {
9697
if let ExprKind::Block(block, _) = &cond.kind {
9798
if block.rules == BlockCheckMode::DefaultBlock {
9899
if block.stmts.is_empty() {

‎clippy_lints/src/collapsible_match.rs

Lines changed: 62 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::visitors::LocalUsedVisitor;
3-
use clippy_utils::{is_lang_ctor, path_to_local, peel_ref_operators, SpanlessEq};
3+
use clippy_utils::{higher, is_lang_ctor, path_to_local, peel_ref_operators, SpanlessEq};
44
use if_chain::if_chain;
55
use rustc_hir::LangItem::OptionNone;
6-
use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind, StmtKind};
6+
use rustc_hir::{Expr, ExprKind, Guard, HirId, Pat, PatKind, StmtKind};
77
use rustc_lint::{LateContext, LateLintPass};
88
use rustc_session::{declare_lint_pass, declare_tool_lint};
99
use rustc_span::{MultiSpan, Span};
@@ -49,22 +49,44 @@ declare_lint_pass!(CollapsibleMatch => [COLLAPSIBLE_MATCH]);
4949

5050
impl<'tcx> LateLintPass<'tcx> for CollapsibleMatch {
5151
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
52+
if let Some(higher::IfLet {
53+
let_pat,
54+
if_then,
55+
if_else,
56+
..
57+
}) = higher::IfLet::hir(expr)
58+
{
59+
check_arm(cx, if_then, None, let_pat, if_else);
60+
61+
check_if_let(cx, if_then, let_pat);
62+
}
63+
5264
if let ExprKind::Match(_expr, arms, _source) = expr.kind {
53-
if let Some(wild_arm) = arms.iter().rfind(|arm| arm_is_wild_like(cx, arm)) {
65+
if let Some(wild_arm) = arms.iter().rfind(|arm| is_wild_like(cx, &arm.pat.kind, &arm.guard)) {
5466
for arm in arms {
55-
check_arm(arm, wild_arm, cx);
67+
check_arm(cx, arm.body, arm.guard.as_ref(), arm.pat, Some(wild_arm.body));
5668
}
5769
}
70+
71+
if let Some(first_arm) = arms.get(0) {
72+
check_if_let(cx, &first_arm.body, &first_arm.pat);
73+
}
5874
}
5975
}
6076
}
6177

62-
fn check_arm<'tcx>(arm: &Arm<'tcx>, wild_outer_arm: &Arm<'tcx>, cx: &LateContext<'tcx>) {
63-
let expr = strip_singleton_blocks(arm.body);
78+
fn check_arm<'tcx>(
79+
cx: &LateContext<'tcx>,
80+
outer_block: &'tcx Expr<'tcx>,
81+
outer_guard: Option<&Guard<'tcx>>,
82+
outer_pat: &'tcx Pat<'tcx>,
83+
wild_outer_block: Option<&'tcx Expr<'tcx>>,
84+
) {
85+
let expr = strip_singleton_blocks(outer_block);
6486
if_chain! {
6587
if let ExprKind::Match(expr_in, arms_inner, _) = expr.kind;
6688
// the outer arm pattern and the inner match
67-
if expr_in.span.ctxt() == arm.pat.span.ctxt();
89+
if expr_in.span.ctxt() == outer_pat.span.ctxt();
6890
// there must be no more than two arms in the inner match for this lint
6991
if arms_inner.len() == 2;
7092
// 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
7395
// match <local> { .. }
7496
if let Some(binding_id) = path_to_local(peel_ref_operators(cx, expr_in));
7597
// one of the branches must be "wild-like"
76-
if let Some(wild_inner_arm_idx) = arms_inner.iter().rposition(|arm_inner| arm_is_wild_like(cx, arm_inner));
98+
if let Some(wild_inner_arm_idx) = arms_inner.iter().rposition(|arm_inner| is_wild_like(cx, &arm_inner.pat.kind, &arm_inner.guard));
7799
let (wild_inner_arm, non_wild_inner_arm) =
78100
(&arms_inner[wild_inner_arm_idx], &arms_inner[1 - wild_inner_arm_idx]);
79101
if !pat_contains_or(non_wild_inner_arm.pat);
80102
// the binding must come from the pattern of the containing match arm
81103
// ..<local>.. => match <local> { .. }
82-
if let Some(binding_span) = find_pat_binding(arm.pat, binding_id);
104+
if let Some(binding_span) = find_pat_binding(outer_pat, binding_id);
83105
// the "wild-like" branches must be equal
84-
if SpanlessEq::new(cx).eq_expr(wild_inner_arm.body, wild_outer_arm.body);
106+
if wild_outer_block.map(|el| SpanlessEq::new(cx).eq_expr(wild_inner_arm.body, el)).unwrap_or(true);
85107
// the binding must not be used in the if guard
86108
let mut used_visitor = LocalUsedVisitor::new(cx, binding_id);
87-
if match arm.guard {
109+
if match outer_guard {
88110
None => true,
89111
Some(Guard::If(expr) | Guard::IfLet(_, expr)) => !used_visitor.check_expr(expr),
90112
};
@@ -107,6 +129,31 @@ fn check_arm<'tcx>(arm: &Arm<'tcx>, wild_outer_arm: &Arm<'tcx>, cx: &LateContext
107129
}
108130
}
109131

132+
fn check_if_let<'tcx>(cx: &LateContext<'tcx>, outer_expr: &'tcx Expr<'tcx>, outer_pat: &'tcx Pat<'tcx>) {
133+
let block_inner = strip_singleton_blocks(outer_expr);
134+
if_chain! {
135+
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);
136+
if let Some(binding_id) = path_to_local(peel_ref_operators(cx, inner_let_expr));
137+
if let Some(binding_span) = find_pat_binding(outer_pat, binding_id);
138+
let mut used_visitor = LocalUsedVisitor::new(cx, binding_id);
139+
if !used_visitor.check_expr(inner_if_then);
140+
then {
141+
span_lint_and_then(
142+
cx,
143+
COLLAPSIBLE_MATCH,
144+
block_inner.span,
145+
"unnecessary nested `if let` or `match`",
146+
|diag| {
147+
let mut help_span = MultiSpan::from_spans(vec![binding_span, inner_let_pat.span]);
148+
help_span.push_span_label(binding_span, "replace this binding".into());
149+
help_span.push_span_label(inner_let_pat.span, "with this pattern".into());
150+
diag.span_help(help_span, "the outer pattern can be modified to include the inner pattern");
151+
},
152+
);
153+
}
154+
}
155+
}
156+
110157
fn strip_singleton_blocks<'hir>(mut expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> {
111158
while let ExprKind::Block(block, _) = expr.kind {
112159
match (block.stmts, block.expr) {
@@ -122,13 +169,13 @@ fn strip_singleton_blocks<'hir>(mut expr: &'hir Expr<'hir>) -> &'hir Expr<'hir>
122169
}
123170

124171
/// A "wild-like" pattern is wild ("_") or `None`.
125-
/// For this lint to apply, both the outer and inner match expressions
172+
/// For this lint to apply, both the outer and inner patterns
126173
/// must have "wild-like" branches that can be combined.
127-
fn arm_is_wild_like(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {
128-
if arm.guard.is_some() {
174+
fn is_wild_like(cx: &LateContext<'_>, pat_kind: &PatKind<'_>, arm_guard: &Option<Guard<'_>>) -> bool {
175+
if arm_guard.is_some() {
129176
return false;
130177
}
131-
match arm.pat.kind {
178+
match pat_kind {
132179
PatKind::Binding(..) | PatKind::Wild => true,
133180
PatKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone),
134181
_ => false,

‎clippy_lints/src/copies.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -316,9 +316,10 @@ fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> Option<
316316
let mut start_eq = usize::MAX;
317317
let mut end_eq = usize::MAX;
318318
let mut expr_eq = true;
319-
for win in blocks.windows(2) {
320-
let l_stmts = win[0].stmts;
321-
let r_stmts = win[1].stmts;
319+
let mut iter = blocks.windows(2);
320+
while let Some(&[win0, win1]) = iter.next() {
321+
let l_stmts = win0.stmts;
322+
let r_stmts = win1.stmts;
322323

323324
// `SpanlessEq` now keeps track of the locals and is therefore context sensitive clippy#6752.
324325
// 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<
335336
it1.zip(it2)
336337
.fold(0, |acc, (l, r)| if evaluator.eq_stmt(l, r) { acc + 1 } else { 0 })
337338
};
338-
let block_expr_eq = both(&win[0].expr, &win[1].expr, |l, r| evaluator.eq_expr(l, r));
339+
let block_expr_eq = both(&win0.expr, &win1.expr, |l, r| evaluator.eq_expr(l, r));
339340

340341
// IF_SAME_THEN_ELSE
341342
if_chain! {
342343
if block_expr_eq;
343344
if l_stmts.len() == r_stmts.len();
344345
if l_stmts.len() == current_start_eq;
345-
if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win[0].hir_id);
346-
if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win[1].hir_id);
346+
if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win0.hir_id);
347+
if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win1.hir_id);
347348
then {
348349
span_lint_and_note(
349350
cx,
350351
IF_SAME_THEN_ELSE,
351-
win[0].span,
352+
win0.span,
352353
"this `if` has identical blocks",
353-
Some(win[1].span),
354+
Some(win1.span),
354355
"same as this",
355356
);
356357

‎clippy_lints/src/dereference.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ fn is_linted_explicit_deref_position(parent: Option<Node<'_>>, child_id: HirId,
232232
| ExprKind::If(..)
233233
| ExprKind::Loop(..)
234234
| ExprKind::Match(..)
235+
| ExprKind::Let(..)
235236
| ExprKind::Closure(..)
236237
| ExprKind::Block(..)
237238
| ExprKind::Assign(..)

‎clippy_lints/src/entry.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1+
use clippy_utils::higher;
12
use clippy_utils::{
23
can_move_expr_to_closure_no_visit,
34
diagnostics::span_lint_and_sugg,
45
is_expr_final_block_expr, is_expr_used_or_unified, match_def_path, paths, peel_hir_expr_while,
56
source::{reindent_multiline, snippet_indent, snippet_with_applicability, snippet_with_context},
67
SpanlessEq,
78
};
9+
use core::fmt::Write;
810
use rustc_errors::Applicability;
911
use rustc_hir::{
1012
intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor},
@@ -13,7 +15,6 @@ use rustc_hir::{
1315
use rustc_lint::{LateContext, LateLintPass};
1416
use rustc_session::{declare_lint_pass, declare_tool_lint};
1517
use rustc_span::{Span, SyntaxContext, DUMMY_SP};
16-
use std::fmt::Write;
1718

1819
declare_clippy_lint! {
1920
/// ### What it does
@@ -62,10 +63,11 @@ declare_lint_pass!(HashMapPass => [MAP_ENTRY]);
6263
impl<'tcx> LateLintPass<'tcx> for HashMapPass {
6364
#[allow(clippy::too_many_lines)]
6465
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
65-
let (cond_expr, then_expr, else_expr) = match expr.kind {
66-
ExprKind::If(c, t, e) => (c, t, e),
66+
let (cond_expr, then_expr, else_expr) = match higher::If::hir(expr) {
67+
Some(higher::If { cond, then, r#else }) => (cond, then, r#else),
6768
_ => return,
6869
};
70+
6971
let (map_ty, contains_expr) = match try_parse_contains(cx, cond_expr) {
7072
Some(x) => x,
7173
None => return,

‎clippy_lints/src/eta_reduction.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ fn check_closure<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
100100

101101
if ex.span.ctxt() != expr.span.ctxt() {
102102
if decl.inputs.is_empty() {
103-
if let Some(VecArgs::Vec(&[])) = higher::vec_macro(cx, ex) {
103+
if let Some(VecArgs::Vec(&[])) = higher::VecArgs::hir(cx, ex) {
104104
// replace `|| vec![]` with `Vec::new`
105105
span_lint_and_sugg(
106106
cx,

‎clippy_lints/src/floating_point_arithmetic.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use clippy_utils::consts::{
33
Constant::{Int, F32, F64},
44
};
55
use clippy_utils::diagnostics::span_lint_and_sugg;
6+
use clippy_utils::higher;
67
use clippy_utils::{eq_expr_value, get_parent_expr, numeric_literal, sugg};
78
use if_chain::if_chain;
89
use rustc_errors::Applicability;
@@ -545,11 +546,11 @@ fn are_negated<'a>(cx: &LateContext<'_>, expr1: &'a Expr<'a>, expr2: &'a Expr<'a
545546

546547
fn check_custom_abs(cx: &LateContext<'_>, expr: &Expr<'_>) {
547548
if_chain! {
548-
if let ExprKind::If(cond, body, else_body) = expr.kind;
549-
if let ExprKind::Block(block, _) = body.kind;
549+
if let Some(higher::If { cond, then, r#else }) = higher::If::hir(expr);
550+
if let ExprKind::Block(block, _) = then.kind;
550551
if block.stmts.is_empty();
551552
if let Some(if_body_expr) = block.expr;
552-
if let Some(ExprKind::Block(else_block, _)) = else_body.map(|el| &el.kind);
553+
if let Some(ExprKind::Block(else_block, _)) = r#else.map(|el| &el.kind);
553554
if else_block.stmts.is_empty();
554555
if let Some(else_body_expr) = else_block.expr;
555556
if let Some((if_expr_positive, body)) = are_negated(cx, if_body_expr, else_body_expr);

‎clippy_lints/src/if_let_mutex.rs

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
2+
use clippy_utils::higher;
23
use clippy_utils::ty::is_type_diagnostic_item;
34
use clippy_utils::SpanlessEq;
45
use if_chain::if_chain;
56
use rustc_hir::intravisit::{self as visit, NestedVisitorMap, Visitor};
6-
use rustc_hir::{Expr, ExprKind, MatchSource};
7+
use rustc_hir::{Expr, ExprKind};
78
use rustc_lint::{LateContext, LateLintPass};
89
use rustc_middle::hir::map::Map;
910
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -42,7 +43,7 @@ declare_clippy_lint! {
4243
declare_lint_pass!(IfLetMutex => [IF_LET_MUTEX]);
4344

4445
impl<'tcx> LateLintPass<'tcx> for IfLetMutex {
45-
fn check_expr(&mut self, cx: &LateContext<'tcx>, ex: &'tcx Expr<'tcx>) {
46+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
4647
let mut arm_visit = ArmVisitor {
4748
mutex_lock_called: false,
4849
found_mutex: None,
@@ -53,25 +54,23 @@ impl<'tcx> LateLintPass<'tcx> for IfLetMutex {
5354
found_mutex: None,
5455
cx,
5556
};
56-
if let ExprKind::Match(
57-
op,
58-
arms,
59-
MatchSource::IfLetDesugar {
60-
contains_else_clause: true,
61-
},
62-
) = ex.kind
57+
if let Some(higher::IfLet {
58+
let_expr,
59+
if_then,
60+
if_else: Some(if_else),
61+
..
62+
}) = higher::IfLet::hir(expr)
6363
{
64-
op_visit.visit_expr(op);
64+
op_visit.visit_expr(let_expr);
6565
if op_visit.mutex_lock_called {
66-
for arm in arms {
67-
arm_visit.visit_arm(arm);
68-
}
66+
arm_visit.visit_expr(if_then);
67+
arm_visit.visit_expr(if_else);
6968

7069
if arm_visit.mutex_lock_called && arm_visit.same_mutex(cx, op_visit.found_mutex.unwrap()) {
7170
span_lint_and_help(
7271
cx,
7372
IF_LET_MUTEX,
74-
ex.span,
73+
expr.span,
7574
"calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock",
7675
None,
7776
"move the lock call outside of the `if let ...` expression",

‎clippy_lints/src/if_let_some_result.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::higher;
23
use clippy_utils::method_chain_args;
34
use clippy_utils::source::snippet_with_applicability;
45
use clippy_utils::ty::is_type_diagnostic_item;
56
use if_chain::if_chain;
67
use rustc_errors::Applicability;
7-
use rustc_hir::{Expr, ExprKind, MatchSource, PatKind, QPath};
8+
use rustc_hir::{Expr, ExprKind, PatKind, QPath};
89
use rustc_lint::{LateContext, LateLintPass};
910
use rustc_session::{declare_lint_pass, declare_tool_lint};
1011
use rustc_span::sym;
@@ -44,17 +45,17 @@ declare_lint_pass!(OkIfLet => [IF_LET_SOME_RESULT]);
4445
impl<'tcx> LateLintPass<'tcx> for OkIfLet {
4546
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
4647
if_chain! { //begin checking variables
47-
if let ExprKind::Match(op, body, MatchSource::IfLetDesugar { .. }) = expr.kind; //test if expr is if let
48-
if let ExprKind::MethodCall(_, ok_span, result_types, _) = op.kind; //check is expr.ok() has type Result<T,E>.ok(, _)
49-
if let PatKind::TupleStruct(QPath::Resolved(_, x), y, _) = body[0].pat.kind; //get operation
50-
if method_chain_args(op, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized;
48+
if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(expr);
49+
if let ExprKind::MethodCall(_, ok_span, ref result_types, _) = let_expr.kind; //check is expr.ok() has type Result<T,E>.ok(, _)
50+
if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = let_pat.kind; //get operation
51+
if method_chain_args(let_expr, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized;
5152
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&result_types[0]), sym::result_type);
5253
if rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_path(x, false)) == "Some";
5354

5455
then {
5556
let mut applicability = Applicability::MachineApplicable;
5657
let some_expr_string = snippet_with_applicability(cx, y[0].span, "", &mut applicability);
57-
let trimmed_ok = snippet_with_applicability(cx, op.span.until(ok_span), "", &mut applicability);
58+
let trimmed_ok = snippet_with_applicability(cx, let_expr.span.until(ok_span), "", &mut applicability);
5859
let sugg = format!(
5960
"if let Ok({}) = {}",
6061
some_expr_string,
@@ -63,7 +64,7 @@ impl<'tcx> LateLintPass<'tcx> for OkIfLet {
6364
span_lint_and_sugg(
6465
cx,
6566
IF_LET_SOME_RESULT,
66-
expr.span.with_hi(op.span.hi()),
67+
expr.span.with_hi(let_expr.span.hi()),
6768
"matching on `Some` with `ok()` is redundant",
6869
&format!("consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string),
6970
sugg,

0 commit comments

Comments
 (0)