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
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
use rustc::lint::*;
use rustc::hir::*;
use syntax::ast::RangeLimits;
use syntax::codemap::Spanned;
use utils::{is_integer_literal, paths, snippet, span_lint, span_lint_and_then};
use utils::{get_trait_def_id, higher, implements_trait};
use utils::sugg::Sugg;
/// **What it does:** Checks for calling `.step_by(0)` on iterators,
/// which never terminates.
///
/// **Why is this bad?** This very much looks like an oversight, since with
/// `loop { .. }` there is an obvious better way to endlessly loop.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// for x in (5..5).step_by(0) { .. }
/// ```
declare_lint! {
pub ITERATOR_STEP_BY_ZERO,
Warn,
"using `Iterator::step_by(0)`, which produces an infinite iterator"
}
/// **What it does:** Checks for zipping a collection with the range of
/// `0.._.len()`.
///
/// **Why is this bad?** The code is better expressed with `.enumerate()`.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// x.iter().zip(0..x.len())
/// ```
declare_lint! {
pub RANGE_ZIP_WITH_LEN,
Warn,
"zipping iterator with a range when `enumerate()` would do"
}
/// **What it does:** Checks for exclusive ranges where 1 is added to the
/// upper bound, e.g. `x..(y+1)`.
///
/// **Why is this bad?** The code is more readable with an inclusive range
/// like `x..=y`.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// for x..(y+1) { .. }
/// ```
declare_lint! {
pub RANGE_PLUS_ONE,
Allow,
"`x..(y+1)` reads better as `x..=y`"
}
/// **What it does:** Checks for inclusive ranges where 1 is subtracted from
/// the upper bound, e.g. `x..=(y-1)`.
///
/// **Why is this bad?** The code is more readable with an exclusive range
/// like `x..y`.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// for x..=(y-1) { .. }
/// ```
declare_lint! {
pub RANGE_MINUS_ONE,
Warn,
"`x..=(y-1)` reads better as `x..y`"
}
#[derive(Copy, Clone)]
pub struct Pass;
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(ITERATOR_STEP_BY_ZERO, RANGE_ZIP_WITH_LEN, RANGE_PLUS_ONE, RANGE_MINUS_ONE)
}
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let ExprMethodCall(ref path, _, ref args) = expr.node {
let name = path.name.as_str();
// Range with step_by(0).
if name == "step_by" && args.len() == 2 && has_step_by(cx, &args[0]) {
use consts::{constant, Constant};
use rustc_const_math::ConstInt::Usize;
if let Some((Constant::Int(Usize(us)), _)) = constant(cx, &args[1]) {
if us.as_u64() == 0 {
span_lint(
cx,
ITERATOR_STEP_BY_ZERO,
expr.span,
"Iterator::step_by(0) will panic at runtime",
);
}
}
} else if name == "zip" && args.len() == 2 {
let iter = &args[0].node;
let zip_arg = &args[1];
if_chain! {
// .iter() call
if let ExprMethodCall(ref iter_path, _, ref iter_args ) = *iter;
if iter_path.name == "iter";
// range expression in .zip() call: 0..x.len()
if let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::range(zip_arg);
if is_integer_literal(start, 0);
// .len() call
if let ExprMethodCall(ref len_path, _, ref len_args) = end.node;
if len_path.name == "len" && len_args.len() == 1;
// .iter() and .len() called on same Path
if let ExprPath(QPath::Resolved(_, ref iter_path)) = iter_args[0].node;
if let ExprPath(QPath::Resolved(_, ref len_path)) = len_args[0].node;
if iter_path.segments == len_path.segments;
then {
span_lint(cx,
RANGE_ZIP_WITH_LEN,
expr.span,
&format!("It is more idiomatic to use {}.iter().enumerate()",
snippet(cx, iter_args[0].span, "_")));
}
}
}
}
// exclusive range plus one: x..(y+1)
if_chain! {
if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::HalfOpen }) = higher::range(expr);
if let Some(y) = y_plus_one(end);
then {
span_lint_and_then(
cx,
RANGE_PLUS_ONE,
expr.span,
"an inclusive range would be more readable",
|db| {
let start = start.map_or("".to_owned(), |x| Sugg::hir(cx, x, "x").to_string());
let end = Sugg::hir(cx, y, "y");
db.span_suggestion(expr.span,
"use",
format!("{}..={}", start, end));
},
);
}
}
// inclusive range minus one: x..=(y-1)
if_chain! {
if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::range(expr);
if let Some(y) = y_minus_one(end);
then {
span_lint_and_then(
cx,
RANGE_MINUS_ONE,
expr.span,
"an exclusive range would be more readable",
|db| {
let start = start.map_or("".to_owned(), |x| Sugg::hir(cx, x, "x").to_string());
let end = Sugg::hir(cx, y, "y");
db.span_suggestion(expr.span,
"use",
format!("{}..{}", start, end));
},
);
}
}
}
}
fn has_step_by(cx: &LateContext, expr: &Expr) -> bool {
// No need for walk_ptrs_ty here because step_by moves self, so it
// can't be called on a borrowed range.
let ty = cx.tables.expr_ty_adjusted(expr);
get_trait_def_id(cx, &paths::ITERATOR).map_or(false, |iterator_trait| implements_trait(cx, ty, iterator_trait, &[]))
}
fn y_plus_one(expr: &Expr) -> Option<&Expr> {
match expr.node {
ExprBinary(Spanned { node: BiAdd, .. }, ref lhs, ref rhs) => if is_integer_literal(lhs, 1) {
Some(rhs)
} else if is_integer_literal(rhs, 1) {
Some(lhs)
} else {
None
},
_ => None,
}
}
fn y_minus_one(expr: &Expr) -> Option<&Expr> {
match expr.node {
ExprBinary(Spanned { node: BiSub, .. }, ref lhs, ref rhs) if is_integer_literal(rhs, 1) => Some(lhs),
_ => None,
}
}