Skip to content

Commit f8f9ed7

Browse files
Kither12Kither12
committed
small fix
1 parent 6bee700 commit f8f9ed7

File tree

4 files changed

+112
-118
lines changed

4 files changed

+112
-118
lines changed

‎clippy_lints/src/methods/truncate_with_drain.rs

Lines changed: 69 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -8,89 +8,95 @@ use rustc_errors::Applicability;
88
use rustc_hir::{Expr, ExprKind, LangItem, Path, QPath};
99
use rustc_lint::LateContext;
1010
use rustc_middle::mir::Const;
11-
use rustc_middle::ty::{Adt, Ty};
11+
use rustc_middle::ty::{Adt, Ty, TypeckResults};
1212
use rustc_span::Span;
1313
use rustc_span::symbol::sym;
1414

1515
use super::TRUNCATE_WITH_DRAIN;
1616

1717
// Add `String` here when it is added to diagnostic items
18-
const ACCEPTABLE_TYPES: [rustc_span::Symbol; 2] = [sym::Vec, sym::VecDeque];
18+
const ACCEPTABLE_TYPES_WITH_ARG: [rustc_span::Symbol; 2] = [sym::Vec, sym::VecDeque];
1919

20-
pub fn is_range_open_ended(cx: &LateContext<'_>, expr: &Expr<'_>, container_path: Option<&Path<'_>>) -> bool {
21-
let ty = cx.typeck_results().expr_ty(expr);
22-
if let Some(higher::Range { start, end, limits }) = higher::Range::hir(expr) {
23-
let start_is_none_or_min = start.is_none_or(|start| {
20+
pub fn is_range_open_ended<'a>(
21+
cx: &LateContext<'a>,
22+
range: higher::Range<'_>,
23+
ty: Ty<'a>,
24+
container_path: Option<&Path<'_>>,
25+
) -> bool {
26+
let higher::Range { start, end, limits } = range;
27+
let start_is_none_or_min = start.map_or(true, |start| {
28+
if let Adt(_, subst) = ty.kind()
29+
&& let bnd_ty = subst.type_at(0)
30+
&& let Some(min_val) = bnd_ty.numeric_min_val(cx.tcx)
31+
&& let Some(min_const) = mir_to_const(cx.tcx, Const::from_ty_const(min_val, bnd_ty, cx.tcx))
32+
&& let Some(start_const) = ConstEvalCtxt::new(cx).eval(start)
33+
{
34+
start_const == min_const
35+
} else {
36+
false
37+
}
38+
});
39+
let end_is_none_or_max = end.map_or(true, |end| match limits {
40+
RangeLimits::Closed => {
2441
if let Adt(_, subst) = ty.kind()
2542
&& let bnd_ty = subst.type_at(0)
26-
&& let Some(min_val) = bnd_ty.numeric_min_val(cx.tcx)
27-
&& let Some(min_const) = mir_to_const(cx.tcx, Const::from_ty_const(min_val, bnd_ty, cx.tcx))
28-
&& let Some(start_const) = ConstEvalCtxt::new(cx).eval(start)
43+
&& let Some(max_val) = bnd_ty.numeric_max_val(cx.tcx)
44+
&& let Some(max_const) = mir_to_const(cx.tcx, Const::from_ty_const(max_val, bnd_ty, cx.tcx))
45+
&& let Some(end_const) = ConstEvalCtxt::new(cx).eval(end)
2946
{
30-
start_const == min_const
47+
end_const == max_const
3148
} else {
3249
false
3350
}
34-
});
35-
let end_is_none_or_max = end.is_none_or(|end| match limits {
36-
RangeLimits::Closed => {
37-
if let Adt(_, subst) = ty.kind()
38-
&& let bnd_ty = subst.type_at(0)
39-
&& let Some(max_val) = bnd_ty.numeric_max_val(cx.tcx)
40-
&& let Some(max_const) = mir_to_const(cx.tcx, Const::from_ty_const(max_val, bnd_ty, cx.tcx))
41-
&& let Some(end_const) = ConstEvalCtxt::new(cx).eval(end)
42-
{
43-
end_const == max_const
44-
} else {
45-
false
46-
}
47-
},
48-
RangeLimits::HalfOpen => {
49-
if let Some(container_path) = container_path
50-
&& let ExprKind::MethodCall(name, self_arg, [], _) = end.kind
51-
&& name.ident.name == sym::len
52-
&& let ExprKind::Path(QPath::Resolved(None, path)) = self_arg.kind
53-
{
54-
container_path.res == path.res
55-
} else {
56-
false
57-
}
58-
},
59-
});
60-
return !start_is_none_or_min && end_is_none_or_max;
61-
}
62-
false
63-
}
64-
65-
fn is_handled_collection_type(cx: &LateContext<'_>, expr_ty: Ty<'_>) -> bool {
66-
ACCEPTABLE_TYPES.iter().any(|&ty| is_type_diagnostic_item(cx, expr_ty, ty))
67-
// String type is a lang item but not a diagnostic item for now so we need a separate check
68-
|| is_type_lang_item(cx, expr_ty, LangItem::String)
51+
},
52+
RangeLimits::HalfOpen => {
53+
if let Some(container_path) = container_path
54+
&& let ExprKind::MethodCall(name, self_arg, [], _) = end.kind
55+
&& name.ident.name == sym::len
56+
&& let ExprKind::Path(QPath::Resolved(None, path)) = self_arg.kind
57+
{
58+
container_path.res == path.res
59+
} else {
60+
false
61+
}
62+
},
63+
});
64+
return !start_is_none_or_min && end_is_none_or_max;
6965
}
7066

7167
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span: Span, arg: Option<&Expr<'_>>) {
7268
if let Some(arg) = arg {
73-
let expr_ty = cx.typeck_results().expr_ty(expr);
74-
if is_handled_collection_type(cx, expr_ty)
69+
let typeck_results = cx.typeck_results();
70+
if match_acceptable_type(cx, recv, typeck_results, &ACCEPTABLE_TYPES_WITH_ARG)
7571
&& let ExprKind::Path(QPath::Resolved(None, container_path)) = recv.kind
76-
&& let Some(range) = higher::Range::hir(expr)
77-
&& is_range_open_ended(cx, arg, Some(container_path))
78-
{
79-
if let Some(adt) = expr_ty.ty_adt_def()
72+
&& let Some(range) = higher::Range::hir(arg)
73+
&& let higher::Range { start: Some(start), .. } = range
74+
&& is_range_open_ended(cx, range, typeck_results.expr_ty(arg), Some(container_path))
75+
&& let Some(adt) = typeck_results.expr_ty(recv).ty_adt_def()
8076
// Use `opt_item_name` while `String` is not a diagnostic item
8177
&& let Some(ty_name) = cx.tcx.opt_item_name(adt.did())
82-
{
83-
span_lint_and_sugg(
84-
cx,
85-
TRUNCATE_WITH_DRAIN,
86-
span.with_hi(expr.span.hi()),
87-
format!("`drain` used to truncate a `{ty_name}`"),
88-
"use",
89-
// Can safely unwrap here because we don't lint on 0.. ranges
90-
format!("truncate({})", snippet(cx, range.start.unwrap().span, "0")),
91-
Applicability::MachineApplicable,
92-
);
93-
}
78+
{
79+
span_lint_and_sugg(
80+
cx,
81+
TRUNCATE_WITH_DRAIN,
82+
span.with_hi(expr.span.hi()),
83+
format!("`drain` used to truncate a `{ty_name}`"),
84+
"try",
85+
format!("truncate({})", snippet(cx, start.span, "0")),
86+
Applicability::MachineApplicable,
87+
);
9488
}
9589
}
9690
}
91+
92+
fn match_acceptable_type(
93+
cx: &LateContext<'_>,
94+
expr: &Expr<'_>,
95+
typeck_results: &TypeckResults<'_>,
96+
types: &[rustc_span::Symbol],
97+
) -> bool {
98+
let expr_ty = typeck_results.expr_ty(expr).peel_refs();
99+
types.iter().any(|&ty| is_type_diagnostic_item(cx, expr_ty, ty))
100+
// String type is a lang item but not a diagnostic item for now so we need a separate check
101+
|| is_type_lang_item(cx, expr_ty, LangItem::String)
102+
}

‎tests/ui/truncate_with_drain.fixed

Lines changed: 22 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -10,45 +10,39 @@ fn vec_range() {
1010

1111
// Do not lint because iterator is used
1212
let mut v = vec![1, 2, 3];
13-
let n = v.drain(1..v.len()).count();
14-
15-
// Do not lint because iterator is assigned and used
16-
let mut v = vec![1, 2, 3];
17-
let iter = v.drain(1..v.len());
18-
let n = iter.count();
13+
v.drain(1..v.len()).next();
1914

2015
// Do lint
2116
let mut v = vec![1, 2, 3];
2217
v.truncate(1);
18+
//~^ ERROR: `drain` used to truncate a `Vec`
2319

2420
// Do lint
2521
let x = 1;
2622
let mut v = vec![1, 2, 3];
2723
v.truncate(x);
24+
//~^ ERROR: `drain` used to truncate a `Vec`
2825
}
2926

3027
fn vec_range_from() {
3128
// Do not lint because iterator is assigned
3229
let mut v = vec![1, 2, 3];
3330
let iter = v.drain(1..);
3431

35-
// Do not lint because iterator is assigned and used
36-
let mut v = vec![1, 2, 3];
37-
let mut iter = v.drain(1..);
38-
let next = iter.next();
39-
4032
// Do not lint because iterator is used
4133
let mut v = vec![1, 2, 3];
42-
let next = v.drain(1..).next();
34+
v.drain(1..).next();
4335

4436
// Do lint
4537
let mut v = vec![1, 2, 3];
4638
v.truncate(1);
39+
//~^ ERROR: `drain` used to truncate a `Vec`
4740

4841
// Do lint
4942
let x = 1;
5043
let mut v = vec![1, 2, 3];
5144
v.truncate(x);
45+
//~^ ERROR: `drain` used to truncate a `Vec`
5246
}
5347

5448
fn vec_partial_drains() {
@@ -102,45 +96,39 @@ fn vec_deque_range() {
10296

10397
// Do not lint because iterator is used
10498
let mut deque = VecDeque::from([1, 2, 3]);
105-
let n = deque.drain(1..deque.len()).count();
106-
107-
// Do not lint because iterator is assigned and used
108-
let mut deque = VecDeque::from([1, 2, 3]);
109-
let iter = deque.drain(1..deque.len());
110-
let n = iter.count();
99+
deque.drain(1..deque.len()).next();
111100

112101
// Do lint
113102
let mut v = VecDeque::from([1, 2, 3]);
114103
v.truncate(1);
104+
//~^ ERROR: `drain` used to truncate a `VecDeque`
115105

116106
// Do lint
117107
let x = 1;
118108
let mut v = VecDeque::from([1, 2, 3]);
119109
v.truncate(x);
110+
//~^ ERROR: `drain` used to truncate a `VecDeque`
120111
}
121112

122113
fn vec_deque_range_from() {
123114
// Do not lint because iterator is assigned
124115
let mut deque = VecDeque::from([1, 2, 3]);
125116
let iter = deque.drain(1..);
126117

127-
// Do not lint because iterator is assigned and used
128-
let mut deque = VecDeque::from([1, 2, 3]);
129-
let mut iter = deque.drain(1..);
130-
let next = iter.next();
131-
132118
// Do not lint because iterator is used
133119
let mut deque = VecDeque::from([1, 2, 3]);
134-
let next = deque.drain(1..).next();
120+
deque.drain(1..).next();
135121

136122
// Do lint
137123
let mut deque = VecDeque::from([1, 2, 3]);
138124
deque.truncate(1);
125+
//~^ ERROR: `drain` used to truncate a `VecDeque`
139126

140127
// Do lint
141128
let x = 1;
142129
let mut v = VecDeque::from([1, 2, 3]);
143130
v.truncate(x);
131+
//~^ ERROR: `drain` used to truncate a `VecDeque`
144132
}
145133

146134
fn vec_deque_partial_drains() {
@@ -194,45 +182,39 @@ fn string_range() {
194182

195183
// Do not lint because iterator is used
196184
let mut s = String::from("Hello, world!");
197-
let n = s.drain(1..s.len()).count();
198-
199-
// Do not lint because iterator is assigned and used
200-
let mut s = String::from("Hello, world!");
201-
let iter = s.drain(1..s.len());
202-
let n = iter.count();
185+
s.drain(1..s.len()).next();
203186

204187
// Do lint
205188
let mut s = String::from("Hello, world!");
206189
s.truncate(1);
190+
//~^ ERROR: `drain` used to truncate a `String`
207191

208192
// Do lint
209193
let x = 1;
210-
let mut v = String::from("Hello, world!");
211-
v.drain(x..s.len());
194+
let mut s = String::from("Hello, world!");
195+
s.truncate(x);
196+
//~^ ERROR: `drain` used to truncate a `String`
212197
}
213198

214199
fn string_range_from() {
215200
// Do not lint because iterator is assigned
216201
let mut s = String::from("Hello, world!");
217202
let iter = s.drain(1..);
218203

219-
// Do not lint because iterator is assigned and used
220-
let mut s = String::from("Hello, world!");
221-
let mut iter = s.drain(1..);
222-
let next = iter.next();
223-
224204
// Do not lint because iterator is used
225205
let mut s = String::from("Hello, world!");
226-
let next = s.drain(1..).next();
206+
s.drain(1..).next();
227207

228208
// Do lint
229209
let mut s = String::from("Hello, world!");
230210
s.truncate(1);
211+
//~^ ERROR: `drain` used to truncate a `String`
231212

232213
// Do lint
233214
let x = 1;
234-
let mut v = String::from("Hello, world!");
235-
v.truncate(x);
215+
let mut s = String::from("Hello, world!");
216+
s.truncate(x);
217+
//~^ ERROR: `drain` used to truncate a `String`
236218
}
237219

238220
fn string_partial_drains() {

‎tests/ui/truncate_with_drain.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,8 @@ fn string_range() {
191191

192192
// Do lint
193193
let x = 1;
194-
let mut v = String::from("Hello, world!");
195-
v.drain(x..s.len());
194+
let mut s = String::from("Hello, world!");
195+
s.drain(x..s.len());
196196
//~^ ERROR: `drain` used to truncate a `String`
197197
}
198198

0 commit comments

Comments
 (0)