Skip to content

Commit d02f337

Browse files
committed
fix: if_then_some_else_none FP when require type coercion
1 parent 7e2d26f commit d02f337

File tree

7 files changed

+90
-2
lines changed

7 files changed

+90
-2
lines changed

clippy_lints/src/if_then_some_else_none.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ use clippy_utils::msrvs::{self, Msrv};
55
use clippy_utils::source::snippet_with_context;
66
use clippy_utils::sugg::Sugg;
77
use clippy_utils::{
8-
contains_return, higher, is_else_clause, is_in_const_context, is_res_lang_ctor, path_res, peel_blocks,
8+
contains_return, expr_adjustment_requires_coercion, higher, is_else_clause, is_in_const_context, is_res_lang_ctor,
9+
path_res, peel_blocks,
910
};
1011
use rustc_errors::Applicability;
1112
use rustc_hir::LangItem::{OptionNone, OptionSome};
@@ -92,6 +93,10 @@ impl<'tcx> LateLintPass<'tcx> for IfThenSomeElseNone {
9293
expr.span,
9394
format!("this could be simplified with `bool::{method_name}`"),
9495
|diag| {
96+
if expr_adjustment_requires_coercion(cx, then_arg) {
97+
return;
98+
}
99+
95100
let mut app = Applicability::MachineApplicable;
96101
let cond_snip = Sugg::hir_with_context(cx, cond, expr.span.ctxt(), "[condition]", &mut app)
97102
.maybe_paren()

clippy_utils/src/lib.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3567,3 +3567,14 @@ pub fn potential_return_of_enclosing_body(cx: &LateContext<'_>, expr: &Expr<'_>)
35673567
// enclosing body.
35683568
false
35693569
}
3570+
3571+
/// Checks if the expression has adjustments that require coercion, for example: dereferencing with
3572+
/// overloaded deref, coercing pointers and `dyn` objects.
3573+
pub fn expr_adjustment_requires_coercion(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
3574+
cx.typeck_results().expr_adjustments(expr).iter().any(|adj| {
3575+
!matches!(
3576+
adj.kind,
3577+
Adjust::Deref(None) | Adjust::Borrow(AutoBorrow::Ref(..)) | Adjust::ReborrowPin(..)
3578+
)
3579+
})
3580+
}

tests/ui/if_then_some_else_none.fixed

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,14 @@ const fn issue12103(x: u32) -> Option<u32> {
122122
// Should not issue an error in `const` context
123123
if x > 42 { Some(150) } else { None }
124124
}
125+
126+
mod issue15257 {
127+
struct Range {
128+
start: u8,
129+
end: u8,
130+
}
131+
132+
fn can_be_safely_rewrite(rs: &[&Range]) -> Option<Vec<u8>> {
133+
(rs.len() == 1 && rs[0].start == rs[0].end).then(|| vec![rs[0].start])
134+
}
135+
}

tests/ui/if_then_some_else_none.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,19 @@ const fn issue12103(x: u32) -> Option<u32> {
143143
// Should not issue an error in `const` context
144144
if x > 42 { Some(150) } else { None }
145145
}
146+
147+
mod issue15257 {
148+
struct Range {
149+
start: u8,
150+
end: u8,
151+
}
152+
153+
fn can_be_safely_rewrite(rs: &[&Range]) -> Option<Vec<u8>> {
154+
if rs.len() == 1 && rs[0].start == rs[0].end {
155+
//~^ if_then_some_else_none
156+
Some(vec![rs[0].start])
157+
} else {
158+
None
159+
}
160+
}
161+
}

tests/ui/if_then_some_else_none.stderr

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,16 @@ error: this could be simplified with `bool::then`
5858
LL | if s == "1" { Some(true) } else { None }
5959
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(s == "1").then(|| true)`
6060

61-
error: aborting due to 6 previous errors
61+
error: this could be simplified with `bool::then`
62+
--> tests/ui/if_then_some_else_none.rs:154:9
63+
|
64+
LL | / if rs.len() == 1 && rs[0].start == rs[0].end {
65+
LL | |
66+
LL | | Some(vec![rs[0].start])
67+
LL | | } else {
68+
LL | | None
69+
LL | | }
70+
| |_________^ help: try: `(rs.len() == 1 && rs[0].start == rs[0].end).then(|| vec![rs[0].start])`
71+
72+
error: aborting due to 7 previous errors
6273

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#![warn(clippy::if_then_some_else_none)]
2+
#![allow(clippy::manual_is_multiple_of)]
3+
4+
mod issue15257 {
5+
#[derive(Default)]
6+
pub struct Foo {}
7+
pub trait Bar {}
8+
impl Bar for Foo {}
9+
10+
fn maybe_get_bar(i: u32) -> Option<Box<dyn Bar>> {
11+
if i % 2 == 0 {
12+
//~^ if_then_some_else_none
13+
Some(Box::new(Foo::default()))
14+
} else {
15+
None
16+
}
17+
}
18+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: this could be simplified with `bool::then`
2+
--> tests/ui/if_then_some_else_none_unfixable.rs:11:9
3+
|
4+
LL | / if i % 2 == 0 {
5+
LL | |
6+
LL | | Some(Box::new(Foo::default()))
7+
LL | | } else {
8+
LL | | None
9+
LL | | }
10+
| |_________^
11+
|
12+
= note: `-D clippy::if-then-some-else-none` implied by `-D warnings`
13+
= help: to override `-D warnings` add `#[allow(clippy::if_then_some_else_none)]`
14+
15+
error: aborting due to 1 previous error
16+

0 commit comments

Comments
 (0)