From 30fb8229e13dadbe9f9968108dd46ebb8d43e56e Mon Sep 17 00:00:00 2001 From: J-ZhengLi Date: Fri, 25 Feb 2022 18:08:52 +0800 Subject: [PATCH] add tests, add base bone for the new lint --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_correctness.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/matches/mod.rs | 47 +++++++++++ clippy_lints/src/matches/nop_match.rs | 34 ++++++++ tests/ui/nop_match.fixed | 83 ++++++++++++++++++++ tests/ui/nop_match.rs | 83 ++++++++++++++++++++ tests/ui/nop_match.stderr | 0 9 files changed, 251 insertions(+) create mode 100644 clippy_lints/src/matches/nop_match.rs create mode 100644 tests/ui/nop_match.fixed create mode 100644 tests/ui/nop_match.rs create mode 100644 tests/ui/nop_match.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b52a6fcd05..16fe9454634 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3348,6 +3348,7 @@ Released 2018-09-13 [`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool [`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options [`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces +[`nop_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#nop_match [`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref [`octal_escapes`]: https://rust-lang.github.io/rust-clippy/master/index.html#octal_escapes [`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index c6f8470cd7d..01429bf7ea5 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -134,6 +134,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(matches::MATCH_OVERLAPPING_ARM), LintId::of(matches::MATCH_REF_PATS), LintId::of(matches::MATCH_SINGLE_BINDING), + LintId::of(matches::NOP_MATCH), LintId::of(matches::REDUNDANT_PATTERN_MATCHING), LintId::of(matches::SINGLE_MATCH), LintId::of(matches::WILDCARD_IN_OR_PATTERNS), diff --git a/clippy_lints/src/lib.register_correctness.rs b/clippy_lints/src/lib.register_correctness.rs index 18fe44282ed..191fa96bdfd 100644 --- a/clippy_lints/src/lib.register_correctness.rs +++ b/clippy_lints/src/lib.register_correctness.rs @@ -37,6 +37,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve LintId::of(loops::NEVER_LOOP), LintId::of(loops::WHILE_IMMUTABLE_CONDITION), LintId::of(match_str_case_mismatch::MATCH_STR_CASE_MISMATCH), + LintId::of(matches::NOP_MATCH), LintId::of(mem_replace::MEM_REPLACE_WITH_UNINIT), LintId::of(methods::CLONE_DOUBLE_REF), LintId::of(methods::ITERATOR_STEP_BY_ZERO), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 75ef1b0a9d5..2977a78f257 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -253,6 +253,7 @@ store.register_lints(&[ matches::MATCH_SINGLE_BINDING, matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS, matches::MATCH_WILD_ERR_ARM, + matches::NOP_MATCH, matches::REDUNDANT_PATTERN_MATCHING, matches::REST_PAT_IN_FULLY_BOUND_STRUCTS, matches::SINGLE_MATCH, diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index 92179eb6f0e..48300931dc4 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -21,6 +21,7 @@ mod redundant_pattern_match; mod rest_pat_in_fully_bound_struct; mod single_match; mod wild_in_or_pats; +mod nop_match; declare_clippy_lint! { /// ### What it does @@ -566,6 +567,49 @@ declare_clippy_lint! { "`match` with identical arm bodies" } +declare_clippy_lint! { + /// ### What it does + /// Checks for unnecessary `match` or match-like `if let` returns for `Option` and `Result` + /// when function signatures are the same. + /// + /// ### Why is this bad? + /// This `match` block does nothing and might not be what the coder intended. + /// + /// ### Example + /// ```rust,ignore + /// fn foo() -> Result<(), i32> { + /// match result { + /// Ok(val) => Ok(val), + /// Err(err) => Err(err), + /// } + /// } + /// + /// fn bar() -> Option { + /// if let Some(val) = option { + /// Some(val) + /// } else { + /// None + /// } + /// } + /// ``` + /// + /// Could be replaced as + /// + /// ```rust,ignore + /// fn foo() -> Result<(), i32> { + /// result + /// } + /// + /// fn bar() -> Option { + /// option + /// } + /// ``` + #[clippy::version = "1.61.0"] + pub NOP_MATCH, + correctness, + "`match` or match-like `if let` that are unnecessary" +} + #[derive(Default)] pub struct Matches { msrv: Option, @@ -599,6 +643,7 @@ impl_lint_pass!(Matches => [ REDUNDANT_PATTERN_MATCHING, MATCH_LIKE_MATCHES_MACRO, MATCH_SAME_ARMS, + NOP_MATCH, ]); impl<'tcx> LateLintPass<'tcx> for Matches { @@ -622,6 +667,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { overlapping_arms::check(cx, ex, arms); match_wild_enum::check(cx, ex, arms); match_as_ref::check(cx, ex, arms, expr); + nop_match::check_match(cx, ex, arms); if self.infallible_destructuring_match_linted { self.infallible_destructuring_match_linted = false; @@ -640,6 +686,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { match_like_matches::check(cx, expr); } redundant_pattern_match::check(cx, expr); + nop_match::check(cx, expr); } } diff --git a/clippy_lints/src/matches/nop_match.rs b/clippy_lints/src/matches/nop_match.rs new file mode 100644 index 00000000000..d995caf6cfd --- /dev/null +++ b/clippy_lints/src/matches/nop_match.rs @@ -0,0 +1,34 @@ +#![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; + +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, + ); + } +} + +pub(crate) fn check_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) { + if false { + span_lint_and_sugg( + cx, + NOP_MATCH, + ex.span, + "this match expression is unnecessary", + "replace it with", + "".to_string(), + Applicability::MachineApplicable, + ); + } +} \ No newline at end of file diff --git a/tests/ui/nop_match.fixed b/tests/ui/nop_match.fixed new file mode 100644 index 00000000000..0e37bb85439 --- /dev/null +++ b/tests/ui/nop_match.fixed @@ -0,0 +1,83 @@ +// run-rustfix +#![warn(clippy::nop_match)] +#![allow(clippy::manual_map)] +#![allow(clippy::question_mark)] +#![allow(dead_code)] + +fn option_match() -> Option { + match Some(1) { + Some(a) => Some(a), + None => None + } +} + +fn result_match() -> Result { + match Ok(1) { + Ok(a) => Ok(a), + Err(err) => Err(err) + } +} + +fn option_check() -> Option { + if let Some(a) = Some(1) { + Some(a) + } else { + None + } +} + +fn option_check_no_else() -> Option { + if let Some(a) = Some(1) { + return Some(a); + } + None +} + +fn func_ret_err(err: T) -> Result<(), T> { + Err(err) +} + +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 main() { } \ No newline at end of file diff --git a/tests/ui/nop_match.rs b/tests/ui/nop_match.rs new file mode 100644 index 00000000000..0e37bb85439 --- /dev/null +++ b/tests/ui/nop_match.rs @@ -0,0 +1,83 @@ +// run-rustfix +#![warn(clippy::nop_match)] +#![allow(clippy::manual_map)] +#![allow(clippy::question_mark)] +#![allow(dead_code)] + +fn option_match() -> Option { + match Some(1) { + Some(a) => Some(a), + None => None + } +} + +fn result_match() -> Result { + match Ok(1) { + Ok(a) => Ok(a), + Err(err) => Err(err) + } +} + +fn option_check() -> Option { + if let Some(a) = Some(1) { + Some(a) + } else { + None + } +} + +fn option_check_no_else() -> Option { + if let Some(a) = Some(1) { + return Some(a); + } + None +} + +fn func_ret_err(err: T) -> Result<(), T> { + Err(err) +} + +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 main() { } \ No newline at end of file diff --git a/tests/ui/nop_match.stderr b/tests/ui/nop_match.stderr new file mode 100644 index 00000000000..e69de29bb2d