diff --git a/clippy_lints/src/manual_unwrap_or_default.rs b/clippy_lints/src/manual_unwrap_or_default.rs index 84fb183e3f79..451172db64e3 100644 --- a/clippy_lints/src/manual_unwrap_or_default.rs +++ b/clippy_lints/src/manual_unwrap_or_default.rs @@ -2,7 +2,7 @@ use rustc_errors::Applicability; use rustc_hir::def::Res; use rustc_hir::{Arm, Expr, ExprKind, HirId, LangItem, MatchSource, Pat, PatKind, QPath}; use rustc_lint::{LateContext, LateLintPass, LintContext}; -use rustc_middle::ty::GenericArgKind; +use rustc_middle::ty::{self, GenericArgKind}; use rustc_session::declare_lint_pass; use rustc_span::sym; @@ -148,6 +148,42 @@ fn handle<'tcx>(cx: &LateContext<'tcx>, if_let_or_match: IfLetOrMatch<'tcx>, exp && is_default_equivalent(cx, body_none) && let Some(receiver) = Sugg::hir_opt(cx, condition).map(Sugg::maybe_par) { + // We check if the expression is not a None variant + if let Some(none_def_id) = cx.tcx.lang_items().option_none_variant() { + if let ExprKind::Path(QPath::Resolved(_, path)) = &condition.kind { + if let Some(def_id) = path.res.opt_def_id() { + if cx.tcx.parent(def_id) == none_def_id { + return span_lint_and_sugg( + cx, + MANUAL_UNWRAP_OR_DEFAULT, + expr.span, + format!("{expr_name} can be simplified with `.unwrap_or_default()`"), + "replace it with", + format!("{receiver}::.unwrap_or_default()"), + Applicability::HasPlaceholders, + ); + } + } + } + } + + // We check if the expression is not a method or function with a unspecified return type + if let ExprKind::MethodCall(_, expr, _, _) = &condition.kind { + if let ty::Adt(_, substs) = cx.typeck_results().expr_ty(expr).kind() { + if let Some(ok_type) = substs.first() { + return span_lint_and_sugg( + cx, + MANUAL_UNWRAP_OR_DEFAULT, + expr.span, + format!("{expr_name} can be simplified with `.unwrap_or_default()`"), + format!("explicit the type {ok_type} and replace your expression with"), + format!("{receiver}.unwrap_or_default()"), + Applicability::Unspecified, + ); + } + } + } + // Machine applicable only if there are no comments present let applicability = if span_contains_comment(cx.sess().source_map(), expr.span) { Applicability::MaybeIncorrect diff --git a/tests/ui/manual_unwrap_or_default.fixed b/tests/ui/manual_unwrap_or_default.fixed index d6e736ba9cc2..c5dfd2eb01e7 100644 --- a/tests/ui/manual_unwrap_or_default.fixed +++ b/tests/ui/manual_unwrap_or_default.fixed @@ -72,3 +72,9 @@ fn issue_12569() { 0 }; } + +fn issue_12670() { + let a: Option = None; + let _ = a.unwrap_or_default(); + let _ = Some(99).unwrap_or_default(); +} diff --git a/tests/ui/manual_unwrap_or_default.rs b/tests/ui/manual_unwrap_or_default.rs index 462d5d90ee77..842a26f4fb12 100644 --- a/tests/ui/manual_unwrap_or_default.rs +++ b/tests/ui/manual_unwrap_or_default.rs @@ -96,3 +96,9 @@ fn issue_12569() { 0 }; } + +fn issue_12670() { + let a: Option = None; + let _ = if let Some(x) = a { x } else { i32::default() }; + let _ = if let Some(x) = Some(99) { x } else { i32::default() }; +} diff --git a/tests/ui/manual_unwrap_or_default.stderr b/tests/ui/manual_unwrap_or_default.stderr index 3f1da444301f..e327c27db6f6 100644 --- a/tests/ui/manual_unwrap_or_default.stderr +++ b/tests/ui/manual_unwrap_or_default.stderr @@ -62,5 +62,17 @@ LL | | _ => 0, LL | | }, | |_________^ help: replace it with: `(*b).unwrap_or_default()` -error: aborting due to 6 previous errors +error: if let can be simplified with `.unwrap_or_default()` + --> tests/ui/manual_unwrap_or_default.rs:102:13 + | +LL | let _ = if let Some(x) = a { x } else { i32::default() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.unwrap_or_default()` + +error: if let can be simplified with `.unwrap_or_default()` + --> tests/ui/manual_unwrap_or_default.rs:103:13 + | +LL | let _ = if let Some(x) = Some(99) { x } else { i32::default() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `Some(99).unwrap_or_default()` + +error: aborting due to 8 previous errors diff --git a/tests/ui/manual_unwrap_or_default_unfixable.rs b/tests/ui/manual_unwrap_or_default_unfixable.rs new file mode 100644 index 000000000000..d82dc8688d9c --- /dev/null +++ b/tests/ui/manual_unwrap_or_default_unfixable.rs @@ -0,0 +1,10 @@ +//@no-rustfix +fn issue_12670() { + #[allow(clippy::match_result_ok)] + let _ = if let Some(x) = "1".parse().ok() { + x + } else { + i32::default() + }; + let _ = if let Some(x) = None { x } else { i32::default() }; +} diff --git a/tests/ui/manual_unwrap_or_default_unfixable.stderr b/tests/ui/manual_unwrap_or_default_unfixable.stderr new file mode 100644 index 000000000000..a6027a33638b --- /dev/null +++ b/tests/ui/manual_unwrap_or_default_unfixable.stderr @@ -0,0 +1,17 @@ +error: if let can be simplified with `.unwrap_or_default()` + --> tests/ui/manual_unwrap_or_default_unfixable.rs:4:30 + | +LL | let _ = if let Some(x) = "1".parse().ok() { + | ^^^^^^^^^^^ help: explicit the type i32 and replace your expression with: `"1".parse().ok().unwrap_or_default()` + | + = note: `-D clippy::manual-unwrap-or-default` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::manual_unwrap_or_default)]` + +error: if let can be simplified with `.unwrap_or_default()` + --> tests/ui/manual_unwrap_or_default_unfixable.rs:9:13 + | +LL | let _ = if let Some(x) = None { x } else { i32::default() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `None::.unwrap_or_default()` + +error: aborting due to 2 previous errors +