From 66afdd1f4292e7fda6ea89113c0c8343e3321d99 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Thu, 21 Jan 2021 19:21:12 -0600 Subject: [PATCH] Enhance collapsible_match for adjusted bindings --- clippy_lints/src/collapsible_match.rs | 22 +++++++++++++--- tests/ui/collapsible_match2.rs | 29 ++++++++++++++++++++ tests/ui/collapsible_match2.stderr | 38 ++++++++++++++++++++++++++- 3 files changed, 85 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/collapsible_match.rs b/clippy_lints/src/collapsible_match.rs index c75aa2bde97..604ba102046 100644 --- a/clippy_lints/src/collapsible_match.rs +++ b/clippy_lints/src/collapsible_match.rs @@ -2,7 +2,7 @@ use crate::utils::{span_lint_and_then, SpanlessEq}; use if_chain::if_chain; use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; -use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind, QPath, StmtKind}; +use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind, QPath, StmtKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{DefIdTree, TyCtxt}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -72,8 +72,7 @@ fn check_arm(arm: &Arm<'_>, wild_outer_arm: &Arm<'_>, cx: &LateContext<'_>) { if arms_inner.iter().all(|arm| arm.guard.is_none()); // match expression must be a local binding // match { .. } - if let ExprKind::Path(QPath::Resolved(None, path)) = expr_in.kind; - if let Res::Local(binding_id) = path.res; + if let Some(binding_id) = addr_adjusted_binding(expr_in, cx); // 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(arm_inner, cx.tcx)); let (wild_inner_arm, non_wild_inner_arm) = @@ -175,3 +174,20 @@ fn is_none_ctor(res: Res, tcx: TyCtxt<'_>) -> bool { } false } + +/// Retrieves a binding ID with optional `&` and/or `*` operators removed. (e.g. `&**foo`) +/// Returns `None` if a non-reference type is de-referenced. +/// For example, if `Vec` is de-referenced to a slice, `None` is returned. +fn addr_adjusted_binding(mut expr: &Expr<'_>, cx: &LateContext<'_>) -> Option { + loop { + match expr.kind { + ExprKind::AddrOf(_, _, e) => expr = e, + ExprKind::Path(QPath::Resolved(None, path)) => match path.res { + Res::Local(binding_id) => break Some(binding_id), + _ => break None, + }, + ExprKind::Unary(UnOp::UnDeref, e) if cx.typeck_results().expr_ty(e).is_ref() => expr = e, + _ => break None, + } + } +} diff --git a/tests/ui/collapsible_match2.rs b/tests/ui/collapsible_match2.rs index d571ac4ab69..8372a212477 100644 --- a/tests/ui/collapsible_match2.rs +++ b/tests/ui/collapsible_match2.rs @@ -40,6 +40,35 @@ macro_rules! mac { // there is still a better way to write this. mac!(res_opt => Ok(val), val => Some(n), foo(n)); } + + // deref reference value + match Some(&[1]) { + Some(s) => match *s { + [n] => foo(n), + _ => (), + }, + _ => (), + } + + // ref pattern and deref + match Some(&[1]) { + Some(ref s) => match &*s { + [n] => foo(n), + _ => (), + }, + _ => (), + } +} + +fn no_lint() { + // deref inner value (cannot pattern match with Vec) + match Some(vec![1]) { + Some(s) => match *s { + [n] => foo(n), + _ => (), + }, + _ => (), + } } fn make() -> T { diff --git a/tests/ui/collapsible_match2.stderr b/tests/ui/collapsible_match2.stderr index 490d82d12cd..b2eb457d173 100644 --- a/tests/ui/collapsible_match2.stderr +++ b/tests/ui/collapsible_match2.stderr @@ -57,5 +57,41 @@ LL | mac!(res_opt => Ok(val), val => Some(n), foo(n)); | Replace this binding = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 3 previous errors +error: Unnecessary nested match + --> $DIR/collapsible_match2.rs:46:20 + | +LL | Some(s) => match *s { + | ____________________^ +LL | | [n] => foo(n), +LL | | _ => (), +LL | | }, + | |_________^ + | +help: The outer pattern can be modified to include the inner pattern. + --> $DIR/collapsible_match2.rs:46:14 + | +LL | Some(s) => match *s { + | ^ Replace this binding +LL | [n] => foo(n), + | ^^^ with this pattern + +error: Unnecessary nested match + --> $DIR/collapsible_match2.rs:55:24 + | +LL | Some(ref s) => match &*s { + | ________________________^ +LL | | [n] => foo(n), +LL | | _ => (), +LL | | }, + | |_________^ + | +help: The outer pattern can be modified to include the inner pattern. + --> $DIR/collapsible_match2.rs:55:14 + | +LL | Some(ref s) => match &*s { + | ^^^^^ Replace this binding +LL | [n] => foo(n), + | ^^^ with this pattern + +error: aborting due to 5 previous errors