1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31
 32
 33
 34
 35
 36
 37
 38
 39
 40
 41
 42
 43
 44
 45
 46
 47
 48
 49
 50
 51
 52
 53
 54
 55
 56
 57
 58
 59
 60
 61
 62
 63
 64
 65
 66
 67
 68
 69
 70
 71
 72
 73
 74
 75
 76
 77
 78
 79
 80
 81
 82
 83
 84
 85
 86
 87
 88
 89
 90
 91
 92
 93
 94
 95
 96
 97
 98
 99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125

use rustc::lint::{LateLintPass, LateContext, LintArray, LintPass};
use rustc::hir::*;
use rustc::hir::intravisit::{Visitor, walk_expr, NestedVisitorMap};
use utils::*;

/// **What it does:** Checks for `if` conditions that use blocks to contain an
/// expression.
///
/// **Why is this bad?** It isn't really Rust style, same as using parentheses
/// to contain expressions.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// if { true } ..
/// ```
declare_lint! {
    pub BLOCK_IN_IF_CONDITION_EXPR,
    Warn,
    "braces that can be eliminated in conditions, e.g `if { true } ...`"
}

/// **What it does:** Checks for `if` conditions that use blocks containing
/// statements, or conditions that use closures with blocks.
///
/// **Why is this bad?** Using blocks in the condition makes it hard to read.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// if { let x = somefunc(); x } ..
/// // or
/// if somefunc(|x| { x == 47 }) ..
/// ```
declare_lint! {
    pub BLOCK_IN_IF_CONDITION_STMT,
    Warn,
    "complex blocks in conditions, e.g. `if { let x = true; x } ...`"
}

#[derive(Copy,Clone)]
pub struct BlockInIfCondition;

impl LintPass for BlockInIfCondition {
    fn get_lints(&self) -> LintArray {
        lint_array!(BLOCK_IN_IF_CONDITION_EXPR, BLOCK_IN_IF_CONDITION_STMT)
    }
}

struct ExVisitor<'a, 'tcx: 'a> {
    found_block: Option<&'tcx Expr>,
    cx: &'a LateContext<'a, 'tcx>,
}

impl<'a, 'tcx: 'a> Visitor<'tcx> for ExVisitor<'a, 'tcx> {
    fn visit_expr(&mut self, expr: &'tcx Expr) {
        if let ExprClosure(_, _, eid, _) = expr.node {
            let body = self.cx.tcx.map.body(eid);
            let ex = &body.value;
            if matches!(ex.node, ExprBlock(_)) {
                self.found_block = Some(ex);
                return;
            }
        }
        walk_expr(self, expr);
    }
    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
        NestedVisitorMap::All(&self.cx.tcx.map)
    }
}

const BRACED_EXPR_MESSAGE: &'static str = "omit braces around single expression condition";
const COMPLEX_BLOCK_MESSAGE: &'static str = "in an 'if' condition, avoid complex blocks or closures with blocks; \
                                             instead, move the block or closure higher and bind it with a 'let'";

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition {
    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
        if let ExprIf(ref check, ref then, _) = expr.node {
            if let ExprBlock(ref block) = check.node {
                if block.rules == DefaultBlock {
                    if block.stmts.is_empty() {
                        if let Some(ref ex) = block.expr {
                            // don't dig into the expression here, just suggest that they remove
                            // the block
                            if in_macro(cx, expr.span) || differing_macro_contexts(expr.span, ex.span) {
                                return;
                            }
                            span_help_and_lint(cx,
                                               BLOCK_IN_IF_CONDITION_EXPR,
                                               check.span,
                                               BRACED_EXPR_MESSAGE,
                                               &format!("try\nif {} {} ... ",
                                                        snippet_block(cx, ex.span, ".."),
                                                        snippet_block(cx, then.span, "..")));
                        }
                    } else {
                        let span = block.expr.as_ref().map_or_else(|| block.stmts[0].span, |e| e.span);
                        if in_macro(cx, span) || differing_macro_contexts(expr.span, span) {
                            return;
                        }
                        // move block higher
                        span_help_and_lint(cx,
                                           BLOCK_IN_IF_CONDITION_STMT,
                                           check.span,
                                           COMPLEX_BLOCK_MESSAGE,
                                           &format!("try\nlet res = {};\nif res {} ... ",
                                                    snippet_block(cx, block.span, ".."),
                                                    snippet_block(cx, then.span, "..")));
                    }
                }
            } else {
                let mut visitor = ExVisitor {
                    found_block: None,
                    cx: cx,
                };
                walk_expr(&mut visitor, check);
                if let Some(block) = visitor.found_block {
                    span_lint(cx, BLOCK_IN_IF_CONDITION_STMT, block.span, COMPLEX_BLOCK_MESSAGE);
                }
            }
        }
    }
}