diff options
| -rw-r--r-- | clippy_lints/src/booleans.rs | 122 | ||||
| -rw-r--r-- | tests/ui/booleans.rs | 20 | ||||
| -rw-r--r-- | tests/ui/booleans.stderr | 12 |
3 files changed, 99 insertions, 55 deletions
diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs index b520a3e2576..a85b77475e5 100644 --- a/clippy_lints/src/booleans.rs +++ b/clippy_lints/src/booleans.rs @@ -1,5 +1,6 @@ use crate::utils::{ - get_trait_def_id, implements_trait, in_macro, match_type, paths, snippet_opt, span_lint_and_then, SpanlessEq, + get_trait_def_id, implements_trait, in_macro, match_type, paths, snippet_opt, span_lint_and_sugg, + span_lint_and_then, SpanlessEq, }; use if_chain::if_chain; use rustc::hir::intravisit::*; @@ -159,46 +160,6 @@ struct SuggestContext<'a, 'tcx, 'v> { } impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> { - fn snip(&self, e: &Expr) -> Option<String> { - snippet_opt(self.cx, e.span) - } - - fn simplify_not(&self, expr: &Expr) -> Option<String> { - match &expr.node { - ExprKind::Binary(binop, lhs, rhs) => { - if !implements_ord(self.cx, lhs) { - return None; - } - - match binop.node { - BinOpKind::Eq => Some(" != "), - BinOpKind::Ne => Some(" == "), - BinOpKind::Lt => Some(" >= "), - BinOpKind::Gt => Some(" <= "), - BinOpKind::Le => Some(" > "), - BinOpKind::Ge => Some(" < "), - _ => None, - } - .and_then(|op| Some(format!("{}{}{}", self.snip(lhs)?, op, self.snip(rhs)?))) - }, - ExprKind::MethodCall(path, _, args) if args.len() == 1 => { - let type_of_receiver = self.cx.tables.expr_ty(&args[0]); - if !match_type(self.cx, type_of_receiver, &paths::OPTION) - && !match_type(self.cx, type_of_receiver, &paths::RESULT) - { - return None; - } - METHODS_WITH_NEGATION - .iter() - .cloned() - .flat_map(|(a, b)| vec![(a, b), (b, a)]) - .find(|&(a, _)| a == path.ident.name.as_str()) - .and_then(|(_, neg_method)| Some(format!("{}.{}()", self.snip(&args[0])?, neg_method))) - }, - _ => None, - } - } - fn recurse(&mut self, suggestion: &Bool) -> Option<()> { use quine_mc_cluskey::Bool::*; match suggestion { @@ -217,12 +178,12 @@ impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> { }, Term(n) => { let terminal = self.terminals[n as usize]; - if let Some(str) = self.simplify_not(terminal) { + if let Some(str) = simplify_not(self.cx, terminal) { self.simplified = true; self.output.push_str(&str) } else { self.output.push('!'); - let snip = self.snip(terminal)?; + let snip = snip(self.cx, terminal)?; self.output.push_str(&snip); } }, @@ -254,7 +215,7 @@ impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> { } }, &Term(n) => { - let snip = self.snip(self.terminals[n as usize])?; + let snip = snip(self.cx, self.terminals[n as usize])?; self.output.push_str(&snip); }, } @@ -262,6 +223,44 @@ impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> { } } +fn snip(cx: &LateContext<'_, '_>, e: &Expr) -> Option<String> { + snippet_opt(cx, e.span) +} + +fn simplify_not(cx: &LateContext<'_, '_>, expr: &Expr) -> Option<String> { + match &expr.node { + ExprKind::Binary(binop, lhs, rhs) => { + if !implements_ord(cx, lhs) { + return None; + } + + match binop.node { + BinOpKind::Eq => Some(" != "), + BinOpKind::Ne => Some(" == "), + BinOpKind::Lt => Some(" >= "), + BinOpKind::Gt => Some(" <= "), + BinOpKind::Le => Some(" > "), + BinOpKind::Ge => Some(" < "), + _ => None, + } + .and_then(|op| Some(format!("{}{}{}", snip(cx, lhs)?, op, snip(cx, rhs)?))) + }, + ExprKind::MethodCall(path, _, args) if args.len() == 1 => { + let type_of_receiver = cx.tables.expr_ty(&args[0]); + if !match_type(cx, type_of_receiver, &paths::OPTION) && !match_type(cx, type_of_receiver, &paths::RESULT) { + return None; + } + METHODS_WITH_NEGATION + .iter() + .cloned() + .flat_map(|(a, b)| vec![(a, b), (b, a)]) + .find(|&(a, _)| a == path.ident.name.as_str()) + .and_then(|(_, neg_method)| Some(format!("{}.{}()", snip(cx, &args[0])?, neg_method))) + }, + _ => None, + } +} + // The boolean part of the return indicates whether some simplifications have been applied. fn suggest(cx: &LateContext<'_, '_>, suggestion: &Bool, terminals: &[&Expr]) -> (String, bool) { let mut suggest_context = SuggestContext { @@ -330,7 +329,7 @@ fn terminal_stats(b: &Bool) -> Stats { } impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> { - fn bool_expr(&self, e: &Expr) { + fn bool_expr(&self, e: &'tcx Expr) { let mut h2q = Hir2Qmm { terminals: Vec::new(), cx: self.cx, @@ -420,10 +419,8 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> { ); }; if improvements.is_empty() { - let suggest = suggest(self.cx, &expr, &h2q.terminals); - if suggest.1 { - nonminimal_bool_lint(vec![suggest.0]) - } + let mut visitor = NotSimplificationVisitor { cx: self.cx }; + visitor.visit_expr(e); } else { nonminimal_bool_lint( improvements @@ -464,3 +461,30 @@ fn implements_ord<'a, 'tcx>(cx: &'a LateContext<'a, 'tcx>, expr: &Expr) -> bool let ty = cx.tables.expr_ty(expr); get_trait_def_id(cx, &paths::ORD).map_or(false, |id| implements_trait(cx, ty, id, &[])) } + +struct NotSimplificationVisitor<'a, 'tcx> { + cx: &'a LateContext<'a, 'tcx>, +} + +impl<'a, 'tcx> Visitor<'tcx> for NotSimplificationVisitor<'a, 'tcx> { + fn visit_expr(&mut self, expr: &'tcx Expr) { + if let ExprKind::Unary(UnNot, inner) = &expr.node { + if let Some(suggestion) = simplify_not(self.cx, inner) { + span_lint_and_sugg( + self.cx, + NONMINIMAL_BOOL, + expr.span, + "this boolean expression can be simplified", + "try", + suggestion, + Applicability::MachineApplicable, + ); + } + } + + walk_expr(self, expr); + } + fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { + NestedVisitorMap::None + } +} diff --git a/tests/ui/booleans.rs b/tests/ui/booleans.rs index c8e01d4b258..ece20fb1eab 100644 --- a/tests/ui/booleans.rs +++ b/tests/ui/booleans.rs @@ -142,3 +142,23 @@ fn dont_warn_for_negated_partial_ord_comparison() { let _ = !(a > b); let _ = !(a >= b); } + +fn issue3847(a: u32, b: u32) -> bool { + const THRESHOLD: u32 = 1_000; + + if a < THRESHOLD && b >= THRESHOLD || a >= THRESHOLD && b < THRESHOLD { + return false; + } + true +} + +fn issue4548() { + fn f(_i: u32, _j: u32) -> u32 { + unimplemented!(); + } + + let i = 0; + let j = 0; + + if i != j && f(i, j) != 0 || i == j && f(i, j) != 1 {} +} diff --git a/tests/ui/booleans.stderr b/tests/ui/booleans.stderr index eebab8c3e25..ab0b54e26d7 100644 --- a/tests/ui/booleans.stderr +++ b/tests/ui/booleans.stderr @@ -158,22 +158,22 @@ LL | let _ = !(a.is_some() && !c); | ^^^^^^^^^^^^^^^^^^^^ help: try: `c || a.is_none()` error: this boolean expression can be simplified - --> $DIR/booleans.rs:54:13 + --> $DIR/booleans.rs:54:26 | LL | let _ = !(!c ^ c) || !a.is_some(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `!(!c ^ c) || a.is_none()` + | ^^^^^^^^^^^^ help: try: `a.is_none()` error: this boolean expression can be simplified - --> $DIR/booleans.rs:55:13 + --> $DIR/booleans.rs:55:25 | LL | let _ = (!c ^ c) || !a.is_some(); - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(!c ^ c) || a.is_none()` + | ^^^^^^^^^^^^ help: try: `a.is_none()` error: this boolean expression can be simplified - --> $DIR/booleans.rs:56:13 + --> $DIR/booleans.rs:56:23 | LL | let _ = !c ^ c || !a.is_some(); - | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `!c ^ c || a.is_none()` + | ^^^^^^^^^^^^ help: try: `a.is_none()` error: this boolean expression can be simplified --> $DIR/booleans.rs:128:8 |