diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs index 0fa2c2ac9..c8478c47a 100644 --- a/clippy_lints/src/booleans.rs +++ b/clippy_lints/src/booleans.rs @@ -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)]) diff --git a/tests/ui/booleans.rs b/tests/ui/booleans.rs index 0898de105..78e876e51 100644 --- a/tests/ui/booleans.rs +++ b/tests/ui/booleans.rs @@ -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 { Ok, Err(E) } +enum CustomResultErr { Ok, Err(E) } +enum CustomSomeSome { Some(T), None } +enum CustomSomeNone { Some(T), None } + +impl CustomResultOk { + pub fn is_ok(&self) -> bool { true } +} + +impl CustomResultErr { + pub fn is_err(&self) -> bool { true } +} + +impl CustomSomeSome { + pub fn is_some(&self) -> bool { true } +} + +impl CustomSomeNone { + 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 = Ok(1); + if !res.is_ok() { } + if !res.is_err() { } + + let res = Some(1); + if !res.is_some() { } + if !res.is_none() { } +} diff --git a/tests/ui/booleans.stderr b/tests/ui/booleans.stderr index c88a7a7be..f1996e8a2 100644 --- a/tests/ui/booleans.stderr +++ b/tests/ui/booleans.stderr @@ -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