Skip to content

Commit 93f2de3

Browse files
sjwang05sjwang05
committed
Add manual_option_folding lint
1 parent d38fa1a commit 93f2de3

File tree

6 files changed

+202
-0
lines changed

6 files changed

+202
-0
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5085,6 +5085,7 @@ Released 2018-09-13
50855085
[`manual_next_back`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_next_back
50865086
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
50875087
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
5088+
[`manual_option_folding`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_option_folding
50885089
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
50895090
[`manual_range_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_patterns
50905091
[`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid

‎clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
286286
crate::manual_let_else::MANUAL_LET_ELSE_INFO,
287287
crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO,
288288
crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,
289+
crate::manual_option_folding::MANUAL_OPTION_FOLDING_INFO,
289290
crate::manual_range_patterns::MANUAL_RANGE_PATTERNS_INFO,
290291
crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO,
291292
crate::manual_retain::MANUAL_RETAIN_INFO,

‎clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ mod manual_is_ascii_check;
196196
mod manual_let_else;
197197
mod manual_main_separator_str;
198198
mod manual_non_exhaustive;
199+
mod manual_option_folding;
199200
mod manual_range_patterns;
200201
mod manual_rem_euclid;
201202
mod manual_retain;
@@ -1123,6 +1124,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
11231124
});
11241125
store.register_late_pass(move |_| Box::new(manual_hash_one::ManualHashOne::new(msrv())));
11251126
store.register_late_pass(|_| Box::new(iter_without_into_iter::IterWithoutIntoIter));
1127+
store.register_late_pass(|_| Box::new(manual_option_folding::ManualOptionFolding::new()));
11261128
// add lints here, do not remove this comment, it's used in `new_lint`
11271129
}
11281130

‎clippy_lints/src/manual_option_folding.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_then;
2+
use clippy_utils::ty::is_type_diagnostic_item;
3+
use rustc_hir as hir;
4+
use rustc_lint::{LateContext, LateLintPass};
5+
use rustc_session::{declare_tool_lint, impl_lint_pass};
6+
use rustc_span::{sym, Symbol};
7+
8+
declare_clippy_lint! {
9+
/// ### What it does
10+
/// Checks for cases where `unwrap_or_else` can be used in lieu of
11+
/// `get_or_insert_with` followed by `unwrap`/`unwrap_unchecked`/`expect`.
12+
///
13+
/// ### Why is this bad?
14+
/// It is more concise to use `unwrap_or_else`, and using `unwrap_or_else`
15+
/// instead of `unwrap_unchecked` eliminates the need for an `unsafe`
16+
/// block.
17+
///
18+
/// ### Example
19+
/// ```rust
20+
/// let mut opt: Option<i32> = None;
21+
/// opt.get_or_insert(42);
22+
/// let res = unsafe { opt.unwrap_unchecked() };
23+
/// ```
24+
/// Use instead:
25+
/// ```rust
26+
/// let opt: Option<i32> = None;
27+
/// let res: i32 = opt.unwrap_or(42);
28+
/// ```
29+
#[clippy::version = "1.74.0"]
30+
pub MANUAL_OPTION_FOLDING,
31+
style,
32+
"manual implementation of `Option::unwrap_or_else`"
33+
}
34+
35+
impl_lint_pass!(ManualOptionFolding<'_> => [MANUAL_OPTION_FOLDING]);
36+
37+
pub struct ManualOptionFolding<'tcx> {
38+
get_call: Option<&'tcx hir::Expr<'tcx>>,
39+
recv: Option<&'tcx hir::Expr<'tcx>>,
40+
get_method_name: Option<Symbol>,
41+
}
42+
43+
impl<'tcx> ManualOptionFolding<'tcx> {
44+
pub fn new() -> Self {
45+
Self {
46+
get_call: None,
47+
recv: None,
48+
get_method_name: None,
49+
}
50+
}
51+
}
52+
53+
impl<'tcx> LateLintPass<'tcx> for ManualOptionFolding<'tcx> {
54+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
55+
if !expr.span.from_expansion()
56+
&& let hir::ExprKind::MethodCall(path, recv, ..) = expr.kind
57+
&& let ty = cx.typeck_results().expr_ty(recv).peel_refs()
58+
&& is_type_diagnostic_item(cx, ty, sym::Option)
59+
{
60+
if path.ident.name == sym!(get_or_insert)
61+
|| path.ident.name == sym!(get_or_insert_with)
62+
|| path.ident.name == sym!(get_or_insert_default)
63+
{
64+
self.get_call = Some(expr);
65+
self.recv = Some(recv);
66+
self.get_method_name = Some(path.ident.name);
67+
} else if let Some(get_call) = self.get_call
68+
&& let Some(get_call_recv) = self.recv
69+
&& let Some(get_method_name) = self.get_method_name
70+
&& (path.ident.name == sym::unwrap
71+
|| path.ident.name == sym!(unwrap_unchecked)
72+
|| path.ident.name == sym::expect)
73+
&& let hir::ExprKind::Path(hir::QPath::Resolved(_, recv_path)) = recv.kind
74+
&& let hir::ExprKind::Path(hir::QPath::Resolved(_, get_call_recv_path)) = get_call_recv.kind
75+
&& recv_path.res == get_call_recv_path.res
76+
{
77+
let sugg_method = if get_method_name == sym!(get_or_insert) {
78+
"unwrap_or".to_string()
79+
} else if get_method_name == sym!(get_or_insert_with) {
80+
"unwrap_or_else".to_string()
81+
} else {
82+
"unwrap_or_default".to_string()
83+
};
84+
85+
span_lint_and_then(
86+
cx,
87+
MANUAL_OPTION_FOLDING,
88+
expr.span,
89+
&format!("`{}` used after `{get_method_name}`", path.ident.name),
90+
|diag| {
91+
diag.span_note(get_call.span, format!("`{get_method_name}` used here"));
92+
diag.help(format!("try using `{sugg_method}` instead"));
93+
}
94+
);
95+
}
96+
}
97+
}
98+
}

