diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 81be9254cbe1..e28ad27d9a6f 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -1,5 +1,5 @@ use crate::consts::{constant_context, constant_simple}; -use crate::differing_macro_contexts; +use crate::{differing_macro_contexts, snippet_opt}; use rustc_ast::ast::InlineAsmTemplatePiece; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; @@ -9,6 +9,7 @@ use rustc_hir::{ GenericArg, GenericArgs, Guard, HirId, InlineAsmOperand, Lifetime, LifetimeName, ParamName, Pat, PatKind, Path, PathSegment, QPath, Stmt, StmtKind, Ty, TyKind, TypeBinding, }; +use rustc_lexer::{tokenize, TokenKind}; use rustc_lint::LateContext; use rustc_middle::ich::StableHashingContextProvider; use rustc_middle::ty::TypeckResults; @@ -110,8 +111,54 @@ impl HirEqInterExpr<'_, '_, '_> { /// Checks whether two blocks are the same. fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool { - over(&left.stmts, &right.stmts, |l, r| self.eq_stmt(l, r)) - && both(&left.expr, &right.expr, |l, r| self.eq_expr(l, r)) + match (left.stmts, left.expr, right.stmts, right.expr) { + ([], None, [], None) => { + // For empty blocks, check to see if the tokens are equal. This will catch the case where a macro + // expanded to nothing, or the cfg attribute was used. + let (left, right) = match ( + snippet_opt(self.inner.cx, left.span), + snippet_opt(self.inner.cx, right.span), + ) { + (Some(left), Some(right)) => (left, right), + _ => return true, + }; + let mut left_pos = 0; + let left = tokenize(&left) + .map(|t| { + let end = left_pos + t.len; + let s = &left[left_pos..end]; + left_pos = end; + (t, s) + }) + .filter(|(t, _)| { + !matches!( + t.kind, + TokenKind::LineComment { .. } | TokenKind::BlockComment { .. } | TokenKind::Whitespace + ) + }) + .map(|(_, s)| s); + let mut right_pos = 0; + let right = tokenize(&right) + .map(|t| { + let end = right_pos + t.len; + let s = &right[right_pos..end]; + right_pos = end; + (t, s) + }) + .filter(|(t, _)| { + !matches!( + t.kind, + TokenKind::LineComment { .. } | TokenKind::BlockComment { .. } | TokenKind::Whitespace + ) + }) + .map(|(_, s)| s); + left.eq(right) + }, + _ => { + over(&left.stmts, &right.stmts, |l, r| self.eq_stmt(l, r)) + && both(&left.expr, &right.expr, |l, r| self.eq_expr(l, r)) + }, + } } #[allow(clippy::similar_names)] @@ -131,7 +178,10 @@ impl HirEqInterExpr<'_, '_, '_> { } } - let is_eq = match (&reduce_exprkind(&left.kind), &reduce_exprkind(&right.kind)) { + let is_eq = match ( + &reduce_exprkind(self.inner.cx, &left.kind), + &reduce_exprkind(self.inner.cx, &right.kind), + ) { (&ExprKind::AddrOf(lb, l_mut, ref le), &ExprKind::AddrOf(rb, r_mut, ref re)) => { lb == rb && l_mut == r_mut && self.eq_expr(le, re) }, @@ -360,11 +410,30 @@ impl HirEqInterExpr<'_, '_, '_> { } /// Some simple reductions like `{ return }` => `return` -fn reduce_exprkind<'hir>(kind: &'hir ExprKind<'hir>) -> &ExprKind<'hir> { +fn reduce_exprkind<'hir>(cx: &LateContext<'_>, kind: &'hir ExprKind<'hir>) -> &'hir ExprKind<'hir> { if let ExprKind::Block(block, _) = kind { match (block.stmts, block.expr) { + // From an `if let` expression without an `else` block. The arm for the implicit wild pattern is an empty + // block with an empty span. + ([], None) if block.span.is_empty() => &ExprKind::Tup(&[]), // `{}` => `()` - ([], None) => &ExprKind::Tup(&[]), + ([], None) => match snippet_opt(cx, block.span) { + // Don't reduce if there are any tokens contained in the braces + Some(snip) + if tokenize(&snip) + .map(|t| t.kind) + .filter(|t| { + !matches!( + t, + TokenKind::LineComment { .. } | TokenKind::BlockComment { .. } | TokenKind::Whitespace + ) + }) + .ne([TokenKind::OpenBrace, TokenKind::CloseBrace].iter().cloned()) => + { + kind + }, + _ => &ExprKind::Tup(&[]), + }, ([], Some(expr)) => match expr.kind { // `{ return .. }` => `return ..` ExprKind::Ret(..) => &expr.kind, diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 00455d4102f9..0395079901cb 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -14,6 +14,7 @@ extern crate rustc_errors; extern crate rustc_hir; extern crate rustc_hir_pretty; extern crate rustc_infer; +extern crate rustc_lexer; extern crate rustc_lint; extern crate rustc_middle; extern crate rustc_mir; diff --git a/tests/ui/match_same_arms2.rs b/tests/ui/match_same_arms2.rs index 06d91497242e..da4e3020d5b8 100644 --- a/tests/ui/match_same_arms2.rs +++ b/tests/ui/match_same_arms2.rs @@ -120,6 +120,35 @@ fn match_same_arms() { }, } + // False positive #1390 + macro_rules! empty { + ($e:expr) => {}; + } + match 0 { + 0 => { + empty!(0); + }, + 1 => { + empty!(1); + }, + x => { + empty!(x); + }, + }; + + // still lint if the tokens are the same + match 0 { + 0 => { + empty!(0); + }, + 1 => { + empty!(0); + }, + x => { + empty!(x); + }, + } + match_expr_like_matches_macro_priority(); } diff --git a/tests/ui/match_same_arms2.stderr b/tests/ui/match_same_arms2.stderr index fccaf805616b..95f9494cdc99 100644 --- a/tests/ui/match_same_arms2.stderr +++ b/tests/ui/match_same_arms2.stderr @@ -141,8 +141,31 @@ LL | Ok(3) => println!("ok"), | ^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) +error: this `match` has identical arm bodies + --> $DIR/match_same_arms2.rs:144:14 + | +LL | 1 => { + | ______________^ +LL | | empty!(0); +LL | | }, + | |_________^ + | +note: same as this + --> $DIR/match_same_arms2.rs:141:14 + | +LL | 0 => { + | ______________^ +LL | | empty!(0); +LL | | }, + | |_________^ +help: consider refactoring into `0 | 1` + --> $DIR/match_same_arms2.rs:141:9 + | +LL | 0 => { + | ^ + error: match expression looks like `matches!` macro - --> $DIR/match_same_arms2.rs:133:16 + --> $DIR/match_same_arms2.rs:162:16 | LL | let _ans = match x { | ________________^ @@ -154,5 +177,5 @@ LL | | }; | = note: `-D clippy::match-like-matches-macro` implied by `-D warnings` -error: aborting due to 8 previous errors +error: aborting due to 9 previous errors