diff options
| -rw-r--r-- | CHANGELOG.md | 8 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 11 | ||||
| -rw-r--r-- | clippy_lints/src/match_result_ok.rs (renamed from clippy_lints/src/if_let_some_result.rs) | 53 | ||||
| -rw-r--r-- | tests/ui/if_let_some_result.fixed | 28 | ||||
| -rw-r--r-- | tests/ui/if_let_some_result.rs | 28 | ||||
| -rw-r--r-- | tests/ui/match_result_ok.fixed | 63 | ||||
| -rw-r--r-- | tests/ui/match_result_ok.rs | 63 | ||||
| -rw-r--r-- | tests/ui/match_result_ok.stderr (renamed from tests/ui/if_let_some_result.stderr) | 19 |
8 files changed, 186 insertions, 87 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 040c906a722..def3e741818 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ document. [74d1561...master](https://github.com/rust-lang/rust-clippy/compare/74d1561...master) +### New Lints + +* Renamed Lint: `if_let_some_result` is now called [`match_result_ok`]. Now also handles `while let` case. + ## Rust 1.55 Current beta, release 2021-09-09 @@ -1491,7 +1495,7 @@ Released 2020-03-12 * `unknown_clippy_lints` [#4963](https://github.com/rust-lang/rust-clippy/pull/4963) * [`explicit_into_iter_loop`] [#4978](https://github.com/rust-lang/rust-clippy/pull/4978) * [`useless_attribute`] [#5022](https://github.com/rust-lang/rust-clippy/pull/5022) -* [`if_let_some_result`] [#5032](https://github.com/rust-lang/rust-clippy/pull/5032) +* `if_let_some_result` [#5032](https://github.com/rust-lang/rust-clippy/pull/5032) ### ICE fixes @@ -2685,7 +2689,6 @@ Released 2018-09-13 [`identity_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_op [`if_let_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_mutex [`if_let_redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_redundant_pattern_matching -[`if_let_some_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_some_result [`if_not_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else [`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else [`if_then_panic`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_panic @@ -2775,6 +2778,7 @@ Released 2018-09-13 [`match_on_vec_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_on_vec_items [`match_overlapping_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_overlapping_arm [`match_ref_pats`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_ref_pats +[`match_result_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_result_ok [`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms [`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding [`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1d24d02511b..92ebebd62bf 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -225,7 +225,6 @@ mod future_not_send; mod get_last_with_len; mod identity_op; mod if_let_mutex; -mod if_let_some_result; mod if_not_else; mod if_then_panic; mod if_then_some_else_none; @@ -264,6 +263,7 @@ mod map_clone; mod map_err_ignore; mod map_unit_fn; mod match_on_vec_items; +mod match_result_ok; mod matches; mod mem_discriminant; mod mem_forget; @@ -658,7 +658,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: get_last_with_len::GET_LAST_WITH_LEN, identity_op::IDENTITY_OP, if_let_mutex::IF_LET_MUTEX, - if_let_some_result::IF_LET_SOME_RESULT, if_not_else::IF_NOT_ELSE, if_then_panic::IF_THEN_PANIC, if_then_some_else_none::IF_THEN_SOME_ELSE_NONE, @@ -728,6 +727,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: map_unit_fn::OPTION_MAP_UNIT_FN, map_unit_fn::RESULT_MAP_UNIT_FN, match_on_vec_items::MATCH_ON_VEC_ITEMS, + match_result_ok::MATCH_RESULT_OK, matches::INFALLIBLE_DESTRUCTURING_MATCH, matches::MATCH_AS_REF, matches::MATCH_BOOL, @@ -1259,7 +1259,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(get_last_with_len::GET_LAST_WITH_LEN), LintId::of(identity_op::IDENTITY_OP), LintId::of(if_let_mutex::IF_LET_MUTEX), - LintId::of(if_let_some_result::IF_LET_SOME_RESULT), LintId::of(if_then_panic::IF_THEN_PANIC), LintId::of(indexing_slicing::OUT_OF_BOUNDS_INDEXING), LintId::of(infinite_iter::INFINITE_ITER), @@ -1303,6 +1302,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(map_clone::MAP_CLONE), LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN), + LintId::of(match_result_ok::MATCH_RESULT_OK), LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(matches::MATCH_AS_REF), LintId::of(matches::MATCH_LIKE_MATCHES_MACRO), @@ -1513,7 +1513,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(functions::DOUBLE_MUST_USE), LintId::of(functions::MUST_USE_UNIT), LintId::of(functions::RESULT_UNIT_ERR), - LintId::of(if_let_some_result::IF_LET_SOME_RESULT), LintId::of(if_then_panic::IF_THEN_PANIC), LintId::of(inherent_to_string::INHERENT_TO_STRING), LintId::of(len_zero::COMPARISON_TO_EMPTY), @@ -1530,6 +1529,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(manual_map::MANUAL_MAP), LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE), LintId::of(map_clone::MAP_CLONE), + LintId::of(match_result_ok::MATCH_RESULT_OK), LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(matches::MATCH_LIKE_MATCHES_MACRO), LintId::of(matches::MATCH_OVERLAPPING_ARM), @@ -1985,7 +1985,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(missing_doc::MissingDoc::new())); store.register_late_pass(|| Box::new(missing_inline::MissingInline)); store.register_late_pass(move || Box::new(exhaustive_items::ExhaustiveItems)); - store.register_late_pass(|| Box::new(if_let_some_result::OkIfLet)); + store.register_late_pass(|| Box::new(match_result_ok::MatchResultOk)); store.register_late_pass(|| Box::new(partialeq_ne_impl::PartialEqNeImpl)); store.register_late_pass(|| Box::new(unused_io_amount::UnusedIoAmount)); let enum_variant_size_threshold = conf.enum_variant_size_threshold; @@ -2212,6 +2212,7 @@ pub fn register_renamed(ls: &mut rustc_lint::LintStore) { ls.register_renamed("clippy::identity_conversion", "clippy::useless_conversion"); ls.register_renamed("clippy::zero_width_space", "clippy::invisible_characters"); ls.register_renamed("clippy::single_char_push_str", "clippy::single_char_add_str"); + ls.register_renamed("clippy::if_let_some_result", "clippy::match_result_ok"); // uplifted lints ls.register_renamed("clippy::invalid_ref", "invalid_value"); diff --git a/clippy_lints/src/if_let_some_result.rs b/clippy_lints/src/match_result_ok.rs index adcd78ed0d4..c7de06f0815 100644 --- a/clippy_lints/src/if_let_some_result.rs +++ b/clippy_lints/src/match_result_ok.rs @@ -12,40 +12,51 @@ use rustc_span::sym; declare_clippy_lint! { /// ### What it does - ///* Checks for unnecessary `ok()` in if let. + /// Checks for unnecessary `ok()` in `while let`. /// /// ### Why is this bad? - /// Calling `ok()` in if let is unnecessary, instead match + /// Calling `ok()` in `while let` is unnecessary, instead match /// on `Ok(pat)` /// /// ### Example /// ```ignore - /// for i in iter { - /// if let Some(value) = i.parse().ok() { - /// vec.push(value) - /// } + /// while let Some(value) = iter.next().ok() { + /// vec.push(value) /// } - /// ``` - /// Could be written: /// + /// if let Some(valie) = iter.next().ok() { + /// vec.push(value) + /// } + /// ``` + /// Use instead: /// ```ignore - /// for i in iter { - /// if let Ok(value) = i.parse() { - /// vec.push(value) - /// } + /// while let Ok(value) = iter.next() { + /// vec.push(value) + /// } + /// + /// if let Ok(value) = iter.next() { + /// vec.push_value) /// } /// ``` - pub IF_LET_SOME_RESULT, + pub MATCH_RESULT_OK, style, - "usage of `ok()` in `if let Some(pat)` statements is unnecessary, match on `Ok(pat)` instead" + "usage of `ok()` in `let Some(pat)` statements is unnecessary, match on `Ok(pat)` instead" } -declare_lint_pass!(OkIfLet => [IF_LET_SOME_RESULT]); +declare_lint_pass!(MatchResultOk => [MATCH_RESULT_OK]); -impl<'tcx> LateLintPass<'tcx> for OkIfLet { +impl<'tcx> LateLintPass<'tcx> for MatchResultOk { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if_chain! { //begin checking variables - if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(cx, expr); + let (let_pat, let_expr, ifwhile) = + if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(cx, expr) { + (let_pat, let_expr, "if") + } else if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) { + (let_pat, let_expr, "while") + } else { + return; + }; + + if_chain! { if let ExprKind::MethodCall(_, ok_span, [ref result_types_0, ..], _) = let_expr.kind; //check is expr.ok() has type Result<T,E>.ok(, _) if let PatKind::TupleStruct(QPath::Resolved(_, x), y, _) = let_pat.kind; //get operation if method_chain_args(let_expr, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized; @@ -53,17 +64,19 @@ impl<'tcx> LateLintPass<'tcx> for OkIfLet { if rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_path(x, false)) == "Some"; then { + let mut applicability = Applicability::MachineApplicable; let some_expr_string = snippet_with_applicability(cx, y[0].span, "", &mut applicability); let trimmed_ok = snippet_with_applicability(cx, let_expr.span.until(ok_span), "", &mut applicability); let sugg = format!( - "if let Ok({}) = {}", + "{} let Ok({}) = {}", + ifwhile, some_expr_string, trimmed_ok.trim().trim_end_matches('.'), ); span_lint_and_sugg( cx, - IF_LET_SOME_RESULT, + MATCH_RESULT_OK, expr.span.with_hi(let_expr.span.hi()), "matching on `Some` with `ok()` is redundant", &format!("consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string), diff --git a/tests/ui/if_let_some_result.fixed b/tests/ui/if_let_some_result.fixed deleted file mode 100644 index 1bddc47721e..00000000000 --- a/tests/ui/if_let_some_result.fixed +++ /dev/null @@ -1,28 +0,0 @@ -// run-rustfix - -#![warn(clippy::if_let_some_result)] -#![allow(dead_code)] - -fn str_to_int(x: &str) -> i32 { - if let Ok(y) = x.parse() { y } else { 0 } -} - -fn str_to_int_ok(x: &str) -> i32 { - if let Ok(y) = x.parse() { y } else { 0 } -} - -#[rustfmt::skip] -fn strange_some_no_else(x: &str) -> i32 { - { - if let Ok(y) = x . parse() { - return y; - }; - 0 - } -} - -fn negative() { - while let Some(1) = "".parse().ok() {} -} - -fn main() {} diff --git a/tests/ui/if_let_some_result.rs b/tests/ui/if_let_some_result.rs deleted file mode 100644 index d4a52ec9881..00000000000 --- a/tests/ui/if_let_some_result.rs +++ /dev/null @@ -1,28 +0,0 @@ -// run-rustfix - -#![warn(clippy::if_let_some_result)] -#![allow(dead_code)] - -fn str_to_int(x: &str) -> i32 { - if let Some(y) = x.parse().ok() { y } else { 0 } -} - -fn str_to_int_ok(x: &str) -> i32 { - if let Ok(y) = x.parse() { y } else { 0 } -} - -#[rustfmt::skip] -fn strange_some_no_else(x: &str) -> i32 { - { - if let Some(y) = x . parse() . ok () { - return y; - }; - 0 - } -} - -fn negative() { - while let Some(1) = "".parse().ok() {} -} - -fn main() {} diff --git a/tests/ui/match_result_ok.fixed b/tests/ui/match_result_ok.fixed new file mode 100644 index 00000000000..d4760a97567 --- /dev/null +++ b/tests/ui/match_result_ok.fixed @@ -0,0 +1,63 @@ +// run-rustfix + +#![warn(clippy::match_result_ok)] +#![allow(clippy::boxed_local)] +#![allow(dead_code)] + +// Checking `if` cases + +fn str_to_int(x: &str) -> i32 { + if let Ok(y) = x.parse() { y } else { 0 } +} + +fn str_to_int_ok(x: &str) -> i32 { + if let Ok(y) = x.parse() { y } else { 0 } +} + +#[rustfmt::skip] +fn strange_some_no_else(x: &str) -> i32 { + { + if let Ok(y) = x . parse() { + return y; + }; + 0 + } +} + +// Checking `while` cases + +struct Wat { + counter: i32, +} + +impl Wat { + fn next(&mut self) -> Result<i32, &str> { + self.counter += 1; + if self.counter < 5 { + Ok(self.counter) + } else { + Err("Oh no") + } + } +} + +fn base_1(x: i32) { + let mut wat = Wat { counter: x }; + while let Ok(a) = wat.next() { + println!("{}", a); + } +} + +fn base_2(x: i32) { + let mut wat = Wat { counter: x }; + while let Ok(a) = wat.next() { + println!("{}", a); + } +} + +fn base_3(test_func: Box<Result<i32, &str>>) { + // Expected to stay as is + while let Some(_b) = test_func.ok() {} +} + +fn main() {} diff --git a/tests/ui/match_result_ok.rs b/tests/ui/match_result_ok.rs new file mode 100644 index 00000000000..0b818723d98 --- /dev/null +++ b/tests/ui/match_result_ok.rs @@ -0,0 +1,63 @@ +// run-rustfix + +#![warn(clippy::match_result_ok)] +#![allow(clippy::boxed_local)] +#![allow(dead_code)] + +// Checking `if` cases + +fn str_to_int(x: &str) -> i32 { + if let Some(y) = x.parse().ok() { y } else { 0 } +} + +fn str_to_int_ok(x: &str) -> i32 { + if let Ok(y) = x.parse() { y } else { 0 } +} + +#[rustfmt::skip] +fn strange_some_no_else(x: &str) -> i32 { + { + if let Some(y) = x . parse() . ok () { + return y; + }; + 0 + } +} + +// Checking `while` cases + +struct Wat { + counter: i32, +} + +impl Wat { + fn next(&mut self) -> Result<i32, &str> { + self.counter += 1; + if self.counter < 5 { + Ok(self.counter) + } else { + Err("Oh no") + } + } +} + +fn base_1(x: i32) { + let mut wat = Wat { counter: x }; + while let Some(a) = wat.next().ok() { + println!("{}", a); + } +} + +fn base_2(x: i32) { + let mut wat = Wat { counter: x }; + while let Ok(a) = wat.next() { + println!("{}", a); + } +} + +fn base_3(test_func: Box<Result<i32, &str>>) { + // Expected to stay as is + while let Some(_b) = test_func.ok() {} +} + +fn main() {} diff --git a/tests/ui/if_let_some_result.stderr b/tests/ui/match_result_ok.stderr index bc3a5e7698d..cc3bc8c76ff 100644 --- a/tests/ui/if_let_some_result.stderr +++ b/tests/ui/match_result_ok.stderr @@ -1,17 +1,17 @@ error: matching on `Some` with `ok()` is redundant - --> $DIR/if_let_some_result.rs:7:5 + --> $DIR/match_result_ok.rs:10:5 | LL | if let Some(y) = x.parse().ok() { y } else { 0 } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: `-D clippy::if-let-some-result` implied by `-D warnings` + = note: `-D clippy::match-result-ok` implied by `-D warnings` help: consider matching on `Ok(y)` and removing the call to `ok` instead | LL | if let Ok(y) = x.parse() { y } else { 0 } | ~~~~~~~~~~~~~~~~~~~~~~~~ error: matching on `Some` with `ok()` is redundant - --> $DIR/if_let_some_result.rs:17:9 + --> $DIR/match_result_ok.rs:20:9 | LL | if let Some(y) = x . parse() . ok () { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -21,5 +21,16 @@ help: consider matching on `Ok(y)` and removing the call to `ok` instead LL | if let Ok(y) = x . parse() { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -error: aborting due to 2 previous errors +error: matching on `Some` with `ok()` is redundant + --> $DIR/match_result_ok.rs:46:5 + | +LL | while let Some(a) = wat.next().ok() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider matching on `Ok(a)` and removing the call to `ok` instead + | +LL | while let Ok(a) = wat.next() { + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to 3 previous errors |