]> git.lizzy.rs Git - rust.git/blob - clippy_lints/src/block_in_if_condition.rs
Auto merge of #3946 - rchaser53:issue-3920, r=flip1995
[rust.git] / clippy_lints / src / block_in_if_condition.rs
1 use crate::utils::*;
2 use matches::matches;
3 use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
4 use rustc::hir::*;
5 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
6 use rustc::{declare_tool_lint, lint_array};
7
8 declare_clippy_lint! {
9     /// **What it does:** Checks for `if` conditions that use blocks to contain an
10     /// expression.
11     ///
12     /// **Why is this bad?** It isn't really Rust style, same as using parentheses
13     /// to contain expressions.
14     ///
15     /// **Known problems:** None.
16     ///
17     /// **Example:**
18     /// ```rust
19     /// if { true } { /* ... */ }
20     /// ```
21     pub BLOCK_IN_IF_CONDITION_EXPR,
22     style,
23     "braces that can be eliminated in conditions, e.g., `if { true } ...`"
24 }
25
26 declare_clippy_lint! {
27     /// **What it does:** Checks for `if` conditions that use blocks containing
28     /// statements, or conditions that use closures with blocks.
29     ///
30     /// **Why is this bad?** Using blocks in the condition makes it hard to read.
31     ///
32     /// **Known problems:** None.
33     ///
34     /// **Example:**
35     /// ```ignore
36     /// if { let x = somefunc(); x } {}
37     /// // or
38     /// if somefunc(|x| { x == 47 }) {}
39     /// ```
40     pub BLOCK_IN_IF_CONDITION_STMT,
41     style,
42     "complex blocks in conditions, e.g., `if { let x = true; x } ...`"
43 }
44
45 #[derive(Copy, Clone)]
46 pub struct BlockInIfCondition;
47
48 impl LintPass for BlockInIfCondition {
49     fn get_lints(&self) -> LintArray {
50         lint_array!(BLOCK_IN_IF_CONDITION_EXPR, BLOCK_IN_IF_CONDITION_STMT)
51     }
52
53     fn name(&self) -> &'static str {
54         "BlockInIfCondition"
55     }
56 }
57
58 struct ExVisitor<'a, 'tcx: 'a> {
59     found_block: Option<&'tcx Expr>,
60     cx: &'a LateContext<'a, 'tcx>,
61 }
62
63 impl<'a, 'tcx: 'a> Visitor<'tcx> for ExVisitor<'a, 'tcx> {
64     fn visit_expr(&mut self, expr: &'tcx Expr) {
65         if let ExprKind::Closure(_, _, eid, _, _) = expr.node {
66             let body = self.cx.tcx.hir().body(eid);
67             let ex = &body.value;
68             if matches!(ex.node, ExprKind::Block(_, _)) && !in_macro(body.value.span) {
69                 self.found_block = Some(ex);
70                 return;
71             }
72         }
73         walk_expr(self, expr);
74     }
75     fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
76         NestedVisitorMap::None
77     }
78 }
79
80 const BRACED_EXPR_MESSAGE: &str = "omit braces around single expression condition";
81 const COMPLEX_BLOCK_MESSAGE: &str = "in an 'if' condition, avoid complex blocks or closures with blocks; \
82                                      instead, move the block or closure higher and bind it with a 'let'";
83
84 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition {
85     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
86         if let ExprKind::If(check, then, _) = &expr.node {
87             if let ExprKind::Block(block, _) = &check.node {
88                 if block.rules == DefaultBlock {
89                     if block.stmts.is_empty() {
90                         if let Some(ex) = &block.expr {
91                             // don't dig into the expression here, just suggest that they remove
92                             // the block
93                             if in_macro(expr.span) || differing_macro_contexts(expr.span, ex.span) {
94                                 return;
95                             }
96                             span_help_and_lint(
97                                 cx,
98                                 BLOCK_IN_IF_CONDITION_EXPR,
99                                 check.span,
100                                 BRACED_EXPR_MESSAGE,
101                                 &format!(
102                                     "try\nif {} {} ... ",
103                                     snippet_block(cx, ex.span, ".."),
104                                     snippet_block(cx, then.span, "..")
105                                 ),
106                             );
107                         }
108                     } else {
109                         let span = block.expr.as_ref().map_or_else(|| block.stmts[0].span, |e| e.span);
110                         if in_macro(span) || differing_macro_contexts(expr.span, span) {
111                             return;
112                         }
113                         // move block higher
114                         span_help_and_lint(
115                             cx,
116                             BLOCK_IN_IF_CONDITION_STMT,
117                             check.span,
118                             COMPLEX_BLOCK_MESSAGE,
119                             &format!(
120                                 "try\nlet res = {};\nif res {} ... ",
121                                 snippet_block(cx, block.span, ".."),
122                                 snippet_block(cx, then.span, "..")
123                             ),
124                         );
125                     }
126                 }
127             } else {
128                 let mut visitor = ExVisitor { found_block: None, cx };
129                 walk_expr(&mut visitor, check);
130                 if let Some(block) = visitor.found_block {
131                     span_lint(cx, BLOCK_IN_IF_CONDITION_STMT, block.span, COMPLEX_BLOCK_MESSAGE);
132                 }
133             }
134         }
135     }
136 }