Skip to content

Commit f51e18d

Browse files
Centri3Centri3
authored
feat: new lint for and_then when returning Option or Result (rust-lang#14051)
close rust-lang#6436 changelog: [`return_and_then`]: new lint
2 parents ad05bc0 + 84fb6b1 commit f51e18d

8 files changed

+356
-5
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6026,6 +6026,7 @@ Released 2018-09-13
60266026
[`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else
60276027
[`result_unit_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unit_err
60286028
[`result_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unwrap_used
6029+
[`return_and_then`]: https://rust-lang.github.io/rust-clippy/master/index.html#return_and_then
60296030
[`return_self_not_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#return_self_not_must_use
60306031
[`reverse_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#reverse_range_loop
60316032
[`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
@@ -463,6 +463,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
463463
crate::methods::REPEAT_ONCE_INFO,
464464
crate::methods::RESULT_FILTER_MAP_INFO,
465465
crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO,
466+
crate::methods::RETURN_AND_THEN_INFO,
466467
crate::methods::SEARCH_IS_SOME_INFO,
467468
crate::methods::SEEK_FROM_CURRENT_INFO,
468469
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;
@@ -4392,6 +4393,46 @@ declare_clippy_lint! {
43924393
"slicing a string and immediately calling as_bytes is less efficient and can lead to panics"
43934394
}
43944395

4396+
declare_clippy_lint! {
4397+
/// ### What it does
4398+
///
4399+
/// Detect functions that end with `Option::and_then` or `Result::and_then`, and suggest using a question mark (`?`) instead.
4400+
///
4401+
/// ### Why is this bad?
4402+
///
4403+
/// The `and_then` method is used to chain a computation that returns an `Option` or a `Result`.
4404+
/// This can be replaced with the `?` operator, which is more concise and idiomatic.
4405+
///
4406+
/// ### Example
4407+
///
4408+
/// ```no_run
4409+
/// fn test(opt: Option<i32>) -> Option<i32> {
4410+
/// opt.and_then(|n| {
4411+
/// if n > 1 {
4412+
/// Some(n + 1)
4413+
/// } else {
4414+
/// None
4415+
/// }
4416+
/// })
4417+
/// }
4418+
/// ```
4419+
/// Use instead:
4420+
/// ```no_run
4421+
/// fn test(opt: Option<i32>) -> Option<i32> {
4422+
/// let n = opt?;
4423+
/// if n > 1 {
4424+
/// Some(n + 1)
4425+
/// } else {
4426+
/// None
4427+
/// }
4428+
/// }
4429+
/// ```
4430+
#[clippy::version = "1.86.0"]
4431+
pub RETURN_AND_THEN,
4432+
restriction,
4433+
"using `Option::and_then` or `Result::and_then` to chain a computation that returns an `Option` or a `Result`"
4434+
}
4435+
43954436
pub struct Methods {
43964437
avoid_breaking_exported_api: bool,
43974438
msrv: Msrv,
@@ -4561,6 +4602,7 @@ impl_lint_pass!(Methods => [
45614602
USELESS_NONZERO_NEW_UNCHECKED,
45624603
MANUAL_REPEAT_N,
45634604
SLICED_STRING_AS_BYTES,
4605+
RETURN_AND_THEN,
45644606
]);
45654607

45664608
/// Extracts a method call name, args, and `Span` of the method name.
@@ -4790,7 +4832,10 @@ impl Methods {
47904832
let biom_option_linted = bind_instead_of_map::check_and_then_some(cx, expr, recv, arg);
47914833
let biom_result_linted = bind_instead_of_map::check_and_then_ok(cx, expr, recv, arg);
47924834
if !biom_option_linted && !biom_result_linted {
4793-
unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
4835+
let ule_and_linted = unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
4836+
if !ule_and_linted {
4837+
return_and_then::check(cx, expr, recv, arg);
4838+
}
47944839
}
47954840
},
47964841
("any", [arg]) => {
@@ -5004,7 +5049,9 @@ impl Methods {
50045049
get_first::check(cx, expr, recv, arg);
50055050
get_last_with_len::check(cx, expr, recv, arg);
50065051
},
5007-
("get_or_insert_with", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "get_or_insert"),
5052+
("get_or_insert_with", [arg]) => {
5053+
unnecessary_lazy_eval::check(cx, expr, recv, arg, "get_or_insert");
5054+
},
50085055
("hash", [arg]) => {
50095056
unit_hash::check(cx, expr, recv, arg);
50105057
},
@@ -5145,7 +5192,9 @@ impl Methods {
51455192
},
51465193
_ => iter_nth_zero::check(cx, expr, recv, n_arg),
51475194
},
5148-
("ok_or_else", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or"),
5195+
("ok_or_else", [arg]) => {
5196+
unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or");
5197+
},
51495198
("open", [_]) => {
51505199
open_options::check(cx, expr, recv);
51515200
},

‎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)