Do not suggest using a if let chain if it is not supported (#15746)

This might be due to a low edition (< 2024) or too low a MSRV. In this
case, we will suggest only `match`.

Fixes #15744

changelog: [`unnecessary_unwrap`]: do not suggest using `if let` chains
if this is not supported with the current edition or MSRV
changelog:[`collapsible_if`]: Do not suggest using `if let` if this is
not supported with the current edition or MSRV

Author: blyxyas

book/src/lint_configuration.md

@@ -898,6 +898,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
 * [`unchecked_time_subtraction`](https://rust-lang.github.io/rust-clippy/master/index.html#unchecked_time_subtraction)
 * [`uninlined_format_args`](https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args)
 * [`unnecessary_lazy_evaluations`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations)
+* [`unnecessary_unwrap`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap)
 * [`unnested_or_patterns`](https://rust-lang.github.io/rust-clippy/master/index.html#unnested_or_patterns)
 * [`unused_trait_names`](https://rust-lang.github.io/rust-clippy/master/index.html#unused_trait_names)
 * [`use_self`](https://rust-lang.github.io/rust-clippy/master/index.html#use_self)

clippy_config/src/conf.rs

@@ -794,6 +794,7 @@ define_Conf! {
         unchecked_time_subtraction,
         uninlined_format_args,
         unnecessary_lazy_evaluations,
+        unnecessary_unwrap,
         unnested_or_patterns,
         unused_trait_names,
         use_self,

clippy_lints/src/collapsible_if.rs

@@ -1,8 +1,8 @@
 use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_hir_and_then;
-use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::msrvs::Msrv;
 use clippy_utils::source::{IntoSpan as _, SpanRangeExt, snippet, snippet_block_with_applicability};
-use clippy_utils::{span_contains_non_whitespace, sym, tokenize_with_text};
+use clippy_utils::{can_use_if_let_chains, span_contains_non_whitespace, sym, tokenize_with_text};
 use rustc_ast::{BinOpKind, MetaItemInner};
 use rustc_errors::Applicability;
 use rustc_hir::{Block, Expr, ExprKind, StmtKind};
@@ -216,8 +216,7 @@ impl CollapsibleIf {
     }
 
     fn eligible_condition(&self, cx: &LateContext<'_>, cond: &Expr<'_>) -> bool {
-        !matches!(cond.kind, ExprKind::Let(..))
-            || (cx.tcx.sess.edition().at_least_rust_2024() && self.msrv.meets(cx, msrvs::LET_CHAINS))
+        !matches!(cond.kind, ExprKind::Let(..)) || can_use_if_let_chains(cx, self.msrv)
     }
 
     // Check that nothing significant can be found between the initial `{` of `inner_if` and

clippy_lints/src/lib.rs

@@ -589,7 +589,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
     store.register_late_pass(|_| Box::new(map_unit_fn::MapUnit));
     store.register_late_pass(|_| Box::new(inherent_impl::MultipleInherentImpl));
     store.register_late_pass(|_| Box::new(neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd));
-    store.register_late_pass(|_| Box::new(unwrap::Unwrap));
+    store.register_late_pass(move |_| Box::new(unwrap::Unwrap::new(conf)));
     store.register_late_pass(move |_| Box::new(indexing_slicing::IndexingSlicing::new(conf)));
     store.register_late_pass(move |tcx| Box::new(non_copy_const::NonCopyConst::new(tcx, conf)));
     store.register_late_pass(|_| Box::new(redundant_clone::RedundantClone));

clippy_lints/src/unwrap.rs

@@ -1,7 +1,9 @@
+use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_hir_and_then;
-use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::msrvs::Msrv;
+use clippy_utils::ty::get_type_diagnostic_name;
 use clippy_utils::usage::is_potentially_local_place;
-use clippy_utils::{higher, path_to_local, sym};
+use clippy_utils::{can_use_if_let_chains, higher, path_to_local, sym};
 use rustc_errors::Applicability;
 use rustc_hir::intravisit::{FnKind, Visitor, walk_expr, walk_fn};
 use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, HirId, Node, UnOp};
@@ -10,7 +12,7 @@ use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::hir::nested_filter;
 use rustc_middle::mir::FakeReadCause;
 use rustc_middle::ty::{self, Ty, TyCtxt};
-use rustc_session::declare_lint_pass;
+use rustc_session::impl_lint_pass;
 use rustc_span::def_id::LocalDefId;
 use rustc_span::{Span, Symbol};
 
@@ -72,10 +74,21 @@ declare_clippy_lint! {
     "checks for calls of `unwrap[_err]()` that will always fail"
 }
 
+pub(crate) struct Unwrap {
+    msrv: Msrv,
+}
+
+impl Unwrap {
+    pub fn new(conf: &'static Conf) -> Self {
+        Self { msrv: conf.msrv }
+    }
+}
+
 /// Visitor that keeps track of which variables are unwrappable.
 struct UnwrappableVariablesVisitor<'a, 'tcx> {
     unwrappables: Vec<UnwrapInfo<'tcx>>,
     cx: &'a LateContext<'tcx>,
+    msrv: Msrv,
 }
 
 /// What kind of unwrappable this is.
@@ -133,12 +146,14 @@ fn collect_unwrap_info<'tcx>(
     invert: bool,
     is_entire_condition: bool,
 ) -> Vec<UnwrapInfo<'tcx>> {
-    fn is_relevant_option_call(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> bool {
-        is_type_diagnostic_item(cx, ty, sym::Option) && matches!(method_name, sym::is_none | sym::is_some)
-    }
-
-    fn is_relevant_result_call(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> bool {
-        is_type_diagnostic_item(cx, ty, sym::Result) && matches!(method_name, sym::is_err | sym::is_ok)
+    fn option_or_result_call(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> Option<(UnwrappableKind, bool)> {
+        match (get_type_diagnostic_name(cx, ty)?, method_name) {
+            (sym::Option, sym::is_some) => Some((UnwrappableKind::Option, true)),
+            (sym::Option, sym::is_none) => Some((UnwrappableKind::Option, false)),
+            (sym::Result, sym::is_ok) => Some((UnwrappableKind::Result, true)),
+            (sym::Result, sym::is_err) => Some((UnwrappableKind::Result, false)),
+            _ => None,
+        }
     }
 
     match expr.kind {
@@ -157,15 +172,9 @@ fn collect_unwrap_info<'tcx>(
             if let Some(local_id) = path_to_local(receiver)
                 && let ty = cx.typeck_results().expr_ty(receiver)
                 && let name = method_name.ident.name
-                && (is_relevant_option_call(cx, ty, name) || is_relevant_result_call(cx, ty, name)) =>
+                && let Some((kind, unwrappable)) = option_or_result_call(cx, ty, name) =>
         {
-            let unwrappable = matches!(name, sym::is_some | sym::is_ok);
             let safe_to_unwrap = unwrappable != invert;
-            let kind = if is_type_diagnostic_item(cx, ty, sym::Option) {
-                UnwrappableKind::Option
-            } else {
-                UnwrappableKind::Result
-            };
 
             vec![UnwrapInfo {
                 local_id,
@@ -357,7 +366,11 @@ impl<'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'_, 'tcx> {
                                 );
                             } else {
                                 diag.span_label(unwrappable.check.span, "the check is happening here");
-                                diag.help("try using `if let` or `match`");
+                                if can_use_if_let_chains(self.cx, self.msrv) {
+                                    diag.help("try using `if let` or `match`");
+                                } else {
+                                    diag.help("try using `match`");
+                                }
                             }
                         },
                     );
@@ -383,7 +396,7 @@ impl<'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'_, 'tcx> {
     }
 }
 
-declare_lint_pass!(Unwrap => [PANICKING_UNWRAP, UNNECESSARY_UNWRAP]);
+impl_lint_pass!(Unwrap => [PANICKING_UNWRAP, UNNECESSARY_UNWRAP]);
 
 impl<'tcx> LateLintPass<'tcx> for Unwrap {
     fn check_fn(
@@ -402,6 +415,7 @@ impl<'tcx> LateLintPass<'tcx> for Unwrap {
         let mut v = UnwrappableVariablesVisitor {
             unwrappables: Vec::new(),
             cx,
+            msrv: self.msrv,
         };
 
         walk_fn(&mut v, kind, decl, body.id(), fn_id);

clippy_utils/src/lib.rs

@@ -128,6 +128,7 @@ use visitors::{Visitable, for_each_unconsumed_temporary};
 use crate::ast_utils::unordered_over;
 use crate::consts::{ConstEvalCtxt, Constant, mir_to_const};
 use crate::higher::Range;
+use crate::msrvs::Msrv;
 use crate::ty::{adt_and_variant_of_res, can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type};
 use crate::visitors::for_each_expr_without_closures;
 
@@ -3649,3 +3650,8 @@ pub fn is_expr_async_block(expr: &Expr<'_>) -> bool {
         })
     )
 }
+
+/// Checks if the chosen edition and `msrv` allows using `if let` chains.
+pub fn can_use_if_let_chains(cx: &LateContext<'_>, msrv: Msrv) -> bool {
+    cx.tcx.sess.edition().at_least_rust_2024() && msrv.meets(cx, msrvs::LET_CHAINS)
+}

tests/ui/checked_unwrap/if_let_chains.rs

@@ -0,0 +1,24 @@
+//@require-annotations-for-level: ERROR
+#![deny(clippy::unnecessary_unwrap)]
+
+#[clippy::msrv = "1.85"]
+fn if_let_chains_unsupported(a: Option<u32>, b: Option<u32>) {
+    if a.is_none() || b.is_none() {
+        println!("a or b is not set");
+    } else {
+        println!("the value of a is {}", a.unwrap());
+        //~^ unnecessary_unwrap
+        //~| HELP: try using `match`
+    }
+}
+
+#[clippy::msrv = "1.88"]
+fn if_let_chains_supported(a: Option<u32>, b: Option<u32>) {
+    if a.is_none() || b.is_none() {
+        println!("a or b is not set");
+    } else {
+        println!("the value of a is {}", a.unwrap());
+        //~^ unnecessary_unwrap
+        //~| HELP: try using `if let` or `match`
+    }
+}

tests/ui/checked_unwrap/if_let_chains.stderr

@@ -0,0 +1,29 @@
+error: called `unwrap` on `a` after checking its variant with `is_none`
+  --> tests/ui/checked_unwrap/if_let_chains.rs:9:42
+   |
+LL |     if a.is_none() || b.is_none() {
+   |        ----------- the check is happening here
+...
+LL |         println!("the value of a is {}", a.unwrap());
+   |                                          ^^^^^^^^^^
+   |
+   = help: try using `match`
+note: the lint level is defined here
+  --> tests/ui/checked_unwrap/if_let_chains.rs:2:9
+   |
+LL | #![deny(clippy::unnecessary_unwrap)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: called `unwrap` on `a` after checking its variant with `is_none`
+  --> tests/ui/checked_unwrap/if_let_chains.rs:20:42
+   |
+LL |     if a.is_none() || b.is_none() {
+   |        ----------- the check is happening here
+...
+LL |         println!("the value of a is {}", a.unwrap());
+   |                                          ^^^^^^^^^^
+   |
+   = help: try using `if let` or `match`
+
+error: aborting due to 2 previous errors
+