-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add new
map_with_unused_argument_over_ranges lint
This lint checks for code that looks like
```rust
let something : Vec<_> = (0..100).map(|_| {
1 + 2 + 3
}).collect();
```
which is more clear as
```rust
let something : Vec<_> = std::iter::repeat_with(|| {
1 + 2 + 3
}).take(100).collect();
```
or
```rust
let something : Vec<_> =
std::iter::repeat_n(1 + 2 + 3, 100)
.collect();
```
That is, a map over a range which does nothing with the parameter
passed to it is simply a function (or closure) being called `n`
times and could be more semantically expressed using `take`.- Loading branch information
1 parent
ccb30bf
commit 208a7e6
Showing
18 changed files
with
562 additions
and
10 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
135 changes: 135 additions & 0 deletions
135
clippy_lints/src/methods/map_with_unused_argument_over_ranges.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| use crate::methods::MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES; | ||
| use clippy_config::msrvs::{self, Msrv}; | ||
| use clippy_utils::diagnostics::span_lint_and_then; | ||
| use clippy_utils::source::snippet_with_applicability; | ||
| use clippy_utils::sugg::Sugg; | ||
| use clippy_utils::{eager_or_lazy, higher, usage}; | ||
| use rustc_ast::LitKind; | ||
| use rustc_ast::ast::RangeLimits; | ||
| use rustc_data_structures::packed::Pu128; | ||
| use rustc_errors::Applicability; | ||
| use rustc_hir::{Body, Closure, Expr, ExprKind}; | ||
| use rustc_lint::LateContext; | ||
| use rustc_span::Span; | ||
|
|
||
| fn extract_count_with_applicability( | ||
| cx: &LateContext<'_>, | ||
| range: higher::Range<'_>, | ||
| applicability: &mut Applicability, | ||
| ) -> Option<String> { | ||
| let start = range.start?; | ||
| let end = range.end?; | ||
| // TODO: This doens't handle if either the start or end are negative literals, or if the start is | ||
| // not a literal. In the first case, we need to be careful about how we handle computing the | ||
| // count to avoid overflows. In the second, we may need to add parenthesis to make the | ||
| // suggestion correct. | ||
| if let ExprKind::Lit(lit) = start.kind | ||
| && let LitKind::Int(Pu128(lower_bound), _) = lit.node | ||
| { | ||
| if let ExprKind::Lit(lit) = end.kind | ||
| && let LitKind::Int(Pu128(upper_bound), _) = lit.node | ||
| { | ||
| // Here we can explicitly calculate the number of iterations | ||
| let count = if upper_bound >= lower_bound { | ||
| match range.limits { | ||
| RangeLimits::HalfOpen => upper_bound - lower_bound, | ||
| RangeLimits::Closed => (upper_bound - lower_bound).checked_add(1)?, | ||
| } | ||
| } else { | ||
| 0 | ||
| }; | ||
| return Some(format!("{count}")); | ||
| } | ||
| let end_snippet = Sugg::hir_with_applicability(cx, end, "...", applicability) | ||
| .maybe_par() | ||
| .into_string(); | ||
| if lower_bound == 0 { | ||
| if range.limits == RangeLimits::Closed { | ||
| return Some(format!("{end_snippet} + 1")); | ||
| } | ||
| return Some(end_snippet); | ||
| } | ||
| if range.limits == RangeLimits::Closed { | ||
| return Some(format!("{end_snippet} - {}", lower_bound - 1)); | ||
| } | ||
| return Some(format!("{end_snippet} - {lower_bound}")); | ||
| } | ||
| None | ||
| } | ||
|
|
||
| pub(super) fn check( | ||
| cx: &LateContext<'_>, | ||
| ex: &Expr<'_>, | ||
| receiver: &Expr<'_>, | ||
| arg: &Expr<'_>, | ||
| msrv: &Msrv, | ||
| method_call_span: Span, | ||
| ) { | ||
| let mut applicability = Applicability::MaybeIncorrect; | ||
| if let Some(range) = higher::Range::hir(receiver) | ||
| && let ExprKind::Closure(Closure { body, .. }) = arg.kind | ||
| && let body_hir = cx.tcx.hir().body(*body) | ||
| && let Body { | ||
| params: [param], | ||
| value: body_expr, | ||
| } = body_hir | ||
| && !usage::BindingUsageFinder::are_params_used(cx, body_hir) | ||
| && let Some(count) = extract_count_with_applicability(cx, range, &mut applicability) | ||
| { | ||
| let method_to_use_name; | ||
| let new_span; | ||
| let use_take; | ||
|
|
||
| if eager_or_lazy::switch_to_eager_eval(cx, body_expr) { | ||
| if msrv.meets(msrvs::REPEAT_N) { | ||
| method_to_use_name = "repeat_n"; | ||
| let body_snippet = snippet_with_applicability(cx, body_expr.span, "..", &mut applicability); | ||
| new_span = (arg.span, format!("{body_snippet}, {count}")); | ||
| use_take = false; | ||
| } else { | ||
| method_to_use_name = "repeat"; | ||
| let body_snippet = snippet_with_applicability(cx, body_expr.span, "..", &mut applicability); | ||
| new_span = (arg.span, body_snippet.to_string()); | ||
| use_take = true; | ||
| } | ||
| } else if msrv.meets(msrvs::REPEAT_WITH) { | ||
| method_to_use_name = "repeat_with"; | ||
| new_span = (param.span, String::new()); | ||
| use_take = true; | ||
| } else { | ||
| return; | ||
| } | ||
|
|
||
| span_lint_and_then( | ||
| cx, | ||
| MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES, | ||
| ex.span, | ||
| "map of a closure that does not depend on its parameter over a range", | ||
| |diag| { | ||
| diag.multipart_suggestion( | ||
| if use_take { | ||
| format!("remove the explicit range and use `{method_to_use_name}` and `take`") | ||
| } else { | ||
| format!("remove the explicit range and use `{method_to_use_name}`") | ||
| }, | ||
| vec![ | ||
| ( | ||
| receiver.span.to(method_call_span), | ||
| format!("std::iter::{method_to_use_name}"), | ||
| ), | ||
| new_span, | ||
| ( | ||
| ex.span.shrink_to_hi(), | ||
| if use_take { | ||
| format!(".take({count})") | ||
| } else { | ||
| String::new() | ||
| }, | ||
| ), | ||
| ], | ||
| applicability, | ||
| ); | ||
| }, | ||
| ); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| #![allow( | ||
| unused, | ||
| clippy::redundant_closure, | ||
| clippy::reversed_empty_ranges, | ||
| clippy::identity_op | ||
| )] | ||
| #![warn(clippy::map_with_unused_argument_over_ranges)] | ||
|
|
||
| fn do_something() -> usize { | ||
| todo!() | ||
| } | ||
|
|
||
| fn do_something_interesting(x: usize, y: usize) -> usize { | ||
| todo!() | ||
| } | ||
|
|
||
| macro_rules! gen { | ||
| () => { | ||
| (0..10).map(|_| do_something()); | ||
| }; | ||
| } | ||
|
|
||
| fn main() { | ||
| // These should be raised | ||
| std::iter::repeat_with(|| do_something()).take(10); | ||
| std::iter::repeat_with(|| do_something()).take(10); | ||
| std::iter::repeat_with(|| do_something()).take(11); | ||
| std::iter::repeat_with(|| do_something()).take(7); | ||
| std::iter::repeat_with(|| do_something()).take(8); | ||
| std::iter::repeat_n(3, 10); | ||
| std::iter::repeat_with(|| { | ||
| let x = 3; | ||
| x + 2 | ||
| }).take(10); | ||
| std::iter::repeat_with(|| do_something()).take(10).collect::<Vec<_>>(); | ||
| let upper = 4; | ||
| std::iter::repeat_with(|| do_something()).take(upper); | ||
| let upper_fn = || 4; | ||
| std::iter::repeat_with(|| do_something()).take(upper_fn()); | ||
| std::iter::repeat_with(|| do_something()).take(upper_fn() + 1); | ||
| std::iter::repeat_with(|| do_something()).take(upper_fn() - 2); | ||
| std::iter::repeat_with(|| do_something()).take(upper_fn() - 1); | ||
| (-3..9).map(|_| do_something()); | ||
| std::iter::repeat_with(|| do_something()).take(0); | ||
| std::iter::repeat_with(|| do_something()).take(1); | ||
| std::iter::repeat_with(|| do_something()).take((1 << 4) - 0); | ||
| // These should not be raised | ||
| gen!(); | ||
| let lower = 2; | ||
| let lower_fn = || 2; | ||
| (lower..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled | ||
| (lower_fn()..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled | ||
| (lower_fn()..=upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled | ||
| (0..10).map(|x| do_something_interesting(x, 4)); // Actual map over range | ||
| "Foobar".chars().map(|_| do_something()); // Not a map over range | ||
| // i128::MAX == 340282366920938463463374607431768211455 | ||
| (0..=340282366920938463463374607431768211455).map(|_: u128| do_something()); // Can't be replaced due to overflow | ||
| } | ||
|
|
||
| #[clippy::msrv = "1.27"] | ||
| fn msrv_1_27() { | ||
| (0..10).map(|_| do_something()); | ||
| } | ||
|
|
||
| #[clippy::msrv = "1.28"] | ||
| fn msrv_1_28() { | ||
| std::iter::repeat_with(|| do_something()).take(10); | ||
| } | ||
|
|
||
| #[clippy::msrv = "1.82"] | ||
| fn msrv_1_82() { | ||
| std::iter::repeat(3).take(10); | ||
| } |
Oops, something went wrong.