Fix nonminimal_bool false positive

It was checking any is_ok, is_err, is_some, is_none method for negation
but it should only perform the check for the built-in types, not custom
types.
This commit is contained in:
Philipp Hansch 2018-04-07 12:52:18 +02:00
parent fbb5050d82
commit 90e7d93d6c
No known key found for this signature in database
GPG key ID: B6FA06A6E0E2665B
3 changed files with 88 additions and 2 deletions

View file

@ -4,7 +4,7 @@ use rustc::hir::intravisit::*;
use syntax::ast::{LitKind, NodeId, DUMMY_NODE_ID};
use syntax::codemap::{dummy_spanned, Span, DUMMY_SP};
use syntax::util::ThinVec;
use utils::{in_macro, snippet_opt, span_lint_and_then, SpanlessEq};
use utils::{in_macro, paths, match_type, snippet_opt, span_lint_and_then, SpanlessEq};
/// **What it does:** Checks for boolean expressions that can be written more
/// concisely.
@ -185,6 +185,11 @@ impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> {
}.and_then(|op| Some(format!("{}{}{}", self.snip(lhs)?, op, self.snip(rhs)?)))
},
ExprMethodCall(ref path, _, ref 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)])

View file

@ -57,3 +57,60 @@ fn methods_with_negation() {
let _ = (!c ^ c) || !a.is_some();
let _ = !c ^ c || !a.is_some();
}
// Simplified versions of https://github.com/rust-lang-nursery/rust-clippy/issues/2638
// nonminimal_bool should only check the built-in Result and Some type, not
// any other types like the following.
enum CustomResultOk<E> { Ok, Err(E) }
enum CustomResultErr<E> { Ok, Err(E) }
enum CustomSomeSome<T> { Some(T), None }
enum CustomSomeNone<T> { Some(T), None }
impl<E> CustomResultOk<E> {
pub fn is_ok(&self) -> bool { true }
}
impl<E> CustomResultErr<E> {
pub fn is_err(&self) -> bool { true }
}
impl<T> CustomSomeSome<T> {
pub fn is_some(&self) -> bool { true }
}
impl<T> CustomSomeNone<T> {
pub fn is_none(&self) -> bool { true }
}
fn dont_warn_for_custom_methods_with_negation() {
let res = CustomResultOk::Err("Error");
// Should not warn and suggest 'is_err()' because the type does not
// implement is_err().
if !res.is_ok() { }
let res = CustomResultErr::Err("Error");
// Should not warn and suggest 'is_ok()' because the type does not
// implement is_ok().
if !res.is_err() { }
let res = CustomSomeSome::Some("thing");
// Should not warn and suggest 'is_none()' because the type does not
// implement is_none().
if !res.is_some() { }
let res = CustomSomeNone::Some("thing");
// Should not warn and suggest 'is_some()' because the type does not
// implement is_some().
if !res.is_none() { }
}
// Only Built-in Result and Some types should suggest the negated alternative
fn warn_for_built_in_methods_with_negation() {
let res: Result<usize, usize> = Ok(1);
if !res.is_ok() { }
if !res.is_err() { }
let res = Some(1);
if !res.is_some() { }
if !res.is_none() { }
}

View file

@ -175,5 +175,29 @@ error: this boolean expression can be simplified
58 | let _ = !c ^ c || !a.is_some();
| ^^^^^^^^^^^^^^^^^^^^^^ help: try: `!c ^ c || a.is_none()`
error: aborting due to 21 previous errors
error: this boolean expression can be simplified
--> $DIR/booleans.rs:110:8
|
110 | if !res.is_ok() { }
| ^^^^^^^^^^^^ help: try: `res.is_err()`
error: this boolean expression can be simplified
--> $DIR/booleans.rs:111:8
|
111 | if !res.is_err() { }
| ^^^^^^^^^^^^^ help: try: `res.is_ok()`
error: this boolean expression can be simplified
--> $DIR/booleans.rs:114:8
|
114 | if !res.is_some() { }
| ^^^^^^^^^^^^^^ help: try: `res.is_none()`
error: this boolean expression can be simplified
--> $DIR/booleans.rs:115:8
|
115 | if !res.is_none() { }
| ^^^^^^^^^^^^^^ help: try: `res.is_some()`
error: aborting due to 25 previous errors