-
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
35a7095
commit acc3842
Showing
16 changed files
with
558 additions
and
8 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
134 changes: 134 additions & 0 deletions
134
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,134 @@ | ||
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; | ||
} | ||
|
||
// We need to provide nonempty parts to diag.multipart_suggestion so we | ||
// collate all our parts here and then remove those that are empty. | ||
let mut parts = vec![ | ||
( | ||
receiver.span.to(method_call_span), | ||
format!("std::iter::{method_to_use_name}"), | ||
), | ||
new_span, | ||
]; | ||
if use_take { | ||
parts.push((ex.span.shrink_to_hi(), format!(".take({count})"))); | ||
} | ||
|
||
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}`") | ||
}, | ||
parts, | ||
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.81"] | ||
fn msrv_1_82() { | ||
std::iter::repeat(3).take(10); | ||
} |
Oops, something went wrong.