Raise a lint when suggest has simplified the expression.

This commit is contained in:
laurent 2017-11-14 21:14:08 +00:00
parent bdf3887d22
commit 25783fa485
2 changed files with 66 additions and 26 deletions

View file

@ -159,8 +159,16 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
} }
} }
fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String { // The boolean part of the return indicates whether some simplifications have been applied.
fn recurse(brackets: bool, cx: &LateContext, suggestion: &Bool, terminals: &[&Expr], mut s: String) -> String { fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> (String, bool) {
fn recurse(
brackets: bool,
cx: &LateContext,
suggestion: &Bool,
terminals: &[&Expr],
mut s: String,
simplified: &mut bool,
) -> String {
use quine_mc_cluskey::Bool::*; use quine_mc_cluskey::Bool::*;
let snip = |e: &Expr| snippet_opt(cx, e.span).expect("don't try to improve booleans created by macros"); let snip = |e: &Expr| snippet_opt(cx, e.span).expect("don't try to improve booleans created by macros");
match *suggestion { match *suggestion {
@ -175,7 +183,7 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
Not(ref inner) => match **inner { Not(ref inner) => match **inner {
And(_) | Or(_) => { And(_) | Or(_) => {
s.push('!'); s.push('!');
recurse(true, cx, inner, terminals, s) recurse(true, cx, inner, terminals, s, simplified)
}, },
Term(n) => match terminals[n as usize].node { Term(n) => match terminals[n as usize].node {
ExprBinary(binop, ref lhs, ref rhs) => { ExprBinary(binop, ref lhs, ref rhs) => {
@ -188,9 +196,10 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
BiGe => " < ", BiGe => " < ",
_ => { _ => {
s.push('!'); s.push('!');
return recurse(true, cx, inner, terminals, s); return recurse(true, cx, inner, terminals, s, simplified);
}, },
}; };
*simplified = true;
s.push_str(&snip(lhs)); s.push_str(&snip(lhs));
s.push_str(op); s.push_str(op);
s.push_str(&snip(rhs)); s.push_str(&snip(rhs));
@ -202,6 +211,7 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
.flat_map(|(a, b)| vec![(a, b), (b, a)]) .flat_map(|(a, b)| vec![(a, b), (b, a)])
.find(|&(a, _)| a == path.name.as_str()); .find(|&(a, _)| a == path.name.as_str());
if let Some((_, negation_method)) = negation { if let Some((_, negation_method)) = negation {
*simplified = true;
s.push_str(&snip(&args[0])); s.push_str(&snip(&args[0]));
s.push('.'); s.push('.');
s.push_str(negation_method); s.push_str(negation_method);
@ -209,17 +219,17 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
s s
} else { } else {
s.push('!'); s.push('!');
recurse(false, cx, inner, terminals, s) recurse(false, cx, inner, terminals, s, simplified)
} }
}, },
_ => { _ => {
s.push('!'); s.push('!');
recurse(false, cx, inner, terminals, s) recurse(false, cx, inner, terminals, s, simplified)
}, },
}, },
_ => { _ => {
s.push('!'); s.push('!');
recurse(false, cx, inner, terminals, s) recurse(false, cx, inner, terminals, s, simplified)
}, },
}, },
And(ref v) => { And(ref v) => {
@ -227,16 +237,16 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
s.push('('); s.push('(');
} }
if let Or(_) = v[0] { if let Or(_) = v[0] {
s = recurse(true, cx, &v[0], terminals, s); s = recurse(true, cx, &v[0], terminals, s, simplified);
} else { } else {
s = recurse(false, cx, &v[0], terminals, s); s = recurse(false, cx, &v[0], terminals, s, simplified);
} }
for inner in &v[1..] { for inner in &v[1..] {
s.push_str(" && "); s.push_str(" && ");
if let Or(_) = *inner { if let Or(_) = *inner {
s = recurse(true, cx, inner, terminals, s); s = recurse(true, cx, inner, terminals, s, simplified);
} else { } else {
s = recurse(false, cx, inner, terminals, s); s = recurse(false, cx, inner, terminals, s, simplified);
} }
} }
if brackets { if brackets {
@ -248,10 +258,10 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
if brackets { if brackets {
s.push('('); s.push('(');
} }
s = recurse(false, cx, &v[0], terminals, s); s = recurse(false, cx, &v[0], terminals, s, simplified);
for inner in &v[1..] { for inner in &v[1..] {
s.push_str(" || "); s.push_str(" || ");
s = recurse(false, cx, inner, terminals, s); s = recurse(false, cx, inner, terminals, s, simplified);
} }
if brackets { if brackets {
s.push(')'); s.push(')');
@ -274,7 +284,9 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
}, },
} }
} }
recurse(false, cx, suggestion, terminals, String::new()) let mut simplified = false;
let s = recurse(false, cx, suggestion, terminals, String::new(), &mut simplified);
(s, simplified)
} }
fn simple_negate(b: Bool) -> Bool { fn simple_negate(b: Bool) -> Bool {
@ -384,7 +396,7 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
db.span_suggestion( db.span_suggestion(
e.span, e.span,
"it would look like the following", "it would look like the following",
suggest(self.cx, suggestion, &h2q.terminals), suggest(self.cx, suggestion, &h2q.terminals).0,
); );
}, },
); );
@ -401,22 +413,26 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
improvements.push(suggestion); improvements.push(suggestion);
} }
} }
if !improvements.is_empty() { let nonminimal_bool_lint = |suggestions| {
span_lint_and_then( span_lint_and_then(
self.cx, self.cx,
NONMINIMAL_BOOL, NONMINIMAL_BOOL,
e.span, e.span,
"this boolean expression can be simplified", "this boolean expression can be simplified",
|db| { |db| { db.span_suggestions(e.span, "try", suggestions); },
db.span_suggestions( );
e.span, };
"try", if improvements.is_empty() {
let suggest = suggest(self.cx, &expr, &h2q.terminals);
if suggest.1 {
nonminimal_bool_lint(vec![suggest.0])
}
} else {
nonminimal_bool_lint(
improvements improvements
.into_iter() .into_iter()
.map(|suggestion| suggest(self.cx, suggestion, &h2q.terminals)) .map(|suggestion| suggest(self.cx, suggestion, &h2q.terminals).0)
.collect(), .collect()
);
},
); );
} }
} }

View file

@ -130,6 +130,30 @@ help: try
39 | let _ = !(a == b && c == d); 39 | let _ = !(a == b && c == d);
| ^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^
error: this boolean expression can be simplified
--> $DIR/booleans.rs:47:13
|
47 | let _ = !a.is_some();
| ^^^^^^^^^^^^ help: try: `a.is_none()`
error: this boolean expression can be simplified
--> $DIR/booleans.rs:49:13
|
49 | let _ = !a.is_none();
| ^^^^^^^^^^^^ help: try: `a.is_some()`
error: this boolean expression can be simplified
--> $DIR/booleans.rs:51:13
|
51 | let _ = !b.is_err();
| ^^^^^^^^^^^ help: try: `b.is_ok()`
error: this boolean expression can be simplified
--> $DIR/booleans.rs:53:13
|
53 | let _ = !b.is_ok();
| ^^^^^^^^^^ help: try: `b.is_err()`
error: this boolean expression can be simplified error: this boolean expression can be simplified
--> $DIR/booleans.rs:55:13 --> $DIR/booleans.rs:55:13
| |