Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 3 additions & 4 deletions clippy_lints/src/collapsible_if.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
50 changes: 32 additions & 18 deletions clippy_lints/src/unwrap.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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};

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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`");
}
}
},
);
Expand All @@ -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(
Expand All @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
}
24 changes: 24 additions & 0 deletions tests/ui/checked_unwrap/if_let_chains.rs
Original file line number Diff line number Diff line change
@@ -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`
}
}
29 changes: 29 additions & 0 deletions tests/ui/checked_unwrap/if_let_chains.stderr
Original file line number Diff line number Diff line change
@@ -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