Skip to content

Commit 25783fa

Browse files
LaurentMazareLaurentMazare
committed
Raise a lint when suggest has simplified the expression.
1 parent bdf3887 commit 25783fa

File tree

2 files changed

+66
-26
lines changed

2 files changed

+66
-26
lines changed

‎clippy_lints/src/booleans.rs

Lines changed: 42 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,16 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
159159
}
160160
}
161161

162-
fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
163-
fn recurse(brackets: bool, cx: &LateContext, suggestion: &Bool, terminals: &[&Expr], mut s: String) -> String {
162+
// The boolean part of the return indicates whether some simplifications have been applied.
163+
fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> (String, bool) {
164+
fn recurse(
165+
brackets: bool,
166+
cx: &LateContext,
167+
suggestion: &Bool,
168+
terminals: &[&Expr],
169+
mut s: String,
170+
simplified: &mut bool,
171+
) -> String {
164172
use quine_mc_cluskey::Bool::*;
165173
let snip = |e: &Expr| snippet_opt(cx, e.span).expect("don't try to improve booleans created by macros");
166174
match *suggestion {
@@ -175,7 +183,7 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
175183
Not(ref inner) => match **inner {
176184
And(_) | Or(_) => {
177185
s.push('!');
178-
recurse(true, cx, inner, terminals, s)
186+
recurse(true, cx, inner, terminals, s, simplified)
179187
},
180188
Term(n) => match terminals[n as usize].node {
181189
ExprBinary(binop, ref lhs, ref rhs) => {
@@ -188,9 +196,10 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
188196
BiGe => " < ",
189197
_ => {
190198
s.push('!');
191-
return recurse(true, cx, inner, terminals, s);
199+
return recurse(true, cx, inner, terminals, s, simplified);
192200
},
193201
};
202+
*simplified = true;
194203
s.push_str(&snip(lhs));
195204
s.push_str(op);
196205
s.push_str(&snip(rhs));
@@ -202,41 +211,42 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
202211
.flat_map(|(a, b)| vec![(a, b), (b, a)])
203212
.find(|&(a, _)| a == path.name.as_str());
204213
if let Some((_, negation_method)) = negation {
214+
*simplified = true;
205215
s.push_str(&snip(&args[0]));
206216
s.push('.');
207217
s.push_str(negation_method);
208218
s.push_str("()");
209219
s
210220
} else {
211221
s.push('!');
212-
recurse(false, cx, inner, terminals, s)
222+
recurse(false, cx, inner, terminals, s, simplified)
213223
}
214224
},
215225
_ => {
216226
s.push('!');
217-
recurse(false, cx, inner, terminals, s)
227+
recurse(false, cx, inner, terminals, s, simplified)
218228
},
219229
},
220230
_ => {
221231
s.push('!');
222-
recurse(false, cx, inner, terminals, s)
232+
recurse(false, cx, inner, terminals, s, simplified)
223233
},
224234
},
225235
And(ref v) => {
226236
if brackets {
227237
s.push('(');
228238
}
229239
if let Or(_) = v[0] {
230-
s = recurse(true, cx, &v[0], terminals, s);
240+
s = recurse(true, cx, &v[0], terminals, s, simplified);
231241
} else {
232-
s = recurse(false, cx, &v[0], terminals, s);
242+
s = recurse(false, cx, &v[0], terminals, s, simplified);
233243
}
234244
for inner in &v[1..] {
235245
s.push_str(" && ");
236246
if let Or(_) = *inner {
237-
s = recurse(true, cx, inner, terminals, s);
247+
s = recurse(true, cx, inner, terminals, s, simplified);
238248
} else {
239-
s = recurse(false, cx, inner, terminals, s);
249+
s = recurse(false, cx, inner, terminals, s, simplified);
240250
}
241251
}
242252
if brackets {
@@ -248,10 +258,10 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
248258
if brackets {
249259
s.push('(');
250260
}
251-
s = recurse(false, cx, &v[0], terminals, s);
261+
s = recurse(false, cx, &v[0], terminals, s, simplified);
252262
for inner in &v[1..] {
253263
s.push_str(" || ");
254-
s = recurse(false, cx, inner, terminals, s);
264+
s = recurse(false, cx, inner, terminals, s, simplified);
255265
}
256266
if brackets {
257267
s.push(')');
@@ -274,7 +284,9 @@ fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
274284
},
275285
}
276286
}
277-
recurse(false, cx, suggestion, terminals, String::new())
287+
let mut simplified = false;
288+
let s = recurse(false, cx, suggestion, terminals, String::new(), &mut simplified);
289+
(s, simplified)
278290
}
279291

280292
fn simple_negate(b: Bool) -> Bool {
@@ -384,7 +396,7 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
384396
db.span_suggestion(
385397
e.span,
386398
"it would look like the following",
387-
suggest(self.cx, suggestion, &h2q.terminals),
399+
suggest(self.cx, suggestion, &h2q.terminals).0,
388400
);
389401
},
390402
);
@@ -401,22 +413,26 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
401413
improvements.push(suggestion);
402414
}
403415
}
404-
if !improvements.is_empty() {
416+
let nonminimal_bool_lint = |suggestions| {
405417
span_lint_and_then(
406418
self.cx,
407419
NONMINIMAL_BOOL,
408420
e.span,
409421
"this boolean expression can be simplified",
410-
|db| {
411-
db.span_suggestions(
412-
e.span,
413-
"try",
414-
improvements
415-
.into_iter()
416-
.map(|suggestion| suggest(self.cx, suggestion, &h2q.terminals))
417-
.collect(),
418-
);
419-
},
422+
|db| { db.span_suggestions(e.span, "try", suggestions); },
423+
);
424+
};
425+
if improvements.is_empty() {
426+
let suggest = suggest(self.cx, &expr, &h2q.terminals);
427+
if suggest.1 {
428+
nonminimal_bool_lint(vec![suggest.0])
429+
}
430+
} else {
431+
nonminimal_bool_lint(
432+
improvements
433+
.into_iter()
434+
.map(|suggestion| suggest(self.cx, suggestion, &h2q.terminals).0)
435+
.collect()
420436
);
421437
}
422438
}

‎tests/ui/booleans.stderr

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,30 @@ help: try
130130
39 | let _ = !(a == b && c == d);
131131
| ^^^^^^^^^^^^^^^^^^^
132132

133+
error: this boolean expression can be simplified
134+
--> $DIR/booleans.rs:47:13
135+
|
136+
47 | let _ = !a.is_some();
137+
| ^^^^^^^^^^^^ help: try: `a.is_none()`
138+
139+
error: this boolean expression can be simplified
140+
--> $DIR/booleans.rs:49:13
141+
|
142+
49 | let _ = !a.is_none();
143+
| ^^^^^^^^^^^^ help: try: `a.is_some()`
144+
145+
error: this boolean expression can be simplified
146+
--> $DIR/booleans.rs:51:13
147+
|
148+
51 | let _ = !b.is_err();
149+
| ^^^^^^^^^^^ help: try: `b.is_ok()`
150+
151+
error: this boolean expression can be simplified
152+
--> $DIR/booleans.rs:53:13
153+
|
154+
53 | let _ = !b.is_ok();
155+
| ^^^^^^^^^^ help: try: `b.is_err()`
156+
133157
error: this boolean expression can be simplified
134158
--> $DIR/booleans.rs:55:13
135159
|

0 commit comments

Comments
 (0)