diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index f8263bb8852a..e7e0670357b3 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -898,6 +898,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio * [`unchecked_duration_subtraction`](https://rust-lang.github.io/rust-clippy/master/index.html#unchecked_duration_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) diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index e14e15a022f7..df8495dfe0ec 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -794,6 +794,7 @@ define_Conf! { unchecked_duration_subtraction, uninlined_format_args, unnecessary_lazy_evaluations, + unnecessary_unwrap, unnested_or_patterns, unused_trait_names, use_self, diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index b239c20ab4d8..b13e307a3f9c 100644 --- a/clippy_lints/src/collapsible_if.rs +++ b/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 diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index c56fa257b068..1a706df567e5 100644 --- a/clippy_lints/src/lib.rs +++ b/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)); diff --git a/clippy_lints/src/unwrap.rs b/clippy_lints/src/unwrap.rs index 34dfe5b6546f..aee8028a75de 100644 --- a/clippy_lints/src/unwrap.rs +++ b/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>, 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> { - 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); diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index feadc0ecf659..1e0bd3a2c4dc 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -129,6 +129,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; @@ -3659,3 +3660,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) +} diff --git a/tests/ui/checked_unwrap/if_let_chains.rs b/tests/ui/checked_unwrap/if_let_chains.rs new file mode 100644 index 000000000000..cfa7715965cd --- /dev/null +++ b/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, b: Option) { + 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, b: Option) { + 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` + } +} diff --git a/tests/ui/checked_unwrap/if_let_chains.stderr b/tests/ui/checked_unwrap/if_let_chains.stderr new file mode 100644 index 000000000000..8a4137de37a3 --- /dev/null +++ b/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 +