Skip to content

Commit 28f651d

Browse files
committed
fix: check temporaries in receiver
1 parent 9857a40 commit 28f651d

File tree

4 files changed

+64
-24
lines changed

4 files changed

+64
-24
lines changed

clippy_lints/src/methods/return_and_then.rs

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,41 @@
1-
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::source::{indent_of, reindent_multiline, snippet_with_applicability};
3-
use clippy_utils::ty::is_type_diagnostic_item;
4-
use clippy_utils::{is_expr_final_block_expr, is_expr_used_or_unified, peel_blocks};
51
use rustc_errors::Applicability;
62
use rustc_hir as hir;
73
use rustc_lint::LateContext;
4+
use rustc_middle::ty::{self, GenericArg, Ty};
85
use rustc_span::sym;
6+
use std::ops::ControlFlow;
97

10-
use super::RETURN_AND_THEN;
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};
1113

12-
fn is_final_call(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
13-
if !is_expr_used_or_unified(cx.tcx, expr) {
14-
return false;
15-
}
16-
is_expr_final_block_expr(cx.tcx, expr)
17-
}
14+
use super::RETURN_AND_THEN;
1815

19-
/// lint if `and_then` is the last call in the function
16+
/// lint if `and_then` is the last expression in a block and
17+
/// there are no temporaries in the receiver
2018
pub(super) fn check<'tcx>(
21-
cx: &LateContext<'_>,
19+
cx: &LateContext<'tcx>,
2220
expr: &hir::Expr<'_>,
23-
recv: &'tcx hir::Expr<'_>,
21+
recv: &'tcx hir::Expr<'tcx>,
2422
arg: &'tcx hir::Expr<'_>,
2523
) {
26-
if !is_final_call(cx, expr) {
24+
if !is_expr_final_block_expr(cx.tcx, expr) {
2725
return;
2826
}
2927

3028
let recv_type = cx.typeck_results().expr_ty(recv);
31-
let is_option = is_type_diagnostic_item(cx, recv_type, sym::Option);
32-
let is_result = is_type_diagnostic_item(cx, recv_type, sym::Result);
29+
if !matches!(get_type_diagnostic_name(cx, recv_type), Some(sym::Option | sym::Result)) {
30+
return;
31+
}
3332

34-
if !is_option && !is_result {
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 {
3539
return;
3640
}
3741

@@ -44,7 +48,7 @@ pub(super) fn check<'tcx>(
4448

4549
let mut applicability = Applicability::MachineApplicable;
4650
let arg_snip = snippet_with_applicability(cx, closure_arg.span, "_", &mut applicability);
47-
let recv_snip = snippet_with_applicability(cx, recv.span, "..", &mut applicability);
51+
let recv_snip = snippet_with_applicability(cx, recv.span, "_", &mut applicability);
4852
let body_snip = snippet_with_applicability(cx, closure_expr.span, "..", &mut applicability);
4953
let inner = match body_snip.strip_prefix('{').and_then(|s| s.strip_suffix('}')) {
5054
Some(s) => s.trim_start_matches('\n').trim_end(),

tests/ui/return_and_then.fixed

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ fn main() {
1515

1616
fn test_call_chain() -> Option<i32> {
1717
let n = gen_option(1)?;
18-
test_opt_func(Some(n))
18+
test_opt_block(Some(n))
1919
}
2020

2121
fn test_res_block(opt: Result<i32, i32>) -> Result<i32, i32> {
@@ -27,6 +27,24 @@ fn main() {
2727
let n = opt?;
2828
test_res_block(Ok(n))
2929
}
30+
31+
// should not lint
32+
fn test_tmp_ref() -> Option<String> {
33+
String::from("<BOOM>")
34+
.strip_prefix("<")
35+
.and_then(|s| s.strip_suffix(">").map(String::from))
36+
}
37+
38+
// should not lint
39+
fn test_unconsumed_tmp() -> Option<i32> {
40+
[1, 2, 3]
41+
.iter()
42+
.map(|x| x + 1)
43+
.collect::<Vec<_>>() // temporary Vec created here
44+
.as_slice() // creates temporary slice
45+
.first() // creates temporary reference
46+
.and_then(|x| test_opt_block(Some(*x)))
47+
}
3048
}
3149

3250
fn gen_option(n: i32) -> Option<i32> {

tests/ui/return_and_then.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ fn main() {
1414
}
1515

1616
fn test_call_chain() -> Option<i32> {
17-
gen_option(1).and_then(|n| test_opt_func(Some(n)))
17+
gen_option(1).and_then(|n| test_opt_block(Some(n)))
1818
}
1919

2020
fn test_res_block(opt: Result<i32, i32>) -> Result<i32, i32> {
@@ -24,6 +24,24 @@ fn main() {
2424
fn test_res_func(opt: Result<i32, i32>) -> Result<i32, i32> {
2525
opt.and_then(|n| test_res_block(Ok(n)))
2626
}
27+
28+
// should not lint
29+
fn test_tmp_ref() -> Option<String> {
30+
String::from("<BOOM>")
31+
.strip_prefix("<")
32+
.and_then(|s| s.strip_suffix(">").map(String::from))
33+
}
34+
35+
// should not lint
36+
fn test_unconsumed_tmp() -> Option<i32> {
37+
[1, 2, 3]
38+
.iter()
39+
.map(|x| x + 1)
40+
.collect::<Vec<_>>() // temporary Vec created here
41+
.as_slice() // creates temporary slice
42+
.first() // creates temporary reference
43+
.and_then(|x| test_opt_block(Some(*x)))
44+
}
2745
}
2846

2947
fn gen_option(n: i32) -> Option<i32> {

tests/ui/return_and_then.stderr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,13 @@ LL + test_opt_block(Some(n))
3333
error: use the question mark operator instead of an `and_then` call
3434
--> tests/ui/return_and_then.rs:17:9
3535
|
36-
LL | gen_option(1).and_then(|n| test_opt_func(Some(n)))
37-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
36+
LL | gen_option(1).and_then(|n| test_opt_block(Some(n)))
37+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3838
|
3939
help: try
4040
|
4141
LL ~ let n = gen_option(1)?;
42-
LL + test_opt_func(Some(n))
42+
LL + test_opt_block(Some(n))
4343
|
4444

4545
error: use the question mark operator instead of an `and_then` call

0 commit comments

Comments
 (0)