Skip to content

Commit 4ab2223

Browse files
oli-obkoli-obk
authored
Merge pull request #2060 from mrecachinas/feature/int-plus-one
Addresses #1955 - Suggests >= y + 1 become > y
2 parents 3509b0b + f571cf0 commit 4ab2223

File tree

4 files changed

+211
-0
lines changed

4 files changed

+211
-0
lines changed

‎clippy_lints/src/int_plus_one.rs

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
//! lint on blocks unnecessarily using >= with a + 1 or - 1
2+
3+
use rustc::lint::*;
4+
use syntax::ast::*;
5+
6+
use utils::{span_lint_and_then, snippet_opt};
7+
8+
/// **What it does:** Checks for usage of `x >= y + 1` or `x - 1 >= y` (and `<=`) in a block
9+
///
10+
///
11+
/// **Why is this bad?** Readability -- better to use `> y` instead of `>= y + 1`.
12+
///
13+
/// **Known problems:** None.
14+
///
15+
/// **Example:**
16+
/// ```rust
17+
/// x >= y + 1
18+
/// ```
19+
///
20+
/// Could be written:
21+
///
22+
/// ```rust
23+
/// x > y
24+
/// ```
25+
declare_lint! {
26+
pub INT_PLUS_ONE,
27+
Allow,
28+
"instead of using x >= y + 1, use x > y"
29+
}
30+
31+
pub struct IntPlusOne;
32+
33+
impl LintPass for IntPlusOne {
34+
fn get_lints(&self) -> LintArray {
35+
lint_array!(INT_PLUS_ONE)
36+
}
37+
}
38+
39+
// cases:
40+
// BinOpKind::Ge
41+
// x >= y + 1
42+
// x - 1 >= y
43+
//
44+
// BinOpKind::Le
45+
// x + 1 <= y
46+
// x <= y - 1
47+
48+
enum Side {
49+
LHS,
50+
RHS,
51+
}
52+
53+
impl IntPlusOne {
54+
#[allow(cast_sign_loss)]
55+
fn check_lit(&self, lit: &Lit, target_value: i128) -> bool {
56+
if let LitKind::Int(value, ..) = lit.node {
57+
return value == (target_value as u128)
58+
}
59+
false
60+
}
61+
62+
fn check_binop(&self, cx: &EarlyContext, binop: BinOpKind, lhs: &Expr, rhs: &Expr) -> Option<String> {
63+
match (binop, &lhs.node, &rhs.node) {
64+
// case where `x - 1 >= ...` or `-1 + x >= ...`
65+
(BinOpKind::Ge, &ExprKind::Binary(ref lhskind, ref lhslhs, ref lhsrhs), _) => {
66+
match (lhskind.node, &lhslhs.node, &lhsrhs.node) {
67+
// `-1 + x`
68+
(BinOpKind::Add, &ExprKind::Lit(ref lit), _) if self.check_lit(lit, -1) => self.generate_recommendation(cx, binop, lhsrhs, rhs, Side::LHS),
69+
// `x - 1`
70+
(BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) if self.check_lit(lit, 1) => self.generate_recommendation(cx, binop, lhslhs, rhs, Side::LHS),
71+
_ => None
72+
}
73+
},
74+
// case where `... >= y + 1` or `... >= 1 + y`
75+
(BinOpKind::Ge, _, &ExprKind::Binary(ref rhskind, ref rhslhs, ref rhsrhs)) if rhskind.node == BinOpKind::Add => {
76+
match (&rhslhs.node, &rhsrhs.node) {
77+
// `y + 1` and `1 + y`
78+
(&ExprKind::Lit(ref lit), _) if self.check_lit(lit, 1) => self.generate_recommendation(cx, binop, rhsrhs, lhs, Side::RHS),
79+
(_, &ExprKind::Lit(ref lit)) if self.check_lit(lit, 1) => self.generate_recommendation(cx, binop, rhslhs, lhs, Side::RHS),
80+
_ => None
81+
}
82+
},
83+
// case where `x + 1 <= ...` or `1 + x <= ...`
84+
(BinOpKind::Le, &ExprKind::Binary(ref lhskind, ref lhslhs, ref lhsrhs), _) if lhskind.node == BinOpKind::Add => {
85+
match (&lhslhs.node, &lhsrhs.node) {
86+
// `1 + x` and `x + 1`
87+
(&ExprKind::Lit(ref lit), _) if self.check_lit(lit, 1) => self.generate_recommendation(cx, binop, lhsrhs, rhs, Side::LHS),
88+
(_, &ExprKind::Lit(ref lit)) if self.check_lit(lit, 1) => self.generate_recommendation(cx, binop, lhslhs, rhs, Side::LHS),
89+
_ => None
90+
}
91+
},
92+
// case where `... >= y - 1` or `... >= -1 + y`
93+
(BinOpKind::Le, _, &ExprKind::Binary(ref rhskind, ref rhslhs, ref rhsrhs)) => {
94+
match (rhskind.node, &rhslhs.node, &rhsrhs.node) {
95+
// `-1 + y`
96+
(BinOpKind::Add, &ExprKind::Lit(ref lit), _) if self.check_lit(lit, -1) => self.generate_recommendation(cx, binop, rhsrhs, lhs, Side::RHS),
97+
// `y - 1`
98+
(BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) if self.check_lit(lit, 1) => self.generate_recommendation(cx, binop, rhslhs, lhs, Side::RHS),
99+
_ => None
100+
}
101+
},
102+
_ => None
103+
}
104+
}
105+
106+
fn generate_recommendation(&self, cx: &EarlyContext, binop: BinOpKind, node: &Expr, other_side: &Expr, side: Side) -> Option<String> {
107+
let binop_string = match binop {
108+
BinOpKind::Ge => ">",
109+
BinOpKind::Le => "<",
110+
_ => return None
111+
};
112+
if let Some(snippet) = snippet_opt(cx, node.span) {
113+
if let Some(other_side_snippet) = snippet_opt(cx, other_side.span) {
114+
let rec = match side {
115+
Side::LHS => Some(format!("{} {} {}", snippet, binop_string, other_side_snippet)),
116+
Side::RHS => Some(format!("{} {} {}", other_side_snippet, binop_string, snippet)),
117+
};
118+
return rec;
119+
}
120+
}
121+
None
122+
}
123+
124+
fn emit_warning(&self, cx: &EarlyContext, block: &Expr, recommendation: String) {
125+
span_lint_and_then(cx,
126+
INT_PLUS_ONE,
127+
block.span,
128+
"Unnecessary `>= y + 1` or `x - 1 >=`",
129+
|db| {
130+
db.span_suggestion(block.span, "change `>= y + 1` to `> y` as shown", recommendation);
131+
});
132+
}
133+
}
134+
135+
impl EarlyLintPass for IntPlusOne {
136+
fn check_expr(&mut self, cx: &EarlyContext, item: &Expr) {
137+
if let ExprKind::Binary(ref kind, ref lhs, ref rhs) = item.node {
138+
if let Some(ref rec) = self.check_binop(cx, kind.node, lhs, rhs) {
139+
self.emit_warning(cx, item, rec.clone());
140+
}
141+
}
142+
}
143+
}

