add basic code to check nop match blocks
modify `manual_map_option` uitest because one test case has confliction.
This commit is contained in:
parent
30fb8229e1
commit
db3fcf8df7
@ -16,12 +16,12 @@
|
|||||||
mod match_single_binding;
|
mod match_single_binding;
|
||||||
mod match_wild_enum;
|
mod match_wild_enum;
|
||||||
mod match_wild_err_arm;
|
mod match_wild_err_arm;
|
||||||
|
mod nop_match;
|
||||||
mod overlapping_arms;
|
mod overlapping_arms;
|
||||||
mod redundant_pattern_match;
|
mod redundant_pattern_match;
|
||||||
mod rest_pat_in_fully_bound_struct;
|
mod rest_pat_in_fully_bound_struct;
|
||||||
mod single_match;
|
mod single_match;
|
||||||
mod wild_in_or_pats;
|
mod wild_in_or_pats;
|
||||||
mod nop_match;
|
|
||||||
|
|
||||||
declare_clippy_lint! {
|
declare_clippy_lint! {
|
||||||
/// ### What it does
|
/// ### What it does
|
||||||
@ -569,7 +569,7 @@
|
|||||||
|
|
||||||
declare_clippy_lint! {
|
declare_clippy_lint! {
|
||||||
/// ### What it does
|
/// ### What it does
|
||||||
/// Checks for unnecessary `match` or match-like `if let` returns for `Option` and `Result`
|
/// Checks for unnecessary `match` or match-like `if let` returns for `Option` and `Result`
|
||||||
/// when function signatures are the same.
|
/// when function signatures are the same.
|
||||||
///
|
///
|
||||||
/// ### Why is this bad?
|
/// ### Why is this bad?
|
||||||
@ -583,7 +583,7 @@
|
|||||||
/// Err(err) => Err(err),
|
/// Err(err) => Err(err),
|
||||||
/// }
|
/// }
|
||||||
/// }
|
/// }
|
||||||
///
|
///
|
||||||
/// fn bar() -> Option<i32> {
|
/// fn bar() -> Option<i32> {
|
||||||
/// if let Some(val) = option {
|
/// if let Some(val) = option {
|
||||||
/// Some(val)
|
/// Some(val)
|
||||||
@ -594,12 +594,12 @@
|
|||||||
/// ```
|
/// ```
|
||||||
///
|
///
|
||||||
/// Could be replaced as
|
/// Could be replaced as
|
||||||
///
|
///
|
||||||
/// ```rust,ignore
|
/// ```rust,ignore
|
||||||
/// fn foo() -> Result<(), i32> {
|
/// fn foo() -> Result<(), i32> {
|
||||||
/// result
|
/// result
|
||||||
/// }
|
/// }
|
||||||
///
|
///
|
||||||
/// fn bar() -> Option<i32> {
|
/// fn bar() -> Option<i32> {
|
||||||
/// option
|
/// option
|
||||||
/// }
|
/// }
|
||||||
|
@ -1,9 +1,11 @@
|
|||||||
#![allow(unused_variables)]
|
#![allow(unused_variables)]
|
||||||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
|
||||||
use rustc_lint::LateContext;
|
|
||||||
use rustc_hir::{Arm, Expr};
|
|
||||||
use rustc_errors::Applicability;
|
|
||||||
use super::NOP_MATCH;
|
use super::NOP_MATCH;
|
||||||
|
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||||
|
use clippy_utils::source::snippet_with_applicability;
|
||||||
|
use clippy_utils::{eq_expr_value, get_parent_expr};
|
||||||
|
use rustc_errors::Applicability;
|
||||||
|
use rustc_hir::{Arm, BindingAnnotation, Expr, ExprKind, Pat, PatKind, PathSegment, QPath};
|
||||||
|
use rustc_lint::LateContext;
|
||||||
|
|
||||||
pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>) {
|
pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>) {
|
||||||
if false {
|
if false {
|
||||||
@ -20,15 +22,95 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn check_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
|
pub(crate) fn check_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
|
||||||
if false {
|
// This is for avoiding collision with `match_single_binding`.
|
||||||
|
if arms.len() < 2 {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
for arm in arms {
|
||||||
|
if let PatKind::Wild = arm.pat.kind {
|
||||||
|
let ret_expr = strip_return(arm.body);
|
||||||
|
if !eq_expr_value(cx, ex, ret_expr) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
} else if !pat_same_as_expr(arm.pat, arm.body) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if let Some(match_expr) = get_parent_expr(cx, ex) {
|
||||||
|
let mut applicability = Applicability::MachineApplicable;
|
||||||
span_lint_and_sugg(
|
span_lint_and_sugg(
|
||||||
cx,
|
cx,
|
||||||
NOP_MATCH,
|
NOP_MATCH,
|
||||||
ex.span,
|
match_expr.span,
|
||||||
"this match expression is unnecessary",
|
"this match expression is unnecessary",
|
||||||
"replace it with",
|
"replace it with",
|
||||||
"".to_string(),
|
snippet_with_applicability(cx, ex.span, "..", &mut applicability).to_string(),
|
||||||
Applicability::MachineApplicable,
|
applicability,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn strip_return<'hir>(expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> {
|
||||||
|
if let ExprKind::Ret(Some(ret)) = expr.kind {
|
||||||
|
ret
|
||||||
|
} else {
|
||||||
|
expr
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
|
||||||
|
let expr = strip_return(expr);
|
||||||
|
match (&pat.kind, &expr.kind) {
|
||||||
|
(
|
||||||
|
PatKind::TupleStruct(QPath::Resolved(_, path), [first_pat, ..], _),
|
||||||
|
ExprKind::Call(call_expr, [first_param, ..]),
|
||||||
|
) => {
|
||||||
|
if let ExprKind::Path(QPath::Resolved(_, call_path)) = call_expr.kind {
|
||||||
|
if is_identical_segments(path.segments, call_path.segments)
|
||||||
|
&& has_same_non_ref_symbol(first_pat, first_param)
|
||||||
|
{
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
},
|
||||||
|
(PatKind::Path(QPath::Resolved(_, p_path)), ExprKind::Path(QPath::Resolved(_, e_path))) => {
|
||||||
|
return is_identical_segments(p_path.segments, e_path.segments);
|
||||||
|
},
|
||||||
|
(PatKind::Lit(pat_lit_expr), ExprKind::Lit(expr_spanned)) => {
|
||||||
|
if let ExprKind::Lit(pat_spanned) = &pat_lit_expr.kind {
|
||||||
|
return pat_spanned.node == expr_spanned.node;
|
||||||
|
}
|
||||||
|
},
|
||||||
|
_ => {},
|
||||||
|
}
|
||||||
|
|
||||||
|
false
|
||||||
|
}
|
||||||
|
|
||||||
|
fn is_identical_segments(left_segs: &[PathSegment<'_>], right_segs: &[PathSegment<'_>]) -> bool {
|
||||||
|
if left_segs.len() != right_segs.len() {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
for i in 0..left_segs.len() {
|
||||||
|
if left_segs[i].ident.name != right_segs[i].ident.name {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
true
|
||||||
|
}
|
||||||
|
|
||||||
|
fn has_same_non_ref_symbol(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
|
||||||
|
if_chain! {
|
||||||
|
if let PatKind::Binding(annot, _, pat_ident, _) = pat.kind;
|
||||||
|
if !matches!(annot, BindingAnnotation::Ref | BindingAnnotation::RefMut);
|
||||||
|
if let ExprKind::Path(QPath::Resolved(_, path)) = expr.kind;
|
||||||
|
if let Some(first_seg) = path.segments.first();
|
||||||
|
then {
|
||||||
|
return pat_ident.name == first_seg.ident.name;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
false
|
||||||
|
}
|
||||||
|
@ -148,6 +148,7 @@ fn main() {
|
|||||||
|
|
||||||
// #7077
|
// #7077
|
||||||
let s = &String::new();
|
let s = &String::new();
|
||||||
|
#[allow(clippy::nop_match)]
|
||||||
let _: Option<&str> = match Some(s) {
|
let _: Option<&str> = match Some(s) {
|
||||||
Some(s) => Some(s),
|
Some(s) => Some(s),
|
||||||
None => None,
|
None => None,
|
||||||
|
@ -214,6 +214,7 @@ const fn f4() {
|
|||||||
|
|
||||||
// #7077
|
// #7077
|
||||||
let s = &String::new();
|
let s = &String::new();
|
||||||
|
#[allow(clippy::nop_match)]
|
||||||
let _: Option<&str> = match Some(s) {
|
let _: Option<&str> = match Some(s) {
|
||||||
Some(s) => Some(s),
|
Some(s) => Some(s),
|
||||||
None => None,
|
None => None,
|
||||||
|
@ -4,26 +4,47 @@
|
|||||||
#![allow(clippy::question_mark)]
|
#![allow(clippy::question_mark)]
|
||||||
#![allow(dead_code)]
|
#![allow(dead_code)]
|
||||||
|
|
||||||
fn option_match() -> Option<i32> {
|
fn func_ret_err<T>(err: T) -> Result<(), T> {
|
||||||
match Some(1) {
|
Err(err)
|
||||||
Some(a) => Some(a),
|
}
|
||||||
None => None
|
|
||||||
|
enum SampleEnum {
|
||||||
|
A,
|
||||||
|
B,
|
||||||
|
C,
|
||||||
|
}
|
||||||
|
|
||||||
|
fn useless_prim_type_match(x: i32) -> i32 {
|
||||||
|
x
|
||||||
|
}
|
||||||
|
|
||||||
|
fn useless_custom_type_match(se: SampleEnum) -> SampleEnum {
|
||||||
|
se
|
||||||
|
}
|
||||||
|
|
||||||
|
// Don't trigger
|
||||||
|
fn mingled_custom_type(se: SampleEnum) -> SampleEnum {
|
||||||
|
match se {
|
||||||
|
SampleEnum::A => SampleEnum::B,
|
||||||
|
SampleEnum::B => SampleEnum::C,
|
||||||
|
SampleEnum::C => SampleEnum::A,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn option_match() -> Option<i32> {
|
||||||
|
Some(1)
|
||||||
|
}
|
||||||
|
|
||||||
fn result_match() -> Result<i32, i32> {
|
fn result_match() -> Result<i32, i32> {
|
||||||
match Ok(1) {
|
Ok(1)
|
||||||
Ok(a) => Ok(a),
|
}
|
||||||
Err(err) => Err(err)
|
|
||||||
}
|
fn result_match_func_call() {
|
||||||
|
let _ = func_ret_err(0_i32);
|
||||||
}
|
}
|
||||||
|
|
||||||
fn option_check() -> Option<i32> {
|
fn option_check() -> Option<i32> {
|
||||||
if let Some(a) = Some(1) {
|
if let Some(a) = Some(1) { Some(a) } else { None }
|
||||||
Some(a)
|
|
||||||
} else {
|
|
||||||
None
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn option_check_no_else() -> Option<i32> {
|
fn option_check_no_else() -> Option<i32> {
|
||||||
@ -33,10 +54,6 @@ fn option_check_no_else() -> Option<i32> {
|
|||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
|
||||||
fn func_ret_err<T>(err: T) -> Result<(), T> {
|
|
||||||
Err(err)
|
|
||||||
}
|
|
||||||
|
|
||||||
fn result_check_no_else() -> Result<(), i32> {
|
fn result_check_no_else() -> Result<(), i32> {
|
||||||
if let Err(e) = func_ret_err(0_i32) {
|
if let Err(e) = func_ret_err(0_i32) {
|
||||||
return Err(e);
|
return Err(e);
|
||||||
@ -54,30 +71,18 @@ fn result_check_a() -> Result<(), i32> {
|
|||||||
|
|
||||||
// Don't trigger
|
// Don't trigger
|
||||||
fn result_check_b() -> Result<(), i32> {
|
fn result_check_b() -> Result<(), i32> {
|
||||||
if let Err(e) = Ok(1) {
|
if let Err(e) = Ok(1) { Err(e) } else { Ok(()) }
|
||||||
Err(e)
|
|
||||||
} else {
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn result_check_c() -> Result<(), i32> {
|
fn result_check_c() -> Result<(), i32> {
|
||||||
let example = Ok(());
|
let example = Ok(());
|
||||||
if let Err(e) = example {
|
if let Err(e) = example { Err(e) } else { example }
|
||||||
Err(e)
|
|
||||||
} else {
|
|
||||||
example
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Don't trigger
|
// Don't trigger
|
||||||
fn result_check_d() -> Result<(), i32> {
|
fn result_check_d() -> Result<(), i32> {
|
||||||
let example = Ok(1);
|
let example = Ok(1);
|
||||||
if let Err(e) = example {
|
if let Err(e) = example { Err(e) } else { Ok(()) }
|
||||||
Err(e)
|
|
||||||
} else {
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn main() { }
|
fn main() {}
|
||||||
|
@ -4,26 +4,65 @@
|
|||||||
#![allow(clippy::question_mark)]
|
#![allow(clippy::question_mark)]
|
||||||
#![allow(dead_code)]
|
#![allow(dead_code)]
|
||||||
|
|
||||||
|
fn func_ret_err<T>(err: T) -> Result<(), T> {
|
||||||
|
Err(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
enum SampleEnum {
|
||||||
|
A,
|
||||||
|
B,
|
||||||
|
C,
|
||||||
|
}
|
||||||
|
|
||||||
|
fn useless_prim_type_match(x: i32) -> i32 {
|
||||||
|
match x {
|
||||||
|
0 => 0,
|
||||||
|
1 => 1,
|
||||||
|
2 => 2,
|
||||||
|
_ => x,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn useless_custom_type_match(se: SampleEnum) -> SampleEnum {
|
||||||
|
match se {
|
||||||
|
SampleEnum::A => SampleEnum::A,
|
||||||
|
SampleEnum::B => SampleEnum::B,
|
||||||
|
SampleEnum::C => SampleEnum::C,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Don't trigger
|
||||||
|
fn mingled_custom_type(se: SampleEnum) -> SampleEnum {
|
||||||
|
match se {
|
||||||
|
SampleEnum::A => SampleEnum::B,
|
||||||
|
SampleEnum::B => SampleEnum::C,
|
||||||
|
SampleEnum::C => SampleEnum::A,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn option_match() -> Option<i32> {
|
fn option_match() -> Option<i32> {
|
||||||
match Some(1) {
|
match Some(1) {
|
||||||
Some(a) => Some(a),
|
Some(a) => Some(a),
|
||||||
None => None
|
None => None,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn result_match() -> Result<i32, i32> {
|
fn result_match() -> Result<i32, i32> {
|
||||||
match Ok(1) {
|
match Ok(1) {
|
||||||
Ok(a) => Ok(a),
|
Ok(a) => Ok(a),
|
||||||
Err(err) => Err(err)
|
Err(err) => Err(err),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn result_match_func_call() {
|
||||||
|
let _ = match func_ret_err(0_i32) {
|
||||||
|
Ok(a) => Ok(a),
|
||||||
|
Err(err) => Err(err),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
fn option_check() -> Option<i32> {
|
fn option_check() -> Option<i32> {
|
||||||
if let Some(a) = Some(1) {
|
if let Some(a) = Some(1) { Some(a) } else { None }
|
||||||
Some(a)
|
|
||||||
} else {
|
|
||||||
None
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn option_check_no_else() -> Option<i32> {
|
fn option_check_no_else() -> Option<i32> {
|
||||||
@ -33,10 +72,6 @@ fn option_check_no_else() -> Option<i32> {
|
|||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
|
||||||
fn func_ret_err<T>(err: T) -> Result<(), T> {
|
|
||||||
Err(err)
|
|
||||||
}
|
|
||||||
|
|
||||||
fn result_check_no_else() -> Result<(), i32> {
|
fn result_check_no_else() -> Result<(), i32> {
|
||||||
if let Err(e) = func_ret_err(0_i32) {
|
if let Err(e) = func_ret_err(0_i32) {
|
||||||
return Err(e);
|
return Err(e);
|
||||||
@ -54,30 +89,18 @@ fn result_check_a() -> Result<(), i32> {
|
|||||||
|
|
||||||
// Don't trigger
|
// Don't trigger
|
||||||
fn result_check_b() -> Result<(), i32> {
|
fn result_check_b() -> Result<(), i32> {
|
||||||
if let Err(e) = Ok(1) {
|
if let Err(e) = Ok(1) { Err(e) } else { Ok(()) }
|
||||||
Err(e)
|
|
||||||
} else {
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn result_check_c() -> Result<(), i32> {
|
fn result_check_c() -> Result<(), i32> {
|
||||||
let example = Ok(());
|
let example = Ok(());
|
||||||
if let Err(e) = example {
|
if let Err(e) = example { Err(e) } else { example }
|
||||||
Err(e)
|
|
||||||
} else {
|
|
||||||
example
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Don't trigger
|
// Don't trigger
|
||||||
fn result_check_d() -> Result<(), i32> {
|
fn result_check_d() -> Result<(), i32> {
|
||||||
let example = Ok(1);
|
let example = Ok(1);
|
||||||
if let Err(e) = example {
|
if let Err(e) = example { Err(e) } else { Ok(()) }
|
||||||
Err(e)
|
|
||||||
} else {
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn main() { }
|
fn main() {}
|
||||||
|
@ -0,0 +1,53 @@
|
|||||||
|
error: this match expression is unnecessary
|
||||||
|
--> $DIR/nop_match.rs:18:5
|
||||||
|
|
|
||||||
|
LL | / match x {
|
||||||
|
LL | | 0 => 0,
|
||||||
|
LL | | 1 => 1,
|
||||||
|
LL | | 2 => 2,
|
||||||
|
LL | | _ => x,
|
||||||
|
LL | | }
|
||||||
|
| |_____^ help: replace it with: `x`
|
||||||
|
|
|
||||||
|
= note: `-D clippy::nop-match` implied by `-D warnings`
|
||||||
|
|
||||||
|
error: this match expression is unnecessary
|
||||||
|
--> $DIR/nop_match.rs:27:5
|
||||||
|
|
|
||||||
|
LL | / match se {
|
||||||
|
LL | | SampleEnum::A => SampleEnum::A,
|
||||||
|
LL | | SampleEnum::B => SampleEnum::B,
|
||||||
|
LL | | SampleEnum::C => SampleEnum::C,
|
||||||
|
LL | | }
|
||||||
|
| |_____^ help: replace it with: `se`
|
||||||
|
|
||||||
|
error: this match expression is unnecessary
|
||||||
|
--> $DIR/nop_match.rs:44:5
|
||||||
|
|
|
||||||
|
LL | / match Some(1) {
|
||||||
|
LL | | Some(a) => Some(a),
|
||||||
|
LL | | None => None,
|
||||||
|
LL | | }
|
||||||
|
| |_____^ help: replace it with: `Some(1)`
|
||||||
|
|
||||||
|
error: this match expression is unnecessary
|
||||||
|
--> $DIR/nop_match.rs:51:5
|
||||||
|
|
|
||||||
|
LL | / match Ok(1) {
|
||||||
|
LL | | Ok(a) => Ok(a),
|
||||||
|
LL | | Err(err) => Err(err),
|
||||||
|
LL | | }
|
||||||
|
| |_____^ help: replace it with: `Ok(1)`
|
||||||
|
|
||||||
|
error: this match expression is unnecessary
|
||||||
|
--> $DIR/nop_match.rs:58:13
|
||||||
|
|
|
||||||
|
LL | let _ = match func_ret_err(0_i32) {
|
||||||
|
| _____________^
|
||||||
|
LL | | Ok(a) => Ok(a),
|
||||||
|
LL | | Err(err) => Err(err),
|
||||||
|
LL | | };
|
||||||
|
| |_____^ help: replace it with: `func_ret_err(0_i32)`
|
||||||
|
|
||||||
|
error: aborting due to 5 previous errors
|
||||||
|
|
Loading…
Reference in New Issue
Block a user