Stop linting on macros and correctly use braces for constructs

This commit is contained in:
JarredAllen 2020-04-25 08:32:33 -07:00
parent bf48a2d50d
commit 82f8d4d6f1
4 changed files with 53 additions and 7 deletions

View File

@ -1,3 +1,4 @@
use crate::utils;
use crate::utils::sugg::Sugg;
use crate::utils::{match_type, paths, span_lint_and_sugg};
use if_chain::if_chain;
@ -89,6 +90,7 @@ struct OptionIfLetElseOccurence {
method_sugg: String,
some_expr: String,
none_expr: String,
wrap_braces: bool,
}
struct ReturnBreakContinueVisitor<'tcx> {
@ -140,6 +142,7 @@ fn contains_return_break_continue<'tcx>(expression: &'tcx Expr<'tcx>) -> bool {
fn detect_option_if_let_else<'a>(cx: &LateContext<'_, 'a>, expr: &'a Expr<'a>) -> Option<OptionIfLetElseOccurence> {
//(String, String, String, String)> {
if_chain! {
// if !utils::in_macro(expr.span); // Don't lint macros, because it behaves weirdly
if let ExprKind::Match(let_body, arms, MatchSource::IfLetDesugar{contains_else_clause: true}) = &expr.kind;
if arms.len() == 2;
if match_type(cx, &cx.tables.expr_ty(let_body), &paths::OPTION);
@ -170,11 +173,23 @@ fn detect_option_if_let_else<'a>(cx: &LateContext<'_, 'a>, expr: &'a Expr<'a>) -
return None;
};
let capture_name = id.name.to_ident_string();
let wrap_braces = utils::get_enclosing_block(cx, expr.hir_id).map_or(false, |parent| {
if_chain! {
if let Some(Expr { kind: ExprKind::Match(condition, arms, MatchSource::IfDesugar{contains_else_clause: true}|MatchSource::IfLetDesugar{contains_else_clause: true}), .. } ) = parent.expr;
if expr.hir_id == arms[1].body.hir_id;
then {
true
} else {
false
}
}
});
Some(OptionIfLetElseOccurence {
option: format!("{}", Sugg::hir(cx, let_body, "..")),
method_sugg: format!("{}", method_sugg),
some_expr: format!("|{}| {}", capture_name, Sugg::hir(cx, some_body, "..")),
none_expr: format!("{}{}", if method_sugg == "map_or" { "" } else { "|| " }, Sugg::hir(cx, none_body, ".."))
none_expr: format!("{}{}", if method_sugg == "map_or" { "" } else { "|| " }, Sugg::hir(cx, none_body, "..")),
wrap_braces,
})
} else {
None
@ -192,8 +207,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OptionIfLetElse {
format!("use Option::{} instead of an if let/else", detection.method_sugg).as_str(),
"try",
format!(
"{}.{}({}, {})",
detection.option, detection.method_sugg, detection.none_expr, detection.some_expr
"{}{}.{}({}, {}){}",
if detection.wrap_braces { "{ " } else { "" },
detection.option, detection.method_sugg, detection.none_expr, detection.some_expr,
if detection.wrap_braces { " }" } else { "" },
),
Applicability::MachineApplicable,
);

View File

@ -5,6 +5,12 @@ fn bad1(string: Option<&str>) -> (bool, &str) {
string.map_or((false, "hello"), |x| (true, x))
}
fn bad2(string: Option<&str>) -> Option<(bool, &str)> {
if string.is_none() {
None
} else { string.map_or(Some((false, "")), |x| Some((true, x))) }
}
fn longer_body(arg: Option<u32>) -> u32 {
arg.map_or(13, |x| {
let y = x * x;
@ -42,6 +48,7 @@ fn main() {
let optional = Some(5);
let _ = optional.map_or(5, |x| x + 2);
let _ = bad1(None);
let _ = bad2(None);
let _ = longer_body(None);
test_map_or_else(None);
let _ = negative_tests(None);

View File

@ -9,6 +9,16 @@ fn bad1(string: Option<&str>) -> (bool, &str) {
}
}
fn bad2(string: Option<&str>) -> Option<(bool, &str)> {
if string.is_none() {
None
} else if let Some(x) = string {
Some((true, x))
} else {
Some((false, ""))
}
}
fn longer_body(arg: Option<u32>) -> u32 {
if let Some(x) = arg {
let y = x * x;
@ -50,6 +60,7 @@ fn main() {
let optional = Some(5);
let _ = if let Some(x) = optional { x + 2 } else { 5 };
let _ = bad1(None);
let _ = bad2(None);
let _ = longer_body(None);
test_map_or_else(None);
let _ = negative_tests(None);

View File

@ -11,7 +11,18 @@ LL | | }
= note: `-D clippy::option-if-let-else` implied by `-D warnings`
error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:13:5
--> $DIR/option_if_let_else.rs:15:12
|
LL | } else if let Some(x) = string {
| ____________^
LL | | Some((true, x))
LL | | } else {
LL | | Some((false, ""))
LL | | }
| |_____^ help: try: `{ string.map_or(Some((false, "")), |x| Some((true, x))) }`
error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:23:5
|
LL | / if let Some(x) = arg {
LL | | let y = x * x;
@ -30,7 +41,7 @@ LL | })
|
error: use Option::map_or_else instead of an if let/else
--> $DIR/option_if_let_else.rs:22:13
--> $DIR/option_if_let_else.rs:32:13
|
LL | let _ = if let Some(x) = arg {
| _____________^
@ -53,10 +64,10 @@ LL | }, |x| x * x * x * x);
|
error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:51:13
--> $DIR/option_if_let_else.rs:61:13
|
LL | let _ = if let Some(x) = optional { x + 2 } else { 5 };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)`
error: aborting due to 4 previous errors
error: aborting due to 5 previous errors