Handle non-trivial nil closures

`reduce_nil_closure` mixed together a) 'is this a nil closure?' and b) 'can it
be reduced to a simple expression?'. Split the logic into two functions so we
can still generate a basic warning when the closure can't be simplified.
This commit is contained in:
Phil Turnbull 2017-01-22 20:57:17 -05:00 committed by Philipp Hansch
parent 2f52d1d568
commit d0bdfe5ce3
No known key found for this signature in database
GPG key ID: B6FA06A6E0E2665B
3 changed files with 36 additions and 20 deletions

View file

@ -14,7 +14,7 @@ pub struct Pass;
/// **Why is this bad?** Readability, this can be written more clearly with
/// an if statement
///
/// **Known problems:** Closures with multiple statements are not handled
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
@ -114,16 +114,17 @@ fn reduce_nil_expression<'a>(cx: &LateContext, expr: &'a hir::Expr) -> Option<Sp
}
}
fn reduce_nil_closure<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'a hir::Expr) -> Option<(Span, Span)> {
fn nil_closure<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'a hir::Expr) -> Option<(&'tcx hir::Arg, &'a hir::Expr)> {
if let hir::ExprClosure(_, ref decl, inner_expr_id, _) = expr.node {
let body = cx.tcx.map.body(inner_expr_id);
let body_expr = &body.value;
if_let_chain! {[
decl.inputs.len() == 1,
is_nil_expression(cx, body_expr),
let Some(binding) = iter_input_pats(&decl, body).next(),
let Some(expr_span) = reduce_nil_expression(cx, &body.value),
], {
return Some((binding.pat.span, expr_span))
return Some((binding, body_expr))
}}
}
None
@ -148,12 +149,18 @@ fn lint_map_nil_fn(cx: &LateContext, stmt: &hir::Stmt, expr: &hir::Expr, map_arg
expr.span,
msg,
|db| { db.span_suggestion(stmt.span, "try this", suggestion); });
} else if let Some((binding_span, expr_span)) = reduce_nil_closure(cx, fn_arg) {
} else if let Some((binding, closure_expr)) = nil_closure(cx, fn_arg) {
let msg = "called `map(f)` on an Option value where `f` is a nil closure";
let suggestion = format!("if let Some({0}) = {1} {{ {2} }}",
snippet(cx, binding_span, "_"),
snippet(cx, var_arg.span, "_"),
snippet(cx, expr_span, "_"));
let suggestion = if let Some(expr_span) = reduce_nil_expression(cx, closure_expr) {
format!("if let Some({0}) = {1} {{ {2} }}",
snippet(cx, binding.pat.span, "_"),
snippet(cx, var_arg.span, "_"),
snippet(cx, expr_span, "_"))
} else {
format!("if let Some({0}) = {1} {{ ... }}",
snippet(cx, binding.pat.span, "_"),
snippet(cx, var_arg.span, "_"))
};
span_lint_and_then(cx,
OPTION_MAP_NIL_FN,

View file

@ -432,16 +432,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
fn check_nan(cx: &LateContext, path: &Path, expr: &Expr) {
if !in_constant(cx, expr.id) {
path.segments.last().map(|seg| {
if seg.name == "NAN" {
span_lint(
cx,
CMP_NAN,
expr.span,
"doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead",
);
}
});
if let Some(seg) = path.segments.last() {
path.segments.last().map(|seg| {
if seg.name == "NAN" {
span_lint(
cx,
CMP_NAN,
expr.span,
"doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead",
);
}
});
}
}
}

View file

@ -130,6 +130,13 @@ fn main() {
//~| SUGGESTION if let Some(ref value) = x.field { do_nothing(value + captured) }
// closures with multiple statements are not linted:
x.field.map(|value| { do_nothing(value); do_nothing(value) });
//~^ ERROR called `map(f)` on an Option value where `f` is a nil closure
//~| HELP try this
//~| SUGGESTION if let Some(value) = x.field { ... }
x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) });
//~^ ERROR called `map(f)` on an Option value where `f` is a nil closure
//~| HELP try this
//~| SUGGESTION if let Some(value) = x.field { ... }
}