about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAlejandra González <blyxyas@gmail.com>2025-07-24 23:50:14 +0000
committerGitHub <noreply@github.com>2025-07-24 23:50:14 +0000
commit98205e60ccba4fe98ae42e9eee3c14120ef32fe4 (patch)
treeb50d1b9119e1a6f0322af479bcff7beab4a8fdba
parent7d5833fd84fd00c32d0abe3b75def5ad0c88c882 (diff)
parent1b883197d717db26d66ca3644abb9f4b4d47c08b (diff)
downloadrust-98205e60ccba4fe98ae42e9eee3c14120ef32fe4.tar.gz
rust-98205e60ccba4fe98ae42e9eee3c14120ef32fe4.zip
Fix `if_then_some_else_none` FP when require type coercion (#15267)
Closes rust-lang/rust-clippy#15257

changelog: [`if_then_some_else_none`] fix FP when require type coercion
-rw-r--r--clippy_lints/src/if_then_some_else_none.rs7
-rw-r--r--clippy_utils/src/lib.rs13
-rw-r--r--tests/ui/if_then_some_else_none.fixed43
-rw-r--r--tests/ui/if_then_some_else_none.rs68
-rw-r--r--tests/ui/if_then_some_else_none.stderr60
-rw-r--r--tests/ui/if_then_some_else_none_unfixable.rs35
-rw-r--r--tests/ui/if_then_some_else_none_unfixable.stderr28
7 files changed, 251 insertions, 3 deletions
diff --git a/clippy_lints/src/if_then_some_else_none.rs b/clippy_lints/src/if_then_some_else_none.rs
index 9e94280fc07..7158f9419c1 100644
--- a/clippy_lints/src/if_then_some_else_none.rs
+++ b/clippy_lints/src/if_then_some_else_none.rs
@@ -5,7 +5,8 @@ use clippy_utils::msrvs::{self, Msrv};
 use clippy_utils::source::snippet_with_context;
 use clippy_utils::sugg::Sugg;
 use clippy_utils::{
-    contains_return, higher, is_else_clause, is_in_const_context, is_res_lang_ctor, path_res, peel_blocks,
+    contains_return, expr_adjustment_requires_coercion, higher, is_else_clause, is_in_const_context, is_res_lang_ctor,
+    path_res, peel_blocks,
 };
 use rustc_errors::Applicability;
 use rustc_hir::LangItem::{OptionNone, OptionSome};
@@ -92,6 +93,10 @@ impl<'tcx> LateLintPass<'tcx> for IfThenSomeElseNone {
                 expr.span,
                 format!("this could be simplified with `bool::{method_name}`"),
                 |diag| {
+                    if expr_adjustment_requires_coercion(cx, then_arg) {
+                        return;
+                    }
+
                     let mut app = Applicability::MachineApplicable;
                     let cond_snip = Sugg::hir_with_context(cx, cond, expr.span.ctxt(), "[condition]", &mut app)
                         .maybe_paren()
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index 8b9cd6a54dd..219ffb7b83c 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -113,7 +113,7 @@ use rustc_middle::hir::nested_filter;
 use rustc_middle::hir::place::PlaceBase;
 use rustc_middle::lint::LevelAndSource;
 use rustc_middle::mir::{AggregateKind, Operand, RETURN_PLACE, Rvalue, StatementKind, TerminatorKind};
-use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow};
+use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, PointerCoercion};
 use rustc_middle::ty::layout::IntegerExt;
 use rustc_middle::ty::{
     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<'_>)
     // enclosing body.
     false
 }
+
+/// Checks if the expression has adjustments that require coercion, for example: dereferencing with
+/// overloaded deref, coercing pointers and `dyn` objects.
+pub fn expr_adjustment_requires_coercion(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    cx.typeck_results().expr_adjustments(expr).iter().any(|adj| {
+        matches!(
+            adj.kind,
+            Adjust::Deref(Some(_)) | Adjust::Pointer(PointerCoercion::Unsize) | Adjust::NeverToAny
+        )
+    })
+}
diff --git a/tests/ui/if_then_some_else_none.fixed b/tests/ui/if_then_some_else_none.fixed
index f774608712d..d14a805b666 100644
--- a/tests/ui/if_then_some_else_none.fixed
+++ b/tests/ui/if_then_some_else_none.fixed
@@ -122,3 +122,46 @@ const fn issue12103(x: u32) -> Option<u32> {
     // Should not issue an error in `const` context
     if x > 42 { Some(150) } else { None }
 }
+
+mod issue15257 {
+    struct Range {
+        start: u8,
+        end: u8,
+    }
+
+    fn can_be_safely_rewrite(rs: &[&Range]) -> Option<Vec<u8>> {
+        (rs.len() == 1 && rs[0].start == rs[0].end).then(|| vec![rs[0].start])
+    }
+
+    fn reborrow_as_ptr(i: *mut i32) -> Option<*const i32> {
+        let modulo = unsafe { *i % 2 };
+        (modulo == 0).then_some(i)
+    }
+
+    fn reborrow_as_fn_ptr(i: i32) {
+        fn do_something(fn_: Option<fn(i32)>) {
+            todo!()
+        }
+
+        fn item_fn(i: i32) {
+            todo!()
+        }
+
+        do_something((i % 2 == 0).then_some(item_fn));
+    }
+
+    fn reborrow_as_fn_unsafe(i: i32) {
+        fn do_something(fn_: Option<unsafe fn(i32)>) {
+            todo!()
+        }
+
+        fn item_fn(i: i32) {
+            todo!()
+        }
+
+        do_something((i % 2 == 0).then_some(item_fn));
+
+        let closure_fn = |i: i32| {};
+        do_something((i % 2 == 0).then_some(closure_fn));
+    }
+}
diff --git a/tests/ui/if_then_some_else_none.rs b/tests/ui/if_then_some_else_none.rs
index 8b8ff0a6ea0..bb0072f3157 100644
--- a/tests/ui/if_then_some_else_none.rs
+++ b/tests/ui/if_then_some_else_none.rs
@@ -143,3 +143,71 @@ const fn issue12103(x: u32) -> Option<u32> {
     // Should not issue an error in `const` context
     if x > 42 { Some(150) } else { None }
 }
+
+mod issue15257 {
+    struct Range {
+        start: u8,
+        end: u8,
+    }
+
+    fn can_be_safely_rewrite(rs: &[&Range]) -> Option<Vec<u8>> {
+        if rs.len() == 1 && rs[0].start == rs[0].end {
+            //~^ if_then_some_else_none
+            Some(vec![rs[0].start])
+        } else {
+            None
+        }
+    }
+
+    fn reborrow_as_ptr(i: *mut i32) -> Option<*const i32> {
+        let modulo = unsafe { *i % 2 };
+        if modulo == 0 {
+            //~^ if_then_some_else_none
+            Some(i)
+        } else {
+            None
+        }
+    }
+
+    fn reborrow_as_fn_ptr(i: i32) {
+        fn do_something(fn_: Option<fn(i32)>) {
+            todo!()
+        }
+
+        fn item_fn(i: i32) {
+            todo!()
+        }
+
+        do_something(if i % 2 == 0 {
+            //~^ if_then_some_else_none
+            Some(item_fn)
+        } else {
+            None
+        });
+    }
+
+    fn reborrow_as_fn_unsafe(i: i32) {
+        fn do_something(fn_: Option<unsafe fn(i32)>) {
+            todo!()
+        }
+
+        fn item_fn(i: i32) {
+            todo!()
+        }
+
+        do_something(if i % 2 == 0 {
+            //~^ if_then_some_else_none
+            Some(item_fn)
+        } else {
+            None
+        });
+
+        let closure_fn = |i: i32| {};
+        do_something(if i % 2 == 0 {
+            //~^ if_then_some_else_none
+            Some(closure_fn)
+        } else {
+            None
+        });
+    }
+}
diff --git a/tests/ui/if_then_some_else_none.stderr b/tests/ui/if_then_some_else_none.stderr
index 71285574ef2..c2e624a0a73 100644
--- a/tests/ui/if_then_some_else_none.stderr
+++ b/tests/ui/if_then_some_else_none.stderr
@@ -58,5 +58,63 @@ error: this could be simplified with `bool::then`
 LL |     if s == "1" { Some(true) } else { None }
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(s == "1").then(|| true)`
 
-error: aborting due to 6 previous errors
+error: this could be simplified with `bool::then`
+  --> tests/ui/if_then_some_else_none.rs:154:9
+   |
+LL | /         if rs.len() == 1 && rs[0].start == rs[0].end {
+LL | |
+LL | |             Some(vec![rs[0].start])
+LL | |         } else {
+LL | |             None
+LL | |         }
+   | |_________^ help: try: `(rs.len() == 1 && rs[0].start == rs[0].end).then(|| vec![rs[0].start])`
+
+error: this could be simplified with `bool::then_some`
+  --> tests/ui/if_then_some_else_none.rs:164:9
+   |
+LL | /         if modulo == 0 {
+LL | |
+LL | |             Some(i)
+LL | |         } else {
+LL | |             None
+LL | |         }
+   | |_________^ help: try: `(modulo == 0).then_some(i)`
+
+error: this could be simplified with `bool::then_some`
+  --> tests/ui/if_then_some_else_none.rs:181:22
+   |
+LL |           do_something(if i % 2 == 0 {
+   |  ______________________^
+LL | |
+LL | |             Some(item_fn)
+LL | |         } else {
+LL | |             None
+LL | |         });
+   | |_________^ help: try: `(i % 2 == 0).then_some(item_fn)`
+
+error: this could be simplified with `bool::then_some`
+  --> tests/ui/if_then_some_else_none.rs:198:22
+   |
+LL |           do_something(if i % 2 == 0 {
+   |  ______________________^
+LL | |
+LL | |             Some(item_fn)
+LL | |         } else {
+LL | |             None
+LL | |         });
+   | |_________^ help: try: `(i % 2 == 0).then_some(item_fn)`
+
+error: this could be simplified with `bool::then_some`
+  --> tests/ui/if_then_some_else_none.rs:206:22
+   |
+LL |           do_something(if i % 2 == 0 {
+   |  ______________________^
+LL | |
+LL | |             Some(closure_fn)
+LL | |         } else {
+LL | |             None
+LL | |         });
+   | |_________^ help: try: `(i % 2 == 0).then_some(closure_fn)`
+
+error: aborting due to 11 previous errors
 
diff --git a/tests/ui/if_then_some_else_none_unfixable.rs b/tests/ui/if_then_some_else_none_unfixable.rs
new file mode 100644
index 00000000000..be04299a6ab
--- /dev/null
+++ b/tests/ui/if_then_some_else_none_unfixable.rs
@@ -0,0 +1,35 @@
+#![warn(clippy::if_then_some_else_none)]
+#![allow(clippy::manual_is_multiple_of)]
+
+mod issue15257 {
+    use std::pin::Pin;
+
+    #[derive(Default)]
+    pub struct Foo {}
+    pub trait Bar {}
+    impl Bar for Foo {}
+
+    fn pointer_unsized_coercion(i: u32) -> Option<Box<dyn Bar>> {
+        if i % 2 == 0 {
+            //~^ if_then_some_else_none
+            Some(Box::new(Foo::default()))
+        } else {
+            None
+        }
+    }
+
+    fn reborrow_as_pin(i: Pin<&mut i32>) {
+        use std::ops::Rem;
+
+        fn do_something(i: Option<&i32>) {
+            todo!()
+        }
+
+        do_something(if i.rem(2) == 0 {
+            //~^ if_then_some_else_none
+            Some(&i)
+        } else {
+            None
+        });
+    }
+}
diff --git a/tests/ui/if_then_some_else_none_unfixable.stderr b/tests/ui/if_then_some_else_none_unfixable.stderr
new file mode 100644
index 00000000000..f77ce7910e7
--- /dev/null
+++ b/tests/ui/if_then_some_else_none_unfixable.stderr
@@ -0,0 +1,28 @@
+error: this could be simplified with `bool::then`
+  --> tests/ui/if_then_some_else_none_unfixable.rs:13:9
+   |
+LL | /         if i % 2 == 0 {
+LL | |
+LL | |             Some(Box::new(Foo::default()))
+LL | |         } else {
+LL | |             None
+LL | |         }
+   | |_________^
+   |
+   = note: `-D clippy::if-then-some-else-none` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::if_then_some_else_none)]`
+
+error: this could be simplified with `bool::then`
+  --> tests/ui/if_then_some_else_none_unfixable.rs:28:22
+   |
+LL |           do_something(if i.rem(2) == 0 {
+   |  ______________________^
+LL | |
+LL | |             Some(&i)
+LL | |         } else {
+LL | |             None
+LL | |         });
+   | |_________^
+
+error: aborting due to 2 previous errors
+