‎clippy_lints/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ pub mod identity_op;
9696
pub mod if_let_redundant_pattern_matching;
9797
pub mod if_not_else;
9898
pub mod infinite_iter;
99+
pub mod int_plus_one;
99100
pub mod is_unit_expr;
100101
pub mod items_after_statements;
101102
pub mod large_enum_variant;
@@ -299,6 +300,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
299300
reg.register_early_lint_pass(box formatting::Formatting);
300301
reg.register_late_lint_pass(box swap::Swap);
301302
reg.register_early_lint_pass(box if_not_else::IfNotElse);
303+
reg.register_early_lint_pass(box int_plus_one::IntPlusOne);
302304
reg.register_late_lint_pass(box overflow_check_conditional::OverflowCheckConditional);
303305
reg.register_late_lint_pass(box unused_label::UnusedLabel);
304306
reg.register_late_lint_pass(box new_without_default::NewWithoutDefault);
@@ -341,6 +343,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
341343
enum_variants::PUB_ENUM_VARIANT_NAMES,
342344
enum_variants::STUTTER,
343345
if_not_else::IF_NOT_ELSE,
346+
int_plus_one::INT_PLUS_ONE,
344347
infinite_iter::MAYBE_INFINITE_ITER,
345348
items_after_statements::ITEMS_AFTER_STATEMENTS,
346349
matches::SINGLE_MATCH_ELSE,

‎tests/ui/int_plus_one.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#![feature(plugin)]
2+
#![plugin(clippy)]
3+
4+
#[allow(no_effect, unnecessary_operation)]
5+
#[warn(int_plus_one)]
6+
fn main() {
7+
let x = 1i32;
8+
let y = 0i32;
9+
10+
x >= y + 1;
11+
y + 1 <= x;
12+
13+
x - 1 >= y;
14+
y <= x - 1;
15+
16+
x > y; // should be ok
17+
y < x; // should be ok
18+
}

‎tests/ui/int_plus_one.stderr

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
error: Unnecessary `>= y + 1` or `x - 1 >=`
2+
--> $DIR/int_plus_one.rs:10:5
3+
|
4+
10 | x >= y + 1;
5+
| ^^^^^^^^^^
6+
|
7+
= note: `-D int-plus-one` implied by `-D warnings`
8+
help: change `>= y + 1` to `> y` as shown
9+
|
10+
10 | x > y;
11+
| ^^^^^
12+
13+
error: Unnecessary `>= y + 1` or `x - 1 >=`
14+
--> $DIR/int_plus_one.rs:11:5
15+
|
16+
11 | y + 1 <= x;
17+
| ^^^^^^^^^^
18+
|
19+
help: change `>= y + 1` to `> y` as shown
20+
|
21+
11 | y < x;
22+
| ^^^^^
23+
24+
error: Unnecessary `>= y + 1` or `x - 1 >=`
25+
--> $DIR/int_plus_one.rs:13:5
26+
|
27+
13 | x - 1 >= y;
28+
| ^^^^^^^^^^
29+
|
30+
help: change `>= y + 1` to `> y` as shown
31+
|
32+
13 | x > y;
33+
| ^^^^^
34+
35+
error: Unnecessary `>= y + 1` or `x - 1 >=`
36+
--> $DIR/int_plus_one.rs:14:5
37+
|
38+
14 | y <= x - 1;
39+
| ^^^^^^^^^^
40+
|
41+
help: change `>= y + 1` to `> y` as shown
42+
|
43+
14 | y < x;
44+
| ^^^^^
45+
46+
error: aborting due to 4 previous errors
47+

0 commit comments

Comments
 (0)