Skip to content

Commit 80728a2

Browse files
committed
Reduced scope of nonminimal_bool so that it doesn't evaluate only partially orded types.
1 parent 09ea75b commit 80728a2

File tree

5 files changed

+37
-16
lines changed

5 files changed

+37
-16
lines changed

clippy_lints/src/booleans.rs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use rustc::hir::intravisit::*;
44
use syntax::ast::{LitKind, NodeId, DUMMY_NODE_ID};
55
use syntax::codemap::{dummy_spanned, Span, DUMMY_SP};
66
use syntax::util::ThinVec;
7-
use crate::utils::{in_macro, paths, match_type, snippet_opt, span_lint_and_then, SpanlessEq};
7+
use crate::utils::{in_macro, paths, match_type, snippet_opt, span_lint_and_then, SpanlessEq, get_trait_def_id, implements_trait};
88

99
/// **What it does:** Checks for boolean expressions that can be written more
1010
/// concisely.
@@ -122,6 +122,12 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
122122
}
123123
let negated = match e.node {
124124
ExprBinary(binop, ref lhs, ref rhs) => {
125+
126+
match implements_ord(self.cx, lhs) {
127+
Some(true) => (),
128+
_ => continue,
129+
};
130+
125131
let mk_expr = |op| {
126132
Expr {
127133
id: DUMMY_NODE_ID,
@@ -174,6 +180,12 @@ impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> {
174180
fn simplify_not(&self, expr: &Expr) -> Option<String> {
175181
match expr.node {
176182
ExprBinary(binop, ref lhs, ref rhs) => {
183+
184+
match implements_ord(self.cx, lhs) {
185+
Some(true) => (),
186+
_ => return None,
187+
};
188+
177189
match binop.node {
178190
BiEq => Some(" != "),
179191
BiNe => Some(" == "),
@@ -444,3 +456,14 @@ impl<'a, 'tcx> Visitor<'tcx> for NonminimalBoolVisitor<'a, 'tcx> {
444456
NestedVisitorMap::None
445457
}
446458
}
459+
460+
461+
fn implements_ord<'a, 'tcx>(cx: &'a LateContext<'a, 'tcx>, expr: &Expr) -> Option<bool> {
462+
let ty = cx.tables.expr_ty(expr);
463+
464+
return if let Some(id) = get_trait_def_id(cx, &paths::ORD) {
465+
Some(implements_trait(cx, ty, id, &[]))
466+
} else {
467+
None
468+
};
469+
}

clippy_lints/src/neg_cmp_op_on_partial_ord.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
use rustc::hir::*;
22
use rustc::lint::*;
33

4-
use crate::utils;
5-
6-
const ORD: [&str; 3] = ["core", "cmp", "Ord"];
7-
const PARTIAL_ORD: [&str; 3] = ["core", "cmp", "PartialOrd"];
4+
use crate::utils::{self, paths};
85

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

6764
let implements_ord = {
68-
if let Some(id) = utils::get_trait_def_id(cx, &ORD) {
65+
if let Some(id) = utils::get_trait_def_id(cx, &paths::ORD) {
6966
utils::implements_trait(cx, ty, id, &[])
7067
} else {
7168
return;
7269
}
7370
};
7471

7572
let implements_partial_ord = {
76-
if let Some(id) = utils::get_trait_def_id(cx, &PARTIAL_ORD) {
73+
if let Some(id) = utils::get_trait_def_id(cx, &paths::PARTIAL_ORD) {
7774
utils::implements_trait(cx, ty, id, &[])
7875
} else {
7976
return;

clippy_lints/src/utils/paths.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ pub const OPS_MODULE: [&str; 2] = ["core", "ops"];
5757
pub const OPTION: [&str; 3] = ["core", "option", "Option"];
5858
pub const OPTION_NONE: [&str; 4] = ["core", "option", "Option", "None"];
5959
pub const OPTION_SOME: [&str; 4] = ["core", "option", "Option", "Some"];
60+
pub const ORD: [&str; 3] = ["core", "cmp", "Ord"];
61+
pub const PARTIAL_ORD: [&str; 3] = ["core", "cmp", "PartialOrd"];
6062
pub const PTR_NULL: [&str; 2] = ["ptr", "null"];
6163
pub const PTR_NULL_MUT: [&str; 2] = ["ptr", "null_mut"];
6264
pub const RANGE: [&str; 3] = ["core", "ops", "Range"];

tests/ui/neg_cmp_op_on_partial_ord.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
55
use std::cmp::Ordering;
66

7-
#[allow(nonminimal_bool)]
87
#[warn(neg_cmp_op_on_partial_ord)]
98
fn main() {
109

tests/ui/neg_cmp_op_on_partial_ord.stderr

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,27 @@
11
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.
2-
--> $DIR/neg_cmp_op_on_partial_ord.rs:18:21
2+
--> $DIR/neg_cmp_op_on_partial_ord.rs:17:21
33
|
4-
18 | let _not_less = !(a_value < another_value);
4+
17 | let _not_less = !(a_value < another_value);
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D neg-cmp-op-on-partial-ord` implied by `-D warnings`
88

99
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.
10-
--> $DIR/neg_cmp_op_on_partial_ord.rs:21:30
10+
--> $DIR/neg_cmp_op_on_partial_ord.rs:20:30
1111
|
12-
21 | let _not_less_or_equal = !(a_value <= another_value);
12+
20 | let _not_less_or_equal = !(a_value <= another_value);
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
1414

1515
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.
16-
--> $DIR/neg_cmp_op_on_partial_ord.rs:24:24
16+
--> $DIR/neg_cmp_op_on_partial_ord.rs:23:24
1717
|
18-
24 | let _not_greater = !(a_value > another_value);
18+
23 | let _not_greater = !(a_value > another_value);
1919
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
2020

2121
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.
22-
--> $DIR/neg_cmp_op_on_partial_ord.rs:27:33
22+
--> $DIR/neg_cmp_op_on_partial_ord.rs:26:33
2323
|
24-
27 | let _not_greater_or_equal = !(a_value >= another_value);
24+
26 | let _not_greater_or_equal = !(a_value >= another_value);
2525
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
2626

2727
error: aborting due to 4 previous errors

0 commit comments

Comments
 (0)