Auto merge of #11030 - darklyspaced:master, r=Centri3,xFrednet
suggests `is_some_and` over `map().unwrap` changelog: Enhancement: [`option_map_unwrap_or`] now considers the [`msrv`] config when creating the suggestion. * modified option_map_unwrap_or lint to recognise when an `Option<T>` is mapped to an `Option<bool>` with false being used when `None` is detected; suggests the use of `is_some_and` instead * msrv is set to 1.70.0 for this lint; when `is_some_and` was stabilised fixes #9125
This commit is contained in:
commit
9020937bbe
@ -108,6 +108,7 @@ The minimum rust version that the project supports
|
|||||||
* [`manual_str_repeat`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat)
|
* [`manual_str_repeat`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat)
|
||||||
* [`cloned_instead_of_copied`](https://rust-lang.github.io/rust-clippy/master/index.html#cloned_instead_of_copied)
|
* [`cloned_instead_of_copied`](https://rust-lang.github.io/rust-clippy/master/index.html#cloned_instead_of_copied)
|
||||||
* [`redundant_field_names`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names)
|
* [`redundant_field_names`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names)
|
||||||
|
* [`option_map_unwrap_or`](https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unwrap_or)
|
||||||
* [`redundant_static_lifetimes`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes)
|
* [`redundant_static_lifetimes`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes)
|
||||||
* [`filter_map_next`](https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next)
|
* [`filter_map_next`](https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next)
|
||||||
* [`checked_conversions`](https://rust-lang.github.io/rust-clippy/master/index.html#checked_conversions)
|
* [`checked_conversions`](https://rust-lang.github.io/rust-clippy/master/index.html#checked_conversions)
|
||||||
|
@ -513,6 +513,7 @@
|
|||||||
/// # let result: Result<usize, ()> = Ok(1);
|
/// # let result: Result<usize, ()> = Ok(1);
|
||||||
/// # fn some_function(foo: ()) -> usize { 1 }
|
/// # fn some_function(foo: ()) -> usize { 1 }
|
||||||
/// option.map(|a| a + 1).unwrap_or(0);
|
/// option.map(|a| a + 1).unwrap_or(0);
|
||||||
|
/// option.map(|a| a > 10).unwrap_or(false);
|
||||||
/// result.map(|a| a + 1).unwrap_or_else(some_function);
|
/// result.map(|a| a + 1).unwrap_or_else(some_function);
|
||||||
/// ```
|
/// ```
|
||||||
///
|
///
|
||||||
@ -522,6 +523,7 @@
|
|||||||
/// # let result: Result<usize, ()> = Ok(1);
|
/// # let result: Result<usize, ()> = Ok(1);
|
||||||
/// # fn some_function(foo: ()) -> usize { 1 }
|
/// # fn some_function(foo: ()) -> usize { 1 }
|
||||||
/// option.map_or(0, |a| a + 1);
|
/// option.map_or(0, |a| a + 1);
|
||||||
|
/// option.is_some_and(|a| a > 10);
|
||||||
/// result.map_or_else(some_function, |a| a + 1);
|
/// result.map_or_else(some_function, |a| a + 1);
|
||||||
/// ```
|
/// ```
|
||||||
#[clippy::version = "1.45.0"]
|
#[clippy::version = "1.45.0"]
|
||||||
@ -3904,7 +3906,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
|
|||||||
manual_saturating_arithmetic::check(cx, expr, lhs, rhs, u_arg, &arith["checked_".len()..]);
|
manual_saturating_arithmetic::check(cx, expr, lhs, rhs, u_arg, &arith["checked_".len()..]);
|
||||||
},
|
},
|
||||||
Some(("map", m_recv, [m_arg], span, _)) => {
|
Some(("map", m_recv, [m_arg], span, _)) => {
|
||||||
option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span);
|
option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span, &self.msrv);
|
||||||
},
|
},
|
||||||
Some(("then_some", t_recv, [t_arg], _, _)) => {
|
Some(("then_some", t_recv, [t_arg], _, _)) => {
|
||||||
obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg);
|
obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg);
|
||||||
|
@ -1,4 +1,5 @@
|
|||||||
use clippy_utils::diagnostics::span_lint_and_then;
|
use clippy_utils::diagnostics::span_lint_and_then;
|
||||||
|
use clippy_utils::msrvs::{self, Msrv};
|
||||||
use clippy_utils::source::snippet_with_applicability;
|
use clippy_utils::source::snippet_with_applicability;
|
||||||
use clippy_utils::ty::is_copy;
|
use clippy_utils::ty::is_copy;
|
||||||
use clippy_utils::ty::is_type_diagnostic_item;
|
use clippy_utils::ty::is_type_diagnostic_item;
|
||||||
@ -19,6 +20,7 @@
|
|||||||
use super::MAP_UNWRAP_OR;
|
use super::MAP_UNWRAP_OR;
|
||||||
|
|
||||||
/// lint use of `map().unwrap_or()` for `Option`s
|
/// lint use of `map().unwrap_or()` for `Option`s
|
||||||
|
#[expect(clippy::too_many_arguments)]
|
||||||
pub(super) fn check<'tcx>(
|
pub(super) fn check<'tcx>(
|
||||||
cx: &LateContext<'tcx>,
|
cx: &LateContext<'tcx>,
|
||||||
expr: &rustc_hir::Expr<'_>,
|
expr: &rustc_hir::Expr<'_>,
|
||||||
@ -27,6 +29,7 @@ pub(super) fn check<'tcx>(
|
|||||||
unwrap_recv: &rustc_hir::Expr<'_>,
|
unwrap_recv: &rustc_hir::Expr<'_>,
|
||||||
unwrap_arg: &'tcx rustc_hir::Expr<'_>,
|
unwrap_arg: &'tcx rustc_hir::Expr<'_>,
|
||||||
map_span: Span,
|
map_span: Span,
|
||||||
|
msrv: &Msrv,
|
||||||
) {
|
) {
|
||||||
// lint if the caller of `map()` is an `Option`
|
// lint if the caller of `map()` is an `Option`
|
||||||
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Option) {
|
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Option) {
|
||||||
@ -74,16 +77,29 @@ pub(super) fn check<'tcx>(
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// is_some_and is stabilised && `unwrap_or` argument is false; suggest `is_some_and` instead
|
||||||
|
let suggest_is_some_and = msrv.meets(msrvs::OPTION_IS_SOME_AND)
|
||||||
|
&& matches!(&unwrap_arg.kind, ExprKind::Lit(lit)
|
||||||
|
if matches!(lit.node, rustc_ast::LitKind::Bool(false)));
|
||||||
|
|
||||||
let mut applicability = Applicability::MachineApplicable;
|
let mut applicability = Applicability::MachineApplicable;
|
||||||
// get snippet for unwrap_or()
|
// get snippet for unwrap_or()
|
||||||
let unwrap_snippet = snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability);
|
let unwrap_snippet = snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability);
|
||||||
// lint message
|
// lint message
|
||||||
// comparing the snippet from source to raw text ("None") below is safe
|
// comparing the snippet from source to raw text ("None") below is safe
|
||||||
// because we already have checked the type.
|
// because we already have checked the type.
|
||||||
let arg = if unwrap_snippet == "None" { "None" } else { "<a>" };
|
let arg = if unwrap_snippet == "None" {
|
||||||
|
"None"
|
||||||
|
} else if suggest_is_some_and {
|
||||||
|
"false"
|
||||||
|
} else {
|
||||||
|
"<a>"
|
||||||
|
};
|
||||||
let unwrap_snippet_none = unwrap_snippet == "None";
|
let unwrap_snippet_none = unwrap_snippet == "None";
|
||||||
let suggest = if unwrap_snippet_none {
|
let suggest = if unwrap_snippet_none {
|
||||||
"and_then(<f>)"
|
"and_then(<f>)"
|
||||||
|
} else if suggest_is_some_and {
|
||||||
|
"is_some_and(<f>)"
|
||||||
} else {
|
} else {
|
||||||
"map_or(<a>, <f>)"
|
"map_or(<a>, <f>)"
|
||||||
};
|
};
|
||||||
@ -98,12 +114,18 @@ pub(super) fn check<'tcx>(
|
|||||||
let mut suggestion = vec![
|
let mut suggestion = vec![
|
||||||
(
|
(
|
||||||
map_span,
|
map_span,
|
||||||
String::from(if unwrap_snippet_none { "and_then" } else { "map_or" }),
|
String::from(if unwrap_snippet_none {
|
||||||
|
"and_then"
|
||||||
|
} else if suggest_is_some_and {
|
||||||
|
"is_some_and"
|
||||||
|
} else {
|
||||||
|
"map_or"
|
||||||
|
}),
|
||||||
),
|
),
|
||||||
(expr.span.with_lo(unwrap_recv.span.hi()), String::new()),
|
(expr.span.with_lo(unwrap_recv.span.hi()), String::new()),
|
||||||
];
|
];
|
||||||
|
|
||||||
if !unwrap_snippet_none {
|
if !unwrap_snippet_none && !suggest_is_some_and {
|
||||||
suggestion.push((map_arg_span.with_hi(map_arg_span.lo()), format!("{unwrap_snippet}, ")));
|
suggestion.push((map_arg_span.with_hi(map_arg_span.lo()), format!("{unwrap_snippet}, ")));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -294,7 +294,7 @@ pub fn get_configuration_metadata() -> Vec<ClippyConfiguration> {
|
|||||||
///
|
///
|
||||||
/// Suppress lints whenever the suggested change would cause breakage for other crates.
|
/// Suppress lints whenever the suggested change would cause breakage for other crates.
|
||||||
(avoid_breaking_exported_api: bool = true),
|
(avoid_breaking_exported_api: bool = true),
|
||||||
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS.
|
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS.
|
||||||
///
|
///
|
||||||
/// The minimum rust version that the project supports
|
/// The minimum rust version that the project supports
|
||||||
(msrv: Option<String> = None),
|
(msrv: Option<String> = None),
|
||||||
|
@ -19,6 +19,7 @@ macro_rules! msrv_aliases {
|
|||||||
|
|
||||||
// names may refer to stabilized feature flags or library items
|
// names may refer to stabilized feature flags or library items
|
||||||
msrv_aliases! {
|
msrv_aliases! {
|
||||||
|
1,70,0 { OPTION_IS_SOME_AND }
|
||||||
1,68,0 { PATH_MAIN_SEPARATOR_STR }
|
1,68,0 { PATH_MAIN_SEPARATOR_STR }
|
||||||
1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS }
|
1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS }
|
||||||
1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE }
|
1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE }
|
||||||
|
@ -56,6 +56,10 @@ fn option_methods() {
|
|||||||
.unwrap_or_else(||
|
.unwrap_or_else(||
|
||||||
0
|
0
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// Check for `map(f).unwrap_or(false)` use.
|
||||||
|
let _ = opt.map(|x| x > 5).unwrap_or(false);
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[rustfmt::skip]
|
#[rustfmt::skip]
|
||||||
@ -95,6 +99,20 @@ fn msrv_1_41() {
|
|||||||
let _ = res.map(|x| x + 1).unwrap_or_else(|_e| 0);
|
let _ = res.map(|x| x + 1).unwrap_or_else(|_e| 0);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[clippy::msrv = "1.69"]
|
||||||
|
fn msrv_1_69() {
|
||||||
|
let opt: Option<i32> = Some(1);
|
||||||
|
|
||||||
|
let _ = opt.map(|x| x > 5).unwrap_or(false);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[clippy::msrv = "1.70"]
|
||||||
|
fn msrv_1_70() {
|
||||||
|
let opt: Option<i32> = Some(1);
|
||||||
|
|
||||||
|
let _ = opt.map(|x| x > 5).unwrap_or(false);
|
||||||
|
}
|
||||||
|
|
||||||
mod issue_10579 {
|
mod issue_10579 {
|
||||||
// Different variations of the same issue.
|
// Different variations of the same issue.
|
||||||
fn v1() {
|
fn v1() {
|
||||||
|
@ -126,8 +126,20 @@ LL | | 0
|
|||||||
LL | | );
|
LL | | );
|
||||||
| |_________^
|
| |_________^
|
||||||
|
|
||||||
|
error: called `map(<f>).unwrap_or(false)` on an `Option` value. This can be done more directly by calling `is_some_and(<f>)` instead
|
||||||
|
--> $DIR/map_unwrap_or.rs:61:13
|
||||||
|
|
|
||||||
|
LL | let _ = opt.map(|x| x > 5).unwrap_or(false);
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
|
||||||
|
help: use `is_some_and(<f>)` instead
|
||||||
|
|
|
||||||
|
LL - let _ = opt.map(|x| x > 5).unwrap_or(false);
|
||||||
|
LL + let _ = opt.is_some_and(|x| x > 5);
|
||||||
|
|
|
||||||
|
|
||||||
error: called `map(<f>).unwrap_or_else(<g>)` on a `Result` value. This can be done more directly by calling `.map_or_else(<g>, <f>)` instead
|
error: called `map(<f>).unwrap_or_else(<g>)` on a `Result` value. This can be done more directly by calling `.map_or_else(<g>, <f>)` instead
|
||||||
--> $DIR/map_unwrap_or.rs:67:13
|
--> $DIR/map_unwrap_or.rs:71:13
|
||||||
|
|
|
|
||||||
LL | let _ = res.map(|x| {
|
LL | let _ = res.map(|x| {
|
||||||
| _____________^
|
| _____________^
|
||||||
@ -137,7 +149,7 @@ LL | | ).unwrap_or_else(|_e| 0);
|
|||||||
| |____________________________^
|
| |____________________________^
|
||||||
|
|
||||||
error: called `map(<f>).unwrap_or_else(<g>)` on a `Result` value. This can be done more directly by calling `.map_or_else(<g>, <f>)` instead
|
error: called `map(<f>).unwrap_or_else(<g>)` on a `Result` value. This can be done more directly by calling `.map_or_else(<g>, <f>)` instead
|
||||||
--> $DIR/map_unwrap_or.rs:71:13
|
--> $DIR/map_unwrap_or.rs:75:13
|
||||||
|
|
|
|
||||||
LL | let _ = res.map(|x| x + 1)
|
LL | let _ = res.map(|x| x + 1)
|
||||||
| _____________^
|
| _____________^
|
||||||
@ -147,10 +159,34 @@ LL | | });
|
|||||||
| |__________^
|
| |__________^
|
||||||
|
|
||||||
error: called `map(<f>).unwrap_or_else(<g>)` on a `Result` value. This can be done more directly by calling `.map_or_else(<g>, <f>)` instead
|
error: called `map(<f>).unwrap_or_else(<g>)` on a `Result` value. This can be done more directly by calling `.map_or_else(<g>, <f>)` instead
|
||||||
--> $DIR/map_unwrap_or.rs:95:13
|
--> $DIR/map_unwrap_or.rs:99:13
|
||||||
|
|
|
|
||||||
LL | let _ = res.map(|x| x + 1).unwrap_or_else(|_e| 0);
|
LL | let _ = res.map(|x| x + 1).unwrap_or_else(|_e| 0);
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `res.map_or_else(|_e| 0, |x| x + 1)`
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `res.map_or_else(|_e| 0, |x| x + 1)`
|
||||||
|
|
||||||
error: aborting due to 12 previous errors
|
error: called `map(<f>).unwrap_or(<a>)` on an `Option` value. This can be done more directly by calling `map_or(<a>, <f>)` instead
|
||||||
|
--> $DIR/map_unwrap_or.rs:106:13
|
||||||
|
|
|
||||||
|
LL | let _ = opt.map(|x| x > 5).unwrap_or(false);
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
|
||||||
|
help: use `map_or(<a>, <f>)` instead
|
||||||
|
|
|
||||||
|
LL - let _ = opt.map(|x| x > 5).unwrap_or(false);
|
||||||
|
LL + let _ = opt.map_or(false, |x| x > 5);
|
||||||
|
|
|
||||||
|
|
||||||
|
error: called `map(<f>).unwrap_or(false)` on an `Option` value. This can be done more directly by calling `is_some_and(<f>)` instead
|
||||||
|
--> $DIR/map_unwrap_or.rs:113:13
|
||||||
|
|
|
||||||
|
LL | let _ = opt.map(|x| x > 5).unwrap_or(false);
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
|
||||||
|
help: use `is_some_and(<f>)` instead
|
||||||
|
|
|
||||||
|
LL - let _ = opt.map(|x| x > 5).unwrap_or(false);
|
||||||
|
LL + let _ = opt.is_some_and(|x| x > 5);
|
||||||
|
|
|
||||||
|
|
||||||
|
error: aborting due to 15 previous errors
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user