mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

add nop if-let expression check.

Browse Source 
re-design test cases as some of them are not worth the effort to check.
This commit is contained in:
J-ZhengLi 2022-03-08 17:37:53 +08:00
parent db3fcf8df7
commit 6bfc1120cf
4 changed files with 244 additions and 191 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

95
clippy_lints/src/matches/nop_match.rs
View File

@ -1,25 +1,13 @@
#![allow(unused_variables)]
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 clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{eq_expr_value, get_parent_expr, higher, is_else_clause, is_lang_ctor, peel_blocks_with_stmt};
use rustc_errors::Applicability;
use rustc_hir::{Arm, BindingAnnotation, Expr, ExprKind, Pat, PatKind, PathSegment, QPath};
use rustc_hir::LangItem::OptionNone;
use rustc_hir::{Arm, BindingAnnotation, Expr, ExprKind, Pat, PatKind, Path, PathSegment, QPath};
use rustc_lint::LateContext;
pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>) {
if false {
span_lint_and_sugg(
cx,
NOP_MATCH,
ex.span,
"this if-let expression is unnecessary",
"replace it with",
"".to_string(),
Applicability::MachineApplicable,
);
}
}
use rustc_span::sym;
pub(crate) fn check_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
// This is for avoiding collision with `match_single_binding`.
@ -52,6 +40,70 @@ pub(crate) fn check_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>])
}
}
/// Check for nop `if let` expression that assembled as unnecessary match
///
/// ```rust,ignore
/// if let Some(a) = option {
/// Some(a)
/// } else {
/// None
/// }
/// ```
/// OR
/// ```rust,ignore
/// if let SomeEnum::A = some_enum {
/// SomeEnum::A
/// } else if let SomeEnum::B = some_enum {
/// SomeEnum::B
/// } else {
/// some_enum
/// }
/// ```
pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>) {
if_chain! {
if let Some(ref if_let) = higher::IfLet::hir(cx, ex);
if !is_else_clause(cx.tcx, ex);
if check_if_let(cx, if_let);
then {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
NOP_MATCH,
ex.span,
"this if-let expression is unnecessary",
"replace it with",
snippet_with_applicability(cx, if_let.let_expr.span, "..", &mut applicability).to_string(),
applicability,
);
}
}
}
fn check_if_let(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool {
if let Some(else_block) = if_let.if_else {
if !pat_same_as_expr(if_let.let_pat, peel_blocks_with_stmt(if_let.if_then)) {
return false;
}
let else_expr = peel_blocks_with_stmt(else_block);
// Recurrsively check for each `else if let` phrase,
if let Some(ref nested_if_let) = higher::IfLet::hir(cx, else_expr) {
return check_if_let(cx, nested_if_let);
}
let ret = strip_return(else_expr);
let let_expr_ty = cx.typeck_results().expr_ty(if_let.let_expr);
if is_type_diagnostic_item(cx, let_expr_ty, sym::Option) {
if let ExprKind::Path(ref qpath) = ret.kind {
return is_lang_ctor(cx, qpath, OptionNone) || eq_expr_value(cx, if_let.let_expr, ret);
}
} else {
return eq_expr_value(cx, if_let.let_expr, ret);
}
return true;
}
false
}
fn strip_return<'hir>(expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> {
if let ExprKind::Ret(Some(ret)) = expr.kind {
ret
@ -68,7 +120,7 @@ fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
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)
if has_identical_segments(path.segments, call_path.segments)
&& has_same_non_ref_symbol(first_pat, first_param)
{
return true;
@ -76,7 +128,7 @@ fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
}
},
(PatKind::Path(QPath::Resolved(_, p_path)), ExprKind::Path(QPath::Resolved(_, e_path))) => {
return is_identical_segments(p_path.segments, e_path.segments);
return has_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 {
@ -89,7 +141,7 @@ fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
false
}
fn is_identical_segments(left_segs: &[PathSegment<'_>], right_segs: &[PathSegment<'_>]) -> bool {
fn has_identical_segments(left_segs: &[PathSegment<'_>], right_segs: &[PathSegment<'_>]) -> bool {
if left_segs.len() != right_segs.len() {
return false;
}
@ -105,8 +157,7 @@ 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();
if let ExprKind::Path(QPath::Resolved(_, Path {segments: [first_seg, ..], .. })) = expr.kind;
then {
return pat_ident.name == first_seg.ident.name;
}

107
tests/ui/nop_match.fixed
View File

@ -1,88 +1,67 @@
// run-rustfix
#![warn(clippy::nop_match)]
#![allow(clippy::manual_map)]
#![allow(clippy::question_mark)]
#![allow(dead_code)]
fn func_ret_err<T>(err: T) -> Result<(), T> {
Err(err)
}
enum SampleEnum {
enum Choice {
A,
B,
C,
D,
}
fn useless_prim_type_match(x: i32) -> i32 {
x
fn useless_match(x: i32) {
let _: i32 = x;
}
fn useless_custom_type_match(se: SampleEnum) -> SampleEnum {
se
fn custom_type_match(se: Choice) {
let _: Choice = se;
// Don't trigger
let _: Choice = match se {
Choice::A => Choice::A,
Choice::B => Choice::B,
_ => Choice::C,
};
// Mingled, don't trigger
let _: Choice = match se {
Choice::A => Choice::B,
Choice::B => Choice::C,
Choice::C => Choice::D,
Choice::D => Choice::A,
};
}
// 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(x: Option<i32>) {
let _: Option<i32> = x;
// Don't trigger, this is the case for manual_map_option
let _: Option<i32> = match x {
Some(a) => Some(-a),
None => None,
};
}
fn option_match() -> Option<i32> {
fn func_ret_err<T>(err: T) -> Result<i32, T> {
Err(err)
}
fn result_match() {
let _: Result<i32, i32> = Ok(1);
let _: Result<i32, i32> = func_ret_err(0_i32);
}
fn if_let_option() -> Option<i32> {
Some(1)
}
fn result_match() -> Result<i32, i32> {
Ok(1)
fn if_let_result(x: Result<(), i32>) {
let _: Result<(), i32> = x;
let _: Result<(), i32> = x;
// Input type mismatch, don't trigger
let _: Result<(), i32> = if let Err(e) = Ok(1) { Err(e) } else { x };
}
fn result_match_func_call() {
let _ = func_ret_err(0_i32);
}
fn option_check() -> Option<i32> {
if let Some(a) = Some(1) { Some(a) } else { None }
}
fn option_check_no_else() -> Option<i32> {
if let Some(a) = Some(1) {
return Some(a);
}
None
}
fn result_check_no_else() -> Result<(), i32> {
if let Err(e) = func_ret_err(0_i32) {
return Err(e);
}
Ok(())
}
fn result_check_a() -> Result<(), i32> {
if let Err(e) = func_ret_err(0_i32) {
Err(e)
} else {
Ok(())
}
}
// Don't trigger
fn result_check_b() -> Result<(), i32> {
if let Err(e) = Ok(1) { Err(e) } else { Ok(()) }
}
fn result_check_c() -> Result<(), i32> {
let example = Ok(());
if let Err(e) = example { Err(e) } else { example }
}
// Don't trigger
fn result_check_d() -> Result<(), i32> {
let example = Ok(1);
if let Err(e) = example { Err(e) } else { Ok(()) }
fn custom_enum_a(x: Choice) -> Choice {
x
}
fn main() {}

146
tests/ui/nop_match.rs
View File

@ -1,106 +1,94 @@
// run-rustfix
#![warn(clippy::nop_match)]
#![allow(clippy::manual_map)]
#![allow(clippy::question_mark)]
#![allow(dead_code)]
fn func_ret_err<T>(err: T) -> Result<(), T> {
Err(err)
}
enum SampleEnum {
enum Choice {
A,
B,
C,
D,
}
fn useless_prim_type_match(x: i32) -> i32 {
match x {
fn useless_match(x: i32) {
let _: 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> {
match Some(1) {
Some(a) => Some(a),
None => None,
}
}
fn result_match() -> Result<i32, i32> {
match Ok(1) {
Ok(a) => Ok(a),
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 custom_type_match(se: Choice) {
let _: Choice = match se {
Choice::A => Choice::A,
Choice::B => Choice::B,
Choice::C => Choice::C,
Choice::D => Choice::D,
};
// Don't trigger
let _: Choice = match se {
Choice::A => Choice::A,
Choice::B => Choice::B,
_ => Choice::C,
};
// Mingled, don't trigger
let _: Choice = match se {
Choice::A => Choice::B,
Choice::B => Choice::C,
Choice::C => Choice::D,
Choice::D => Choice::A,
};
}
fn option_match(x: Option<i32>) {
let _: Option<i32> = match x {
Some(a) => Some(a),
None => None,
};
// Don't trigger, this is the case for manual_map_option
let _: Option<i32> = match x {
Some(a) => Some(-a),
None => None,
};
}
fn func_ret_err<T>(err: T) -> Result<i32, T> {
Err(err)
}
fn result_match() {
let _: Result<i32, i32> = match Ok(1) {
Ok(a) => Ok(a),
Err(err) => Err(err),
};
let _: Result<i32, i32> = match func_ret_err(0_i32) {
Err(err) => Err(err),
Ok(a) => Ok(a),
};
}
fn if_let_option() -> Option<i32> {
if let Some(a) = Some(1) { Some(a) } else { None }
}
fn option_check_no_else() -> Option<i32> {
if let Some(a) = Some(1) {
return Some(a);
}
None
fn if_let_result(x: Result<(), i32>) {
let _: Result<(), i32> = if let Err(e) = x { Err(e) } else { x };
let _: Result<(), i32> = if let Ok(val) = x { Ok(val) } else { x };
// Input type mismatch, don't trigger
let _: Result<(), i32> = if let Err(e) = Ok(1) { Err(e) } else { x };
}
fn result_check_no_else() -> Result<(), i32> {
if let Err(e) = func_ret_err(0_i32) {
return Err(e);
}
Ok(())
}
fn result_check_a() -> Result<(), i32> {
if let Err(e) = func_ret_err(0_i32) {
Err(e)
fn custom_enum_a(x: Choice) -> Choice {
if let Choice::A = x {
Choice::A
} else if let Choice::B = x {
Choice::B
} else if let Choice::C = x {
Choice::C
} else {
Ok(())
x
}
}
// Don't trigger
fn result_check_b() -> Result<(), i32> {
if let Err(e) = Ok(1) { Err(e) } else { Ok(()) }
}
fn result_check_c() -> Result<(), i32> {
let example = Ok(());
if let Err(e) = example { Err(e) } else { example }
}
// Don't trigger
fn result_check_d() -> Result<(), i32> {
let example = Ok(1);
if let Err(e) = example { Err(e) } else { Ok(()) }
}
fn main() {}

87
tests/ui/nop_match.stderr
View File

@ -1,53 +1,88 @@
error: this match expression is unnecessary
--> $DIR/nop_match.rs:18:5
--> $DIR/nop_match.rs:14:18
|
LL | / match x {
LL | let _: i32 = match x {
| __________________^
LL | | 0 => 0,
LL | | 1 => 1,
LL | | 2 => 2,
LL | | _ => x,
LL | | }
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
--> $DIR/nop_match.rs:23:21
|
LL | / match se {
LL | | SampleEnum::A => SampleEnum::A,
LL | | SampleEnum::B => SampleEnum::B,
LL | | SampleEnum::C => SampleEnum::C,
LL | | }
LL | let _: Choice = match se {
| _____________________^
LL | | Choice::A => Choice::A,
LL | | Choice::B => Choice::B,
LL | | Choice::C => Choice::C,
LL | | Choice::D => Choice::D,
LL | | };
| |_____^ help: replace it with: `se`
error: this match expression is unnecessary
--> $DIR/nop_match.rs:44:5
--> $DIR/nop_match.rs:45:26
|
LL | / match Some(1) {
LL | let _: Option<i32> = match x {
| __________________________^
LL | | Some(a) => Some(a),
LL | | None => None,
LL | | }
| |_____^ help: replace it with: `Some(1)`
LL | | };
| |_____^ help: replace it with: `x`
error: this match expression is unnecessary
--> $DIR/nop_match.rs:51:5
--> $DIR/nop_match.rs:61:31
|
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 | let _: Result<i32, i32> = 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:65:31
|
LL | let _: Result<i32, i32> = match func_ret_err(0_i32) {
| _______________________________^
LL | | Err(err) => Err(err),
LL | | Ok(a) => Ok(a),
LL | | };
| |_____^ help: replace it with: `func_ret_err(0_i32)`
error: aborting due to 5 previous errors
error: this if-let expression is unnecessary
--> $DIR/nop_match.rs:72:5
|
LL | if let Some(a) = Some(1) { Some(a) } else { None }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `Some(1)`
error: this if-let expression is unnecessary
--> $DIR/nop_match.rs:76:30
|
LL | let _: Result<(), i32> = if let Err(e) = x { Err(e) } else { x };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `x`
error: this if-let expression is unnecessary
--> $DIR/nop_match.rs:77:30
|
LL | let _: Result<(), i32> = if let Ok(val) = x { Ok(val) } else { x };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `x`
error: this if-let expression is unnecessary
--> $DIR/nop_match.rs:83:5
|
LL | / if let Choice::A = x {
LL | | Choice::A
LL | | } else if let Choice::B = x {
LL | | Choice::B
... |
LL | | x
LL | | }
| |_____^ help: replace it with: `x`
error: aborting due to 9 previous errors