From 35f25be9c37fa8396bc7f7fa7c276a0ecd1487ce Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Mon, 22 Sep 2025 18:50:01 +0100 Subject: [PATCH 01/10] Add or_else_then_unwrap This is the same as or_then_unwrap but for or_else calls with a closure immediately calling `Some`. --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/methods/mod.rs | 41 ++++++++++ .../src/methods/or_else_then_unwrap.rs | 81 +++++++++++++++++++ tests/ui/or_else_then_unwrap.fixed | 66 +++++++++++++++ tests/ui/or_else_then_unwrap.rs | 69 ++++++++++++++++ tests/ui/or_else_then_unwrap.stderr | 26 ++++++ 7 files changed, 285 insertions(+) create mode 100644 clippy_lints/src/methods/or_else_then_unwrap.rs create mode 100644 tests/ui/or_else_then_unwrap.fixed create mode 100644 tests/ui/or_else_then_unwrap.rs create mode 100644 tests/ui/or_else_then_unwrap.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 37d46d349667..28e8ffee16b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6643,6 +6643,7 @@ Released 2018-09-13 [`option_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unwrap_or_else [`option_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_option [`option_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_unwrap_used +[`or_else_then_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#or_else_then_unwrap [`or_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#or_fun_call [`or_then_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#or_then_unwrap [`out_of_bounds_indexing`]: https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 375d179681da..5889bb490580 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -442,6 +442,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::methods::OPTION_AS_REF_DEREF_INFO, crate::methods::OPTION_FILTER_MAP_INFO, crate::methods::OPTION_MAP_OR_NONE_INFO, + crate::methods::OR_ELSE_THEN_UNWRAP_INFO, crate::methods::OR_FUN_CALL_INFO, crate::methods::OR_THEN_UNWRAP_INFO, crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index c9066be51c44..bfaece2fa88b 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -89,6 +89,7 @@ mod option_as_ref_cloned; mod option_as_ref_deref; mod option_map_or_none; mod option_map_unwrap_or; +mod or_else_then_unwrap; mod or_fun_call; mod or_then_unwrap; mod path_buf_push_overwrite; @@ -918,6 +919,42 @@ declare_clippy_lint! { "using `.chars().next()` to check if a string starts with a char" } +declare_clippy_lint! { + /// ### What it does + /// Checks for `.or_else(…).unwrap()` calls to Options and Results. + /// + /// ### Why is this bad? + /// You should use `.unwrap_or_else(…)` instead for clarity. + /// + /// ### Example + /// ```no_run + /// # let fallback = "fallback"; + /// // Result + /// # type Error = &'static str; + /// # let result: Result, Error> = Err("error"); + /// let value = result.or_else(|_| Ok::, Error>(vec![fallback])).unwrap(); + /// + /// // Option + /// # let option: Option> = None; + /// let value = option.or_else(|| Some(vec![fallback])).unwrap(); + /// ``` + /// Use instead: + /// ```no_run + /// # let fallback = "fallback"; + /// // Result + /// # let result: Result, &str> = Err("error"); + /// let value = result.unwrap_or_else(|_| vec![fallback]); + /// + /// // Option + /// # let option: Option> = None; + /// let value = option.unwrap_or_else(|| vec![fallback]); + /// ``` + #[clippy::version = "1.90.0"] + pub OR_ELSE_THEN_UNWRAP, + complexity, + "checks for `.or_else(…).unwrap()` calls to Options and Results." +} + declare_clippy_lint! { /// ### What it does /// Checks for calls to `.or(foo(..))`, `.unwrap_or(foo(..))`, @@ -4706,6 +4743,7 @@ impl_lint_pass!(Methods => [ RESULT_MAP_OR_INTO_OPTION, OPTION_MAP_OR_NONE, BIND_INSTEAD_OF_MAP, + OR_ELSE_THEN_UNWRAP, OR_FUN_CALL, OR_THEN_UNWRAP, EXPECT_FUN_CALL, @@ -5500,6 +5538,9 @@ impl Methods { Some((sym::or, recv, [or_arg], or_span, _)) => { or_then_unwrap::check(cx, expr, recv, or_arg, or_span); }, + Some((sym::or_else, recv, [or_arg], or_span, _)) => { + or_else_then_unwrap::check(cx, expr, recv, or_arg, or_span); + }, _ => {}, } unnecessary_literal_unwrap::check(cx, expr, recv, name, args); diff --git a/clippy_lints/src/methods/or_else_then_unwrap.rs b/clippy_lints/src/methods/or_else_then_unwrap.rs new file mode 100644 index 000000000000..b779c4479225 --- /dev/null +++ b/clippy_lints/src/methods/or_else_then_unwrap.rs @@ -0,0 +1,81 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{is_res_lang_ctor, path_res}; +use rustc_errors::Applicability; +use rustc_hir::lang_items::LangItem; +use rustc_hir::{Body, Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_span::{Span, sym}; + +use super::OR_ELSE_THEN_UNWRAP; + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + unwrap_expr: &Expr<'_>, + recv: &'tcx Expr<'tcx>, + or_else_arg: &'tcx Expr<'_>, + or_span: Span, +) { + let ty = cx.typeck_results().expr_ty(recv); // get type of x (we later check if it's Option or Result) + let title; + let or_else_arg_content: Span; + + if is_type_diagnostic_item(cx, ty, sym::Option) { + title = "found `.or_else(|| Some(…)).unwrap()`"; + if let Some(content) = get_content_if_ctor_matches_in_closure(cx, or_else_arg, LangItem::OptionSome) { + or_else_arg_content = content; + } else { + return; + } + } else if is_type_diagnostic_item(cx, ty, sym::Result) { + title = "found `.or_else(|| Ok(…)).unwrap()`"; + if let Some(content) = get_content_if_ctor_matches_in_closure(cx, or_else_arg, LangItem::ResultOk) { + or_else_arg_content = content; + } else { + return; + } + } else { + // Someone has implemented a struct with .or(...).unwrap() chaining, + // but it's not an Option or a Result, so bail + return; + } + + let mut applicability = Applicability::MachineApplicable; + let suggestion = format!( + "unwrap_or_else(|| {})", + snippet_with_applicability(cx, or_else_arg_content, "..", &mut applicability) + ); + + span_lint_and_sugg( + cx, + OR_ELSE_THEN_UNWRAP, + unwrap_expr.span.with_lo(or_span.lo()), + title, + "try", + suggestion, + applicability, + ); +} + +fn get_content_if_ctor_matches_in_closure(cx: &LateContext<'_>, expr: &Expr<'_>, item: LangItem) -> Option { + if let ExprKind::Closure(closure) = expr.kind { + if let Body { + params: [], + value: body, + } = cx.tcx.hir_body(closure.body) + { + if let ExprKind::Call(some_expr, [arg]) = body.kind + && is_res_lang_ctor(cx, path_res(cx, some_expr), item) + { + Some(arg.span.source_callsite()) + } else { + None + } + } else { + None + } + } else { + None + } +} diff --git a/tests/ui/or_else_then_unwrap.fixed b/tests/ui/or_else_then_unwrap.fixed new file mode 100644 index 000000000000..f796fea94121 --- /dev/null +++ b/tests/ui/or_else_then_unwrap.fixed @@ -0,0 +1,66 @@ +#![warn(clippy::or_then_unwrap)] +#![allow(clippy::map_identity, clippy::let_unit_value, clippy::unnecessary_literal_unwrap)] + +struct SomeStruct; +impl SomeStruct { + fn or_else Option>(self, _: F) -> Self { + self + } + fn unwrap(&self) {} +} + +struct SomeOtherStruct; +impl SomeOtherStruct { + fn or_else(self) -> Self { + self + } + fn unwrap(&self) {} +} + +struct Wrapper { + inner: &'static str, +} +impl Wrapper { + fn new(inner: &'static str) -> Self { + Self { inner } + } +} + +fn main() { + let option: Option = None; + let _ = option.unwrap_or_else(|| Wrapper::new("fallback")); // should trigger lint + // + //~^^ or_else_then_unwrap + + // as part of a method chain + let option: Option = None; + let _ = option + .map(|v| v) + .unwrap_or_else(|| Wrapper::new("fallback")) + .inner + .to_string() + .chars(); + + // Call with macro should preserve the macro call rather than expand it + let option: Option> = None; + let _ = option.unwrap_or_else(|| vec!["fallback"]); // should trigger lint + // + //~^^ or_else_then_unwrap + + // Not Option/Result + let instance = SomeStruct {}; + let _ = instance.or_else(|| Some(SomeStruct {})).unwrap(); // should not trigger lint + + // or takes no argument + let instance = SomeOtherStruct {}; + let _ = instance.or_else().unwrap(); // should not trigger lint and should not panic + + // None in or + let option: Option = None; + #[allow(clippy::unnecessary_lazy_evaluations)] + let _ = option.or_else(|| None).unwrap(); // should not trigger lint + + // other function between + let option: Option = None; + let _ = option.or_else(|| Some(Wrapper::new("fallback"))).map(|v| v).unwrap(); // should not trigger lint +} diff --git a/tests/ui/or_else_then_unwrap.rs b/tests/ui/or_else_then_unwrap.rs new file mode 100644 index 000000000000..be6d3f68d4bd --- /dev/null +++ b/tests/ui/or_else_then_unwrap.rs @@ -0,0 +1,69 @@ +#![warn(clippy::or_then_unwrap)] +#![allow(clippy::map_identity, clippy::let_unit_value, clippy::unnecessary_literal_unwrap)] + +struct SomeStruct; +impl SomeStruct { + fn or_else Option>(self, _: F) -> Self { + self + } + fn unwrap(&self) {} +} + +struct SomeOtherStruct; +impl SomeOtherStruct { + fn or_else(self) -> Self { + self + } + fn unwrap(&self) {} +} + +struct Wrapper { + inner: &'static str, +} +impl Wrapper { + fn new(inner: &'static str) -> Self { + Self { inner } + } +} + +fn main() { + let option: Option = None; + let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); // should trigger lint + // + //~^^ or_else_then_unwrap + + // as part of a method chain + let option: Option = None; + let _ = option + .map(|v| v) + .or_else(|| Some(Wrapper::new("fallback"))) // should trigger lint + // + //~^^ or_else_then_unwrap + .unwrap() + .inner + .to_string() + .chars(); + + // Call with macro should preserve the macro call rather than expand it + let option: Option> = None; + let _ = option.or_else(|| Some(vec!["fallback"])).unwrap(); // should trigger lint + // + //~^^ or_else_then_unwrap + + // Not Option/Result + let instance = SomeStruct {}; + let _ = instance.or_else(|| Some(SomeStruct {})).unwrap(); // should not trigger lint + + // or takes no argument + let instance = SomeOtherStruct {}; + let _ = instance.or_else().unwrap(); // should not trigger lint and should not panic + + // None in or + let option: Option = None; + #[allow(clippy::unnecessary_lazy_evaluations)] + let _ = option.or_else(|| None).unwrap(); // should not trigger lint + + // other function between + let option: Option = None; + let _ = option.or_else(|| Some(Wrapper::new("fallback"))).map(|v| v).unwrap(); // should not trigger lint +} diff --git a/tests/ui/or_else_then_unwrap.stderr b/tests/ui/or_else_then_unwrap.stderr new file mode 100644 index 000000000000..b062ae4cb19b --- /dev/null +++ b/tests/ui/or_else_then_unwrap.stderr @@ -0,0 +1,26 @@ +error: found `.or_else(|| Some(…)).unwrap()` + --> tests/ui/or_else_then_unwrap.rs:31:20 + | +LL | let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); // should trigger lint + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| Wrapper::new("fallback"))` + | + = note: `-D clippy::or-else-then-unwrap` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::or_else_then_unwrap)]` + +error: found `.or_else(|| Some(…)).unwrap()` + --> tests/ui/or_else_then_unwrap.rs:39:10 + | +LL | .or_else(|| Some(Wrapper::new("fallback"))) // should trigger lint + | __________^ +... | +LL | | .unwrap() + | |_________________^ help: try: `unwrap_or_else(|| Wrapper::new("fallback"))` + +error: found `.or_else(|| Some(…)).unwrap()` + --> tests/ui/or_else_then_unwrap.rs:49:20 + | +LL | let _ = option.or_else(|| Some(vec!["fallback"])).unwrap(); // should trigger lint + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| vec!["fallback"])` + +error: aborting due to 3 previous errors + From 6c280d2040acdbf1c39ba31b9644ae3085cde58d Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Thu, 25 Sep 2025 22:26:29 +0100 Subject: [PATCH 02/10] Collapse nested `if`s Co-authored-by: Ada Alakbarova <58857108+ada4a@users.noreply.github.com> --- .../src/methods/or_else_then_unwrap.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/methods/or_else_then_unwrap.rs b/clippy_lints/src/methods/or_else_then_unwrap.rs index b779c4479225..3437625ad25c 100644 --- a/clippy_lints/src/methods/or_else_then_unwrap.rs +++ b/clippy_lints/src/methods/or_else_then_unwrap.rs @@ -59,22 +59,15 @@ pub(super) fn check<'tcx>( } fn get_content_if_ctor_matches_in_closure(cx: &LateContext<'_>, expr: &Expr<'_>, item: LangItem) -> Option { - if let ExprKind::Closure(closure) = expr.kind { - if let Body { + if let ExprKind::Closure(closure) = expr.kind + && let Body { params: [], value: body, } = cx.tcx.hir_body(closure.body) - { - if let ExprKind::Call(some_expr, [arg]) = body.kind - && is_res_lang_ctor(cx, path_res(cx, some_expr), item) - { - Some(arg.span.source_callsite()) - } else { - None - } - } else { - None - } + && let ExprKind::Call(some_expr, [arg]) = body.kind + && is_res_lang_ctor(cx, path_res(cx, some_expr), item) + { + Some(arg.span.source_callsite()) } else { None } From c69f2ebb5bcef291f3ed9ceb586ab813c4f2c4b2 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Sun, 19 Oct 2025 22:40:18 +0100 Subject: [PATCH 03/10] Switch to let-chain Co-authored-by: Ada Alakbarova <58857108+ada4a@users.noreply.github.com> --- .../src/methods/or_else_then_unwrap.rs | 37 +++++++++---------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/clippy_lints/src/methods/or_else_then_unwrap.rs b/clippy_lints/src/methods/or_else_then_unwrap.rs index 3437625ad25c..e5244ddbe244 100644 --- a/clippy_lints/src/methods/or_else_then_unwrap.rs +++ b/clippy_lints/src/methods/or_else_then_unwrap.rs @@ -18,28 +18,25 @@ pub(super) fn check<'tcx>( or_span: Span, ) { let ty = cx.typeck_results().expr_ty(recv); // get type of x (we later check if it's Option or Result) - let title; - let or_else_arg_content: Span; - - if is_type_diagnostic_item(cx, ty, sym::Option) { - title = "found `.or_else(|| Some(…)).unwrap()`"; - if let Some(content) = get_content_if_ctor_matches_in_closure(cx, or_else_arg, LangItem::OptionSome) { - or_else_arg_content = content; - } else { - return; - } - } else if is_type_diagnostic_item(cx, ty, sym::Result) { - title = "found `.or_else(|| Ok(…)).unwrap()`"; - if let Some(content) = get_content_if_ctor_matches_in_closure(cx, or_else_arg, LangItem::ResultOk) { - or_else_arg_content = content; - } else { - return; - } - } else { + let (title, or_else_arg_content) = match ty + .ty_adt_def() + .map(AdtDef::did) + .and_then(|did| cx.tcx.get_diagnostic_name(did)) + { + Some(sym::Option) + if let Some(content) = get_content_if_ctor_matches_in_closure(cx, or_else_arg, LangItem::OptionSome) => + { + ("found `.or_else(|| Some(…)).unwrap()`", content) + }, + Some(sym::Result) + if let Some(content) = get_content_if_ctor_matches_in_closure(cx, or_else_arg, LangItem::ResultOk) => + { + ("found `.or_else(|| Ok(…)).unwrap()`", content) + }, // Someone has implemented a struct with .or(...).unwrap() chaining, // but it's not an Option or a Result, so bail - return; - } + _ => return, + }; let mut applicability = Applicability::MachineApplicable; let suggestion = format!( From 0275d74ab501d580fbef516342c3e1fbcb58f97a Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Sun, 19 Oct 2025 22:40:39 +0100 Subject: [PATCH 04/10] Make comment more concise Co-authored-by: Ada Alakbarova <58857108+ada4a@users.noreply.github.com> --- tests/ui/or_else_then_unwrap.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/ui/or_else_then_unwrap.rs b/tests/ui/or_else_then_unwrap.rs index be6d3f68d4bd..44d9d5e9604c 100644 --- a/tests/ui/or_else_then_unwrap.rs +++ b/tests/ui/or_else_then_unwrap.rs @@ -28,9 +28,7 @@ impl Wrapper { fn main() { let option: Option = None; - let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); // should trigger lint - // - //~^^ or_else_then_unwrap + let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); //~ or_else_then_unwrap // as part of a method chain let option: Option = None; From b8bcd8d901bb210a01e032bb4b6d0d69d84ebeb3 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Sun, 19 Oct 2025 22:40:55 +0100 Subject: [PATCH 05/10] Fix function name Co-authored-by: Ada Alakbarova <58857108+ada4a@users.noreply.github.com> --- tests/ui/or_else_then_unwrap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ui/or_else_then_unwrap.rs b/tests/ui/or_else_then_unwrap.rs index 44d9d5e9604c..225ae999a0db 100644 --- a/tests/ui/or_else_then_unwrap.rs +++ b/tests/ui/or_else_then_unwrap.rs @@ -52,7 +52,7 @@ fn main() { let instance = SomeStruct {}; let _ = instance.or_else(|| Some(SomeStruct {})).unwrap(); // should not trigger lint - // or takes no argument + // `or_else` takes no argument let instance = SomeOtherStruct {}; let _ = instance.or_else().unwrap(); // should not trigger lint and should not panic From 5a9e1733d2afef15384da271ba67462a5b2941f6 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Sun, 19 Oct 2025 22:41:08 +0100 Subject: [PATCH 06/10] Improve naming Co-authored-by: Ada Alakbarova <58857108+ada4a@users.noreply.github.com> --- clippy_lints/src/methods/or_else_then_unwrap.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/or_else_then_unwrap.rs b/clippy_lints/src/methods/or_else_then_unwrap.rs index e5244ddbe244..d842963d3f1b 100644 --- a/clippy_lints/src/methods/or_else_then_unwrap.rs +++ b/clippy_lints/src/methods/or_else_then_unwrap.rs @@ -61,8 +61,8 @@ fn get_content_if_ctor_matches_in_closure(cx: &LateContext<'_>, expr: &Expr<'_>, params: [], value: body, } = cx.tcx.hir_body(closure.body) - && let ExprKind::Call(some_expr, [arg]) = body.kind - && is_res_lang_ctor(cx, path_res(cx, some_expr), item) + && let ExprKind::Call(some_or_ok, [arg]) = body.kind + && is_res_lang_ctor(cx, path_res(cx, some_or_ok), item) { Some(arg.span.source_callsite()) } else { From 48f6cd4394528f4e9861375c8b54f37ef15e7451 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Sun, 19 Oct 2025 23:14:40 +0100 Subject: [PATCH 07/10] Make suggestion verbose --- .../src/methods/or_else_then_unwrap.rs | 20 ++++++++---- tests/ui/or_else_then_unwrap.fixed | 6 ++-- tests/ui/or_else_then_unwrap.stderr | 32 +++++++++++++++---- 3 files changed, 41 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/methods/or_else_then_unwrap.rs b/clippy_lints/src/methods/or_else_then_unwrap.rs index d842963d3f1b..a01c55229de8 100644 --- a/clippy_lints/src/methods/or_else_then_unwrap.rs +++ b/clippy_lints/src/methods/or_else_then_unwrap.rs @@ -1,11 +1,11 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::{is_res_lang_ctor, path_res}; use rustc_errors::Applicability; use rustc_hir::lang_items::LangItem; use rustc_hir::{Body, Expr, ExprKind}; use rustc_lint::LateContext; +use rustc_middle::ty::AdtDef; use rustc_span::{Span, sym}; use super::OR_ELSE_THEN_UNWRAP; @@ -44,14 +44,20 @@ pub(super) fn check<'tcx>( snippet_with_applicability(cx, or_else_arg_content, "..", &mut applicability) ); - span_lint_and_sugg( + let span = unwrap_expr.span.with_lo(or_span.lo()); + span_lint_and_then( cx, OR_ELSE_THEN_UNWRAP, - unwrap_expr.span.with_lo(or_span.lo()), + span, title, - "try", - suggestion, - applicability, + |diag| { + diag.span_suggestion_verbose( + span, + "try", + suggestion, + applicability, + ); + } ); } diff --git a/tests/ui/or_else_then_unwrap.fixed b/tests/ui/or_else_then_unwrap.fixed index f796fea94121..778c500dfbb6 100644 --- a/tests/ui/or_else_then_unwrap.fixed +++ b/tests/ui/or_else_then_unwrap.fixed @@ -28,9 +28,7 @@ impl Wrapper { fn main() { let option: Option = None; - let _ = option.unwrap_or_else(|| Wrapper::new("fallback")); // should trigger lint - // - //~^^ or_else_then_unwrap + let _ = option.unwrap_or_else(|| Wrapper::new("fallback")); //~ or_else_then_unwrap // as part of a method chain let option: Option = None; @@ -51,7 +49,7 @@ fn main() { let instance = SomeStruct {}; let _ = instance.or_else(|| Some(SomeStruct {})).unwrap(); // should not trigger lint - // or takes no argument + // `or_else` takes no argument let instance = SomeOtherStruct {}; let _ = instance.or_else().unwrap(); // should not trigger lint and should not panic diff --git a/tests/ui/or_else_then_unwrap.stderr b/tests/ui/or_else_then_unwrap.stderr index b062ae4cb19b..24d49db51fed 100644 --- a/tests/ui/or_else_then_unwrap.stderr +++ b/tests/ui/or_else_then_unwrap.stderr @@ -1,26 +1,46 @@ error: found `.or_else(|| Some(…)).unwrap()` --> tests/ui/or_else_then_unwrap.rs:31:20 | -LL | let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); // should trigger lint - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| Wrapper::new("fallback"))` +LL | let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::or-else-then-unwrap` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::or_else_then_unwrap)]` +help: try + | +LL - let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); +LL + let _ = option.unwrap_or_else(|| Wrapper::new("fallback")); + | error: found `.or_else(|| Some(…)).unwrap()` - --> tests/ui/or_else_then_unwrap.rs:39:10 + --> tests/ui/or_else_then_unwrap.rs:37:10 | LL | .or_else(|| Some(Wrapper::new("fallback"))) // should trigger lint | __________^ ... | LL | | .unwrap() - | |_________________^ help: try: `unwrap_or_else(|| Wrapper::new("fallback"))` + | |_________________^ + | +help: try + | +LL - .or_else(|| Some(Wrapper::new("fallback"))) // should trigger lint +LL - // +LL - +LL - .unwrap() +LL + .unwrap_or_else(|| Wrapper::new("fallback")) + | error: found `.or_else(|| Some(…)).unwrap()` - --> tests/ui/or_else_then_unwrap.rs:49:20 + --> tests/ui/or_else_then_unwrap.rs:47:20 | LL | let _ = option.or_else(|| Some(vec!["fallback"])).unwrap(); // should trigger lint - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| vec!["fallback"])` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: try + | +LL - let _ = option.or_else(|| Some(vec!["fallback"])).unwrap(); // should trigger lint +LL + let _ = option.unwrap_or_else(|| vec!["fallback"]); // should trigger lint + | error: aborting due to 3 previous errors From 5dd81d74133a76ab946f335faacaeb9b6021460e Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Sun, 19 Oct 2025 23:18:29 +0100 Subject: [PATCH 08/10] Add example macro call --- tests/ui/or_else_then_unwrap.fixed | 10 ++++++++++ tests/ui/or_else_then_unwrap.rs | 10 ++++++++++ tests/ui/or_else_then_unwrap.stderr | 6 +++--- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/tests/ui/or_else_then_unwrap.fixed b/tests/ui/or_else_then_unwrap.fixed index 778c500dfbb6..3c7009740550 100644 --- a/tests/ui/or_else_then_unwrap.fixed +++ b/tests/ui/or_else_then_unwrap.fixed @@ -1,6 +1,10 @@ +//@aux-build:proc_macros.rs + #![warn(clippy::or_then_unwrap)] #![allow(clippy::map_identity, clippy::let_unit_value, clippy::unnecessary_literal_unwrap)] +extern crate proc_macros; + struct SomeStruct; impl SomeStruct { fn or_else Option>(self, _: F) -> Self { @@ -61,4 +65,10 @@ fn main() { // other function between let option: Option = None; let _ = option.or_else(|| Some(Wrapper::new("fallback"))).map(|v| v).unwrap(); // should not trigger lint + + // We don't lint external macros + proc_macros::external!{ + let option: Option = None; + let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); + }; } diff --git a/tests/ui/or_else_then_unwrap.rs b/tests/ui/or_else_then_unwrap.rs index 225ae999a0db..e350cb559df9 100644 --- a/tests/ui/or_else_then_unwrap.rs +++ b/tests/ui/or_else_then_unwrap.rs @@ -1,6 +1,10 @@ +//@aux-build:proc_macros.rs + #![warn(clippy::or_then_unwrap)] #![allow(clippy::map_identity, clippy::let_unit_value, clippy::unnecessary_literal_unwrap)] +extern crate proc_macros; + struct SomeStruct; impl SomeStruct { fn or_else Option>(self, _: F) -> Self { @@ -64,4 +68,10 @@ fn main() { // other function between let option: Option = None; let _ = option.or_else(|| Some(Wrapper::new("fallback"))).map(|v| v).unwrap(); // should not trigger lint + + // We don't lint external macros + proc_macros::external!{ + let option: Option = None; + let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); + }; } diff --git a/tests/ui/or_else_then_unwrap.stderr b/tests/ui/or_else_then_unwrap.stderr index 24d49db51fed..850e4214cb5c 100644 --- a/tests/ui/or_else_then_unwrap.stderr +++ b/tests/ui/or_else_then_unwrap.stderr @@ -1,5 +1,5 @@ error: found `.or_else(|| Some(…)).unwrap()` - --> tests/ui/or_else_then_unwrap.rs:31:20 + --> tests/ui/or_else_then_unwrap.rs:35:20 | LL | let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -13,7 +13,7 @@ LL + let _ = option.unwrap_or_else(|| Wrapper::new("fallback")); | error: found `.or_else(|| Some(…)).unwrap()` - --> tests/ui/or_else_then_unwrap.rs:37:10 + --> tests/ui/or_else_then_unwrap.rs:41:10 | LL | .or_else(|| Some(Wrapper::new("fallback"))) // should trigger lint | __________^ @@ -31,7 +31,7 @@ LL + .unwrap_or_else(|| Wrapper::new("fallback")) | error: found `.or_else(|| Some(…)).unwrap()` - --> tests/ui/or_else_then_unwrap.rs:47:20 + --> tests/ui/or_else_then_unwrap.rs:51:20 | LL | let _ = option.or_else(|| Some(vec!["fallback"])).unwrap(); // should trigger lint | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From c90a482edd4e181a410b89313ed9f041f988347b Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Sun, 19 Oct 2025 23:23:01 +0100 Subject: [PATCH 09/10] fmt --- clippy_lints/src/methods/or_else_then_unwrap.rs | 17 +++-------------- tests/ui/or_else_then_unwrap.fixed | 8 ++++---- tests/ui/or_else_then_unwrap.rs | 8 ++++---- 3 files changed, 11 insertions(+), 22 deletions(-) diff --git a/clippy_lints/src/methods/or_else_then_unwrap.rs b/clippy_lints/src/methods/or_else_then_unwrap.rs index a01c55229de8..fbeb22e92f6b 100644 --- a/clippy_lints/src/methods/or_else_then_unwrap.rs +++ b/clippy_lints/src/methods/or_else_then_unwrap.rs @@ -45,20 +45,9 @@ pub(super) fn check<'tcx>( ); let span = unwrap_expr.span.with_lo(or_span.lo()); - span_lint_and_then( - cx, - OR_ELSE_THEN_UNWRAP, - span, - title, - |diag| { - diag.span_suggestion_verbose( - span, - "try", - suggestion, - applicability, - ); - } - ); + span_lint_and_then(cx, OR_ELSE_THEN_UNWRAP, span, title, |diag| { + diag.span_suggestion_verbose(span, "try", suggestion, applicability); + }); } fn get_content_if_ctor_matches_in_closure(cx: &LateContext<'_>, expr: &Expr<'_>, item: LangItem) -> Option { diff --git a/tests/ui/or_else_then_unwrap.fixed b/tests/ui/or_else_then_unwrap.fixed index 3c7009740550..2218af23d7ce 100644 --- a/tests/ui/or_else_then_unwrap.fixed +++ b/tests/ui/or_else_then_unwrap.fixed @@ -67,8 +67,8 @@ fn main() { let _ = option.or_else(|| Some(Wrapper::new("fallback"))).map(|v| v).unwrap(); // should not trigger lint // We don't lint external macros - proc_macros::external!{ - let option: Option = None; - let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); - }; + proc_macros::external! { + let option: Option = None; + let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); + }; } diff --git a/tests/ui/or_else_then_unwrap.rs b/tests/ui/or_else_then_unwrap.rs index e350cb559df9..22d6d5807b93 100644 --- a/tests/ui/or_else_then_unwrap.rs +++ b/tests/ui/or_else_then_unwrap.rs @@ -70,8 +70,8 @@ fn main() { let _ = option.or_else(|| Some(Wrapper::new("fallback"))).map(|v| v).unwrap(); // should not trigger lint // We don't lint external macros - proc_macros::external!{ - let option: Option = None; - let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); - }; + proc_macros::external! { + let option: Option = None; + let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); + }; } From 0f37275df7ff038382638b5d5e6d3120a0fbbff9 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Sun, 19 Oct 2025 23:30:50 +0100 Subject: [PATCH 10/10] Update calls since merge --- clippy_lints/src/methods/or_else_then_unwrap.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/or_else_then_unwrap.rs b/clippy_lints/src/methods/or_else_then_unwrap.rs index fbeb22e92f6b..fa6327dbcf3d 100644 --- a/clippy_lints/src/methods/or_else_then_unwrap.rs +++ b/clippy_lints/src/methods/or_else_then_unwrap.rs @@ -1,6 +1,6 @@ +use crate::clippy_utils::res::{MaybeDef, MaybeQPath}; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::{is_res_lang_ctor, path_res}; use rustc_errors::Applicability; use rustc_hir::lang_items::LangItem; use rustc_hir::{Body, Expr, ExprKind}; @@ -57,7 +57,7 @@ fn get_content_if_ctor_matches_in_closure(cx: &LateContext<'_>, expr: &Expr<'_>, value: body, } = cx.tcx.hir_body(closure.body) && let ExprKind::Call(some_or_ok, [arg]) = body.kind - && is_res_lang_ctor(cx, path_res(cx, some_or_ok), item) + && some_or_ok.res(cx).ctor_parent(cx).is_lang_item(cx, item) { Some(arg.span.source_callsite()) } else {