Skip to content

Commit 718e6eb

Browse files
aaron-angaaron-ang
committed
feat: new lint for and_then when returning Option or Result
1 parent d1b5fa2 commit 718e6eb

File tree

8 files changed

+356
-5
lines changed

8 files changed

+356
-5
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6024,6 +6024,7 @@ Released 2018-09-13
60246024
[`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else
60256025
[`result_unit_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unit_err
60266026
[`result_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unwrap_used
6027+
[`return_and_then`]: https://rust-lang.github.io/rust-clippy/master/index.html#return_and_then
60276028
[`return_self_not_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#return_self_not_must_use
60286029
[`reverse_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#reverse_range_loop
60296030
[`reversed_empty_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges

‎clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
462462
crate::methods::REPEAT_ONCE_INFO,
463463
crate::methods::RESULT_FILTER_MAP_INFO,
464464
crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO,
465+
crate::methods::RETURN_AND_THEN_INFO,
465466
crate::methods::SEARCH_IS_SOME_INFO,
466467
crate::methods::SEEK_FROM_CURRENT_INFO,
467468
crate::methods::SEEK_TO_START_INSTEAD_OF_REWIND_INFO,

‎clippy_lints/src/methods/mod.rs

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ mod readonly_write_lock;
9595
mod redundant_as_str;
9696
mod repeat_once;
9797
mod result_map_or_else_none;
98+
mod return_and_then;
9899
mod search_is_some;
99100
mod seek_from_current;
100101
mod seek_to_start_instead_of_rewind;
@@ -4363,6 +4364,46 @@ declare_clippy_lint! {
43634364
"detect `repeat().take()` that can be replaced with `repeat_n()`"
43644365
}
43654366

4367+
declare_clippy_lint! {
4368+
/// ### What it does
4369+
///
4370+
/// Detect functions that end with `Option::and_then` or `Result::and_then`, and suggest using a question mark (`?`) instead.
4371+
///
4372+
/// ### Why is this bad?
4373+
///
4374+
/// The `and_then` method is used to chain a computation that returns an `Option` or a `Result`.
4375+
/// This can be replaced with the `?` operator, which is more concise and idiomatic.
4376+
///
4377+
/// ### Example
4378+
///
4379+
/// ```no_run
4380+
/// fn test(opt: Option<i32>) -> Option<i32> {
4381+
/// opt.and_then(|n| {
4382+
/// if n > 1 {
4383+
/// Some(n + 1)
4384+
/// } else {
4385+
/// None
4386+
/// }
4387+
/// })
4388+
/// }
4389+
/// ```
4390+
/// Use instead:
4391+
/// ```no_run
4392+
/// fn test(opt: Option<i32>) -> Option<i32> {
4393+
/// let n = opt?;
4394+
/// if n > 1 {
4395+
/// Some(n + 1)
4396+
/// } else {
4397+
/// None
4398+
/// }
4399+
/// }
4400+
/// ```
4401+
#[clippy::version = "1.86.0"]
4402+
pub RETURN_AND_THEN,
4403+
style,
4404+
"using `Option::and_then` or `Result::and_then` to chain a computation that returns an `Option` or a `Result`"
4405+
}
4406+
43664407
pub struct Methods {
43674408
avoid_breaking_exported_api: bool,
43684409
msrv: Msrv,
@@ -4531,6 +4572,7 @@ impl_lint_pass!(Methods => [
45314572
DOUBLE_ENDED_ITERATOR_LAST,
45324573
USELESS_NONZERO_NEW_UNCHECKED,
45334574
MANUAL_REPEAT_N,
4575+
RETURN_AND_THEN,
45344576
]);
45354577

45364578
/// Extracts a method call name, args, and `Span` of the method name.
@@ -4760,7 +4802,10 @@ impl Methods {
47604802
let biom_option_linted = bind_instead_of_map::check_and_then_some(cx, expr, recv, arg);
47614803
let biom_result_linted = bind_instead_of_map::check_and_then_ok(cx, expr, recv, arg);
47624804
if !biom_option_linted && !biom_result_linted {
4763-
unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
4805+
let ule_and_linted = unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
4806+
if !ule_and_linted {
4807+
return_and_then::check(cx, expr, recv, arg);
4808+
}
47644809
}
47654810
},
47664811
("any", [arg]) => {
@@ -4973,7 +5018,9 @@ impl Methods {
49735018
get_first::check(cx, expr, recv, arg);
49745019
get_last_with_len::check(cx, expr, recv, arg);
49755020
},
4976-
("get_or_insert_with", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "get_or_insert"),
5021+
("get_or_insert_with", [arg]) => {
5022+
unnecessary_lazy_eval::check(cx, expr, recv, arg, "get_or_insert");
5023+
},
49775024
("hash", [arg]) => {
49785025
unit_hash::check(cx, expr, recv, arg);
49795026
},
@@ -5114,7 +5161,9 @@ impl Methods {
51145161
},
51155162
_ => iter_nth_zero::check(cx, expr, recv, n_arg),
51165163
},
5117-
("ok_or_else", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or"),
5164+
("ok_or_else", [arg]) => {
5165+
unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or");
5166+
},
51185167
("open", [_]) => {
51195168
open_options::check(cx, expr, recv);
51205169
},

‎clippy_lints/src/methods/return_and_then.rs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
use rustc_errors::Applicability;
2+
use rustc_hir as hir;
3+
use rustc_lint::LateContext;
4+
use rustc_middle::ty::{self, GenericArg, Ty};
5+
use rustc_span::sym;
6+
use std::ops::ControlFlow;
7+
8+
use clippy_utils::diagnostics::span_lint_and_sugg;
9+
use clippy_utils::source::{indent_of, reindent_multiline, snippet_with_applicability};
10+
use clippy_utils::ty::get_type_diagnostic_name;
11+
use clippy_utils::visitors::for_each_unconsumed_temporary;
12+
use clippy_utils::{is_expr_final_block_expr, peel_blocks};
13+
14+
use super::RETURN_AND_THEN;
15+
16+
/// lint if `and_then` is the last expression in a block, and
17+
/// there are no references or temporaries in the receiver
18+
pub(super) fn check<'tcx>(
19+
cx: &LateContext<'tcx>,
20+
expr: &hir::Expr<'_>,
21+
recv: &'tcx hir::Expr<'tcx>,
22+
arg: &'tcx hir::Expr<'_>,
23+
) {
24+
if !is_expr_final_block_expr(cx.tcx, expr) {
25+
return;
26+
}
27+
28+
let recv_type = cx.typeck_results().expr_ty(recv);
29+
if !matches!(get_type_diagnostic_name(cx, recv_type), Some(sym::Option | sym::Result)) {
30+
return;
31+
}
32+
33+
let has_ref_type = matches!(recv_type.kind(), ty::Adt(_, args) if args
34+
.first()
35+
.and_then(|arg0: &GenericArg<'tcx>| GenericArg::as_type(*arg0))
36+
.is_some_and(Ty::is_ref));
37+
let has_temporaries = for_each_unconsumed_temporary(cx, recv, |_| ControlFlow::Break(())).is_break();
38+
if has_ref_type && has_temporaries {
39+
return;
40+
}
41+
42+
let hir::ExprKind::Closure(&hir::Closure { body, fn_decl, .. }) = arg.kind else {
43+
return;
44+
};
45+
46+
let closure_arg = fn_decl.inputs[0];
47+
let closure_expr = peel_blocks(cx.tcx.hir().body(body).value);
48+
49+
let mut applicability = Applicability::MachineApplicable;
50+
let arg_snip = snippet_with_applicability(cx, closure_arg.span, "_", &mut applicability);
51+
let recv_snip = snippet_with_applicability(cx, recv.span, "_", &mut applicability);
52+
let body_snip = snippet_with_applicability(cx, closure_expr.span, "..", &mut applicability);
53+
let inner = match body_snip.strip_prefix('{').and_then(|s| s.strip_suffix('}')) {
54+
Some(s) => s.trim_start_matches('\n').trim_end(),
55+
None => &body_snip,
56+
};
57+
58+
let msg = "use the question mark operator instead of an `and_then` call";
59+
let sugg = format!(
60+
"let {} = {}?;\n{}",
61+
arg_snip,
62+
recv_snip,
63+
reindent_multiline(inner.into(), false, indent_of(cx, expr.span))
64+
);
65+
66+
span_lint_and_sugg(cx, RETURN_AND_THEN, expr.span, msg, "try", sugg, applicability);
67+
}

‎clippy_lints/src/methods/unnecessary_lazy_eval.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pub(super) fn check<'tcx>(
1818
recv: &'tcx hir::Expr<'_>,
1919
arg: &'tcx hir::Expr<'_>,
2020
simplify_using: &str,
21-
) {
21+
) -> bool {
2222
let is_option = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Option);
2323
let is_result = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Result);
2424
let is_bool = cx.typeck_results().expr_ty(recv).is_bool();
@@ -29,7 +29,7 @@ pub(super) fn check<'tcx>(
2929
let body_expr = &body.value;
3030

3131
if usage::BindingUsageFinder::are_params_used(cx, body) || is_from_proc_macro(cx, expr) {
32-
return;
32+
return false;
3333
}
3434

3535
if eager_or_lazy::switch_to_eager_eval(cx, body_expr) {
@@ -71,8 +71,10 @@ pub(super) fn check<'tcx>(
7171
applicability,
7272
);
7373
});
74+
return true;
7475
}
7576
}
7677
}
7778
}
79+
false
7880
}

