Skip to content

Commit

Permalink
When checking if two empty hir blocks are equal also check to see if …
Browse files Browse the repository at this point in the history
…the tokens used are the same as well
  • Loading branch information
Jarcho committed Mar 4, 2021
1 parent ff51964 commit 39c5e86
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 8 deletions.
81 changes: 75 additions & 6 deletions clippy_utils/src/hir_utils.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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)]
Expand All @@ -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)
},
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
29 changes: 29 additions & 0 deletions tests/ui/match_same_arms2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
27 changes: 25 additions & 2 deletions tests/ui/match_same_arms2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
| ________________^
Expand All @@ -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

0 comments on commit 39c5e86

Please sign in to comment.