Skip to content

Commit

Permalink
Reduced scope of nonminimal_bool so that it doesn't evaluate only p…
Browse files Browse the repository at this point in the history
…artially orded types.
  • Loading branch information
0ndorio committed Jun 3, 2018
1 parent 09ea75b commit 80728a2
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 16 deletions.
25 changes: 24 additions & 1 deletion clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 crate::utils::{in_macro, paths, match_type, snippet_opt, span_lint_and_then, SpanlessEq};
use crate::utils::{in_macro, paths, match_type, snippet_opt, span_lint_and_then, SpanlessEq, get_trait_def_id, implements_trait};

/// **What it does:** Checks for boolean expressions that can be written more
/// concisely.
Expand Down Expand Up @@ -122,6 +122,12 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
}
let negated = match e.node {
ExprBinary(binop, ref lhs, ref rhs) => {

match implements_ord(self.cx, lhs) {
Some(true) => (),
_ => continue,
};

let mk_expr = |op| {
Expr {
id: DUMMY_NODE_ID,
Expand Down Expand Up @@ -174,6 +180,12 @@ impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> {
fn simplify_not(&self, expr: &Expr) -> Option<String> {
match expr.node {
ExprBinary(binop, ref lhs, ref rhs) => {

match implements_ord(self.cx, lhs) {
Some(true) => (),
_ => return None,
};

match binop.node {
BiEq => Some(" != "),
BiNe => Some(" == "),
Expand Down Expand Up @@ -444,3 +456,14 @@ impl<'a, 'tcx> Visitor<'tcx> for NonminimalBoolVisitor<'a, 'tcx> {
NestedVisitorMap::None
}
}


fn implements_ord<'a, 'tcx>(cx: &'a LateContext<'a, 'tcx>, expr: &Expr) -> Option<bool> {
let ty = cx.tables.expr_ty(expr);

return if let Some(id) = get_trait_def_id(cx, &paths::ORD) {
Some(implements_trait(cx, ty, id, &[]))
} else {
None
};
}
9 changes: 3 additions & 6 deletions clippy_lints/src/neg_cmp_op_on_partial_ord.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
use rustc::hir::*;
use rustc::lint::*;

use crate::utils;

const ORD: [&str; 3] = ["core", "cmp", "Ord"];
const PARTIAL_ORD: [&str; 3] = ["core", "cmp", "PartialOrd"];
use crate::utils::{self, paths};

/// **What it does:**
/// Checks for the usage of negated comparision operators on types which only implement
Expand Down Expand Up @@ -65,15 +62,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NoNegCompOpForPartialOrd {
let ty = cx.tables.expr_ty(left);

let implements_ord = {
if let Some(id) = utils::get_trait_def_id(cx, &ORD) {
if let Some(id) = utils::get_trait_def_id(cx, &paths::ORD) {
utils::implements_trait(cx, ty, id, &[])
} else {
return;
}
};

let implements_partial_ord = {
if let Some(id) = utils::get_trait_def_id(cx, &PARTIAL_ORD) {
if let Some(id) = utils::get_trait_def_id(cx, &paths::PARTIAL_ORD) {
utils::implements_trait(cx, ty, id, &[])
} else {
return;
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/utils/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ pub const OPS_MODULE: [&str; 2] = ["core", "ops"];
pub const OPTION: [&str; 3] = ["core", "option", "Option"];
pub const OPTION_NONE: [&str; 4] = ["core", "option", "Option", "None"];
pub const OPTION_SOME: [&str; 4] = ["core", "option", "Option", "Some"];
pub const ORD: [&str; 3] = ["core", "cmp", "Ord"];
pub const PARTIAL_ORD: [&str; 3] = ["core", "cmp", "PartialOrd"];
pub const PTR_NULL: [&str; 2] = ["ptr", "null"];
pub const PTR_NULL_MUT: [&str; 2] = ["ptr", "null_mut"];
pub const RANGE: [&str; 3] = ["core", "ops", "Range"];
Expand Down
1 change: 0 additions & 1 deletion tests/ui/neg_cmp_op_on_partial_ord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
use std::cmp::Ordering;

#[allow(nonminimal_bool)]
#[warn(neg_cmp_op_on_partial_ord)]
fn main() {

Expand Down
16 changes: 8 additions & 8 deletions tests/ui/neg_cmp_op_on_partial_ord.stderr
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
error: The use of negated comparision operators on partially orded types produces code that is hard to read and refactor. Please consider to use the `partial_cmp` instead, to make it clear that the two values could be incomparable.
--> $DIR/neg_cmp_op_on_partial_ord.rs:18:21
--> $DIR/neg_cmp_op_on_partial_ord.rs:17:21
|
18 | let _not_less = !(a_value < another_value);
17 | let _not_less = !(a_value < another_value);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D neg-cmp-op-on-partial-ord` implied by `-D warnings`

error: The use of negated comparision operators on partially orded types produces code that is hard to read and refactor. Please consider to use the `partial_cmp` instead, to make it clear that the two values could be incomparable.
--> $DIR/neg_cmp_op_on_partial_ord.rs:21:30
--> $DIR/neg_cmp_op_on_partial_ord.rs:20:30
|
21 | let _not_less_or_equal = !(a_value <= another_value);
20 | let _not_less_or_equal = !(a_value <= another_value);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: The use of negated comparision operators on partially orded types produces code that is hard to read and refactor. Please consider to use the `partial_cmp` instead, to make it clear that the two values could be incomparable.
--> $DIR/neg_cmp_op_on_partial_ord.rs:24:24
--> $DIR/neg_cmp_op_on_partial_ord.rs:23:24
|
24 | let _not_greater = !(a_value > another_value);
23 | let _not_greater = !(a_value > another_value);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

error: The use of negated comparision operators on partially orded types produces code that is hard to read and refactor. Please consider to use the `partial_cmp` instead, to make it clear that the two values could be incomparable.
--> $DIR/neg_cmp_op_on_partial_ord.rs:27:33
--> $DIR/neg_cmp_op_on_partial_ord.rs:26:33
|
27 | let _not_greater_or_equal = !(a_value >= another_value);
26 | let _not_greater_or_equal = !(a_value >= another_value);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 4 previous errors
Expand Down

0 comments on commit 80728a2

Please sign in to comment.