‎tests/ui/return_and_then.fixed

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
#![warn(clippy::return_and_then)]
2+
3+
fn main() {
4+
fn test_opt_block(opt: Option<i32>) -> Option<i32> {
5+
let n = opt?;
6+
let mut ret = n + 1;
7+
ret += n;
8+
if n > 1 { Some(ret) } else { None }
9+
}
10+
11+
fn test_opt_func(opt: Option<i32>) -> Option<i32> {
12+
let n = opt?;
13+
test_opt_block(Some(n))
14+
}
15+
16+
fn test_call_chain() -> Option<i32> {
17+
let n = gen_option(1)?;
18+
test_opt_block(Some(n))
19+
}
20+
21+
fn test_res_block(opt: Result<i32, i32>) -> Result<i32, i32> {
22+
let n = opt?;
23+
if n > 1 { Ok(n + 1) } else { Err(n) }
24+
}
25+
26+
fn test_res_func(opt: Result<i32, i32>) -> Result<i32, i32> {
27+
let n = opt?;
28+
test_res_block(Ok(n))
29+
}
30+
31+
fn test_ref_only() -> Option<i32> {
32+
// ref: empty string
33+
let x = Some("")?;
34+
if x.len() > 2 { Some(3) } else { None }
35+
}
36+
37+
fn test_tmp_only() -> Option<i32> {
38+
// unused temporary: vec![1, 2, 4]
39+
let x = Some(match (vec![1, 2, 3], vec![1, 2, 4]) {
40+
(a, _) if a.len() > 1 => a,
41+
(_, b) => b,
42+
})?;
43+
if x.len() > 2 { Some(3) } else { None }
44+
}
45+
46+
// should not lint
47+
fn test_tmp_ref() -> Option<String> {
48+
String::from("<BOOM>")
49+
.strip_prefix("<")
50+
.and_then(|s| s.strip_suffix(">").map(String::from))
51+
}
52+
53+
// should not lint
54+
fn test_unconsumed_tmp() -> Option<i32> {
55+
[1, 2, 3]
56+
.iter()
57+
.map(|x| x + 1)
58+
.collect::<Vec<_>>() // temporary Vec created here
59+
.as_slice() // creates temporary slice
60+
.first() // creates temporary reference
61+
.and_then(|x| test_opt_block(Some(*x)))
62+
}
63+
}
64+
65+
fn gen_option(n: i32) -> Option<i32> {
66+
Some(n)
67+
}

