Auto merge of rust-lang#9148 - arieluy:then_some_unwrap_or, r=Jarcho

Add new lint `obfuscated_if_else`

part of rust-lang#9100, additional commits could make it work with `then` and `unwrap_or_else` as well

changelog: Add new lint `obfuscated_if_else`

Author: bors

CHANGELOG.md

@@ -3793,6 +3793,7 @@ Released 2018-09-13
 [`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options
 [`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces
 [`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref
+[`obfuscated_if_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#obfuscated_if_else
 [`octal_escapes`]: https://rust-lang.github.io/rust-clippy/master/index.html#octal_escapes
 [`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect
 [`only_used_in_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#only_used_in_recursion

clippy_lints/src/lib.register_all.rs

@@ -187,6 +187,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(methods::NEEDLESS_SPLITN),
     LintId::of(methods::NEW_RET_NO_SELF),
     LintId::of(methods::NO_EFFECT_REPLACE),
+    LintId::of(methods::OBFUSCATED_IF_ELSE),
     LintId::of(methods::OK_EXPECT),
     LintId::of(methods::OPTION_AS_REF_DEREF),
     LintId::of(methods::OPTION_FILTER_MAP),

clippy_lints/src/lib.register_lints.rs

@@ -330,6 +330,7 @@ store.register_lints(&[
     methods::NEEDLESS_SPLITN,
     methods::NEW_RET_NO_SELF,
     methods::NO_EFFECT_REPLACE,
+    methods::OBFUSCATED_IF_ELSE,
     methods::OK_EXPECT,
     methods::OPTION_AS_REF_DEREF,
     methods::OPTION_FILTER_MAP,

clippy_lints/src/lib.register_style.rs

@@ -70,6 +70,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(methods::MANUAL_SATURATING_ARITHMETIC),
     LintId::of(methods::MAP_COLLECT_RESULT_UNIT),
     LintId::of(methods::NEW_RET_NO_SELF),
+    LintId::of(methods::OBFUSCATED_IF_ELSE),
     LintId::of(methods::OK_EXPECT),
     LintId::of(methods::OPTION_MAP_OR_NONE),
     LintId::of(methods::RESULT_MAP_OR_INTO_OPTION),

clippy_lints/src/methods/mod.rs

@@ -46,6 +46,7 @@ mod map_unwrap_or;
 mod needless_option_as_deref;
 mod needless_option_take;
 mod no_effect_replace;
+mod obfuscated_if_else;
 mod ok_expect;
 mod option_as_ref_deref;
 mod option_map_or_none;
@@ -2263,6 +2264,35 @@ declare_clippy_lint! {
     "replace with no effect"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usages of `.then_some(..).unwrap_or(..)`
+    ///
+    /// ### Why is this bad?
+    /// This can be written more clearly with `if .. else ..`
+    ///
+    /// ### Limitations
+    /// This lint currently only looks for usages of
+    /// `.then_some(..).unwrap_or(..)`, but will be expanded
+    /// to account for similar patterns.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let x = true;
+    /// x.then_some("a").unwrap_or("b");
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let x = true;
+    /// if x { "a" } else { "b" };
+    /// ```
+    #[clippy::version = "1.64.0"]
+    pub OBFUSCATED_IF_ELSE,
+    style,
+    "use of `.then_some(..).unwrap_or(..)` can be written \
+    more clearly with `if .. else ..`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Option<RustcVersion>,
@@ -2364,6 +2394,7 @@ impl_lint_pass!(Methods => [
     IS_DIGIT_ASCII_RADIX,
     NEEDLESS_OPTION_TAKE,
     NO_EFFECT_REPLACE,
+    OBFUSCATED_IF_ELSE,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -2772,6 +2803,9 @@ impl Methods {
                     Some(("map", [m_recv, m_arg], span)) => {
                         option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span);
                     },
+                    Some(("then_some", [t_recv, t_arg], _)) => {
+                        obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg);
+                    },
                     _ => {},
                 },
                 ("unwrap_or_else", [u_arg]) => match method_call(recv) {

clippy_lints/src/methods/obfuscated_if_else.rs

@@ -0,0 +1,42 @@
+// run-rustfix
+
+use super::OBFUSCATED_IF_ELSE;
+use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet_with_applicability};
+use rustc_errors::Applicability;
+use rustc_hir as hir;
+use rustc_lint::LateContext;
+
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx hir::Expr<'_>,
+    then_recv: &'tcx hir::Expr<'_>,
+    then_arg: &'tcx hir::Expr<'_>,
+    unwrap_arg: &'tcx hir::Expr<'_>,
+) {
+    // something.then_some(blah).unwrap_or(blah)
+    // ^^^^^^^^^-then_recv ^^^^-then_arg   ^^^^- unwrap_arg
+    // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- expr
+
+    let recv_ty = cx.typeck_results().expr_ty(then_recv);
+
+    if recv_ty.is_bool() {
+        let mut applicability = Applicability::MachineApplicable;
+        let sugg = format!(
+            "if {} {{ {} }} else {{ {} }}",
+            snippet_with_applicability(cx, then_recv.span, "..", &mut applicability),
+            snippet_with_applicability(cx, then_arg.span, "..", &mut applicability),
+            snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability)
+        );
+
+        span_lint_and_sugg(
+            cx,
+            OBFUSCATED_IF_ELSE,
+            expr.span,
+            "use of `.then_some(..).unwrap_or(..)` can be written \
+            more clearly with `if .. else ..`",
+            "try",
+            sugg,
+            applicability,
+        );
+    }
+}

tests/ui/obfuscated_if_else.fixed

@@ -0,0 +1,7 @@
+// run-rustfix
+
+#![warn(clippy::obfuscated_if_else)]
+
+fn main() {
+    if true { "a" } else { "b" };
+}

tests/ui/obfuscated_if_else.rs

@@ -0,0 +1,7 @@
+// run-rustfix
+
+#![warn(clippy::obfuscated_if_else)]
+
+fn main() {
+    true.then_some("a").unwrap_or("b");
+}

tests/ui/obfuscated_if_else.stderr

@@ -0,0 +1,10 @@
+error: use of `.then_some(..).unwrap_or(..)` can be written more clearly with `if .. else ..`
+  --> $DIR/obfuscated_if_else.rs:6:5
+   |
+LL |     true.then_some("a").unwrap_or("b");
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { "a" } else { "b" }`
+   |
+   = note: `-D clippy::obfuscated-if-else` implied by `-D warnings`
+
+error: aborting due to previous error
+