]>
Commit | Line | Data |
---|---|---|
ea8adc8c XL |
1 | //! Checks for needless boolean results of if-else expressions |
2 | //! | |
3 | //! This lint is **warn** by default | |
4 | ||
5 | use rustc::lint::*; | |
6 | use rustc::hir::*; | |
7 | use syntax::ast::LitKind; | |
8 | use syntax::codemap::Spanned; | |
abe05a73 | 9 | use utils::{snippet, span_lint, span_lint_and_sugg}; |
ea8adc8c XL |
10 | use utils::sugg::Sugg; |
11 | ||
12 | /// **What it does:** Checks for expressions of the form `if c { true } else { | |
13 | /// false }` | |
14 | /// (or vice versa) and suggest using the condition directly. | |
15 | /// | |
16 | /// **Why is this bad?** Redundant code. | |
17 | /// | |
18 | /// **Known problems:** Maybe false positives: Sometimes, the two branches are | |
19 | /// painstakingly documented (which we of course do not detect), so they *may* | |
20 | /// have some value. Even then, the documentation can be rewritten to match the | |
21 | /// shorter code. | |
22 | /// | |
23 | /// **Example:** | |
24 | /// ```rust | |
25 | /// if x { false } else { true } | |
26 | /// ``` | |
27 | declare_lint! { | |
28 | pub NEEDLESS_BOOL, | |
29 | Warn, | |
30 | "if-statements with plain booleans in the then- and else-clause, e.g. \ | |
31 | `if p { true } else { false }`" | |
32 | } | |
33 | ||
34 | /// **What it does:** Checks for expressions of the form `x == true` (or vice | |
35 | /// versa) and suggest using the variable directly. | |
36 | /// | |
37 | /// **Why is this bad?** Unnecessary code. | |
38 | /// | |
39 | /// **Known problems:** None. | |
40 | /// | |
41 | /// **Example:** | |
42 | /// ```rust | |
43 | /// if x == true { } // could be `if x { }` | |
44 | /// ``` | |
45 | declare_lint! { | |
46 | pub BOOL_COMPARISON, | |
47 | Warn, | |
48 | "comparing a variable to a boolean, e.g. `if x == true`" | |
49 | } | |
50 | ||
51 | #[derive(Copy, Clone)] | |
52 | pub struct NeedlessBool; | |
53 | ||
54 | impl LintPass for NeedlessBool { | |
55 | fn get_lints(&self) -> LintArray { | |
56 | lint_array!(NEEDLESS_BOOL) | |
57 | } | |
58 | } | |
59 | ||
60 | impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBool { | |
61 | fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { | |
62 | use self::Expression::*; | |
63 | if let ExprIf(ref pred, ref then_block, Some(ref else_expr)) = e.node { | |
64 | let reduce = |ret, not| { | |
65 | let snip = Sugg::hir(cx, pred, "<predicate>"); | |
66 | let snip = if not { !snip } else { snip }; | |
67 | ||
68 | let hint = if ret { | |
69 | format!("return {}", snip) | |
70 | } else { | |
71 | snip.to_string() | |
72 | }; | |
73 | ||
74 | span_lint_and_sugg( | |
75 | cx, | |
76 | NEEDLESS_BOOL, | |
77 | e.span, | |
78 | "this if-then-else expression returns a bool literal", | |
79 | "you can reduce it to", | |
80 | hint, | |
81 | ); | |
82 | }; | |
83 | if let ExprBlock(ref then_block) = then_block.node { | |
84 | match (fetch_bool_block(then_block), fetch_bool_expr(else_expr)) { | |
abe05a73 | 85 | (RetBool(true), RetBool(true)) | (Bool(true), Bool(true)) => { |
ea8adc8c XL |
86 | span_lint( |
87 | cx, | |
88 | NEEDLESS_BOOL, | |
89 | e.span, | |
90 | "this if-then-else expression will always return true", | |
91 | ); | |
92 | }, | |
abe05a73 | 93 | (RetBool(false), RetBool(false)) | (Bool(false), Bool(false)) => { |
ea8adc8c XL |
94 | span_lint( |
95 | cx, | |
96 | NEEDLESS_BOOL, | |
97 | e.span, | |
98 | "this if-then-else expression will always return false", | |
99 | ); | |
100 | }, | |
101 | (RetBool(true), RetBool(false)) => reduce(true, false), | |
102 | (Bool(true), Bool(false)) => reduce(false, false), | |
103 | (RetBool(false), RetBool(true)) => reduce(true, true), | |
104 | (Bool(false), Bool(true)) => reduce(false, true), | |
105 | _ => (), | |
106 | } | |
107 | } else { | |
108 | panic!("IfExpr 'then' node is not an ExprBlock"); | |
109 | } | |
110 | } | |
111 | } | |
112 | } | |
113 | ||
114 | #[derive(Copy, Clone)] | |
115 | pub struct BoolComparison; | |
116 | ||
117 | impl LintPass for BoolComparison { | |
118 | fn get_lints(&self) -> LintArray { | |
119 | lint_array!(BOOL_COMPARISON) | |
120 | } | |
121 | } | |
122 | ||
123 | impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoolComparison { | |
124 | fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { | |
125 | use self::Expression::*; | |
126 | if let ExprBinary(Spanned { node: BiEq, .. }, ref left_side, ref right_side) = e.node { | |
127 | match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) { | |
128 | (Bool(true), Other) => { | |
129 | let hint = snippet(cx, right_side.span, "..").into_owned(); | |
130 | span_lint_and_sugg( | |
131 | cx, | |
132 | BOOL_COMPARISON, | |
133 | e.span, | |
134 | "equality checks against true are unnecessary", | |
135 | "try simplifying it as shown", | |
136 | hint, | |
137 | ); | |
138 | }, | |
139 | (Other, Bool(true)) => { | |
140 | let hint = snippet(cx, left_side.span, "..").into_owned(); | |
141 | span_lint_and_sugg( | |
142 | cx, | |
143 | BOOL_COMPARISON, | |
144 | e.span, | |
145 | "equality checks against true are unnecessary", | |
146 | "try simplifying it as shown", | |
147 | hint, | |
148 | ); | |
149 | }, | |
150 | (Bool(false), Other) => { | |
151 | let hint = Sugg::hir(cx, right_side, ".."); | |
152 | span_lint_and_sugg( | |
153 | cx, | |
154 | BOOL_COMPARISON, | |
155 | e.span, | |
156 | "equality checks against false can be replaced by a negation", | |
157 | "try simplifying it as shown", | |
158 | (!hint).to_string(), | |
159 | ); | |
160 | }, | |
161 | (Other, Bool(false)) => { | |
162 | let hint = Sugg::hir(cx, left_side, ".."); | |
163 | span_lint_and_sugg( | |
164 | cx, | |
165 | BOOL_COMPARISON, | |
166 | e.span, | |
167 | "equality checks against false can be replaced by a negation", | |
168 | "try simplifying it as shown", | |
169 | (!hint).to_string(), | |
170 | ); | |
171 | }, | |
172 | _ => (), | |
173 | } | |
174 | } | |
175 | } | |
176 | } | |
177 | ||
178 | enum Expression { | |
179 | Bool(bool), | |
180 | RetBool(bool), | |
181 | Other, | |
182 | } | |
183 | ||
184 | fn fetch_bool_block(block: &Block) -> Expression { | |
185 | match (&*block.stmts, block.expr.as_ref()) { | |
186 | (&[], Some(e)) => fetch_bool_expr(&**e), | |
abe05a73 XL |
187 | (&[ref e], None) => if let StmtSemi(ref e, _) = e.node { |
188 | if let ExprRet(_) = e.node { | |
189 | fetch_bool_expr(&**e) | |
ea8adc8c XL |
190 | } else { |
191 | Expression::Other | |
192 | } | |
abe05a73 XL |
193 | } else { |
194 | Expression::Other | |
ea8adc8c XL |
195 | }, |
196 | _ => Expression::Other, | |
197 | } | |
198 | } | |
199 | ||
200 | fn fetch_bool_expr(expr: &Expr) -> Expression { | |
201 | match expr.node { | |
202 | ExprBlock(ref block) => fetch_bool_block(block), | |
abe05a73 XL |
203 | ExprLit(ref lit_ptr) => if let LitKind::Bool(value) = lit_ptr.node { |
204 | Expression::Bool(value) | |
205 | } else { | |
206 | Expression::Other | |
ea8adc8c | 207 | }, |
abe05a73 XL |
208 | ExprRet(Some(ref expr)) => match fetch_bool_expr(expr) { |
209 | Expression::Bool(value) => Expression::RetBool(value), | |
210 | _ => Expression::Other, | |
ea8adc8c XL |
211 | }, |
212 | _ => Expression::Other, | |
213 | } | |
214 | } |