‎tests/ui/return_and_then.rs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
#![warn(clippy::return_and_then)]
2+
3+
fn main() {
4+
fn test_opt_block(opt: Option<i32>) -> Option<i32> {
5+
opt.and_then(|n| {
6+
let mut ret = n + 1;
7+
ret += n;
8+
if n > 1 { Some(ret) } else { None }
9+
})
10+
}
11+
12+
fn test_opt_func(opt: Option<i32>) -> Option<i32> {
13+
opt.and_then(|n| test_opt_block(Some(n)))
14+
}
15+
16+
fn test_call_chain() -> Option<i32> {
17+
gen_option(1).and_then(|n| test_opt_block(Some(n)))
18+
}
19+
20+
fn test_res_block(opt: Result<i32, i32>) -> Result<i32, i32> {
21+
opt.and_then(|n| if n > 1 { Ok(n + 1) } else { Err(n) })
22+
}
23+
24+
fn test_res_func(opt: Result<i32, i32>) -> Result<i32, i32> {
25+
opt.and_then(|n| test_res_block(Ok(n)))
26+
}
27+
28+
fn test_ref_only() -> Option<i32> {
29+
// ref: empty string
30+
Some("").and_then(|x| if x.len() > 2 { Some(3) } else { None })
31+
}
32+
33+
fn test_tmp_only() -> Option<i32> {
34+
// unused temporary: vec![1, 2, 4]
35+
Some(match (vec![1, 2, 3], vec![1, 2, 4]) {
36+
(a, _) if a.len() > 1 => a,
37+
(_, b) => b,
38+
})
39+
.and_then(|x| if x.len() > 2 { Some(3) } else { None })
40+
}
41+
42+
// should not lint
43+
fn test_tmp_ref() -> Option<String> {
44+
String::from("<BOOM>")
45+
.strip_prefix("<")
46+
.and_then(|s| s.strip_suffix(">").map(String::from))
47+
}
48+
49+
// should not lint
50+
fn test_unconsumed_tmp() -> Option<i32> {
51+
[1, 2, 3]
52+
.iter()
53+
.map(|x| x + 1)
54+
.collect::<Vec<_>>() // temporary Vec created here
55+
.as_slice() // creates temporary slice
56+
.first() // creates temporary reference
57+
.and_then(|x| test_opt_block(Some(*x)))
58+
}
59+
}
60+
61+
fn gen_option(n: i32) -> Option<i32> {
62+
Some(n)
63+
}

0 commit comments

Comments
 (0)