Auto merge of rust-lang#8163 - pmnoxx:piotr-improve-unwrap-or-default, r=Manishearth

Improve `unwrap_or_else_default` when handling `unwrap_or_else(XXX::new)`

changelog:  change `unwrap_or_else_default` to work with std constructors like `Vec::new`, `HashSet::new`, `HashMap::new`.

Notes:
- Code to handle detecting those constructors is already there. I moved it out to `is_default_equivalent_call`

Author: bors

clippy_lints/src/methods/unwrap_or_else_default.rs

@@ -2,7 +2,8 @@
 
 use super::UNWRAP_OR_ELSE_DEFAULT;
 use clippy_utils::{
-    diagnostics::span_lint_and_sugg, is_trait_item, source::snippet_with_applicability, ty::is_type_diagnostic_item,
+    diagnostics::span_lint_and_sugg, is_default_equivalent_call, source::snippet_with_applicability,
+    ty::is_type_diagnostic_item,
 };
 use rustc_errors::Applicability;
 use rustc_hir as hir;
@@ -24,7 +25,7 @@ pub(super) fn check<'tcx>(
 
     if_chain! {
         if is_option || is_result;
-        if is_trait_item(cx, u_arg, sym::Default);
+        if is_default_equivalent_call(cx, u_arg);
         then {
             let mut applicability = Applicability::MachineApplicable;
 

clippy_utils/src/lib.rs

@@ -664,6 +664,22 @@ fn is_default_equivalent_ctor(cx: &LateContext<'_>, def_id: DefId, path: &QPath<
     false
 }
 
+/// Return true if the expr is equal to `Default::default` when evaluated.
+pub fn is_default_equivalent_call(cx: &LateContext<'_>, repl_func: &Expr<'_>) -> bool {
+    if_chain! {
+        if let hir::ExprKind::Path(ref repl_func_qpath) = repl_func.kind;
+        if let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id();
+        if is_diag_trait_item(cx, repl_def_id, sym::Default)
+            || is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath);
+        then {
+            true
+        }
+        else {
+            false
+        }
+    }
+}
+
 /// Returns true if the expr is equal to `Default::default()` of it's type when evaluated.
 /// It doesn't cover all cases, for example indirect function calls (some of std
 /// functions are supported) but it is the best we have.
@@ -686,18 +702,7 @@ pub fn is_default_equivalent(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
                 false
             }
         },
-        ExprKind::Call(repl_func, _) => if_chain! {
-            if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind;
-            if let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id();
-            if is_diag_trait_item(cx, repl_def_id, sym::Default)
-                || is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath);
-            then {
-                true
-            }
-            else {
-                false
-            }
-        },
+        ExprKind::Call(repl_func, _) => is_default_equivalent_call(cx, repl_func),
         ExprKind::Path(qpath) => is_lang_ctor(cx, qpath, OptionNone),
         ExprKind::AddrOf(rustc_hir::BorrowKind::Ref, _, expr) => matches!(expr.kind, ExprKind::Array([])),
         _ => false,

tests/ui/unwrap_or_else_default.fixed

@@ -45,7 +45,7 @@ fn unwrap_or_else_default() {
     with_enum.unwrap_or_else(Enum::A);
 
     let with_new = Some(vec![1]);
-    with_new.unwrap_or_else(Vec::new);
+    with_new.unwrap_or_default();
 
     let with_err: Result<_, ()> = Ok(vec![1]);
     with_err.unwrap_or_else(make);
@@ -66,6 +66,9 @@ fn unwrap_or_else_default() {
 
     let with_default_type = Some(1);
     with_default_type.unwrap_or_default();
+
+    let with_default_type: Option<Vec<u64>> = None;
+    with_default_type.unwrap_or_default();
 }
 
 fn main() {}

tests/ui/unwrap_or_else_default.rs

@@ -66,6 +66,9 @@ fn unwrap_or_else_default() {
 
     let with_default_type = Some(1);
     with_default_type.unwrap_or_else(u64::default);
+
+    let with_default_type: Option<Vec<u64>> = None;
+    with_default_type.unwrap_or_else(Vec::new);
 }
 
 fn main() {}

tests/ui/unwrap_or_else_default.stderr

@@ -1,10 +1,16 @@
+error: use of `.unwrap_or_else(..)` to construct default value
+  --> $DIR/unwrap_or_else_default.rs:48:5
+   |
+LL |     with_new.unwrap_or_else(Vec::new);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_new.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:62:5
    |
 LL |     with_real_default.unwrap_or_else(<HasDefaultAndDuplicate as Default>::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
@@ -18,5 +24,11 @@ error: use of `.unwrap_or_else(..)` to construct default value
 LL |     with_default_type.unwrap_or_else(u64::default);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_default_type.unwrap_or_default()`
 
-error: aborting due to 3 previous errors
+error: use of `.unwrap_or_else(..)` to construct default value
+  --> $DIR/unwrap_or_else_default.rs:71:5
+   |
+LL |     with_default_type.unwrap_or_else(Vec::new);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_default_type.unwrap_or_default()`
+
+error: aborting due to 5 previous errors