-
Notifications
You must be signed in to change notification settings - Fork 2k
Expand file tree
/
Copy pathint_plus_one.rs
More file actions
167 lines (155 loc) · 5.34 KB
/
int_plus_one.rs
File metadata and controls
167 lines (155 loc) · 5.34 KB
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::sugg;
use rustc_ast::ast::{BinOpKind, Expr, ExprKind, LitKind, UnOp};
use rustc_ast::util::parser::AssocOp;
use rustc_data_structures::packed::Pu128;
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::declare_lint_pass;
declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `x >= y + 1` or `x - 1 >= y` (and `<=`) in a block
///
/// ### Why is this bad?
/// Readability -- better to use `> y` instead of `>= y + 1`.
///
/// ### Example
/// ```no_run
/// # let x = 1;
/// # let y = 1;
/// if x >= y + 1 {}
/// ```
///
/// Use instead:
/// ```no_run
/// # let x = 1;
/// # let y = 1;
/// if x > y {}
/// ```
#[clippy::version = "pre 1.29.0"]
pub INT_PLUS_ONE,
complexity,
"instead of using `x >= y + 1`, use `x > y`"
}
declare_lint_pass!(IntPlusOne => [INT_PLUS_ONE]);
// cases:
// LeOrGe::Ge
// x >= y + 1
// x - 1 >= y
//
// LeOrGe::Le
// x + 1 <= y
// x <= y - 1
#[derive(Copy, Clone)]
enum LeOrGe {
Le,
Ge,
}
impl TryFrom<BinOpKind> for LeOrGe {
type Error = ();
fn try_from(value: BinOpKind) -> Result<Self, Self::Error> {
match value {
BinOpKind::Le => Ok(Self::Le),
BinOpKind::Ge => Ok(Self::Ge),
_ => Err(()),
}
}
}
impl IntPlusOne {
fn is_one(expr: &Expr) -> bool {
if let ExprKind::Lit(token_lit) = expr.kind
&& matches!(LitKind::from_token_lit(token_lit), Ok(LitKind::Int(Pu128(1), ..)))
{
return true;
}
false
}
fn is_neg_one(expr: &Expr) -> bool {
if let ExprKind::Unary(UnOp::Neg, expr) = &expr.kind {
Self::is_one(expr)
} else {
false
}
}
/// Checks whether `expr` is `x + 1` or `1 + x`, and if so, returns `x`
fn as_x_plus_one(expr: &Expr) -> Option<&Expr> {
if let ExprKind::Binary(op, lhs, rhs) = &expr.kind
&& op.node == BinOpKind::Add
{
if Self::is_one(rhs) {
// x + 1
return Some(lhs);
} else if Self::is_one(lhs) {
// 1 + x
return Some(rhs);
}
}
None
}
/// Checks whether `expr` is `x - 1` or `-1 + x`, and if so, returns `x`
fn as_x_minus_one(expr: &Expr) -> Option<&Expr> {
if let ExprKind::Binary(op, lhs, rhs) = &expr.kind {
if op.node == BinOpKind::Sub && Self::is_one(rhs) {
// x - 1
return Some(lhs);
} else if op.node == BinOpKind::Add && Self::is_neg_one(lhs) {
// -1 + x
return Some(rhs);
}
}
None
}
fn check_binop<'tcx>(le_or_ge: LeOrGe, lhs: &'tcx Expr, rhs: &'tcx Expr) -> Option<(&'tcx Expr, &'tcx Expr)> {
match le_or_ge {
LeOrGe::Ge => {
// case where `x - 1 >= ...` or `-1 + x >= ...`
(Self::as_x_minus_one(lhs).map(|new_lhs| (new_lhs, rhs)))
// case where `... >= y + 1` or `... >= 1 + y`
.or_else(|| Self::as_x_plus_one(rhs).map(|new_rhs| (lhs, new_rhs)))
},
LeOrGe::Le => {
// case where `x + 1 <= ...` or `1 + x <= ...`
(Self::as_x_plus_one(lhs).map(|new_lhs| (new_lhs, rhs)))
// case where `... <= y - 1` or `... <= -1 + y`
.or_else(|| Self::as_x_minus_one(rhs).map(|new_rhs| (lhs, new_rhs)))
},
}
}
fn emit_warning(cx: &EarlyContext<'_>, expr: &Expr, new_lhs: &Expr, le_or_ge: LeOrGe, new_rhs: &Expr) {
span_lint_and_then(
cx,
INT_PLUS_ONE,
expr.span,
"unnecessary `>= y + 1` or `x - 1 >=`",
|diag| {
let mut app = Applicability::MachineApplicable;
let ctxt = expr.span.ctxt();
let mut new_lhs = sugg::Sugg::ast(cx, new_lhs, "_", ctxt, &mut app);
let new_rhs = sugg::Sugg::ast(cx, new_rhs, "_", ctxt, &mut app);
let new_binop = match le_or_ge {
LeOrGe::Ge => BinOpKind::Gt,
LeOrGe::Le => BinOpKind::Lt,
};
// When the replacement operator is `<`, an `as` cast on the LHS
// must be parenthesized. Otherwise, the parser interprets the `<`
// as the start of generic arguments on the cast type
// (e.g., `x as usize < y` is parsed as `x as usize<y>`).
if matches!(new_lhs, sugg::Sugg::BinOp(AssocOp::Cast, ..)) && new_binop == BinOpKind::Lt {
new_lhs = new_lhs.maybe_paren();
}
let rec = sugg::make_binop(new_binop, &new_lhs, &new_rhs);
diag.span_suggestion(expr.span, "change it to", rec, app);
},
);
}
}
impl EarlyLintPass for IntPlusOne {
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
if let ExprKind::Binary(binop, lhs, rhs) = &expr.kind
&& let Ok(le_or_ge) = LeOrGe::try_from(binop.node)
&& let Some((new_lhs, new_rhs)) = Self::check_binop(le_or_ge, lhs, rhs)
{
Self::emit_warning(cx, expr, new_lhs, le_or_ge, new_rhs);
}
}
}