Skip to content

Commit d10171f

Browse files
andrewpollackandrewpollack
committed
PR followups to make match_result_ok
1 parent 475dbeb commit d10171f

10 files changed

+183
-213
lines changed

‎CHANGELOG.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ document.
88

99
[74d1561...master](https://github.com/rust-lang/rust-clippy/compare/74d1561...master)
1010

11+
### New Lints
12+
13+
* Renamed Lint: `if_let_some_result` is now called [`match_result_ok`].
14+
1115
## Rust 1.55
1216

1317
Current beta, release 2021-09-09
@@ -2684,7 +2688,6 @@ Released 2018-09-13
26842688
[`identity_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_op
26852689
[`if_let_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_mutex
26862690
[`if_let_redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_redundant_pattern_matching
2687-
[`if_let_some_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_some_result
26882691
[`if_not_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else
26892692
[`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else
26902693
[`if_then_some_else_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none
@@ -2773,6 +2776,7 @@ Released 2018-09-13
27732776
[`match_on_vec_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_on_vec_items
27742777
[`match_overlapping_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_overlapping_arm
27752778
[`match_ref_pats`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_ref_pats
2779+
[`match_result_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_result_ok
27762780
[`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms
27772781
[`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
27782782
[`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm
@@ -3026,7 +3030,6 @@ Released 2018-09-13
30263030
[`while_immutable_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_immutable_condition
30273031
[`while_let_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_loop
30283032
[`while_let_on_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_on_iterator
3029-
[`while_let_some_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_some_result
30303033
[`wildcard_dependencies`]: https://rust-lang.github.io/rust-clippy/master/index.html#wildcard_dependencies
30313034
[`wildcard_enum_match_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#wildcard_enum_match_arm
30323035
[`wildcard_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#wildcard_imports

‎clippy_lints/src/lib.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,7 @@ mod map_clone;
262262
mod map_err_ignore;
263263
mod map_unit_fn;
264264
mod match_on_vec_items;
265+
mod match_result_ok;
265266
mod matches;
266267
mod mem_discriminant;
267268
mod mem_forget;
@@ -290,7 +291,6 @@ mod needless_borrow;
290291
mod needless_borrowed_ref;
291292
mod needless_continue;
292293
mod needless_for_each;
293-
mod needless_ok;
294294
mod needless_pass_by_value;
295295
mod needless_question_mark;
296296
mod needless_update;
@@ -722,6 +722,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
722722
map_unit_fn::OPTION_MAP_UNIT_FN,
723723
map_unit_fn::RESULT_MAP_UNIT_FN,
724724
match_on_vec_items::MATCH_ON_VEC_ITEMS,
725+
match_result_ok::MATCH_RESULT_OK,
725726
matches::INFALLIBLE_DESTRUCTURING_MATCH,
726727
matches::MATCH_AS_REF,
727728
matches::MATCH_BOOL,
@@ -849,8 +850,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
849850
needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE,
850851
needless_continue::NEEDLESS_CONTINUE,
851852
needless_for_each::NEEDLESS_FOR_EACH,
852-
needless_ok::IF_LET_SOME_RESULT,
853-
needless_ok::WHILE_LET_SOME_RESULT,
854853
needless_pass_by_value::NEEDLESS_PASS_BY_VALUE,
855854
needless_question_mark::NEEDLESS_QUESTION_MARK,
856855
needless_update::NEEDLESS_UPDATE,
@@ -1291,6 +1290,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12911290
LintId::of(map_clone::MAP_CLONE),
12921291
LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN),
12931292
LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN),
1293+
LintId::of(match_result_ok::MATCH_RESULT_OK),
12941294
LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH),
12951295
LintId::of(matches::MATCH_AS_REF),
12961296
LintId::of(matches::MATCH_LIKE_MATCHES_MACRO),
@@ -1378,8 +1378,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13781378
LintId::of(needless_bool::NEEDLESS_BOOL),
13791379
LintId::of(needless_borrow::NEEDLESS_BORROW),
13801380
LintId::of(needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
1381-
LintId::of(needless_ok::IF_LET_SOME_RESULT),
1382-
LintId::of(needless_ok::WHILE_LET_SOME_RESULT),
13831381
LintId::of(needless_question_mark::NEEDLESS_QUESTION_MARK),
13841382
LintId::of(needless_update::NEEDLESS_UPDATE),
13851383
LintId::of(neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
@@ -1519,6 +1517,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15191517
LintId::of(manual_map::MANUAL_MAP),
15201518
LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
15211519
LintId::of(map_clone::MAP_CLONE),
1520+
LintId::of(match_result_ok::MATCH_RESULT_OK),
15221521
LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH),
15231522
LintId::of(matches::MATCH_LIKE_MATCHES_MACRO),
15241523
LintId::of(matches::MATCH_OVERLAPPING_ARM),
@@ -1558,8 +1557,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15581557
LintId::of(mut_mutex_lock::MUT_MUTEX_LOCK),
15591558
LintId::of(mut_reference::UNNECESSARY_MUT_PASSED),
15601559
LintId::of(needless_borrow::NEEDLESS_BORROW),
1561-
LintId::of(needless_ok::IF_LET_SOME_RESULT),
1562-
LintId::of(needless_ok::WHILE_LET_SOME_RESULT),
15631560
LintId::of(neg_multiply::NEG_MULTIPLY),
15641561
LintId::of(new_without_default::NEW_WITHOUT_DEFAULT),
15651562
LintId::of(non_copy_const::BORROW_INTERIOR_MUTABLE_CONST),
@@ -1974,7 +1971,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
19741971
store.register_late_pass(|| box missing_doc::MissingDoc::new());
19751972
store.register_late_pass(|| box missing_inline::MissingInline);
19761973
store.register_late_pass(move || box exhaustive_items::ExhaustiveItems);
1977-
store.register_late_pass(|| box needless_ok::NeedlessOk);
1974+
store.register_late_pass(|| box match_result_ok::MatchResultOk);
19781975
store.register_late_pass(|| box partialeq_ne_impl::PartialEqNeImpl);
19791976
store.register_late_pass(|| box unused_io_amount::UnusedIoAmount);
19801977
let enum_variant_size_threshold = conf.enum_variant_size_threshold;

‎clippy_lints/src/match_result_ok.rs

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::method_chain_args;
3+
use clippy_utils::source::snippet_with_applicability;
4+
use clippy_utils::ty::is_type_diagnostic_item;
5+
use if_chain::if_chain;
6+
use rustc_errors::Applicability;
7+
use rustc_hir::{Expr, ExprKind, MatchSource, PatKind, QPath};
8+
use rustc_lint::{LateContext, LateLintPass};
9+
use rustc_session::{declare_lint_pass, declare_tool_lint};
10+
use rustc_span::sym;
11+
12+
declare_clippy_lint! {
13+
/// ### What it does
14+
/// Checks for unnecessary `ok()` in `while let`.
15+
///
16+
/// ### Why is this bad?
17+
/// Calling `ok()` in `while let` is unnecessary, instead match
18+
/// on `Ok(pat)`
19+
///
20+
/// ### Example
21+
/// ```ignore
22+
/// while let Some(value) = iter.next().ok() {
23+
/// vec.push(value)
24+
/// }
25+
///
26+
/// if let Some(valie) = iter.next().ok() {
27+
/// vec.push(value)
28+
/// }
29+
/// ```
30+
/// Use instead:
31+
/// ```ignore
32+
/// while let Ok(value) = iter.next() {
33+
/// vec.push(value)
34+
/// }
35+
///
36+
/// if let Ok(value) = iter.next() {
37+
/// vec.push_value)
38+
/// }
39+
/// ```
40+
pub MATCH_RESULT_OK,
41+
style,
42+
"usage of `ok()` in `let Some(pat)` statements is unnecessary, match on `Ok(pat)` instead"
43+
}
44+
45+
declare_lint_pass!(MatchResultOk => [MATCH_RESULT_OK]);
46+
47+
impl<'tcx> LateLintPass<'tcx> for MatchResultOk {
48+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
49+
if_chain! {
50+
if let ExprKind::Match(op, body, MatchSource::IfLetDesugar { .. }
51+
| MatchSource::WhileLetDesugar { .. }) = expr.kind;
52+
if let ExprKind::MethodCall(_, ok_span, result_types, _) = op.kind;
53+
if let PatKind::TupleStruct(QPath::Resolved(_, x), y, _) = body[0].pat.kind; // get operation
54+
if method_chain_args(op, &["ok"]).is_some(); // test to see if using ok() methoduse std::marker::Sized;
55+
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&result_types[0]), sym::result_type);
56+
if rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_path(x, false)) == "Some";
57+
58+
then {
59+
60+
let mut applicability = Applicability::MachineApplicable;
61+
let some_expr_string = snippet_with_applicability(cx, y[0].span, "", &mut applicability);
62+
let trimmed_ok = snippet_with_applicability(cx, op.span.until(ok_span), "", &mut applicability);
63+
64+
if let ExprKind::Match(op, _, MatchSource::IfLetDesugar { .. }) = expr.kind {
65+
let sugg = format!(
66+
"if let Ok({}) = {}",
67+
some_expr_string,
68+
trimmed_ok.trim().trim_end_matches('.'),
69+
);
70+
span_lint_and_sugg(
71+
cx,
72+
MATCH_RESULT_OK,
73+
expr.span.with_hi(op.span.hi()),
74+
"matching on `Some` with `ok()` is redundant",
75+
&format!("consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string),
76+
sugg,
77+
applicability,
78+
);
79+
} else if let ExprKind::Match(op, _, MatchSource::WhileLetDesugar { .. }) = expr.kind {
80+
let sugg = format!(
81+
"while let Ok({}) = {}",
82+
some_expr_string,
83+
trimmed_ok.trim().trim_end_matches('.'),
84+
);
85+
span_lint_and_sugg(
86+
cx,
87+
MATCH_RESULT_OK,
88+
expr.span.with_hi(op.span.hi()),
89+
"matching on `Some` with `ok()` is redundant",
90+
&format!("consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string),
91+
sugg,
92+
applicability,
93+
);
94+
}
95+
}
96+
}
97+
}
98+
}

‎clippy_lints/src/needless_ok.rs

Lines changed: 0 additions & 129 deletions
This file was deleted.

‎tests/ui/if_let_some_result.fixed

Lines changed: 0 additions & 27 deletions
This file was deleted.

‎tests/ui/if_let_some_result.rs

Lines changed: 0 additions & 27 deletions
This file was deleted.

0 commit comments

Comments
 (0)