Skip to content

Commit 1b88319

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

File tree

7 files changed

+251
-3
lines changed

7 files changed

+251
-3
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: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ use rustc_middle::hir::nested_filter;
113113
use rustc_middle::hir::place::PlaceBase;
114114
use rustc_middle::lint::LevelAndSource;
115115
use rustc_middle::mir::{AggregateKind, Operand, RETURN_PLACE, Rvalue, StatementKind, TerminatorKind};
116-
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow};
116+
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, PointerCoercion};
117117
use rustc_middle::ty::layout::IntegerExt;
118118
use rustc_middle::ty::{
119119
self as rustc_ty, Binder, BorrowKind, ClosureKind, EarlyBinder, GenericArgKind, GenericArgsRef, IntTy, Ty, TyCtxt,
@@ -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(Some(_)) | Adjust::Pointer(PointerCoercion::Unsize) | Adjust::NeverToAny
3578+
)
3579+
})
3580+
}

tests/ui/if_then_some_else_none.fixed

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,46 @@ 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+
136+
fn reborrow_as_ptr(i: *mut i32) -> Option<*const i32> {
137+
let modulo = unsafe { *i % 2 };
138+
(modulo == 0).then_some(i)
139+
}
140+
141+
fn reborrow_as_fn_ptr(i: i32) {
142+
fn do_something(fn_: Option<fn(i32)>) {
143+
todo!()
144+
}
145+
146+
fn item_fn(i: i32) {
147+
todo!()
148+
}
149+
150+
do_something((i % 2 == 0).then_some(item_fn));
151+
}
152+
153+
fn reborrow_as_fn_unsafe(i: i32) {
154+
fn do_something(fn_: Option<unsafe fn(i32)>) {
155+
todo!()
156+
}
157+
158+
fn item_fn(i: i32) {
159+
todo!()
160+
}
161+
162+
do_something((i % 2 == 0).then_some(item_fn));
163+
164+
let closure_fn = |i: i32| {};
165+
do_something((i % 2 == 0).then_some(closure_fn));
166+
}
167+
}

tests/ui/if_then_some_else_none.rs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,71 @@ 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+
162+
fn reborrow_as_ptr(i: *mut i32) -> Option<*const i32> {
163+
let modulo = unsafe { *i % 2 };
164+
if modulo == 0 {
165+
//~^ if_then_some_else_none
166+
Some(i)
167+
} else {
168+
None
169+
}
170+
}
171+
172+
fn reborrow_as_fn_ptr(i: i32) {
173+
fn do_something(fn_: Option<fn(i32)>) {
174+
todo!()
175+
}
176+
177+
fn item_fn(i: i32) {
178+
todo!()
179+
}
180+
181+
do_something(if i % 2 == 0 {
182+
//~^ if_then_some_else_none
183+
Some(item_fn)
184+
} else {
185+
None
186+
});
187+
}
188+
189+
fn reborrow_as_fn_unsafe(i: i32) {
190+
fn do_something(fn_: Option<unsafe fn(i32)>) {
191+
todo!()
192+
}
193+
194+
fn item_fn(i: i32) {
195+
todo!()
196+
}
197+
198+
do_something(if i % 2 == 0 {
199+
//~^ if_then_some_else_none
200+
Some(item_fn)
201+
} else {
202+
None
203+
});
204+
205+
let closure_fn = |i: i32| {};
206+
do_something(if i % 2 == 0 {
207+
//~^ if_then_some_else_none
208+
Some(closure_fn)
209+
} else {
210+
None
211+
});
212+
}
213+
}

tests/ui/if_then_some_else_none.stderr

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,63 @@ 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: this could be simplified with `bool::then_some`
73+
--> tests/ui/if_then_some_else_none.rs:164:9
74+
|
75+
LL | / if modulo == 0 {
76+
LL | |
77+
LL | | Some(i)
78+
LL | | } else {
79+
LL | | None
80+
LL | | }
81+
| |_________^ help: try: `(modulo == 0).then_some(i)`
82+
83+
error: this could be simplified with `bool::then_some`
84+
--> tests/ui/if_then_some_else_none.rs:181:22
85+
|
86+
LL | do_something(if i % 2 == 0 {
87+
| ______________________^
88+
LL | |
89+
LL | | Some(item_fn)
90+
LL | | } else {
91+
LL | | None
92+
LL | | });
93+
| |_________^ help: try: `(i % 2 == 0).then_some(item_fn)`
94+
95+
error: this could be simplified with `bool::then_some`
96+
--> tests/ui/if_then_some_else_none.rs:198:22
97+
|
98+
LL | do_something(if i % 2 == 0 {
99+
| ______________________^
100+
LL | |
101+
LL | | Some(item_fn)
102+
LL | | } else {
103+
LL | | None
104+
LL | | });
105+
| |_________^ help: try: `(i % 2 == 0).then_some(item_fn)`
106+
107+
error: this could be simplified with `bool::then_some`
108+
--> tests/ui/if_then_some_else_none.rs:206:22
109+
|
110+
LL | do_something(if i % 2 == 0 {
111+
| ______________________^
112+
LL | |
113+
LL | | Some(closure_fn)
114+
LL | | } else {
115+
LL | | None
116+
LL | | });
117+
| |_________^ help: try: `(i % 2 == 0).then_some(closure_fn)`
118+
119+
error: aborting due to 11 previous errors
62120

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
#![warn(clippy::if_then_some_else_none)]
2+
#![allow(clippy::manual_is_multiple_of)]
3+
4+
mod issue15257 {
5+
use std::pin::Pin;
6+
7+
#[derive(Default)]
8+
pub struct Foo {}
9+
pub trait Bar {}
10+
impl Bar for Foo {}
11+
12+
fn pointer_unsized_coercion(i: u32) -> Option<Box<dyn Bar>> {
13+
if i % 2 == 0 {
14+
//~^ if_then_some_else_none
15+
Some(Box::new(Foo::default()))
16+
} else {
17+
None
18+
}
19+
}
20+
21+
fn reborrow_as_pin(i: Pin<&mut i32>) {
22+
use std::ops::Rem;
23+
24+
fn do_something(i: Option<&i32>) {
25+
todo!()
26+
}
27+
28+
do_something(if i.rem(2) == 0 {
29+
//~^ if_then_some_else_none
30+
Some(&i)
31+
} else {
32+
None
33+
});
34+
}
35+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
error: this could be simplified with `bool::then`
2+
--> tests/ui/if_then_some_else_none_unfixable.rs:13: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: this could be simplified with `bool::then`
16+
--> tests/ui/if_then_some_else_none_unfixable.rs:28:22
17+
|
18+
LL | do_something(if i.rem(2) == 0 {
19+
| ______________________^
20+
LL | |
21+
LL | | Some(&i)
22+
LL | | } else {
23+
LL | | None
24+
LL | | });
25+
| |_________^
26+
27+
error: aborting due to 2 previous errors
28+

0 commit comments

Comments
 (0)