Skip to content

Commit 79b9982

Browse files
committed
Do not suggest using a if let chain if it is not supported
This might be due to a low edition (< 2024) or too low a MSRV.
1 parent 11b19dc commit 79b9982

File tree

6 files changed

+81
-8
lines changed

6 files changed

+81
-8
lines changed

book/src/lint_configuration.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -897,6 +897,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
897897
* [`unchecked_duration_subtraction`](https://rust-lang.github.io/rust-clippy/master/index.html#unchecked_duration_subtraction)
898898
* [`uninlined_format_args`](https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args)
899899
* [`unnecessary_lazy_evaluations`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations)
900+
* [`unnecessary_unwrap`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap)
900901
* [`unnested_or_patterns`](https://rust-lang.github.io/rust-clippy/master/index.html#unnested_or_patterns)
901902
* [`unused_trait_names`](https://rust-lang.github.io/rust-clippy/master/index.html#unused_trait_names)
902903
* [`use_self`](https://rust-lang.github.io/rust-clippy/master/index.html#use_self)

clippy_config/src/conf.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,7 @@ define_Conf! {
793793
unchecked_duration_subtraction,
794794
uninlined_format_args,
795795
unnecessary_lazy_evaluations,
796+
unnecessary_unwrap,
796797
unnested_or_patterns,
797798
unused_trait_names,
798799
use_self,

clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
589589
store.register_late_pass(|_| Box::new(map_unit_fn::MapUnit));
590590
store.register_late_pass(|_| Box::new(inherent_impl::MultipleInherentImpl));
591591
store.register_late_pass(|_| Box::new(neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd));
592-
store.register_late_pass(|_| Box::new(unwrap::Unwrap));
592+
store.register_late_pass(move |_| Box::new(unwrap::Unwrap::new(conf)));
593593
store.register_late_pass(move |_| Box::new(indexing_slicing::IndexingSlicing::new(conf)));
594594
store.register_late_pass(move |tcx| Box::new(non_copy_const::NonCopyConst::new(tcx, conf)));
595595
store.register_late_pass(|_| Box::new(redundant_clone::RedundantClone));

clippy_lints/src/unwrap.rs

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1+
use clippy_config::Conf;
12
use clippy_utils::diagnostics::span_lint_hir_and_then;
3+
use clippy_utils::msrvs::Msrv;
24
use clippy_utils::ty::get_type_diagnostic_name;
35
use clippy_utils::usage::is_potentially_local_place;
4-
use clippy_utils::{higher, path_to_local, sym};
6+
use clippy_utils::{can_use_if_let_chains, higher, path_to_local, sym};
57
use rustc_errors::Applicability;
68
use rustc_hir::intravisit::{FnKind, Visitor, walk_expr, walk_fn};
79
use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, HirId, Node, UnOp};
@@ -10,7 +12,7 @@ use rustc_lint::{LateContext, LateLintPass};
1012
use rustc_middle::hir::nested_filter;
1113
use rustc_middle::mir::FakeReadCause;
1214
use rustc_middle::ty::{self, Ty, TyCtxt};
13-
use rustc_session::declare_lint_pass;
15+
use rustc_session::impl_lint_pass;
1416
use rustc_span::def_id::LocalDefId;
1517
use rustc_span::{Span, Symbol};
1618

@@ -72,10 +74,21 @@ declare_clippy_lint! {
7274
"checks for calls of `unwrap[_err]()` that will always fail"
7375
}
7476

77+
pub(crate) struct Unwrap {
78+
msrv: Msrv,
79+
}
80+
81+
impl Unwrap {
82+
pub fn new(conf: &'static Conf) -> Self {
83+
Self { msrv: conf.msrv }
84+
}
85+
}
86+
7587
/// Visitor that keeps track of which variables are unwrappable.
76-
struct UnwrappableVariablesVisitor<'a, 'tcx> {
88+
struct UnwrappableVariablesVisitor<'a, 'tcx, 'unwrap> {
7789
unwrappables: Vec<UnwrapInfo<'tcx>>,
7890
cx: &'a LateContext<'tcx>,
91+
unwrap: &'unwrap Unwrap,
7992
}
8093

8194
/// What kind of unwrappable this is.
@@ -231,7 +244,7 @@ impl<'tcx> Delegate<'tcx> for MutationVisitor<'tcx> {
231244
fn fake_read(&mut self, _: &PlaceWithHirId<'tcx>, _: FakeReadCause, _: HirId) {}
232245
}
233246

234-
impl<'tcx> UnwrappableVariablesVisitor<'_, 'tcx> {
247+
impl<'tcx> UnwrappableVariablesVisitor<'_, 'tcx, '_> {
235248
fn visit_branch(
236249
&mut self,
237250
if_expr: &'tcx Expr<'_>,
@@ -282,7 +295,7 @@ fn consume_option_as_ref<'tcx>(expr: &'tcx Expr<'tcx>) -> (&'tcx Expr<'tcx>, Opt
282295
}
283296
}
284297

285-
impl<'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'_, 'tcx> {
298+
impl<'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'_, 'tcx, '_> {
286299
type NestedFilter = nested_filter::OnlyBodies;
287300

288301
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
@@ -353,7 +366,11 @@ impl<'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'_, 'tcx> {
353366
);
354367
} else {
355368
diag.span_label(unwrappable.check.span, "the check is happening here");
356-
diag.help("try using `if let` or `match`");
369+
if can_use_if_let_chains(self.cx, self.unwrap.msrv) {
370+
diag.help("try using `if let` or `match`");
371+
} else {
372+
diag.help("try using `match`");
373+
}
357374
}
358375
},
359376
);
@@ -379,7 +396,7 @@ impl<'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'_, 'tcx> {
379396
}
380397
}
381398

