]> git.proxmox.com Git - rustc.git/blame - src/tools/clippy/clippy_lints/src/booleans.rs
New upstream version 1.27.1+dfsg1
[rustc.git] / src / tools / clippy / clippy_lints / src / booleans.rs
CommitLineData
abe05a73 1use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
ea8adc8c
XL
2use rustc::hir::*;
3use rustc::hir::intravisit::*;
abe05a73
XL
4use syntax::ast::{LitKind, NodeId, DUMMY_NODE_ID};
5use syntax::codemap::{dummy_spanned, Span, DUMMY_SP};
ea8adc8c 6use syntax::util::ThinVec;
83c7162d 7use utils::{in_macro, paths, match_type, snippet_opt, span_lint_and_then, SpanlessEq};
ea8adc8c
XL
8
9/// **What it does:** Checks for boolean expressions that can be written more
10/// concisely.
11///
12/// **Why is this bad?** Readability of boolean expressions suffers from
13/// unnecessary duplication.
14///
15/// **Known problems:** Ignores short circuiting behavior of `||` and
16/// `&&`. Ignores `|`, `&` and `^`.
17///
18/// **Example:**
19/// ```rust
20/// if a && true // should be: if a
21/// if !(a == b) // should be: if a != b
22/// ```
0531ce1d 23declare_clippy_lint! {
ea8adc8c 24 pub NONMINIMAL_BOOL,
0531ce1d 25 complexity,
ea8adc8c
XL
26 "boolean expressions that can be written more concisely"
27}
28
29/// **What it does:** Checks for boolean expressions that contain terminals that
30/// can be eliminated.
31///
32/// **Why is this bad?** This is most likely a logic bug.
33///
34/// **Known problems:** Ignores short circuiting behavior.
35///
36/// **Example:**
37/// ```rust
38/// if a && b || a { ... }
39/// ```
40/// The `b` is unnecessary, the expression is equivalent to `if a`.
0531ce1d 41declare_clippy_lint! {
ea8adc8c 42 pub LOGIC_BUG,
0531ce1d 43 correctness,
ea8adc8c
XL
44 "boolean expressions that contain terminals which can be eliminated"
45}
46
abe05a73
XL
47// For each pairs, both orders are considered.
48const METHODS_WITH_NEGATION: [(&str, &str); 2] = [
49 ("is_some", "is_none"),
50 ("is_err", "is_ok"),
51];
52
ea8adc8c
XL
53#[derive(Copy, Clone)]
54pub struct NonminimalBool;
55
56impl LintPass for NonminimalBool {
57 fn get_lints(&self) -> LintArray {
58 lint_array!(NONMINIMAL_BOOL, LOGIC_BUG)
59 }
60}
61
62impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonminimalBool {
63 fn check_fn(
64 &mut self,
65 cx: &LateContext<'a, 'tcx>,
66 _: intravisit::FnKind<'tcx>,
67 _: &'tcx FnDecl,
68 body: &'tcx Body,
69 _: Span,
70 _: NodeId,
71 ) {
0531ce1d 72 NonminimalBoolVisitor { cx }.visit_body(body)
ea8adc8c
XL
73 }
74}
75
76struct NonminimalBoolVisitor<'a, 'tcx: 'a> {
77 cx: &'a LateContext<'a, 'tcx>,
78}
79
80use quine_mc_cluskey::Bool;
81struct Hir2Qmm<'a, 'tcx: 'a, 'v> {
82 terminals: Vec<&'v Expr>,
83 cx: &'a LateContext<'a, 'tcx>,
84}
85
86impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
87 fn extract(&mut self, op: BinOp_, a: &[&'v Expr], mut v: Vec<Bool>) -> Result<Vec<Bool>, String> {
88 for a in a {
89 if let ExprBinary(binop, ref lhs, ref rhs) = a.node {
90 if binop.node == op {
91 v = self.extract(op, &[lhs, rhs], v)?;
92 continue;
93 }
94 }
95 v.push(self.run(a)?);
96 }
97 Ok(v)
98 }
99
100 fn run(&mut self, e: &'v Expr) -> Result<Bool, String> {
101 // prevent folding of `cfg!` macros and the like
102 if !in_macro(e.span) {
103 match e.node {
104 ExprUnary(UnNot, ref inner) => return Ok(Bool::Not(box self.run(inner)?)),
abe05a73
XL
105 ExprBinary(binop, ref lhs, ref rhs) => match binop.node {
106 BiOr => return Ok(Bool::Or(self.extract(BiOr, &[lhs, rhs], Vec::new())?)),
107 BiAnd => return Ok(Bool::And(self.extract(BiAnd, &[lhs, rhs], Vec::new())?)),
108 _ => (),
ea8adc8c 109 },
abe05a73
XL
110 ExprLit(ref lit) => match lit.node {
111 LitKind::Bool(true) => return Ok(Bool::True),
112 LitKind::Bool(false) => return Ok(Bool::False),
113 _ => (),
ea8adc8c
XL
114 },
115 _ => (),
116 }
117 }
118 for (n, expr) in self.terminals.iter().enumerate() {
119 if SpanlessEq::new(self.cx).ignore_fn().eq_expr(e, expr) {
abe05a73
XL
120 #[allow(cast_possible_truncation)]
121 return Ok(Bool::Term(n as u8));
ea8adc8c
XL
122 }
123 let negated = match e.node {
124 ExprBinary(binop, ref lhs, ref rhs) => {
125 let mk_expr = |op| {
126 Expr {
127 id: DUMMY_NODE_ID,
128 hir_id: DUMMY_HIR_ID,
129 span: DUMMY_SP,
130 attrs: ThinVec::new(),
131 node: ExprBinary(dummy_spanned(op), lhs.clone(), rhs.clone()),
132 }
133 };
134 match binop.node {
135 BiEq => mk_expr(BiNe),
136 BiNe => mk_expr(BiEq),
137 BiGt => mk_expr(BiLe),
138 BiGe => mk_expr(BiLt),
139 BiLt => mk_expr(BiGe),
140 BiLe => mk_expr(BiGt),
141 _ => continue,
142 }
143 },
144 _ => continue,
145 };
146 if SpanlessEq::new(self.cx).ignore_fn().eq_expr(&negated, expr) {
abe05a73
XL
147 #[allow(cast_possible_truncation)]
148 return Ok(Bool::Not(Box::new(Bool::Term(n as u8))));
ea8adc8c
XL
149 }
150 }
151 let n = self.terminals.len();
152 self.terminals.push(e);
153 if n < 32 {
abe05a73
XL
154 #[allow(cast_possible_truncation)]
155 Ok(Bool::Term(n as u8))
ea8adc8c
XL
156 } else {
157 Err("too many literals".to_owned())
158 }
159 }
160}
161
ff7c6d11
XL
162struct SuggestContext<'a, 'tcx: 'a, 'v> {
163 terminals: &'v [&'v Expr],
164 cx: &'a LateContext<'a, 'tcx>,
165 output: String,
166 simplified: bool,
167}
168
169impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> {
170 fn snip(&self, e: &Expr) -> Option<String> {
171 snippet_opt(self.cx, e.span)
172 }
173
174 fn simplify_not(&self, expr: &Expr) -> Option<String> {
175 match expr.node {
176 ExprBinary(binop, ref lhs, ref rhs) => {
177 match binop.node {
178 BiEq => Some(" != "),
179 BiNe => Some(" == "),
180 BiLt => Some(" >= "),
181 BiGt => Some(" <= "),
182 BiLe => Some(" > "),
183 BiGe => Some(" < "),
184 _ => None,
185 }.and_then(|op| Some(format!("{}{}{}", self.snip(lhs)?, op, self.snip(rhs)?)))
186 },
187 ExprMethodCall(ref path, _, ref args) if args.len() == 1 => {
83c7162d
XL
188 let type_of_receiver = self.cx.tables.expr_ty(&args[0]);
189 if !match_type(self.cx, type_of_receiver, &paths::OPTION) &&
190 !match_type(self.cx, type_of_receiver, &paths::RESULT) {
191 return None;
192 }
ff7c6d11
XL
193 METHODS_WITH_NEGATION
194 .iter().cloned()
195 .flat_map(|(a, b)| vec![(a, b), (b, a)])
196 .find(|&(a, _)| a == path.name.as_str())
197 .and_then(|(_, neg_method)| Some(format!("{}.{}()", self.snip(&args[0])?, neg_method)))
198 },
199 _ => None,
200 }
201 }
202
203 fn recurse(&mut self, suggestion: &Bool) -> Option<()> {
ea8adc8c 204 use quine_mc_cluskey::Bool::*;
ea8adc8c
XL
205 match *suggestion {
206 True => {
ff7c6d11 207 self.output.push_str("true");
ea8adc8c
XL
208 },
209 False => {
ff7c6d11 210 self.output.push_str("false");
ea8adc8c 211 },
abe05a73
XL
212 Not(ref inner) => match **inner {
213 And(_) | Or(_) => {
ff7c6d11
XL
214 self.output.push('!');
215 self.output.push('(');
216 self.recurse(inner);
217 self.output.push(')');
abe05a73 218 },
ff7c6d11
XL
219 Term(n) => {
220 let terminal = self.terminals[n as usize];
221 if let Some(str) = self.simplify_not(terminal) {
222 self.simplified = true;
223 self.output.push_str(&str)
224 } else {
225 self.output.push('!');
226 let snip = self.snip(terminal)?;
227 self.output.push_str(&snip);
228 }
abe05a73 229 },
ff7c6d11
XL
230 True | False | Not(_) => {
231 self.output.push('!');
232 self.recurse(inner)?;
abe05a73 233 },
ea8adc8c
XL
234 },
235 And(ref v) => {
ff7c6d11
XL
236 for (index, inner) in v.iter().enumerate() {
237 if index > 0 {
238 self.output.push_str(" && ");
239 }
ea8adc8c 240 if let Or(_) = *inner {
ff7c6d11
XL
241 self.output.push('(');
242 self.recurse(inner);
243 self.output.push(')');
ea8adc8c 244 } else {
ff7c6d11 245 self.recurse(inner);
ea8adc8c
XL
246 }
247 }
ea8adc8c
XL
248 },
249 Or(ref v) => {
ff7c6d11
XL
250 for (index, inner) in v.iter().enumerate() {
251 if index > 0 {
252 self.output.push_str(" || ");
253 }
254 self.recurse(inner);
ea8adc8c 255 }
ea8adc8c
XL
256 },
257 Term(n) => {
ff7c6d11
XL
258 let snip = self.snip(self.terminals[n as usize])?;
259 self.output.push_str(&snip);
ea8adc8c
XL
260 },
261 }
ff7c6d11 262 Some(())
ea8adc8c 263 }
ff7c6d11
XL
264}
265
266// The boolean part of the return indicates whether some simplifications have been applied.
267fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> (String, bool) {
268 let mut suggest_context = SuggestContext {
0531ce1d
XL
269 terminals,
270 cx,
ff7c6d11
XL
271 output: String::new(),
272 simplified: false,
273 };
274 suggest_context.recurse(suggestion);
275 (suggest_context.output, suggest_context.simplified)
ea8adc8c
XL
276}
277
278fn simple_negate(b: Bool) -> Bool {
279 use quine_mc_cluskey::Bool::*;
280 match b {
281 True => False,
282 False => True,
283 t @ Term(_) => Not(Box::new(t)),
284 And(mut v) => {
285 for el in &mut v {
286 *el = simple_negate(::std::mem::replace(el, True));
287 }
288 Or(v)
289 },
290 Or(mut v) => {
291 for el in &mut v {
292 *el = simple_negate(::std::mem::replace(el, True));
293 }
294 And(v)
295 },
296 Not(inner) => *inner,
297 }
298}
299
300#[derive(Default)]
301struct Stats {
302 terminals: [usize; 32],
303 negations: usize,
304 ops: usize,
305}
306
307fn terminal_stats(b: &Bool) -> Stats {
308 fn recurse(b: &Bool, stats: &mut Stats) {
309 match *b {
310 True | False => stats.ops += 1,
311 Not(ref inner) => {
312 match **inner {
313 And(_) | Or(_) => stats.ops += 1, // brackets are also operations
314 _ => stats.negations += 1,
315 }
316 recurse(inner, stats);
317 },
318 And(ref v) | Or(ref v) => {
319 stats.ops += v.len() - 1;
320 for inner in v {
321 recurse(inner, stats);
322 }
323 },
324 Term(n) => stats.terminals[n as usize] += 1,
325 }
326 }
327 use quine_mc_cluskey::Bool::*;
328 let mut stats = Stats::default();
329 recurse(b, &mut stats);
330 stats
331}
332
333impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
334 fn bool_expr(&self, e: &Expr) {
335 let mut h2q = Hir2Qmm {
336 terminals: Vec::new(),
337 cx: self.cx,
338 };
339 if let Ok(expr) = h2q.run(e) {
ea8adc8c
XL
340 if h2q.terminals.len() > 8 {
341 // QMC has exponentially slow behavior as the number of terminals increases
342 // 8 is reasonable, it takes approximately 0.2 seconds.
343 // See #825
344 return;
345 }
346
347 let stats = terminal_stats(&expr);
348 let mut simplified = expr.simplify();
349 for simple in Bool::Not(Box::new(expr.clone())).simplify() {
350 match simple {
351 Bool::Not(_) | Bool::True | Bool::False => {},
352 _ => simplified.push(Bool::Not(Box::new(simple.clone()))),
353 }
354 let simple_negated = simple_negate(simple);
355 if simplified.iter().any(|s| *s == simple_negated) {
356 continue;
357 }
358 simplified.push(simple_negated);
359 }
360 let mut improvements = Vec::new();
361 'simplified: for suggestion in &simplified {
362 let simplified_stats = terminal_stats(suggestion);
363 let mut improvement = false;
364 for i in 0..32 {
365 // ignore any "simplifications" that end up requiring a terminal more often
366 // than in the original expression
367 if stats.terminals[i] < simplified_stats.terminals[i] {
368 continue 'simplified;
369 }
370 if stats.terminals[i] != 0 && simplified_stats.terminals[i] == 0 {
371 span_lint_and_then(
372 self.cx,
373 LOGIC_BUG,
374 e.span,
375 "this boolean expression contains a logic bug",
376 |db| {
377 db.span_help(
378 h2q.terminals[i].span,
379 "this expression can be optimized out by applying boolean operations to the \
abe05a73 380 outer expression",
ea8adc8c
XL
381 );
382 db.span_suggestion(
383 e.span,
384 "it would look like the following",
abe05a73 385 suggest(self.cx, suggestion, &h2q.terminals).0,
ea8adc8c
XL
386 );
387 },
388 );
389 // don't also lint `NONMINIMAL_BOOL`
390 return;
391 }
392 // if the number of occurrences of a terminal decreases or any of the stats
393 // decreases while none increases
abe05a73
XL
394 improvement |= (stats.terminals[i] > simplified_stats.terminals[i])
395 || (stats.negations > simplified_stats.negations && stats.ops == simplified_stats.ops)
396 || (stats.ops > simplified_stats.ops && stats.negations == simplified_stats.negations);
ea8adc8c
XL
397 }
398 if improvement {
399 improvements.push(suggestion);
400 }
401 }
abe05a73 402 let nonminimal_bool_lint = |suggestions| {
ea8adc8c
XL
403 span_lint_and_then(
404 self.cx,
405 NONMINIMAL_BOOL,
406 e.span,
407 "this boolean expression can be simplified",
abe05a73
XL
408 |db| { db.span_suggestions(e.span, "try", suggestions); },
409 );
410 };
411 if improvements.is_empty() {
412 let suggest = suggest(self.cx, &expr, &h2q.terminals);
413 if suggest.1 {
414 nonminimal_bool_lint(vec![suggest.0])
415 }
416 } else {
417 nonminimal_bool_lint(
418 improvements
419 .into_iter()
420 .map(|suggestion| suggest(self.cx, suggestion, &h2q.terminals).0)
421 .collect()
ea8adc8c
XL
422 );
423 }
424 }
425 }
426}
427
428impl<'a, 'tcx> Visitor<'tcx> for NonminimalBoolVisitor<'a, 'tcx> {
429 fn visit_expr(&mut self, e: &'tcx Expr) {
430 if in_macro(e.span) {
431 return;
432 }
433 match e.node {
434 ExprBinary(binop, _, _) if binop.node == BiOr || binop.node == BiAnd => self.bool_expr(e),
abe05a73
XL
435 ExprUnary(UnNot, ref inner) => if self.cx.tables.node_types()[inner.hir_id].is_bool() {
436 self.bool_expr(e);
437 } else {
438 walk_expr(self, e);
ea8adc8c
XL
439 },
440 _ => walk_expr(self, e),
441 }
442 }
443 fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
444 NestedVisitorMap::None
445 }
446}