]> git.lizzy.rs Git - rust.git/blob - clippy_lints/src/block_in_if_condition.rs
Auto merge of #4522 - mikerite:fix-4514, r=phansch
[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::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass};
6 use rustc::{declare_lint_pass, declare_tool_lint};
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 declare_lint_pass!(BlockInIfCondition => [BLOCK_IN_IF_CONDITION_EXPR, BLOCK_IN_IF_CONDITION_STMT]);
46
47 struct ExVisitor<'a, 'tcx> {
48     found_block: Option<&'tcx Expr>,
49     cx: &'a LateContext<'a, 'tcx>,
50 }
51
52 impl<'a, 'tcx> Visitor<'tcx> for ExVisitor<'a, 'tcx> {
53     fn visit_expr(&mut self, expr: &'tcx Expr) {
54         if let ExprKind::Closure(_, _, eid, _, _) = expr.node {
55             let body = self.cx.tcx.hir().body(eid);
56             let ex = &body.value;
57             if matches!(ex.node, ExprKind::Block(_, _)) && !body.value.span.from_expansion() {
58                 self.found_block = Some(ex);
59                 return;
60             }
61         }
62         walk_expr(self, expr);
63     }
64     fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
65         NestedVisitorMap::None
66     }
67 }
68
69 const BRACED_EXPR_MESSAGE: &str = "omit braces around single expression condition";
70 const COMPLEX_BLOCK_MESSAGE: &str = "in an 'if' condition, avoid complex blocks or closures with blocks; \
71                                      instead, move the block or closure higher and bind it with a 'let'";
72
73 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition {
74     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
75         if in_external_macro(cx.sess(), expr.span) {
76             return;
77         }
78         if let Some((check, then, _)) = higher::if_block(&expr) {
79             if let ExprKind::Block(block, _) = &check.node {
80                 if block.rules == DefaultBlock {
81                     if block.stmts.is_empty() {
82                         if let Some(ex) = &block.expr {
83                             // don't dig into the expression here, just suggest that they remove
84                             // the block
85                             if expr.span.from_expansion() || differing_macro_contexts(expr.span, ex.span) {
86                                 return;
87                             }
88                             span_help_and_lint(
89                                 cx,
90                                 BLOCK_IN_IF_CONDITION_EXPR,
91                                 check.span,
92                                 BRACED_EXPR_MESSAGE,
93                                 &format!(
94                                     "try\nif {} {} ... ",
95                                     snippet_block(cx, ex.span, ".."),
96                                     snippet_block(cx, then.span, "..")
97                                 ),
98                             );
99                         }
100                     } else {
101                         let span = block.expr.as_ref().map_or_else(|| block.stmts[0].span, |e| e.span);
102                         if span.from_expansion() || differing_macro_contexts(expr.span, span) {
103                             return;
104                         }
105                         // move block higher
106                         span_help_and_lint(
107                             cx,
108                             BLOCK_IN_IF_CONDITION_STMT,
109                             check.span,
110                             COMPLEX_BLOCK_MESSAGE,
111                             &format!(
112                                 "try\nlet res = {};\nif res {} ... ",
113                                 snippet_block(cx, block.span, ".."),
114                                 snippet_block(cx, then.span, "..")
115                             ),
116                         );
117                     }
118                 }
119             } else {
120                 let mut visitor = ExVisitor { found_block: None, cx };
121                 walk_expr(&mut visitor, check);
122                 if let Some(block) = visitor.found_block {
123                     span_lint(cx, BLOCK_IN_IF_CONDITION_STMT, block.span, COMPLEX_BLOCK_MESSAGE);
124                 }
125             }
126         }
127     }
128 }