Skip to content

Commit f230cdc

Browse files
committed
Auto merge of #8297 - Jarcho:if_same_then_else_7579, r=Manishearth
Don't lint `if_same_then_else` with `if let` conditions fixes #7579 changelog: Don't lint `if_same_then_else` with `if let` conditions
2 parents 0d27bd8 + aaf49a5 commit f230cdc

File tree

2 files changed

+38
-18
lines changed

2 files changed

+38
-18
lines changed

clippy_lints/src/copies.rs

+21-18
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
183183
lint_same_cond(cx, &conds);
184184
lint_same_fns_in_if_cond(cx, &conds);
185185
// Block duplication
186-
lint_same_then_else(cx, &blocks, conds.len() == blocks.len(), expr);
186+
lint_same_then_else(cx, &conds, &blocks, conds.len() == blocks.len(), expr);
187187
}
188188
}
189189
}
@@ -192,6 +192,7 @@ impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
192192
/// Implementation of `BRANCHES_SHARING_CODE` and `IF_SAME_THEN_ELSE` if the blocks are equal.
193193
fn lint_same_then_else<'tcx>(
194194
cx: &LateContext<'tcx>,
195+
conds: &[&'tcx Expr<'_>],
195196
blocks: &[&Block<'tcx>],
196197
has_conditional_else: bool,
197198
expr: &'tcx Expr<'_>,
@@ -204,7 +205,7 @@ fn lint_same_then_else<'tcx>(
204205
// Check if each block has shared code
205206
let has_expr = blocks[0].expr.is_some();
206207

207-
let (start_eq, mut end_eq, expr_eq) = if let Some(block_eq) = scan_block_for_eq(cx, blocks) {
208+
let (start_eq, mut end_eq, expr_eq) = if let Some(block_eq) = scan_block_for_eq(cx, conds, blocks) {
208209
(block_eq.start_eq, block_eq.end_eq, block_eq.expr_eq)
209210
} else {
210211
return;
@@ -316,14 +317,14 @@ struct BlockEqual {
316317

317318
/// This function can also trigger the `IF_SAME_THEN_ELSE` in which case it'll return `None` to
318319
/// abort any further processing and avoid duplicate lint triggers.
319-
fn scan_block_for_eq(cx: &LateContext<'_>, blocks: &[&Block<'_>]) -> Option<BlockEqual> {
320+
fn scan_block_for_eq(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&Block<'_>]) -> Option<BlockEqual> {
320321
let mut start_eq = usize::MAX;
321322
let mut end_eq = usize::MAX;
322323
let mut expr_eq = true;
323-
let mut iter = blocks.windows(2);
324-
while let Some(&[win0, win1]) = iter.next() {
325-
let l_stmts = win0.stmts;
326-
let r_stmts = win1.stmts;
324+
let mut iter = conds.windows(2).zip(blocks.windows(2));
325+
while let Some((&[cond0, cond1], &[block0, block1])) = iter.next() {
326+
let l_stmts = block0.stmts;
327+
let r_stmts = block1.stmts;
327328

328329
// `SpanlessEq` now keeps track of the locals and is therefore context sensitive clippy#6752.
329330
// The comparison therefore needs to be done in a way that builds the correct context.
@@ -340,24 +341,26 @@ fn scan_block_for_eq(cx: &LateContext<'_>, blocks: &[&Block<'_>]) -> Option<Bloc
340341
it1.zip(it2)
341342
.fold(0, |acc, (l, r)| if evaluator.eq_stmt(l, r) { acc + 1 } else { 0 })
342343
};
343-
let block_expr_eq = both(&win0.expr, &win1.expr, |l, r| evaluator.eq_expr(l, r));
344+
let block_expr_eq = both(&block0.expr, &block1.expr, |l, r| evaluator.eq_expr(l, r));
344345

345346
// IF_SAME_THEN_ELSE
346347
if_chain! {
347348
if block_expr_eq;
348349
if l_stmts.len() == r_stmts.len();
349350
if l_stmts.len() == current_start_eq;
350-
if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win0.hir_id);
351-
if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win1.hir_id);
351+
if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, block0.hir_id);
352+
if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, block1.hir_id);
352353
then {
353-
span_lint_and_note(
354-
cx,
355-
IF_SAME_THEN_ELSE,
356-
win0.span,
357-
"this `if` has identical blocks",
358-
Some(win1.span),
359-
"same as this",
360-
);
354+
if !(matches!(cond0.kind, ExprKind::Let(..)) || !matches!(cond1.kind, ExprKind::Let(..))) {
355+
span_lint_and_note(
356+
cx,
357+
IF_SAME_THEN_ELSE,
358+
block0.span,
359+
"this `if` has identical blocks",
360+
Some(block1.span),
361+
"same as this",
362+
);
363+
}
361364

362365
return None;
363366
}

tests/ui/if_then_some_else_none.rs

+17
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,23 @@ fn main() {
6464

6565
// Should not issue an error since the `then` block doesn't use `Some` directly.
6666
let _ = if foo() { into_some("foo") } else { None };
67+
68+
// Issue #7579
69+
let _ = if let Some(0) = None { 0 } else { 0 };
70+
71+
let _ = if true {
72+
0
73+
} else if let Some(0) = None {
74+
0
75+
};
76+
77+
let _ = if let Some(0) = None {
78+
0
79+
} else if let None = None {
80+
0
81+
} else {
82+
0
83+
};
6784
}
6885

6986
fn _msrv_1_49() {

0 commit comments

Comments
 (0)