Skip to content

Commit b7d40bc

Browse files
andrewpollackManishearth
authored andcommitted
Adding new linting
1 parent 0c8799d commit b7d40bc

File tree

9 files changed

+261
-142
lines changed

9 files changed

+261
-142
lines changed

CHANGELOG.md

Lines changed: 5 additions & 1 deletion
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
@@ -2685,7 +2689,6 @@ Released 2018-09-13
26852689
[`identity_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_op
26862690
[`if_let_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_mutex
26872691
[`if_let_redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_redundant_pattern_matching
2688-
[`if_let_some_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_some_result
26892692
[`if_not_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else
26902693
[`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else
26912694
[`if_then_panic`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_panic
@@ -2775,6 +2778,7 @@ Released 2018-09-13
27752778
[`match_on_vec_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_on_vec_items
27762779
[`match_overlapping_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_overlapping_arm
27772780
[`match_ref_pats`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_ref_pats
2781+
[`match_result_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_result_ok
27782782
[`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms
27792783
[`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
27802784
[`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm

clippy_lints/src/if_let_some_result.rs

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

clippy_lints/src/lib.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,6 @@ mod future_not_send;
225225
mod get_last_with_len;
226226
mod identity_op;
227227
mod if_let_mutex;
228-
mod if_let_some_result;
229228
mod if_not_else;
230229
mod if_then_panic;
231230
mod if_then_some_else_none;
@@ -264,6 +263,7 @@ mod map_clone;
264263
mod map_err_ignore;
265264
mod map_unit_fn;
266265
mod match_on_vec_items;
266+
mod match_result_ok;
267267
mod matches;
268268
mod mem_discriminant;
269269
mod mem_forget;
@@ -658,7 +658,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
658658
get_last_with_len::GET_LAST_WITH_LEN,
659659
identity_op::IDENTITY_OP,
660660
if_let_mutex::IF_LET_MUTEX,
661-
if_let_some_result::IF_LET_SOME_RESULT,
662661
if_not_else::IF_NOT_ELSE,
663662
if_then_panic::IF_THEN_PANIC,
664663
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:
728727
map_unit_fn::OPTION_MAP_UNIT_FN,
729728
map_unit_fn::RESULT_MAP_UNIT_FN,
730729
match_on_vec_items::MATCH_ON_VEC_ITEMS,
730+
match_result_ok::MATCH_RESULT_OK,
731731
matches::INFALLIBLE_DESTRUCTURING_MATCH,
732732
matches::MATCH_AS_REF,
733733
matches::MATCH_BOOL,
@@ -1259,7 +1259,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12591259
LintId::of(get_last_with_len::GET_LAST_WITH_LEN),
12601260
LintId::of(identity_op::IDENTITY_OP),
12611261
LintId::of(if_let_mutex::IF_LET_MUTEX),
1262-
LintId::of(if_let_some_result::IF_LET_SOME_RESULT),
12631262
LintId::of(if_then_panic::IF_THEN_PANIC),
12641263
LintId::of(indexing_slicing::OUT_OF_BOUNDS_INDEXING),
12651264
LintId::of(infinite_iter::INFINITE_ITER),
@@ -1303,6 +1302,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13031302
LintId::of(map_clone::MAP_CLONE),
13041303
LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN),
13051304
LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN),
1305+
LintId::of(match_result_ok::MATCH_RESULT_OK),
13061306
LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH),
13071307
LintId::of(matches::MATCH_AS_REF),
13081308
LintId::of(matches::MATCH_LIKE_MATCHES_MACRO),
@@ -1513,7 +1513,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15131513
LintId::of(functions::DOUBLE_MUST_USE),
15141514
LintId::of(functions::MUST_USE_UNIT),
15151515
LintId::of(functions::RESULT_UNIT_ERR),
1516-
LintId::of(if_let_some_result::IF_LET_SOME_RESULT),
15171516
LintId::of(if_then_panic::IF_THEN_PANIC),
15181517
LintId::of(inherent_to_string::INHERENT_TO_STRING),
15191518
LintId::of(len_zero::COMPARISON_TO_EMPTY),
@@ -1530,6 +1529,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15301529
LintId::of(manual_map::MANUAL_MAP),
15311530
LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
15321531
LintId::of(map_clone::MAP_CLONE),
1532+
LintId::of(match_result_ok::MATCH_RESULT_OK),
15331533
LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH),
15341534
LintId::of(matches::MATCH_LIKE_MATCHES_MACRO),
15351535
LintId::of(matches::MATCH_OVERLAPPING_ARM),
@@ -1985,7 +1985,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
19851985
store.register_late_pass(|| Box::new(missing_doc::MissingDoc::new()));
19861986
store.register_late_pass(|| Box::new(missing_inline::MissingInline));
19871987
store.register_late_pass(move || Box::new(exhaustive_items::ExhaustiveItems));
1988-
store.register_late_pass(|| Box::new(if_let_some_result::OkIfLet));
1988+
store.register_late_pass(|| Box::new(match_result_ok::MatchResultOk));
19891989
store.register_late_pass(|| Box::new(partialeq_ne_impl::PartialEqNeImpl));
19901990
store.register_late_pass(|| Box::new(unused_io_amount::UnusedIoAmount));
19911991
let enum_variant_size_threshold = conf.enum_variant_size_threshold;

clippy_lints/src/match_result_ok.rs

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

tests/ui/if_let_some_result.fixed

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

tests/ui/if_let_some_result.rs

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

0 commit comments

Comments
 (0)