Auto merge of #7971 - togami2864:fix/option-map-or-none, r=llogiq
fix suggestion in option_map_or_none fix: #7960 changelog: change suggestion in the lint rule `option_map_or_none`
This commit is contained in:
bors
commit
6ac42fe6fa
clippy_lints/src/methods
tests/ui
@ -1,7 +1,7 @@
|
||||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||
use clippy_utils::is_lang_ctor;
|
||||
use clippy_utils::source::snippet;
|
||||
use clippy_utils::ty::is_type_diagnostic_item;
|
||||
use clippy_utils::{is_lang_ctor, single_segment_path};
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir as hir;
|
||||
use rustc_hir::LangItem::{OptionNone, OptionSome};
|
||||
@ -11,6 +11,28 @@ use rustc_span::symbol::sym;
|
||||
use super::OPTION_MAP_OR_NONE;
|
||||
use super::RESULT_MAP_OR_INTO_OPTION;
|
||||
|
||||
// The expression inside a closure may or may not have surrounding braces
|
||||
// which causes problems when generating a suggestion.
|
||||
fn reduce_unit_expression<'a>(
|
||||
cx: &LateContext<'_>,
|
||||
expr: &'a hir::Expr<'_>,
|
||||
) -> Option<(&'a hir::Expr<'a>, &'a [hir::Expr<'a>])> {
|
||||
match expr.kind {
|
||||
hir::ExprKind::Call(func, arg_char) => Some((func, arg_char)),
|
||||
hir::ExprKind::Block(block, _) => {
|
||||
match (block.stmts, block.expr) {
|
||||
(&[], Some(inner_expr)) => {
|
||||
// If block only contains an expression,
|
||||
// reduce `|x| { x + 1 }` to `|x| x + 1`
|
||||
reduce_unit_expression(cx, inner_expr)
|
||||
},
|
||||
_ => None,
|
||||
}
|
||||
},
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// lint use of `_.map_or(None, _)` for `Option`s and `Result`s
|
||||
pub(super) fn check<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
@ -31,58 +53,75 @@ pub(super) fn check<'tcx>(
|
||||
return;
|
||||
}
|
||||
|
||||
let (lint_name, msg, instead, hint) = {
|
||||
let default_arg_is_none = if let hir::ExprKind::Path(ref qpath) = def_arg.kind {
|
||||
is_lang_ctor(cx, qpath, OptionNone)
|
||||
} else {
|
||||
return;
|
||||
};
|
||||
|
||||
if !default_arg_is_none {
|
||||
// nothing to lint!
|
||||
return;
|
||||
}
|
||||
|
||||
let f_arg_is_some = if let hir::ExprKind::Path(ref qpath) = map_arg.kind {
|
||||
is_lang_ctor(cx, qpath, OptionSome)
|
||||
} else {
|
||||
false
|
||||
};
|
||||
|
||||
if is_option {
|
||||
let self_snippet = snippet(cx, recv.span, "..");
|
||||
let func_snippet = snippet(cx, map_arg.span, "..");
|
||||
let msg = "called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling \
|
||||
`and_then(..)` instead";
|
||||
(
|
||||
OPTION_MAP_OR_NONE,
|
||||
msg,
|
||||
"try using `and_then` instead",
|
||||
format!("{0}.and_then({1})", self_snippet, func_snippet),
|
||||
)
|
||||
} else if f_arg_is_some {
|
||||
let msg = "called `map_or(None, Some)` on a `Result` value. This can be done more directly by calling \
|
||||
`ok()` instead";
|
||||
let self_snippet = snippet(cx, recv.span, "..");
|
||||
(
|
||||
RESULT_MAP_OR_INTO_OPTION,
|
||||
msg,
|
||||
"try using `ok` instead",
|
||||
format!("{0}.ok()", self_snippet),
|
||||
)
|
||||
} else {
|
||||
// nothing to lint!
|
||||
return;
|
||||
}
|
||||
let default_arg_is_none = if let hir::ExprKind::Path(ref qpath) = def_arg.kind {
|
||||
is_lang_ctor(cx, qpath, OptionNone)
|
||||
} else {
|
||||
return;
|
||||
};
|
||||
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
lint_name,
|
||||
expr.span,
|
||||
msg,
|
||||
instead,
|
||||
hint,
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
if !default_arg_is_none {
|
||||
// nothing to lint!
|
||||
return;
|
||||
}
|
||||
|
||||
let f_arg_is_some = if let hir::ExprKind::Path(ref qpath) = map_arg.kind {
|
||||
is_lang_ctor(cx, qpath, OptionSome)
|
||||
} else {
|
||||
false
|
||||
};
|
||||
|
||||
if is_option {
|
||||
let self_snippet = snippet(cx, recv.span, "..");
|
||||
if_chain! {
|
||||
if let hir::ExprKind::Closure(_, _, id, span, _) = map_arg.kind;
|
||||
let arg_snippet = snippet(cx, span, "..");
|
||||
let body = cx.tcx.hir().body(id);
|
||||
if let Some((func, arg_char)) = reduce_unit_expression(cx, &body.value);
|
||||
if arg_char.len() == 1;
|
||||
if let hir::ExprKind::Path(ref qpath) = func.kind;
|
||||
if let Some(segment) = single_segment_path(qpath);
|
||||
if segment.ident.name == sym::Some;
|
||||
then {
|
||||
let func_snippet = snippet(cx, arg_char[0].span, "..");
|
||||
let msg = "called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling \
|
||||
`map(..)` instead";
|
||||
return span_lint_and_sugg(
|
||||
cx,
|
||||
OPTION_MAP_OR_NONE,
|
||||
expr.span,
|
||||
msg,
|
||||
"try using `map` instead",
|
||||
format!("{0}.map({1} {2})", self_snippet, arg_snippet,func_snippet),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
let func_snippet = snippet(cx, map_arg.span, "..");
|
||||
let msg = "called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling \
|
||||
`and_then(..)` instead";
|
||||
return span_lint_and_sugg(
|
||||
cx,
|
||||
OPTION_MAP_OR_NONE,
|
||||
expr.span,
|
||||
msg,
|
||||
"try using `and_then` instead",
|
||||
format!("{0}.and_then({1})", self_snippet, func_snippet),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
} else if f_arg_is_some {
|
||||
let msg = "called `map_or(None, Some)` on a `Result` value. This can be done more directly by calling \
|
||||
`ok()` instead";
|
||||
let self_snippet = snippet(cx, recv.span, "..");
|
||||
return span_lint_and_sugg(
|
||||
cx,
|
||||
RESULT_MAP_OR_INTO_OPTION,
|
||||
expr.span,
|
||||
msg,
|
||||
"try using `ok` instead",
|
||||
format!("{0}.ok()", self_snippet),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@ -4,13 +4,19 @@
|
||||
|
||||
fn main() {
|
||||
let opt = Some(1);
|
||||
let bar = |_| Some(1);
|
||||
|
||||
// Check `OPTION_MAP_OR_NONE`.
|
||||
// Single line case.
|
||||
let _ = opt.and_then(|x| Some(x + 1));
|
||||
let _: Option<i32> = opt.map(|x| x + 1);
|
||||
// Multi-line case.
|
||||
#[rustfmt::skip]
|
||||
let _ = opt.and_then(|x| {
|
||||
Some(x + 1)
|
||||
});
|
||||
let _: Option<i32> = opt.map(|x| x + 1);
|
||||
// function returning `Option`
|
||||
let _: Option<i32> = opt.and_then(bar);
|
||||
let _: Option<i32> = opt.and_then(|x| {
|
||||
let offset = 0;
|
||||
let height = x;
|
||||
Some(offset + height)
|
||||
});
|
||||
}
|
||||
|
||||
@ -4,13 +4,21 @@
|
||||
|
||||
fn main() {
|
||||
let opt = Some(1);
|
||||
let bar = |_| Some(1);
|
||||
|
||||
// Check `OPTION_MAP_OR_NONE`.
|
||||
// Single line case.
|
||||
let _ = opt.map_or(None, |x| Some(x + 1));
|
||||
let _: Option<i32> = opt.map_or(None, |x| Some(x + 1));
|
||||
// Multi-line case.
|
||||
#[rustfmt::skip]
|
||||
let _ = opt.map_or(None, |x| {
|
||||
let _: Option<i32> = opt.map_or(None, |x| {
|
||||
Some(x + 1)
|
||||
});
|
||||
// function returning `Option`
|
||||
let _: Option<i32> = opt.map_or(None, bar);
|
||||
let _: Option<i32> = opt.map_or(None, |x| {
|
||||
let offset = 0;
|
||||
let height = x;
|
||||
Some(offset + height)
|
||||
});
|
||||
}
|
||||
|
||||
@ -1,26 +1,45 @@
|
||||
error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `and_then(..)` instead
|
||||
--> $DIR/option_map_or_none.rs:10:13
|
||||
error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `map(..)` instead
|
||||
--> $DIR/option_map_or_none.rs:11:26
|
||||
|
|
||||
LL | let _ = opt.map_or(None, |x| Some(x + 1));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `opt.and_then(|x| Some(x + 1))`
|
||||
LL | let _: Option<i32> = opt.map_or(None, |x| Some(x + 1));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `map` instead: `opt.map(|x| x + 1)`
|
||||
|
|
||||
= note: `-D clippy::option-map-or-none` implied by `-D warnings`
|
||||
|
||||
error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `and_then(..)` instead
|
||||
--> $DIR/option_map_or_none.rs:13:13
|
||||
error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `map(..)` instead
|
||||
--> $DIR/option_map_or_none.rs:14:26
|
||||
|
|
||||
LL | let _ = opt.map_or(None, |x| {
|
||||
| _____________^
|
||||
LL | let _: Option<i32> = opt.map_or(None, |x| {
|
||||
| __________________________^
|
||||
LL | | Some(x + 1)
|
||||
LL | | });
|
||||
| |_________________________^
|
||||
| |_________________________^ help: try using `map` instead: `opt.map(|x| x + 1)`
|
||||
|
||||
error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `and_then(..)` instead
|
||||
--> $DIR/option_map_or_none.rs:18:26
|
||||
|
|
||||
LL | let _: Option<i32> = opt.map_or(None, bar);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `opt.and_then(bar)`
|
||||
|
||||
error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `and_then(..)` instead
|
||||
--> $DIR/option_map_or_none.rs:19:26
|
||||
|
|
||||
LL | let _: Option<i32> = opt.map_or(None, |x| {
|
||||
| __________________________^
|
||||
LL | | let offset = 0;
|
||||
LL | | let height = x;
|
||||
LL | | Some(offset + height)
|
||||
LL | | });
|
||||
| |______^
|
||||
|
|
||||
help: try using `and_then` instead
|
||||
|
|
||||
LL ~ let _ = opt.and_then(|x| {
|
||||
LL + Some(x + 1)
|
||||
LL ~ });
|
||||
LL ~ let _: Option<i32> = opt.and_then(|x| {
|
||||
LL + let offset = 0;
|
||||
LL + let height = x;
|
||||
LL + Some(offset + height)
|
||||
LL ~ });
|
||||
|
|
||||
|
||||
error: aborting due to 2 previous errors
|
||||
error: aborting due to 4 previous errors
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user