Skip to content

Commit ed44449

Browse files
rspencer01rspencer01
committed
Use higher::Range to match the receiver
This lets us handle half-closed ranges naturally and makes the pattern matching slightly more clean.
1 parent d19cd65 commit ed44449

File tree

4 files changed

+54
-16
lines changed

4 files changed

+54
-16
lines changed

‎clippy_lints/src/map_with_unused_argument_over_ranges.rs‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::higher;
23
use clippy_utils::source::snippet_with_applicability;
4+
use rustc_ast::ast::RangeLimits;
35
use rustc_ast::LitKind;
46
use rustc_data_structures::packed::Pu128;
57
use rustc_errors::Applicability;
6-
use rustc_hir::{Body, Closure, Expr, ExprKind, LangItem, PatKind, QPath};
8+
use rustc_hir::{Body, Closure, Expr, ExprKind, PatKind};
79
use rustc_lint::{LateContext, LateLintPass};
810
use rustc_session::declare_lint_pass;
911

@@ -39,19 +41,25 @@ impl LateLintPass<'_> for MapWithUnusedArgumentOverRanges {
3941
fn check_expr(&mut self, cx: &LateContext<'_>, ex: &Expr<'_>) {
4042
if let ExprKind::MethodCall(path, receiver, [map_arg_expr], _call_span) = ex.kind
4143
&& path.ident.name == rustc_span::sym::map
42-
&& let ExprKind::Struct(qpath, fields, _) = receiver.kind
43-
&& matches!(qpath, QPath::LangItem(LangItem::Range, _))
44-
&& fields.len() == 2
44+
&& let Some(higher::Range {
45+
start: Some(start),
46+
end: Some(end),
47+
limits,
48+
}) = higher::Range::hir(receiver)
4549
&& let ExprKind::Closure(Closure { body, .. }) = map_arg_expr.kind
4650
&& let Body { params: [param], .. } = cx.tcx.hir().body(*body)
4751
&& matches!(param.pat.kind, PatKind::Wild)
48-
&& let ExprKind::Lit(lit) = fields[0].expr.kind
52+
&& let ExprKind::Lit(lit) = start.kind
4953
&& let LitKind::Int(Pu128(lower_bound), _) = lit.node
5054
{
51-
if let ExprKind::Lit(lit) = fields[1].expr.kind
55+
if let ExprKind::Lit(lit) = end.kind
5256
&& let LitKind::Int(Pu128(upper_bound), _) = lit.node
5357
{
54-
let count = upper_bound - lower_bound;
58+
let count = if limits == RangeLimits::Closed {
59+
upper_bound - lower_bound + 1
60+
} else {
61+
upper_bound - lower_bound
62+
};
5563
let mut applicability = Applicability::MaybeIncorrect;
5664
let snippet = snippet_with_applicability(cx, map_arg_expr.span, "|| { ... }", &mut applicability)
5765
.replacen("|_|", "||", 1);
@@ -66,7 +74,11 @@ impl LateLintPass<'_> for MapWithUnusedArgumentOverRanges {
6674
);
6775
} else if lower_bound == 0 {
6876
let mut applicability = Applicability::MaybeIncorrect;
69-
let count = snippet_with_applicability(cx, fields[1].expr.span, "...", &mut applicability);
77+
let count = if limits == RangeLimits::Closed {
78+
snippet_with_applicability(cx, end.span, "...", &mut applicability) + " + 1"
79+
} else {
80+
snippet_with_applicability(cx, end.span, "...", &mut applicability)
81+
};
7082
let snippet = snippet_with_applicability(cx, map_arg_expr.span, "|| { ... }", &mut applicability)
7183
.replacen("|_|", "||", 1);
7284
span_lint_and_sugg(

‎tests/ui/map_with_unused_argument_over_ranges.fixed‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ fn do_something_interesting(x: usize, y: usize) -> usize {
1212
fn main() {
1313
// These should be raised
1414
std::iter::repeat_with(|| do_something()).take(10);
15+
std::iter::repeat_with(|| do_something()).take(11);
1516
std::iter::repeat_with(|| do_something()).take(7);
1617
std::iter::repeat_with(|| 3).take(10);
1718
std::iter::repeat_with(|| {
@@ -23,8 +24,14 @@ fn main() {
2324
std::iter::repeat_with(|| do_something()).take(upper);
2425
let upper_fn = || 4;
2526
std::iter::repeat_with(|| do_something()).take(upper_fn());
27+
std::iter::repeat_with(|| do_something()).take(upper_fn() + 1);
2628
// These should not be raised
27-
(0..=10).map(|_| do_something()); // Inclusive bounds not yet handled
29+
let lower = 2;
30+
let lower_fn = || 2;
31+
(2..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
32+
(lower..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
33+
(lower_fn()..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
34+
(lower_fn()..=upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
2835
(0..10).map(|x| do_something()); // We do not detect unused parameters
2936
(0..10).map(|x| do_something()).collect::<Vec<_>>(); // We do not detect unused parameters
3037
(0..10).map(|x| do_something_interesting(x, 4)); // Actual map over range

‎tests/ui/map_with_unused_argument_over_ranges.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ fn do_something_interesting(x: usize, y: usize) -> usize {
1212
fn main() {
1313
// These should be raised
1414
(0..10).map(|_| do_something());
15+
(0..=10).map(|_| do_something());
1516
(3..10).map(|_| do_something());
1617
(0..10).map(|_| 3);
1718
(0..10).map(|_| {
@@ -23,8 +24,14 @@ fn main() {
2324
(0..upper).map(|_| do_something());
2425
let upper_fn = || 4;
2526
(0..upper_fn()).map(|_| do_something());
27+
(0..=upper_fn()).map(|_| do_something());
2628
// These should not be raised
27-
(0..=10).map(|_| do_something()); // Inclusive bounds not yet handled
29+
let lower = 2;
30+
let lower_fn = || 2;
31+
(2..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
32+
(lower..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
33+
(lower_fn()..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
34+
(lower_fn()..=upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
2835
(0..10).map(|x| do_something()); // We do not detect unused parameters
2936
(0..10).map(|x| do_something()).collect::<Vec<_>>(); // We do not detect unused parameters
3037
(0..10).map(|x| do_something_interesting(x, 4)); // Actual map over range

‎tests/ui/map_with_unused_argument_over_ranges.stderr‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,23 @@ LL | (0..10).map(|_| do_something());
1010
error: map of a trivial closure (not dependent on parameter) over a range
1111
--> tests/ui/map_with_unused_argument_over_ranges.rs:15:5
1212
|
13+
LL | (0..=10).map(|_| do_something());
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(11)`
15+
16+
error: map of a trivial closure (not dependent on parameter) over a range
17+
--> tests/ui/map_with_unused_argument_over_ranges.rs:16:5
18+
|
1319
LL | (3..10).map(|_| do_something());
1420
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(7)`
1521

1622
error: map of a trivial closure (not dependent on parameter) over a range
17-
--> tests/ui/map_with_unused_argument_over_ranges.rs:16:5
23+
--> tests/ui/map_with_unused_argument_over_ranges.rs:17:5
1824
|
1925
LL | (0..10).map(|_| 3);
2026
| ^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| 3).take(10)`
2127

2228
error: map of a trivial closure (not dependent on parameter) over a range
23-
--> tests/ui/map_with_unused_argument_over_ranges.rs:17:5
29+
--> tests/ui/map_with_unused_argument_over_ranges.rs:18:5
2430
|
2531
LL | / (0..10).map(|_| {
2632
LL | | let x = 3;
@@ -37,22 +43,28 @@ LL ~ }).take(10);
3743
|
3844

3945
error: map of a trivial closure (not dependent on parameter) over a range
40-
--> tests/ui/map_with_unused_argument_over_ranges.rs:21:5
46+
--> tests/ui/map_with_unused_argument_over_ranges.rs:22:5
4147
|
4248
LL | (0..10).map(|_| do_something()).collect::<Vec<_>>();
4349
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(10)`
4450

4551
error: map of a trivial closure (not dependent on parameter) over a range
46-
--> tests/ui/map_with_unused_argument_over_ranges.rs:23:5
52+
--> tests/ui/map_with_unused_argument_over_ranges.rs:24:5
4753
|
4854
LL | (0..upper).map(|_| do_something());
4955
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper)`
5056

5157
error: map of a trivial closure (not dependent on parameter) over a range
52-
--> tests/ui/map_with_unused_argument_over_ranges.rs:25:5
58+
--> tests/ui/map_with_unused_argument_over_ranges.rs:26:5
5359
|
5460
LL | (0..upper_fn()).map(|_| do_something());
5561
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper_fn())`
5662

57-
error: aborting due to 7 previous errors
63+
error: map of a trivial closure (not dependent on parameter) over a range
64+
--> tests/ui/map_with_unused_argument_over_ranges.rs:27:5
65+
|
66+
LL | (0..=upper_fn()).map(|_| do_something());
67+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper_fn() + 1)`
68+
69+
error: aborting due to 9 previous errors
5870

0 commit comments

Comments
 (0)