-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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(); ``` 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
c412528
commit 91a94cf
Showing
10 changed files
with
214 additions
and
3 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
use clippy_utils::diagnostics::span_lint_and_sugg; | ||
use clippy_utils::source::snippet_with_applicability; | ||
use rustc_ast::LitKind; | ||
use rustc_data_structures::packed::Pu128; | ||
use rustc_errors::Applicability; | ||
use rustc_hir::{Body, Closure, Expr, ExprKind, LangItem, PatKind, QPath}; | ||
use rustc_lint::{LateContext, LateLintPass}; | ||
use rustc_session::declare_lint_pass; | ||
|
||
declare_clippy_lint! { | ||
/// ### What it does | ||
/// | ||
/// Lint to prevent trivial `map`s over ranges when one could use `take`. | ||
/// | ||
/// ### Why is this bad? | ||
/// | ||
/// It expreses the intent more clearly to `take` the correct number of times | ||
/// from a generating function than to apply a closure to each number in a | ||
/// range only to discard them. | ||
/// | ||
/// ### Example | ||
/// ```no_run | ||
/// let random_numbers : Vec<_> = (0..10).map(|_| { 3 + 1 }).collect(); | ||
/// ``` | ||
/// Use instead: | ||
/// ```no_run | ||
/// let f : Vec<_> = std::iter::from_fn(|| { 3 + 1 }).take(10).collect(); | ||
/// ``` | ||
#[clippy::version = "1.81.0"] | ||
pub TRIVIAL_MAP_OVER_RANGE, | ||
style, | ||
"map of a trivial closure (not dependent on parameter) over a range" | ||
} | ||
|
||
declare_lint_pass!(TrivialMapOverRange => [TRIVIAL_MAP_OVER_RANGE]); | ||
|
||
impl LateLintPass<'_> for TrivialMapOverRange { | ||
fn check_expr(&mut self, cx: &LateContext<'_>, ex: &Expr<'_>) { | ||
if let ExprKind::MethodCall(path, receiver, [map_arg_expr], _call_span) = ex.kind | ||
&& path.ident.name == rustc_span::sym::map | ||
&& let ExprKind::Struct(qpath, fields, _) = receiver.kind | ||
&& matches!(qpath, QPath::LangItem(LangItem::Range, _)) | ||
&& fields.len() == 2 | ||
&& let ExprKind::Closure(Closure { body, .. }) = map_arg_expr.kind | ||
&& let Body { params: [param], .. } = cx.tcx.hir().body(*body) | ||
&& matches!(param.pat.kind, PatKind::Wild) | ||
&& let ExprKind::Lit(lit) = fields[0].expr.kind | ||
&& let LitKind::Int(Pu128(lower_bound), _) = lit.node | ||
{ | ||
if let ExprKind::Lit(lit) = fields[1].expr.kind | ||
&& let LitKind::Int(Pu128(upper_bound), _) = lit.node | ||
{ | ||
let count = upper_bound - lower_bound; | ||
let mut applicability = Applicability::MaybeIncorrect; | ||
let snippet = snippet_with_applicability(cx, map_arg_expr.span, "|| { ... }", &mut applicability) | ||
.replacen("|_|", "||", 1); | ||
span_lint_and_sugg( | ||
cx, | ||
TRIVIAL_MAP_OVER_RANGE, | ||
ex.span, | ||
"map of a trivial closure (not dependent on parameter) over a range", | ||
"use", | ||
format!("std::iter::repeat_with({snippet}).take({count})"), | ||
applicability, | ||
); | ||
} else if lower_bound == 0 { | ||
let mut applicability = Applicability::MaybeIncorrect; | ||
let count = snippet_with_applicability(cx, fields[1].expr.span, "...", &mut applicability); | ||
let snippet = snippet_with_applicability(cx, map_arg_expr.span, "|| { ... }", &mut applicability) | ||
.replacen("|_|", "||", 1); | ||
span_lint_and_sugg( | ||
cx, | ||
TRIVIAL_MAP_OVER_RANGE, | ||
ex.span, | ||
"map of a trivial closure (not dependent on parameter) over a range", | ||
"use", | ||
format!("std::iter::repeat_with({snippet}).take({count})"), | ||
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
#![allow(clippy::trivial_map_over_range)] | ||
#![warn(clippy::repeat_vec_with_capacity)] | ||
|
||
fn main() { | ||
|
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 |
---|---|---|
@@ -1,3 +1,4 @@ | ||
#![allow(clippy::trivial_map_over_range)] | ||
#![warn(clippy::repeat_vec_with_capacity)] | ||
|
||
fn main() { | ||
|
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,32 @@ | ||
#![allow(unused, clippy::redundant_closure)] | ||
#![warn(clippy::trivial_map_over_range)] | ||
|
||
fn do_something() -> usize { | ||
todo!() | ||
} | ||
|
||
fn do_something_interesting(x: usize, y: usize) -> usize { | ||
todo!() | ||
} | ||
|
||
fn main() { | ||
// These should be raised | ||
std::iter::repeat_with(|| do_something()).take(10); | ||
std::iter::repeat_with(|| do_something()).take(7); | ||
std::iter::repeat_with(|| 3).take(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()); | ||
// These should not be raised | ||
(0..=10).map(|_| do_something()); // Inclusive bounds not yet handled | ||
(0..10).map(|x| do_something()); // We do not detect unused parameters | ||
(0..10).map(|x| do_something()).collect::<Vec<_>>(); // We do not detect unused parameters | ||
(0..10).map(|x| do_something_interesting(x, 4)); // Actual map over range | ||
"Foobar".chars().map(|_| do_something()); // Not a map over range | ||
} |
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,32 @@ | ||
#![allow(unused, clippy::redundant_closure)] | ||
#![warn(clippy::trivial_map_over_range)] | ||
|
||
fn do_something() -> usize { | ||
todo!() | ||
} | ||
|
||
fn do_something_interesting(x: usize, y: usize) -> usize { | ||
todo!() | ||
} | ||
|
||
fn main() { | ||
// These should be raised | ||
(0..10).map(|_| do_something()); | ||
(3..10).map(|_| do_something()); | ||
(0..10).map(|_| 3); | ||
(0..10).map(|_| { | ||
let x = 3; | ||
x + 2 | ||
}); | ||
(0..10).map(|_| do_something()).collect::<Vec<_>>(); | ||
let upper = 4; | ||
(0..upper).map(|_| do_something()); | ||
let upper_fn = || 4; | ||
(0..upper_fn()).map(|_| do_something()); | ||
// These should not be raised | ||
(0..=10).map(|_| do_something()); // Inclusive bounds not yet handled | ||
(0..10).map(|x| do_something()); // We do not detect unused parameters | ||
(0..10).map(|x| do_something()).collect::<Vec<_>>(); // We do not detect unused parameters | ||
(0..10).map(|x| do_something_interesting(x, 4)); // Actual map over range | ||
"Foobar".chars().map(|_| do_something()); // Not a map over range | ||
} |
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,58 @@ | ||
error: map of a trivial closure (not dependent on parameter) over a range | ||
--> tests/ui/trivial_map_over_range.rs:14:5 | ||
| | ||
LL | (0..10).map(|_| do_something()); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(10)` | ||
| | ||
= note: `-D clippy::trivial-map-over-range` implied by `-D warnings` | ||
= help: to override `-D warnings` add `#[allow(clippy::trivial_map_over_range)]` | ||
|
||
error: map of a trivial closure (not dependent on parameter) over a range | ||
--> tests/ui/trivial_map_over_range.rs:15:5 | ||
| | ||
LL | (3..10).map(|_| do_something()); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(7)` | ||
|
||
error: map of a trivial closure (not dependent on parameter) over a range | ||
--> tests/ui/trivial_map_over_range.rs:16:5 | ||
| | ||
LL | (0..10).map(|_| 3); | ||
| ^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| 3).take(10)` | ||
|
||
error: map of a trivial closure (not dependent on parameter) over a range | ||
--> tests/ui/trivial_map_over_range.rs:17:5 | ||
| | ||
LL | / (0..10).map(|_| { | ||
LL | | let x = 3; | ||
LL | | x + 2 | ||
LL | | }); | ||
| |______^ | ||
| | ||
help: use | ||
| | ||
LL ~ std::iter::repeat_with(|| { | ||
LL + let x = 3; | ||
LL + x + 2 | ||
LL ~ }).take(10); | ||
| | ||
|
||
error: map of a trivial closure (not dependent on parameter) over a range | ||
--> tests/ui/trivial_map_over_range.rs:21:5 | ||
| | ||
LL | (0..10).map(|_| do_something()).collect::<Vec<_>>(); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(10)` | ||
|
||
error: map of a trivial closure (not dependent on parameter) over a range | ||
--> tests/ui/trivial_map_over_range.rs:23:5 | ||
| | ||
LL | (0..upper).map(|_| do_something()); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper)` | ||
|
||
error: map of a trivial closure (not dependent on parameter) over a range | ||
--> tests/ui/trivial_map_over_range.rs:25:5 | ||
| | ||
LL | (0..upper_fn()).map(|_| do_something()); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper_fn())` | ||
|
||
error: aborting due to 7 previous errors | ||
|