‎tests/ui/manual_option_folding.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#![feature(option_get_or_insert_default)]
2+
#![allow(dead_code, clippy::unnecessary_lazy_evaluations, clippy::unnecessary_literal_unwrap)]
3+
#![warn(clippy::manual_option_folding)]
4+
5+
fn main() {
6+
let mut opt: Option<i32> = Some(42);
7+
opt.get_or_insert_with(|| 21);
8+
9+
let opt: Option<i32> = Some(42);
10+
opt.unwrap();
11+
12+
let mut opt: Option<i32> = Some(42);
13+
opt.get_or_insert_with(|| 21);
14+
let _res: i32 = unsafe { opt.unwrap_unchecked() };
15+
16+
let mut opt: Option<i32> = Some(42);
17+
opt.get_or_insert_with(|| 21);
18+
let _res: i32 = opt.unwrap();
19+
20+
let mut opt: Option<i32> = Some(42);
21+
opt.get_or_insert_with(|| 21);
22+
let _res: i32 = opt.expect("msg");
23+
24+
let mut opt: Option<i32> = Some(42);
25+
opt.get_or_insert(21);
26+
let _res: i32 = opt.unwrap();
27+
28+
let mut opt: Option<i32> = Some(42);
29+
opt.get_or_insert_default();
30+
let _res: i32 = opt.unwrap();
31+
}

‎tests/ui/manual_option_folding.stderr

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
error: `unwrap_unchecked` used after `get_or_insert_with`
2+
--> $DIR/manual_option_folding.rs:14:30
3+
|
4+
LL | let _res: i32 = unsafe { opt.unwrap_unchecked() };
5+
| ^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
note: `get_or_insert_with` used here
8+
--> $DIR/manual_option_folding.rs:13:5
9+
|
10+
LL | opt.get_or_insert_with(|| 21);
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
= help: try using `unwrap_or_else` instead
13+
= note: `-D clippy::manual-option-folding` implied by `-D warnings`
14+
= help: to override `-D warnings` add `#[allow(clippy::manual_option_folding)]`
15+
16+
error: `unwrap` used after `get_or_insert_with`
17+
--> $DIR/manual_option_folding.rs:18:21
18+
|
19+
LL | let _res: i32 = opt.unwrap();
20+
| ^^^^^^^^^^^^
21+
|
22+
note: `get_or_insert_with` used here
23+
--> $DIR/manual_option_folding.rs:17:5
24+
|
25+
LL | opt.get_or_insert_with(|| 21);
26+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
27+
= help: try using `unwrap_or_else` instead
28+
29+
error: `expect` used after `get_or_insert_with`
30+
--> $DIR/manual_option_folding.rs:22:21
31+
|
32+
LL | let _res: i32 = opt.expect("msg");
33+
| ^^^^^^^^^^^^^^^^^
34+
|
35+
note: `get_or_insert_with` used here
36+
--> $DIR/manual_option_folding.rs:21:5
37+
|
38+
LL | opt.get_or_insert_with(|| 21);
39+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
40+
= help: try using `unwrap_or_else` instead
41+
42+
error: `unwrap` used after `get_or_insert`
43+
--> $DIR/manual_option_folding.rs:26:21
44+
|
45+
LL | let _res: i32 = opt.unwrap();
46+
| ^^^^^^^^^^^^
47+
|
48+
note: `get_or_insert` used here
49+
--> $DIR/manual_option_folding.rs:25:5
50+
|
51+
LL | opt.get_or_insert(21);
52+
| ^^^^^^^^^^^^^^^^^^^^^
53+
= help: try using `unwrap_or` instead
54+
55+
error: `unwrap` used after `get_or_insert_default`
56+
--> $DIR/manual_option_folding.rs:30:21
57+
|
58+
LL | let _res: i32 = opt.unwrap();
59+
| ^^^^^^^^^^^^
60+
|
61+
note: `get_or_insert_default` used here
62+
--> $DIR/manual_option_folding.rs:29:5
63+
|
64+
LL | opt.get_or_insert_default();
65+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
66+
= help: try using `unwrap_or_default` instead
67+
68+
error: aborting due to 5 previous errors
69+

0 commit comments

Comments
 (0)