From c9f0374177892e9b21017190153958b66a26591c Mon Sep 17 00:00:00 2001 From: Tom Webber Date: Thu, 18 Apr 2024 01:02:21 +0200 Subject: [PATCH 1/3] fix: manual_unwrap_or_default --- clippy_lints/src/manual_unwrap_or_default.rs | 38 +++++++++++++++++++- tests/ui/manual_unwrap_or_default.rs | 16 +++++++++ tests/ui/manual_unwrap_or_default.stderr | 26 +++++++++++++- 3 files changed, 78 insertions(+), 2 deletions(-) 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.rs b/tests/ui/manual_unwrap_or_default.rs index 462d5d90ee77..42b3370024d3 100644 --- a/tests/ui/manual_unwrap_or_default.rs +++ b/tests/ui/manual_unwrap_or_default.rs @@ -96,3 +96,19 @@ fn issue_12569() { 0 }; } + +//@no-rustfix +fn issue_12670() { + // no auto: type not found + #[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() }; + // auto fix with unwrap_or_default + 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..182ed9a7ba23 100644 --- a/tests/ui/manual_unwrap_or_default.stderr +++ b/tests/ui/manual_unwrap_or_default.stderr @@ -62,5 +62,29 @@ 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:104: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()` + +error: if let can be simplified with `.unwrap_or_default()` + --> tests/ui/manual_unwrap_or_default.rs:109:13 + | +LL | let _ = if let Some(x) = None { x } else { i32::default() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `None::.unwrap_or_default()` + +error: if let can be simplified with `.unwrap_or_default()` + --> tests/ui/manual_unwrap_or_default.rs:112: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:113: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 10 previous errors From 73168455f4bc037d550bd96d1cbb9dedc36ea473 Mon Sep 17 00:00:00 2001 From: Tom Webber Date: Thu, 18 Apr 2024 23:27:28 +0200 Subject: [PATCH 2/3] update tests --- tests/ui/manual_unwrap_or_default.fixed | 74 ------------------------- 1 file changed, 74 deletions(-) delete mode 100644 tests/ui/manual_unwrap_or_default.fixed diff --git a/tests/ui/manual_unwrap_or_default.fixed b/tests/ui/manual_unwrap_or_default.fixed deleted file mode 100644 index d6e736ba9cc2..000000000000 --- a/tests/ui/manual_unwrap_or_default.fixed +++ /dev/null @@ -1,74 +0,0 @@ -#![warn(clippy::manual_unwrap_or_default)] -#![allow(clippy::unnecessary_literal_unwrap)] - -fn main() { - let x: Option> = None; - x.unwrap_or_default(); - - let x: Option> = None; - x.unwrap_or_default(); - - let x: Option = None; - x.unwrap_or_default(); - - let x: Option> = None; - x.unwrap_or_default(); - - let x: Option> = None; - x.unwrap_or_default(); - - // Issue #12564 - // No error as &Vec<_> doesn't implement std::default::Default - let mut map = std::collections::HashMap::from([(0, vec![0; 3]), (1, vec![1; 3]), (2, vec![2])]); - let x: &[_] = if let Some(x) = map.get(&0) { x } else { &[] }; - // Same code as above written using match. - let x: &[_] = match map.get(&0) { - Some(x) => x, - None => &[], - }; -} - -// Issue #12531 -unsafe fn no_deref_ptr(a: Option, b: *const Option) -> i32 { - match a { - // `*b` being correct depends on `a == Some(_)` - Some(_) => (*b).unwrap_or_default(), - _ => 0, - } -} - -const fn issue_12568(opt: Option) -> bool { - match opt { - Some(s) => s, - None => false, - } -} - -fn issue_12569() { - let match_none_se = match 1u32.checked_div(0) { - Some(v) => v, - None => { - println!("important"); - 0 - }, - }; - let match_some_se = match 1u32.checked_div(0) { - Some(v) => { - println!("important"); - v - }, - None => 0, - }; - let iflet_else_se = if let Some(v) = 1u32.checked_div(0) { - v - } else { - println!("important"); - 0 - }; - let iflet_then_se = if let Some(v) = 1u32.checked_div(0) { - println!("important"); - v - } else { - 0 - }; -} From d7572b8fa6f7cb8e713f3ee6c4482b90021e8bed Mon Sep 17 00:00:00 2001 From: Tom Webber Date: Mon, 17 Jun 2024 09:04:01 +0200 Subject: [PATCH 3/3] add: tests configuration in separate file --- tests/ui/manual_unwrap_or_default.fixed | 80 +++++++++++++++++++ tests/ui/manual_unwrap_or_default.rs | 10 --- tests/ui/manual_unwrap_or_default.stderr | 18 +---- .../ui/manual_unwrap_or_default_unfixable.rs | 10 +++ .../manual_unwrap_or_default_unfixable.stderr | 17 ++++ 5 files changed, 110 insertions(+), 25 deletions(-) create mode 100644 tests/ui/manual_unwrap_or_default.fixed create mode 100644 tests/ui/manual_unwrap_or_default_unfixable.rs create mode 100644 tests/ui/manual_unwrap_or_default_unfixable.stderr diff --git a/tests/ui/manual_unwrap_or_default.fixed b/tests/ui/manual_unwrap_or_default.fixed new file mode 100644 index 000000000000..c5dfd2eb01e7 --- /dev/null +++ b/tests/ui/manual_unwrap_or_default.fixed @@ -0,0 +1,80 @@ +#![warn(clippy::manual_unwrap_or_default)] +#![allow(clippy::unnecessary_literal_unwrap)] + +fn main() { + let x: Option> = None; + x.unwrap_or_default(); + + let x: Option> = None; + x.unwrap_or_default(); + + let x: Option = None; + x.unwrap_or_default(); + + let x: Option> = None; + x.unwrap_or_default(); + + let x: Option> = None; + x.unwrap_or_default(); + + // Issue #12564 + // No error as &Vec<_> doesn't implement std::default::Default + let mut map = std::collections::HashMap::from([(0, vec![0; 3]), (1, vec![1; 3]), (2, vec![2])]); + let x: &[_] = if let Some(x) = map.get(&0) { x } else { &[] }; + // Same code as above written using match. + let x: &[_] = match map.get(&0) { + Some(x) => x, + None => &[], + }; +} + +// Issue #12531 +unsafe fn no_deref_ptr(a: Option, b: *const Option) -> i32 { + match a { + // `*b` being correct depends on `a == Some(_)` + Some(_) => (*b).unwrap_or_default(), + _ => 0, + } +} + +const fn issue_12568(opt: Option) -> bool { + match opt { + Some(s) => s, + None => false, + } +} + +fn issue_12569() { + let match_none_se = match 1u32.checked_div(0) { + Some(v) => v, + None => { + println!("important"); + 0 + }, + }; + let match_some_se = match 1u32.checked_div(0) { + Some(v) => { + println!("important"); + v + }, + None => 0, + }; + let iflet_else_se = if let Some(v) = 1u32.checked_div(0) { + v + } else { + println!("important"); + 0 + }; + let iflet_then_se = if let Some(v) = 1u32.checked_div(0) { + println!("important"); + v + } else { + 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 42b3370024d3..842a26f4fb12 100644 --- a/tests/ui/manual_unwrap_or_default.rs +++ b/tests/ui/manual_unwrap_or_default.rs @@ -97,17 +97,7 @@ fn issue_12569() { }; } -//@no-rustfix fn issue_12670() { - // no auto: type not found - #[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() }; - // auto fix with unwrap_or_default 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 182ed9a7ba23..e327c27db6f6 100644 --- a/tests/ui/manual_unwrap_or_default.stderr +++ b/tests/ui/manual_unwrap_or_default.stderr @@ -63,28 +63,16 @@ LL | | }, | |_________^ help: replace it with: `(*b).unwrap_or_default()` error: if let can be simplified with `.unwrap_or_default()` - --> tests/ui/manual_unwrap_or_default.rs:104: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()` - -error: if let can be simplified with `.unwrap_or_default()` - --> tests/ui/manual_unwrap_or_default.rs:109:13 - | -LL | let _ = if let Some(x) = None { x } else { i32::default() }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `None::.unwrap_or_default()` - -error: if let can be simplified with `.unwrap_or_default()` - --> tests/ui/manual_unwrap_or_default.rs:112:13 + --> 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:113:13 + --> 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 10 previous errors +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 +