Merge pull request #2216 from LaurentMazare/master
Handle methods with an obvious negation in the non-minimal bool lint
This commit is contained in:
Oliver Schneider
GitHub
commit
f975fb54f8
3 changed files with 129 additions and 42 deletions
|
|
@ -44,6 +44,12 @@ declare_lint! {
|
|||
"boolean expressions that contain terminals which can be eliminated"
|
||||
}
|
||||
|
||||
// For each pairs, both orders are considered.
|
||||
const METHODS_WITH_NEGATION: [(&str, &str); 2] = [
|
||||
("is_some", "is_none"),
|
||||
("is_err", "is_ok"),
|
||||
];
|
||||
|
||||
#[derive(Copy, Clone)]
|
||||
pub struct NonminimalBool;
|
||||
|
||||
|
|
@ -153,8 +159,16 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
|
|||
}
|
||||
}
|
||||
|
||||
fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
|
||||
fn recurse(brackets: bool, cx: &LateContext, suggestion: &Bool, terminals: &[&Expr], mut s: String) -> String {
|
||||
// The boolean part of the return indicates whether some simplifications have been applied.
|
||||
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::*;
|
||||
let snip = |e: &Expr| snippet_opt(cx, e.span).expect("don't try to improve booleans created by macros");
|
||||
match *suggestion {
|
||||
|
|
@ -169,32 +183,53 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
|
|||
Not(ref inner) => match **inner {
|
||||
And(_) | Or(_) => {
|
||||
s.push('!');
|
||||
recurse(true, cx, inner, terminals, s)
|
||||
recurse(true, cx, inner, terminals, s, simplified)
|
||||
},
|
||||
Term(n) => if let ExprBinary(binop, ref lhs, ref rhs) = terminals[n as usize].node {
|
||||
let op = match binop.node {
|
||||
BiEq => " != ",
|
||||
BiNe => " == ",
|
||||
BiLt => " >= ",
|
||||
BiGt => " <= ",
|
||||
BiLe => " > ",
|
||||
BiGe => " < ",
|
||||
_ => {
|
||||
Term(n) => match terminals[n as usize].node {
|
||||
ExprBinary(binop, ref lhs, ref rhs) => {
|
||||
let op = match binop.node {
|
||||
BiEq => " != ",
|
||||
BiNe => " == ",
|
||||
BiLt => " >= ",
|
||||
BiGt => " <= ",
|
||||
BiLe => " > ",
|
||||
BiGe => " < ",
|
||||
_ => {
|
||||
s.push('!');
|
||||
return recurse(true, cx, inner, terminals, s, simplified);
|
||||
},
|
||||
};
|
||||
*simplified = true;
|
||||
s.push_str(&snip(lhs));
|
||||
s.push_str(op);
|
||||
s.push_str(&snip(rhs));
|
||||
s
|
||||
},
|
||||
ExprMethodCall(ref path, _, ref args) if args.len() == 1 => {
|
||||
let negation = METHODS_WITH_NEGATION
|
||||
.iter().cloned()
|
||||
.flat_map(|(a, b)| vec![(a, b), (b, a)])
|
||||
.find(|&(a, _)| a == path.name.as_str());
|
||||
if let Some((_, negation_method)) = negation {
|
||||
*simplified = true;
|
||||
s.push_str(&snip(&args[0]));
|
||||
s.push('.');
|
||||
s.push_str(negation_method);
|
||||
s.push_str("()");
|
||||
s
|
||||
} else {
|
||||
s.push('!');
|
||||
return recurse(true, cx, inner, terminals, s);
|
||||
},
|
||||
};
|
||||
s.push_str(&snip(lhs));
|
||||
s.push_str(op);
|
||||
s.push_str(&snip(rhs));
|
||||
s
|
||||
} else {
|
||||
s.push('!');
|
||||
recurse(false, cx, inner, terminals, s)
|
||||
recurse(false, cx, inner, terminals, s, simplified)
|
||||
}
|
||||
},
|
||||
_ => {
|
||||
s.push('!');
|
||||
recurse(false, cx, inner, terminals, s, simplified)
|
||||
},
|
||||
},
|
||||
_ => {
|
||||
s.push('!');
|
||||
recurse(false, cx, inner, terminals, s)
|
||||
recurse(false, cx, inner, terminals, s, simplified)
|
||||
},
|
||||
},
|
||||
And(ref v) => {
|
||||
|
|
@ -202,16 +237,16 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
|
|||
s.push('(');
|
||||
}
|
||||
if let Or(_) = v[0] {
|
||||
s = recurse(true, cx, &v[0], terminals, s);
|
||||
s = recurse(true, cx, &v[0], terminals, s, simplified);
|
||||
} else {
|
||||
s = recurse(false, cx, &v[0], terminals, s);
|
||||
s = recurse(false, cx, &v[0], terminals, s, simplified);
|
||||
}
|
||||
for inner in &v[1..] {
|
||||
s.push_str(" && ");
|
||||
if let Or(_) = *inner {
|
||||
s = recurse(true, cx, inner, terminals, s);
|
||||
s = recurse(true, cx, inner, terminals, s, simplified);
|
||||
} else {
|
||||
s = recurse(false, cx, inner, terminals, s);
|
||||
s = recurse(false, cx, inner, terminals, s, simplified);
|
||||
}
|
||||
}
|
||||
if brackets {
|
||||
|
|
@ -223,10 +258,10 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
|
|||
if brackets {
|
||||
s.push('(');
|
||||
}
|
||||
s = recurse(false, cx, &v[0], terminals, s);
|
||||
s = recurse(false, cx, &v[0], terminals, s, simplified);
|
||||
for inner in &v[1..] {
|
||||
s.push_str(" || ");
|
||||
s = recurse(false, cx, inner, terminals, s);
|
||||
s = recurse(false, cx, inner, terminals, s, simplified);
|
||||
}
|
||||
if brackets {
|
||||
s.push(')');
|
||||
|
|
@ -249,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 {
|
||||
|
|
@ -359,7 +396,7 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
|
|||
db.span_suggestion(
|
||||
e.span,
|
||||
"it would look like the following",
|
||||
suggest(self.cx, suggestion, &h2q.terminals),
|
||||
suggest(self.cx, suggestion, &h2q.terminals).0,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
|
@ -376,22 +413,26 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
|
|||
improvements.push(suggestion);
|
||||
}
|
||||
}
|
||||
if !improvements.is_empty() {
|
||||
let nonminimal_bool_lint = |suggestions| {
|
||||
span_lint_and_then(
|
||||
self.cx,
|
||||
NONMINIMAL_BOOL,
|
||||
e.span,
|
||||
"this boolean expression can be simplified",
|
||||
|db| {
|
||||
db.span_suggestions(
|
||||
e.span,
|
||||
"try",
|
||||
improvements
|
||||
.into_iter()
|
||||
.map(|suggestion| suggest(self.cx, suggestion, &h2q.terminals))
|
||||
.collect(),
|
||||
);
|
||||
},
|
||||
|db| { db.span_suggestions(e.span, "try", suggestions); },
|
||||
);
|
||||
};
|
||||
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
|
||||
.into_iter()
|
||||
.map(|suggestion| suggest(self.cx, suggestion, &h2q.terminals).0)
|
||||
.collect()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -38,3 +38,19 @@ fn equality_stuff() {
|
|||
let _ = a > b && a == b;
|
||||
let _ = a != b || !(a != b || c == d);
|
||||
}
|
||||
|
||||
#[allow(unused, many_single_char_names)]
|
||||
fn methods_with_negation() {
|
||||
let a: Option<i32> = unimplemented!();
|
||||
let b: Result<i32, i32> = unimplemented!();
|
||||
let _ = a.is_some();
|
||||
let _ = !a.is_some();
|
||||
let _ = a.is_none();
|
||||
let _ = !a.is_none();
|
||||
let _ = b.is_err();
|
||||
let _ = !b.is_err();
|
||||
let _ = b.is_ok();
|
||||
let _ = !b.is_ok();
|
||||
let c = false;
|
||||
let _ = !(a.is_some() && !c);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -130,3 +130,33 @@ help: try
|
|||
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
|
||||
--> $DIR/booleans.rs:55:13
|
||||
|
|
||||
55 | let _ = !(a.is_some() && !c);
|
||||
| ^^^^^^^^^^^^^^^^^^^^ help: try: `c || a.is_none()`
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue