Skip to content

Commit 77fc280

Browse files
authored
Replace is_range_full with is_full_collection_range (#17213)
Based on #17212 This was only ever used for collections and the extra work it did was incorrect for that use case. This will no longer const eval the start, but we shouldn't doing that on style lints anyways. changelog: none
2 parents 65be426 + a71d118 commit 77fc280

7 files changed

Lines changed: 38 additions & 178 deletions

File tree

clippy_lints/src/methods/clear_with_drain.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::res::MaybeDef;
3-
use clippy_utils::{is_range_full, sym};
2+
use clippy_utils::res::{MaybeDef, MaybeResPath};
3+
use clippy_utils::{is_full_collection_range, sym};
44
use rustc_errors::Applicability;
5-
use rustc_hir::{Expr, ExprKind, LangItem, QPath};
5+
use rustc_hir::{Expr, LangItem};
66
use rustc_lint::LateContext;
77
use rustc_span::Span;
88

@@ -16,8 +16,7 @@ const ACCEPTABLE_TYPES_WITHOUT_ARG: [rustc_span::Symbol; 3] = [sym::BinaryHeap,
1616
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span: Span, arg: Option<&Expr<'_>>) {
1717
if let Some(arg) = arg {
1818
if match_acceptable_type(cx, recv, &ACCEPTABLE_TYPES_WITH_ARG)
19-
&& let ExprKind::Path(QPath::Resolved(None, container_path)) = recv.kind
20-
&& is_range_full(cx, arg, Some(container_path))
19+
&& is_full_collection_range(cx, recv.res_local_id(), arg)
2120
{
2221
suggest(cx, expr, recv, span);
2322
}

clippy_lints/src/methods/drain_collect.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
use crate::methods::DRAIN_COLLECT;
22
use clippy_utils::diagnostics::span_lint_and_sugg;
3-
use clippy_utils::res::MaybeDef;
3+
use clippy_utils::res::{MaybeDef, MaybeResPath};
44
use clippy_utils::source::snippet;
5-
use clippy_utils::{is_range_full, std_or_core, sym};
5+
use clippy_utils::{is_full_collection_range, std_or_core, sym};
66
use rustc_errors::Applicability;
7-
use rustc_hir::{Expr, ExprKind, QPath};
7+
use rustc_hir::Expr;
88
use rustc_lint::LateContext;
99
use rustc_middle::ty;
1010

@@ -21,13 +21,7 @@ pub(super) fn check(cx: &LateContext<'_>, arg: Option<&Expr<'_>>, expr: &Expr<'_
2121
ty.opt_diag_name(cx),
2222
Some(sym::HashMap | sym::HashSet | sym::BinaryHeap | sym::Vec | sym::VecDeque)
2323
))
24-
&& arg.is_none_or(|arg| {
25-
if let ExprKind::Path(QPath::Resolved(None, path)) = recv.kind {
26-
is_range_full(cx, arg, Some(path))
27-
} else {
28-
false
29-
}
30-
})
24+
&& arg.is_none_or(|arg| is_full_collection_range(cx, recv.res_local_id(), arg))
3125
&& let Some(exec_context) = std_or_core(cx)
3226
{
3327
let recv = snippet(cx, recv.span, "<expr>");

clippy_lints/src/methods/iter_with_drain.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::{is_range_full, sym};
2+
use clippy_utils::res::MaybeResPath;
3+
use clippy_utils::{is_full_collection_range, sym};
34
use rustc_errors::Applicability;
4-
use rustc_hir::{Expr, ExprKind, QPath};
5+
use rustc_hir::{Expr, ExprKind};
56
use rustc_lint::LateContext;
67
use rustc_span::Span;
78

@@ -12,8 +13,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span
1213
&& let Some(adt) = cx.typeck_results().expr_ty(recv).ty_adt_def()
1314
&& let Some(ty_name) = cx.tcx.get_diagnostic_name(adt.did())
1415
&& matches!(ty_name, sym::Vec | sym::VecDeque)
15-
&& let ExprKind::Path(QPath::Resolved(None, container_path)) = recv.kind
16-
&& is_range_full(cx, arg, Some(container_path))
16+
&& is_full_collection_range(cx, recv.res_local_id(), arg)
1717
{
1818
span_lint_and_sugg(
1919
cx,

clippy_utils/src/lib.rs

Lines changed: 11 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ use source::{SpanExt, walk_span_to_context};
120120
use visitors::{Visitable, for_each_unconsumed_temporary};
121121

122122
use crate::ast_utils::unordered_over;
123-
use crate::consts::ConstEvalCtxt;
124123
use crate::higher::Range;
125124
use crate::msrvs::Msrv;
126125
use crate::res::{MaybeDef, MaybeQPath, MaybeResPath};
@@ -1330,59 +1329,23 @@ pub fn is_else_clause_in_let_else(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
13301329
})
13311330
}
13321331

1333-
/// Checks whether the given `Expr` is a range equivalent to a `RangeFull`.
1334-
///
1335-
/// For the lower bound, this means that:
1336-
/// - either there is none
1337-
/// - or it is the smallest value that can be represented by the range's integer type
1338-
///
1339-
/// For the upper bound, this means that:
1340-
/// - either there is none
1341-
/// - or it is the largest value that can be represented by the range's integer type and is
1342-
/// inclusive
1343-
/// - or it is a call to some container's `len` method and is exclusive, and the range is passed to
1344-
/// a method call on that same container (e.g. `v.drain(..v.len())`)
1345-
///
1346-
/// If the given `Expr` is not some kind of range, the function returns `false`.
1347-
pub fn is_range_full(cx: &LateContext<'_>, expr: &Expr<'_>, container_path: Option<&Path<'_>>) -> bool {
1348-
let ty = cx.typeck_results().expr_ty(expr);
1332+
/// Checks whether the given `Expr` is a range over the entire container.
1333+
pub fn is_full_collection_range(cx: &LateContext<'_>, container: Option<HirId>, expr: &Expr<'_>) -> bool {
13491334
if let Some(Range { start, end, limits, .. }) = Range::hir(cx, expr) {
1350-
let start_is_none_or_min = start.is_none_or(|start| {
1351-
if let rustc_ty::Adt(_, subst) = ty.kind()
1352-
&& let bnd_ty = subst.type_at(0)
1353-
&& let Some(start_const) = ConstEvalCtxt::new(cx).eval(start)
1354-
{
1355-
start_const.is_numeric_min(cx.tcx, bnd_ty)
1356-
} else {
1357-
false
1358-
}
1359-
});
1360-
let end_is_none_or_max = end.is_none_or(|end| match limits {
1361-
RangeLimits::Closed => {
1362-
if let rustc_ty::Adt(_, subst) = ty.kind()
1363-
&& let bnd_ty = subst.type_at(0)
1364-
&& let Some(end_const) = ConstEvalCtxt::new(cx).eval(end)
1335+
start.is_none_or(|start| is_integer_literal(start, 0))
1336+
&& end.is_none_or(|end| {
1337+
if limits == RangeLimits::HalfOpen
1338+
&& let Some(container) = container
1339+
&& let ExprKind::MethodCall(seg, recv, [], _) = end.kind
13651340
{
1366-
end_const.is_numeric_max(cx.tcx, bnd_ty)
1341+
seg.ident.name == sym::len && recv.res_local_id() == Some(container)
13671342
} else {
13681343
false
13691344
}
1370-
},
1371-
RangeLimits::HalfOpen => {
1372-
if let Some(container_path) = container_path
1373-
&& let ExprKind::MethodCall(name, self_arg, [], _) = end.kind
1374-
&& name.ident.name == sym::len
1375-
&& let ExprKind::Path(QPath::Resolved(None, path)) = self_arg.kind
1376-
{
1377-
container_path.res == path.res
1378-
} else {
1379-
false
1380-
}
1381-
},
1382-
});
1383-
return start_is_none_or_min && end_is_none_or_max;
1345+
})
1346+
} else {
1347+
false
13841348
}
1385-
false
13861349
}
13871350

13881351
/// Checks whether the given expression is a constant literal of the given value.

tests/ui/clear_with_drain.fixed

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,6 @@ fn vec_range() {
2020
let mut v = vec![1, 2, 3];
2121
v.clear();
2222
//~^ clear_with_drain
23-
24-
// Do lint
25-
let mut v = vec![1, 2, 3];
26-
v.clear();
27-
//~^ clear_with_drain
2823
}
2924

3025
fn vec_range_from() {
@@ -45,11 +40,6 @@ fn vec_range_from() {
4540
let mut v = vec![1, 2, 3];
4641
v.clear();
4742
//~^ clear_with_drain
48-
49-
// Do lint
50-
let mut v = vec![1, 2, 3];
51-
v.clear();
52-
//~^ clear_with_drain
5343
}
5444

5545
fn vec_range_full() {
@@ -124,11 +114,6 @@ fn vec_deque_range() {
124114
let mut deque = VecDeque::from([1, 2, 3]);
125115
deque.clear();
126116
//~^ clear_with_drain
127-
128-
// Do lint
129-
let mut deque = VecDeque::from([1, 2, 3]);
130-
deque.clear();
131-
//~^ clear_with_drain
132117
}
133118

134119
fn vec_deque_range_from() {
@@ -149,11 +134,6 @@ fn vec_deque_range_from() {
149134
let mut deque = VecDeque::from([1, 2, 3]);
150135
deque.clear();
151136
//~^ clear_with_drain
152-
153-
// Do lint
154-
let mut deque = VecDeque::from([1, 2, 3]);
155-
deque.clear();
156-
//~^ clear_with_drain
157137
}
158138

159139
fn vec_deque_range_full() {
@@ -228,11 +208,6 @@ fn string_range() {
228208
let mut s = String::from("Hello, world!");
229209
s.clear();
230210
//~^ clear_with_drain
231-
232-
// Do lint
233-
let mut s = String::from("Hello, world!");
234-
s.clear();
235-
//~^ clear_with_drain
236211
}
237212

238213
fn string_range_from() {
@@ -253,11 +228,6 @@ fn string_range_from() {
253228
let mut s = String::from("Hello, world!");
254229
s.clear();
255230
//~^ clear_with_drain
256-
257-
// Do lint
258-
let mut s = String::from("Hello, world!");
259-
s.clear();
260-
//~^ clear_with_drain
261231
}
262232

263233
fn string_range_full() {

tests/ui/clear_with_drain.rs

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,6 @@ fn vec_range() {
2020
let mut v = vec![1, 2, 3];
2121
v.drain(0..v.len());
2222
//~^ clear_with_drain
23-
24-
// Do lint
25-
let mut v = vec![1, 2, 3];
26-
v.drain(usize::MIN..v.len());
27-
//~^ clear_with_drain
2823
}
2924

3025
fn vec_range_from() {
@@ -45,11 +40,6 @@ fn vec_range_from() {
4540
let mut v = vec![1, 2, 3];
4641
v.drain(0..);
4742
//~^ clear_with_drain
48-
49-
// Do lint
50-
let mut v = vec![1, 2, 3];
51-
v.drain(usize::MIN..);
52-
//~^ clear_with_drain
5343
}
5444

5545
fn vec_range_full() {
@@ -124,11 +114,6 @@ fn vec_deque_range() {
124114
let mut deque = VecDeque::from([1, 2, 3]);
125115
deque.drain(0..deque.len());
126116
//~^ clear_with_drain
127-
128-
// Do lint
129-
let mut deque = VecDeque::from([1, 2, 3]);
130-
deque.drain(usize::MIN..deque.len());
131-
//~^ clear_with_drain
132117
}
133118

134119
fn vec_deque_range_from() {
@@ -149,11 +134,6 @@ fn vec_deque_range_from() {
149134
let mut deque = VecDeque::from([1, 2, 3]);
150135
deque.drain(0..);
151136
//~^ clear_with_drain
152-
153-
// Do lint
154-
let mut deque = VecDeque::from([1, 2, 3]);
155-
deque.drain(usize::MIN..);
156-
//~^ clear_with_drain
157137
}
158138

159139
fn vec_deque_range_full() {
@@ -228,11 +208,6 @@ fn string_range() {
228208
let mut s = String::from("Hello, world!");
229209
s.drain(0..s.len());
230210
//~^ clear_with_drain
231-
232-
// Do lint
233-
let mut s = String::from("Hello, world!");
234-
s.drain(usize::MIN..s.len());
235-
//~^ clear_with_drain
236211
}
237212

238213
fn string_range_from() {
@@ -253,11 +228,6 @@ fn string_range_from() {
253228
let mut s = String::from("Hello, world!");
254229
s.drain(0..);
255230
//~^ clear_with_drain
256-
257-
// Do lint
258-
let mut s = String::from("Hello, world!");
259-
s.drain(usize::MIN..);
260-
//~^ clear_with_drain
261231
}
262232

263233
fn string_range_full() {

0 commit comments

Comments
 (0)