From 11ef04728c90aa6105b20c681e3c12f231ba4f12 Mon Sep 17 00:00:00 2001 From: Jade Date: Thu, 29 Jul 2021 23:56:47 -0700 Subject: [PATCH] Add unwrap_or_else_default lint This will catch `unwrap_or_else(Default::default)` on Result and Option and suggest `unwrap_or_default()` instead. --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 3 + clippy_lints/src/methods/mod.rs | 32 ++++++++- clippy_lints/src/methods/or_fun_call.rs | 21 ++++-- .../src/methods/unwrap_or_else_default.rs | 50 +++++++++++++ clippy_utils/src/source.rs | 2 +- clippy_utils/src/ty.rs | 18 ++++- tests/ui/or_fun_call.fixed | 19 +++++ tests/ui/or_fun_call.rs | 19 +++++ tests/ui/or_fun_call.stderr | 58 +++++++++------ tests/ui/unwrap_or_else_default.fixed | 71 +++++++++++++++++++ tests/ui/unwrap_or_else_default.rs | 71 +++++++++++++++++++ tests/ui/unwrap_or_else_default.stderr | 22 ++++++ 13 files changed, 356 insertions(+), 31 deletions(-) create mode 100644 clippy_lints/src/methods/unwrap_or_else_default.rs create mode 100644 tests/ui/unwrap_or_else_default.fixed create mode 100644 tests/ui/unwrap_or_else_default.rs create mode 100644 tests/ui/unwrap_or_else_default.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index ac745793dda..2b89170073b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2999,6 +2999,7 @@ Released 2018-09-13 [`unused_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit [`unusual_byte_groupings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unusual_byte_groupings [`unwrap_in_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_in_result +[`unwrap_or_else_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_or_else_default [`unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used [`upper_case_acronyms`]: https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms [`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f49b382c5ea..dbdb4251b3b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -797,6 +797,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: methods::UNNECESSARY_FILTER_MAP, methods::UNNECESSARY_FOLD, methods::UNNECESSARY_LAZY_EVALUATIONS, + methods::UNWRAP_OR_ELSE_DEFAULT, methods::UNWRAP_USED, methods::USELESS_ASREF, methods::WRONG_SELF_CONVENTION, @@ -1341,6 +1342,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(methods::UNNECESSARY_FILTER_MAP), LintId::of(methods::UNNECESSARY_FOLD), LintId::of(methods::UNNECESSARY_LAZY_EVALUATIONS), + LintId::of(methods::UNWRAP_OR_ELSE_DEFAULT), LintId::of(methods::USELESS_ASREF), LintId::of(methods::WRONG_SELF_CONVENTION), LintId::of(methods::ZST_OFFSET), @@ -1535,6 +1537,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(methods::STRING_EXTEND_CHARS), LintId::of(methods::UNNECESSARY_FOLD), LintId::of(methods::UNNECESSARY_LAZY_EVALUATIONS), + LintId::of(methods::UNWRAP_OR_ELSE_DEFAULT), LintId::of(methods::WRONG_SELF_CONVENTION), LintId::of(misc::TOPLEVEL_REF_ARG), LintId::of(misc::ZERO_PTR), diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 5aa29424349..12f7987fd3a 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -56,6 +56,7 @@ mod uninit_assumed_init; mod unnecessary_filter_map; mod unnecessary_fold; mod unnecessary_lazy_eval; +mod unwrap_or_else_default; mod unwrap_used; mod useless_asref; mod utils; @@ -310,6 +311,31 @@ declare_clippy_lint! { "using `ok().expect()`, which gives worse error messages than calling `expect` directly on the Result" } +declare_clippy_lint! { + /// ### What it does + /// Checks for usages of `_.unwrap_or_else(Default::default)` on Option and + /// Result values. + /// + /// ### Why is this bad? + /// Readability, these can be written as `option.unwrap_or_default` or + /// `result.unwrap_or_default`. + /// + /// ### Examples + /// ```rust + /// # let x = Some(1); + /// + /// // Bad + /// x.unwrap_or_else(Default::default); + /// x.unwrap_or_else(u32::default); + /// + /// // Good + /// x.unwrap_or_default(); + /// ``` + pub UNWRAP_OR_ELSE_DEFAULT, + style, + "using `.unwrap_or_else(Default::default)`, which is more succinctly expressed as `.unwrap_or_default()`" +} + declare_clippy_lint! { /// ### What it does /// Checks for usage of `option.map(_).unwrap_or(_)` or `option.map(_).unwrap_or_else(_)` or @@ -1766,6 +1792,7 @@ impl_lint_pass!(Methods => [ SHOULD_IMPLEMENT_TRAIT, WRONG_SELF_CONVENTION, OK_EXPECT, + UNWRAP_OR_ELSE_DEFAULT, MAP_UNWRAP_OR, RESULT_MAP_OR_INTO_OPTION, OPTION_MAP_OR_NONE, @@ -2172,7 +2199,10 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio }, ("unwrap_or_else", [u_arg]) => match method_call!(recv) { Some(("map", [recv, map_arg], _)) if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, msrv) => {}, - _ => unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or"), + _ => { + unwrap_or_else_default::check(cx, expr, recv, u_arg); + unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or"); + }, }, _ => {}, } diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index ef615b0aa40..378b0724170 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -1,7 +1,8 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::eager_or_lazy::is_lazyness_candidate; use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_macro_callsite}; -use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type}; +use clippy_utils::ty::{implements_trait, qpath_target_trait}; +use clippy_utils::ty::{is_type_diagnostic_item, match_type}; use clippy_utils::{contains_return, last_path_segment, paths}; use if_chain::if_chain; use rustc_errors::Applicability; @@ -34,15 +35,25 @@ pub(super) fn check<'tcx>( or_has_args: bool, span: Span, ) -> bool { + let is_default_default = |qpath, default_trait_id| { + qpath_target_trait(cx, qpath, fun.hir_id).map_or(false, |target_trait| target_trait == default_trait_id) + }; + + let implements_default = |arg, default_trait_id| { + let arg_ty = cx.typeck_results().expr_ty(arg); + implements_trait(cx, arg_ty, default_trait_id, &[]) + }; + if_chain! { if !or_has_args; if name == "unwrap_or"; if let hir::ExprKind::Path(ref qpath) = fun.kind; - let path = last_path_segment(qpath).ident.name; - if matches!(path, kw::Default | sym::new); - let arg_ty = cx.typeck_results().expr_ty(arg); if let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default); - if implements_trait(cx, arg_ty, default_trait_id, &[]); + let path = last_path_segment(qpath).ident.name; + // needs to target Default::default in particular or be *::new and have a Default impl + // available + if (matches!(path, kw::Default) && is_default_default(qpath, default_trait_id)) + || (matches!(path, sym::new) && implements_default(arg, default_trait_id)); then { let mut applicability = Applicability::MachineApplicable; diff --git a/clippy_lints/src/methods/unwrap_or_else_default.rs b/clippy_lints/src/methods/unwrap_or_else_default.rs new file mode 100644 index 00000000000..f99ae6cae93 --- /dev/null +++ b/clippy_lints/src/methods/unwrap_or_else_default.rs @@ -0,0 +1,50 @@ +//! Lint for `some_result_or_option.unwrap_or_else(Default::default)` + +use super::UNWRAP_OR_ELSE_DEFAULT; +use clippy_utils::{ + diagnostics::span_lint_and_sugg, + source::snippet_with_applicability, + ty::{is_type_diagnostic_item, qpath_target_trait}, +}; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::LateContext; +use rustc_span::sym; + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx hir::Expr<'_>, + recv: &'tcx hir::Expr<'_>, + u_arg: &'tcx hir::Expr<'_>, +) { + // something.unwrap_or_else(Default::default) + // ^^^^^^^^^- recv ^^^^^^^^^^^^^^^^- u_arg + // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- expr + let recv_ty = cx.typeck_results().expr_ty(recv); + let is_option = is_type_diagnostic_item(cx, recv_ty, sym::option_type); + let is_result = is_type_diagnostic_item(cx, recv_ty, sym::result_type); + + if_chain! { + if is_option || is_result; + if let hir::ExprKind::Path(ref qpath) = u_arg.kind; + if let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default); + if let Some(target_trait) = qpath_target_trait(cx, qpath, u_arg.hir_id); + if target_trait == default_trait_id; + then { + let mut applicability = Applicability::MachineApplicable; + + span_lint_and_sugg( + cx, + UNWRAP_OR_ELSE_DEFAULT, + expr.span, + "use of `.unwrap_or_else(..)` to construct default value", + "try", + format!( + "{}.unwrap_or_default()", + snippet_with_applicability(cx, recv.span, "..", &mut applicability) + ), + applicability, + ); + } + } +} diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs index 4d49b43bde9..789079510c5 100644 --- a/clippy_utils/src/source.rs +++ b/clippy_utils/src/source.rs @@ -168,7 +168,7 @@ pub fn snippet<'a, T: LintContext>(cx: &T, span: Span, default: &'a str) -> Cow< snippet_opt(cx, span).map_or_else(|| Cow::Borrowed(default), From::from) } -/// Same as `snippet`, but it adapts the applicability level by following rules: +/// Same as [`snippet`], but it adapts the applicability level by following rules: /// /// - Applicability level `Unspecified` will never be changed. /// - If the span is inside a macro, change the applicability level to `MaybeIncorrect`. diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index e914dc1c222..bc31bea8b9f 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -2,6 +2,7 @@ #![allow(clippy::module_name_repetitions)] +use hir::{HirId, QPath}; use rustc_ast::ast::Mutability; use rustc_data_structures::fx::FxHashMap; use rustc_hir as hir; @@ -114,7 +115,7 @@ pub fn has_iter_method(cx: &LateContext<'_>, probably_ref_ty: Ty<'_>) -> Option< /// Checks whether a type implements a trait. /// The function returns false in case the type contains an inference variable. -/// See also `get_trait_def_id`. +/// See also [`get_trait_def_id`]. pub fn implements_trait<'tcx>( cx: &LateContext<'tcx>, ty: Ty<'tcx>, @@ -136,6 +137,21 @@ pub fn implements_trait<'tcx>( }) } +/// Gets the trait that a path targets. For example `::a` would return the +/// [`DefId`] for `Trait`. +/// +/// `cx` must be in a body. +pub fn qpath_target_trait<'tcx>(cx: &LateContext<'tcx>, qpath: &QPath<'_>, expr_id: HirId) -> Option { + let method_res = cx.typeck_results().qpath_res(qpath, expr_id); + let method_id = match method_res { + hir::def::Res::Def(_kind, id) => Some(id), + _ => None, + }; + let method_id = method_id?; + + cx.tcx.trait_of_item(method_id) +} + /// Checks whether this type implements `Drop`. pub fn has_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { match ty.ty_adt_def() { diff --git a/tests/ui/or_fun_call.fixed b/tests/ui/or_fun_call.fixed index 4390ff7dc30..c2f94d0e857 100644 --- a/tests/ui/or_fun_call.fixed +++ b/tests/ui/or_fun_call.fixed @@ -18,6 +18,19 @@ fn or_fun_call() { } } + struct FakeDefault; + impl FakeDefault { + fn default() -> Self { + FakeDefault + } + } + + impl Default for FakeDefault { + fn default() -> Self { + FakeDefault + } + } + enum Enum { A(i32), } @@ -53,6 +66,12 @@ fn or_fun_call() { let with_default_type = Some(1); with_default_type.unwrap_or_default(); + let self_default = None::; + self_default.unwrap_or_else(::default); + + let real_default = None::; + real_default.unwrap_or_default(); + let with_vec = Some(vec![1]); with_vec.unwrap_or_default(); diff --git a/tests/ui/or_fun_call.rs b/tests/ui/or_fun_call.rs index 75908c974cc..afaf92961b0 100644 --- a/tests/ui/or_fun_call.rs +++ b/tests/ui/or_fun_call.rs @@ -18,6 +18,19 @@ fn or_fun_call() { } } + struct FakeDefault; + impl FakeDefault { + fn default() -> Self { + FakeDefault + } + } + + impl Default for FakeDefault { + fn default() -> Self { + FakeDefault + } + } + enum Enum { A(i32), } @@ -53,6 +66,12 @@ fn or_fun_call() { let with_default_type = Some(1); with_default_type.unwrap_or(u64::default()); + let self_default = None::; + self_default.unwrap_or(::default()); + + let real_default = None::; + real_default.unwrap_or(::default()); + let with_vec = Some(vec![1]); with_vec.unwrap_or(vec![]); diff --git a/tests/ui/or_fun_call.stderr b/tests/ui/or_fun_call.stderr index 9905029ce91..b2bcbd38c2d 100644 --- a/tests/ui/or_fun_call.stderr +++ b/tests/ui/or_fun_call.stderr @@ -1,5 +1,5 @@ error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:33:19 + --> $DIR/or_fun_call.rs:46:19 | LL | with_const_fn.unwrap_or(Duration::from_secs(5)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| Duration::from_secs(5))` @@ -7,130 +7,142 @@ LL | with_const_fn.unwrap_or(Duration::from_secs(5)); = note: `-D clippy::or-fun-call` implied by `-D warnings` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:36:22 + --> $DIR/or_fun_call.rs:49:22 | LL | with_constructor.unwrap_or(make()); | ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(make)` error: use of `unwrap_or` followed by a call to `new` - --> $DIR/or_fun_call.rs:39:5 + --> $DIR/or_fun_call.rs:52:5 | LL | with_new.unwrap_or(Vec::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_new.unwrap_or_default()` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:42:21 + --> $DIR/or_fun_call.rs:55:21 | LL | with_const_args.unwrap_or(Vec::with_capacity(12)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| Vec::with_capacity(12))` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:45:14 + --> $DIR/or_fun_call.rs:58:14 | LL | with_err.unwrap_or(make()); | ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| make())` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:48:19 + --> $DIR/or_fun_call.rs:61:19 | LL | with_err_args.unwrap_or(Vec::with_capacity(12)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| Vec::with_capacity(12))` error: use of `unwrap_or` followed by a call to `default` - --> $DIR/or_fun_call.rs:51:5 + --> $DIR/or_fun_call.rs:64:5 | LL | with_default_trait.unwrap_or(Default::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_trait.unwrap_or_default()` error: use of `unwrap_or` followed by a call to `default` - --> $DIR/or_fun_call.rs:54:5 + --> $DIR/or_fun_call.rs:67:5 | LL | with_default_type.unwrap_or(u64::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_type.unwrap_or_default()` +error: use of `unwrap_or` followed by a function call + --> $DIR/or_fun_call.rs:70:18 + | +LL | self_default.unwrap_or(::default()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(::default)` + +error: use of `unwrap_or` followed by a call to `default` + --> $DIR/or_fun_call.rs:73:5 + | +LL | real_default.unwrap_or(::default()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `real_default.unwrap_or_default()` + error: use of `unwrap_or` followed by a call to `new` - --> $DIR/or_fun_call.rs:57:5 + --> $DIR/or_fun_call.rs:76:5 | LL | with_vec.unwrap_or(vec![]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_vec.unwrap_or_default()` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:60:21 + --> $DIR/or_fun_call.rs:79:21 | LL | without_default.unwrap_or(Foo::new()); | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(Foo::new)` error: use of `or_insert` followed by a function call - --> $DIR/or_fun_call.rs:63:19 + --> $DIR/or_fun_call.rs:82:19 | LL | map.entry(42).or_insert(String::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)` error: use of `or_insert` followed by a function call - --> $DIR/or_fun_call.rs:66:23 + --> $DIR/or_fun_call.rs:85:23 | LL | map_vec.entry(42).or_insert(vec![]); | ^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(Vec::new)` error: use of `or_insert` followed by a function call - --> $DIR/or_fun_call.rs:69:21 + --> $DIR/or_fun_call.rs:88:21 | LL | btree.entry(42).or_insert(String::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)` error: use of `or_insert` followed by a function call - --> $DIR/or_fun_call.rs:72:25 + --> $DIR/or_fun_call.rs:91:25 | LL | btree_vec.entry(42).or_insert(vec![]); | ^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(Vec::new)` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:75:21 + --> $DIR/or_fun_call.rs:94:21 | LL | let _ = stringy.unwrap_or("".to_owned()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "".to_owned())` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:83:21 + --> $DIR/or_fun_call.rs:102:21 | LL | let _ = Some(1).unwrap_or(map[&1]); | ^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| map[&1])` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:85:21 + --> $DIR/or_fun_call.rs:104:21 | LL | let _ = Some(1).unwrap_or(map[&1]); | ^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| map[&1])` error: use of `or` followed by a function call - --> $DIR/or_fun_call.rs:109:35 + --> $DIR/or_fun_call.rs:128:35 | LL | let _ = Some("a".to_string()).or(Some("b".to_string())); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some("b".to_string()))` error: use of `or` followed by a function call - --> $DIR/or_fun_call.rs:113:10 + --> $DIR/or_fun_call.rs:132:10 | LL | .or(Some(Bar(b, Duration::from_secs(2)))); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some(Bar(b, Duration::from_secs(2))))` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:141:14 + --> $DIR/or_fun_call.rs:160:14 | LL | None.unwrap_or(s.as_mut_vec()); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| s.as_mut_vec())` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:146:14 + --> $DIR/or_fun_call.rs:165:14 | LL | None.unwrap_or(unsafe { s.as_mut_vec() }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { s.as_mut_vec() })` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:148:14 + --> $DIR/or_fun_call.rs:167:14 | LL | None.unwrap_or( unsafe { s.as_mut_vec() } ); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { s.as_mut_vec() })` -error: aborting due to 22 previous errors +error: aborting due to 24 previous errors diff --git a/tests/ui/unwrap_or_else_default.fixed b/tests/ui/unwrap_or_else_default.fixed new file mode 100644 index 00000000000..7ac3f426c97 --- /dev/null +++ b/tests/ui/unwrap_or_else_default.fixed @@ -0,0 +1,71 @@ +// run-rustfix + +#![warn(clippy::unwrap_or_else_default)] +#![allow(dead_code)] +#![allow(clippy::unnecessary_wraps)] + +/// Checks implementation of the `UNWRAP_OR_ELSE_DEFAULT` lint. +fn unwrap_or_else_default() { + struct Foo; + + impl Foo { + fn new() -> Foo { + Foo + } + + // fake default, we should not trigger on this + fn default() -> Foo { + Foo + } + } + + struct HasDefaultAndDuplicate; + + impl HasDefaultAndDuplicate { + fn default() -> Self { + HasDefaultAndDuplicate + } + } + + impl Default for HasDefaultAndDuplicate { + fn default() -> Self { + HasDefaultAndDuplicate + } + } + + enum Enum { + A(), + } + + fn make(_: V) -> T { + unimplemented!(); + } + + let with_enum = Some(Enum::A()); + with_enum.unwrap_or_else(Enum::A); + + let with_new = Some(vec![1]); + with_new.unwrap_or_else(Vec::new); + + let with_err: Result<_, ()> = Ok(vec![1]); + with_err.unwrap_or_else(make); + + // should not be changed + let with_fake_default = None::; + with_fake_default.unwrap_or_else(Foo::default); + + // should not be changed + let with_fake_default2 = None::; + with_fake_default2.unwrap_or_else(::default); + + let with_real_default = None::; + with_real_default.unwrap_or_default(); + + let with_default_trait = Some(1); + with_default_trait.unwrap_or_default(); + + let with_default_type = Some(1); + with_default_type.unwrap_or_default(); +} + +fn main() {} diff --git a/tests/ui/unwrap_or_else_default.rs b/tests/ui/unwrap_or_else_default.rs new file mode 100644 index 00000000000..82b727a039e --- /dev/null +++ b/tests/ui/unwrap_or_else_default.rs @@ -0,0 +1,71 @@ +// run-rustfix + +#![warn(clippy::unwrap_or_else_default)] +#![allow(dead_code)] +#![allow(clippy::unnecessary_wraps)] + +/// Checks implementation of the `UNWRAP_OR_ELSE_DEFAULT` lint. +fn unwrap_or_else_default() { + struct Foo; + + impl Foo { + fn new() -> Foo { + Foo + } + + // fake default, we should not trigger on this + fn default() -> Foo { + Foo + } + } + + struct HasDefaultAndDuplicate; + + impl HasDefaultAndDuplicate { + fn default() -> Self { + HasDefaultAndDuplicate + } + } + + impl Default for HasDefaultAndDuplicate { + fn default() -> Self { + HasDefaultAndDuplicate + } + } + + enum Enum { + A(), + } + + fn make(_: V) -> T { + unimplemented!(); + } + + let with_enum = Some(Enum::A()); + with_enum.unwrap_or_else(Enum::A); + + let with_new = Some(vec![1]); + with_new.unwrap_or_else(Vec::new); + + let with_err: Result<_, ()> = Ok(vec![1]); + with_err.unwrap_or_else(make); + + // should not be changed + let with_fake_default = None::; + with_fake_default.unwrap_or_else(Foo::default); + + // should not be changed + let with_fake_default2 = None::; + with_fake_default2.unwrap_or_else(::default); + + let with_real_default = None::; + with_real_default.unwrap_or_else(::default); + + let with_default_trait = Some(1); + with_default_trait.unwrap_or_else(Default::default); + + let with_default_type = Some(1); + with_default_type.unwrap_or_else(u64::default); +} + +fn main() {} diff --git a/tests/ui/unwrap_or_else_default.stderr b/tests/ui/unwrap_or_else_default.stderr new file mode 100644 index 00000000000..feb215b09f6 --- /dev/null +++ b/tests/ui/unwrap_or_else_default.stderr @@ -0,0 +1,22 @@ +error: use of `.unwrap_or_else(..)` to construct default value + --> $DIR/unwrap_or_else_default.rs:62:5 + | +LL | with_real_default.unwrap_or_else(::default); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_real_default.unwrap_or_default()` + | + = note: `-D clippy::unwrap-or-else-default` implied by `-D warnings` + +error: use of `.unwrap_or_else(..)` to construct default value + --> $DIR/unwrap_or_else_default.rs:65:5 + | +LL | with_default_trait.unwrap_or_else(Default::default); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_default_trait.unwrap_or_default()` + +error: use of `.unwrap_or_else(..)` to construct default value + --> $DIR/unwrap_or_else_default.rs:68:5 + | +LL | with_default_type.unwrap_or_else(u64::default); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_default_type.unwrap_or_default()` + +error: aborting due to 3 previous errors +