382-
declare_lint_pass!(Unwrap => [PANICKING_UNWRAP, UNNECESSARY_UNWRAP]);
399+
impl_lint_pass!(Unwrap => [PANICKING_UNWRAP, UNNECESSARY_UNWRAP]);
383400

384401
impl<'tcx> LateLintPass<'tcx> for Unwrap {
385402
fn check_fn(
@@ -398,6 +415,7 @@ impl<'tcx> LateLintPass<'tcx> for Unwrap {
398415
let mut v = UnwrappableVariablesVisitor {
399416
unwrappables: Vec::new(),
400417
cx,
418+
unwrap: self,
401419
};
402420

403421
walk_fn(&mut v, kind, decl, body.id(), fn_id);
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//@require-annotations-for-level: ERROR
2+
#![deny(clippy::unnecessary_unwrap)]
3+
4+
#[clippy::msrv = "1.85"]
5+
fn if_let_chains_unsupported(a: Option<u32>, b: Option<u32>) {
6+
if a.is_none() || b.is_none() {
7+
println!("a or b is not set");
8+
} else {
9+
println!("the value of a is {}", a.unwrap());
10+
//~^ unnecessary_unwrap
11+
//~| HELP: try using `match`
12+
}
13+
}
14+
15+
#[clippy::msrv = "1.88"]
16+
fn if_let_chains_supported(a: Option<u32>, b: Option<u32>) {
17+
if a.is_none() || b.is_none() {
18+
println!("a or b is not set");
19+
} else {
20+
println!("the value of a is {}", a.unwrap());
21+
//~^ unnecessary_unwrap
22+
//~| HELP: try using `if let` or `match`
23+
}
24+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
error: called `unwrap` on `a` after checking its variant with `is_none`
2+
--> tests/ui/checked_unwrap/if_let_chains.rs:9:42
3+
|
4+
LL | if a.is_none() || b.is_none() {
5+
| ----------- the check is happening here
6+
...
7+
LL | println!("the value of a is {}", a.unwrap());
8+
| ^^^^^^^^^^
9+
|
10+
= help: try using `match`
11+
note: the lint level is defined here
12+
--> tests/ui/checked_unwrap/if_let_chains.rs:2:9
13+
|
14+
LL | #![deny(clippy::unnecessary_unwrap)]
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
16+
17+
error: called `unwrap` on `a` after checking its variant with `is_none`
18+
--> tests/ui/checked_unwrap/if_let_chains.rs:20:42
19+
|
20+
LL | if a.is_none() || b.is_none() {
21+
| ----------- the check is happening here
22+
...
23+
LL | println!("the value of a is {}", a.unwrap());
24+
| ^^^^^^^^^^
25+
|
26+
= help: try using `if let` or `match`
27+
28+
error: aborting due to 2 previous errors
29+

0 commit comments

Comments
 (0)