From b21d9d307b5fdb1e423d786f9899757b42957b07 Mon Sep 17 00:00:00 2001 From: xphoniex Date: Tue, 6 Sep 2022 04:38:29 +0000 Subject: [PATCH] Suggest `unwrap_or_default` when closure returns `"".to_string` Signed-off-by: xphoniex --- .../src/methods/unwrap_or_else_default.rs | 24 +++++++++++++++++-- tests/ui/unwrap_or_else_default.fixed | 3 +++ tests/ui/unwrap_or_else_default.rs | 3 +++ tests/ui/unwrap_or_else_default.stderr | 8 ++++++- 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/methods/unwrap_or_else_default.rs b/clippy_lints/src/methods/unwrap_or_else_default.rs index f3af281d6ca..d3630b7d08e 100644 --- a/clippy_lints/src/methods/unwrap_or_else_default.rs +++ b/clippy_lints/src/methods/unwrap_or_else_default.rs @@ -5,10 +5,11 @@ use clippy_utils::{ diagnostics::span_lint_and_sugg, is_default_equivalent_call, source::snippet_with_applicability, ty::is_type_diagnostic_item, }; +use rustc_ast::ast::LitKind; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; -use rustc_span::sym; +use rustc_span::{sym, symbol}; pub(super) fn check<'tcx>( cx: &LateContext<'tcx>, @@ -25,7 +26,7 @@ pub(super) fn check<'tcx>( if_chain! { if is_option || is_result; - if is_default_equivalent_call(cx, u_arg); + if closure_body_returns_empty_to_string(cx, u_arg) || is_default_equivalent_call(cx, u_arg); then { let mut applicability = Applicability::MachineApplicable; @@ -44,3 +45,22 @@ pub(super) fn check<'tcx>( } } } + +fn closure_body_returns_empty_to_string(cx: &LateContext<'_>, e: &hir::Expr<'_>) -> bool { + if let hir::ExprKind::Closure(&hir::Closure { body, .. }) = e.kind { + let body = cx.tcx.hir().body(body); + + if body.params.is_empty() + && let hir::Expr{ kind, .. } = &body.value + && let hir::ExprKind::MethodCall(hir::PathSegment {ident, ..}, [self_arg], _) = kind + && ident == &symbol::Ident::from_str("to_string") + && let hir::Expr{ kind, .. } = self_arg + && let hir::ExprKind::Lit(lit) = kind + && let LitKind::Str(symbol::kw::Empty, _) = lit.node + { + return true; + } + } + + false +} diff --git a/tests/ui/unwrap_or_else_default.fixed b/tests/ui/unwrap_or_else_default.fixed index c2b9bd2c881..84f779569ff 100644 --- a/tests/ui/unwrap_or_else_default.fixed +++ b/tests/ui/unwrap_or_else_default.fixed @@ -69,6 +69,9 @@ fn unwrap_or_else_default() { let with_default_type: Option> = None; with_default_type.unwrap_or_default(); + + let empty_string = None::; + empty_string.unwrap_or_default(); } fn main() {} diff --git a/tests/ui/unwrap_or_else_default.rs b/tests/ui/unwrap_or_else_default.rs index d55664990ae..1735bd5808e 100644 --- a/tests/ui/unwrap_or_else_default.rs +++ b/tests/ui/unwrap_or_else_default.rs @@ -69,6 +69,9 @@ fn unwrap_or_else_default() { let with_default_type: Option> = None; with_default_type.unwrap_or_else(Vec::new); + + let empty_string = None::; + empty_string.unwrap_or_else(|| "".to_string()); } fn main() {} diff --git a/tests/ui/unwrap_or_else_default.stderr b/tests/ui/unwrap_or_else_default.stderr index 53e31d85edf..d2b9212223f 100644 --- a/tests/ui/unwrap_or_else_default.stderr +++ b/tests/ui/unwrap_or_else_default.stderr @@ -30,5 +30,11 @@ error: use of `.unwrap_or_else(..)` to construct default value LL | with_default_type.unwrap_or_else(Vec::new); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_default_type.unwrap_or_default()` -error: aborting due to 5 previous errors +error: use of `.unwrap_or_else(..)` to construct default value + --> $DIR/unwrap_or_else_default.rs:74:5 + | +LL | empty_string.unwrap_or_else(|| "".to_string()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `empty_string.unwrap_or_default()` + +error: aborting due to 6 previous errors