Skip to content

Add new lints: const_sized_chunks_exact, const_sized_chunks_exact_mut, and const_sized_windows #15097

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5696,6 +5696,9 @@ Released 2018-09-13
[`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty
[`confusing_method_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#confusing_method_to_numeric_cast
[`const_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_is_empty
[`const_sized_chunks_exact`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_sized_chunks_exact
[`const_sized_chunks_exact_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_sized_chunks_exact_mut
[`const_sized_windows`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_sized_windows
[`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime
[`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator
[`crate_in_macro_def`]: https://rust-lang.github.io/rust-clippy/master/index.html#crate_in_macro_def
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,9 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
crate::literal_representation::UNUSUAL_BYTE_GROUPINGS_INFO,
crate::literal_string_with_formatting_args::LITERAL_STRING_WITH_FORMATTING_ARGS_INFO,
crate::loops::CHAR_INDICES_AS_BYTE_INDICES_INFO,
crate::loops::CONST_SIZED_CHUNKS_EXACT_INFO,
crate::loops::CONST_SIZED_CHUNKS_EXACT_MUT_INFO,
crate::loops::CONST_SIZED_WINDOWS_INFO,
crate::loops::EMPTY_LOOP_INFO,
crate::loops::EXPLICIT_COUNTER_LOOP_INFO,
crate::loops::EXPLICIT_INTO_ITER_LOOP_INFO,
Expand Down
60 changes: 60 additions & 0 deletions clippy_lints/src/loops/const_sized_chunks.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
use clippy_utils::consts::{ConstEvalCtxt, Constant};
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::expr_or_init;
use clippy_utils::source::snippet;
use clippy_utils::ty::is_slice_like;
use itertools::Itertools;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, Pat, PatKind};
use rustc_lint::LateContext;

use super::{CONST_SIZED_CHUNKS_EXACT, CONST_SIZED_CHUNKS_EXACT_MUT, CONST_SIZED_WINDOWS};

/// Checks for the `/CONST_SIZED(_CHUNKS_EXACT(_MUT)?|_WINDOWS)/` lint.
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, arg: &'tcx Expr<'_>) {
if !arg.span.from_expansion()
// The `for` loop pattern should be a binding.
&& let PatKind::Binding(..) = pat.kind
// The `for` loop argument expression must be a method call.
&& let ExprKind::MethodCall(method, self_arg, [arg], span) = arg.kind
// The receiver of the method call must be slice-like.
&& is_slice_like(cx, cx.typeck_results().expr_ty(self_arg).peel_refs())
// The parameter to the method call must be a constant.
&& let Some(Constant::Int(n)) = ConstEvalCtxt::new(cx).eval(expr_or_init(cx, arg))
// The number of elements should be limited.
&& let Ok(n) = n.try_into() && n >= 1 && n <= 1 + 'z' as usize - 'a' as usize
{
let method = method.ident.name;
let new_method;

let lint = match method {
clippy_utils::sym::chunks_exact => {
new_method = clippy_utils::sym::array_chunks;
CONST_SIZED_CHUNKS_EXACT
},
clippy_utils::sym::chunks_exact_mut => {
new_method = clippy_utils::sym::array_chunks_mut;
CONST_SIZED_CHUNKS_EXACT_MUT
},
rustc_span::sym::windows => {
new_method = clippy_utils::sym::array_windows;
CONST_SIZED_WINDOWS
},
_ => return,
};

let bindings = ('a'..).take(n).join(", ");
let self_arg = snippet(cx, self_arg.span, "..");
let arg = snippet(cx, arg.span, "_");
let msg = format!("iterating over `{method}()` with constant parameter `{arg}`");

span_lint_and_then(cx, lint, span, msg, |diag| {
diag.span_suggestion_verbose(
span.with_lo(pat.span.lo()),
format!("use `{new_method}::<{n}>()` instead"),
format!("[{bindings}] in {self_arg}.{new_method}()"),
Applicability::Unspecified,
);
});
}
}
106 changes: 106 additions & 0 deletions clippy_lints/src/loops/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mod char_indices_as_byte_indices;
mod const_sized_chunks;
mod empty_loop;
mod explicit_counter_loop;
mod explicit_into_iter_loop;
Expand Down Expand Up @@ -784,6 +785,107 @@ declare_clippy_lint! {
"using the character position yielded by `.chars().enumerate()` in a context where a byte index is expected"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `.chunks_exact()` on slices where the `chunk_size` is a constant and suggests
/// replacing it with its const generic equivalent, `.array_chunks()`, in a way that the value of the
/// const parameter `N` can be inferred.
///
/// Specifically, in a for-loop, consuming the iterator over the (non-overlapping) chunks, the lint
/// suggests destructuring the chunks to allow for `N`, the number of elements in a chunk, to be inferred.
///
/// ### Why is this bad?
/// When `.chunks_exact()` is used, a bounds check is required before an element can be accessed.
///
/// ### Example
/// ```no_run
/// let numbers = Vec::from_iter(0..10);
/// for chunk in numbers.chunks_exact(2) {
/// println!("{n} is an odd number", n = chunk[1]);
/// }
/// ```
/// Use instead:
/// ```no_run
/// #![feature(array_chunks)]
/// let numbers = Vec::from_iter(0..10);
/// for [_, n] in numbers.array_chunks() {
/// println!("{n} is an odd number");
/// }
/// ```
#[clippy::version = "1.89.0"]
pub CONST_SIZED_CHUNKS_EXACT,
nursery,
"calling `.chunks_exact()` with a constant `chunk_size` where `.array_chunks()` could be used instead"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `.chunks_exact_mut()` on slices where the `chunk_size` is a constant and suggests
/// replacing it with its const generic equivalent, `.array_chunks_mut()`, in a way that the value of the
/// const parameter `N` can be inferred.
///
/// Specifically, in a for-loop, consuming the iterator over the (non-overlapping) chunks, the lint
/// suggests destructuring the chunks to allow for `N`, the number of elements in a chunk, to be inferred.
///
/// ### Why is this bad?
/// When `.chunks_exact_mut()` is used, a bounds check is required before an element can be accessed.
///
/// ### Example
/// ```no_run
/// let mut numbers = Vec::from_iter(0..10);
/// for chunk in numbers.chunks_exact_mut(2) {
/// chunk[1] += chunk[0];
/// println!("{n} is an odd number", n = chunk[1]);
/// }
/// ```
/// Use instead:
/// ```no_run
/// #![feature(array_chunks)]
/// let mut numbers = Vec::from_iter(0..10);
/// for [m, n] in numbers.array_chunks_mut() {
/// *n += *m;
/// println!("{n} is an odd number");
/// }
/// ```
#[clippy::version = "1.89.0"]
pub CONST_SIZED_CHUNKS_EXACT_MUT,
nursery,
"calling `.chunks_exact_mut()` with a constant `chunk_size` where `.array_chunks_mut()` could be used instead"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `.windows()` on slices where the `size` is a constant and suggests replacing it with
/// its const generic equivalent, `.array_windows()`, in a way that the value of the const parameter `N` can
/// be inferred.
///
/// Specifically, in a for-loop, consuming the windowed iterator over the overlapping chunks, the lint
/// suggests destructuring the chunks to allow for `N`, the number of elements in a chunk, to be inferred.
///
/// ### Why is this bad?
/// When `.windows()` is used, a bounds check is required before an element can be accessed.
///
/// ### Example
/// ```no_run
/// let numbers = Vec::from_iter(0..10);
/// for chunk in numbers.windows(2) {
/// println!("{n} is an odd number", n = chunk[0] + chunk[1]);
/// }
/// ```
/// Use instead:
/// ```no_run
/// #![feature(array_windows)]
/// let numbers = Vec::from_iter(0..10);
/// for [m, n] in numbers.array_windows() {
/// println!("{n} is an odd number", n = m + n);
/// }
/// ```
#[clippy::version = "1.89.0"]
pub CONST_SIZED_WINDOWS,
nursery,
"calling `.windows()` with a constant `size` where `.array_windows()` could be used instead"
}

pub struct Loops {
msrv: Msrv,
enforce_iter_loop_reborrow: bool,
Expand Down Expand Up @@ -822,6 +924,9 @@ impl_lint_pass!(Loops => [
INFINITE_LOOP,
MANUAL_SLICE_FILL,
CHAR_INDICES_AS_BYTE_INDICES,
CONST_SIZED_CHUNKS_EXACT,
CONST_SIZED_CHUNKS_EXACT_MUT,
CONST_SIZED_WINDOWS,
]);

impl<'tcx> LateLintPass<'tcx> for Loops {
Expand Down Expand Up @@ -906,6 +1011,7 @@ impl Loops {
manual_find::check(cx, pat, arg, body, span, expr);
unused_enumerate_index::check(cx, pat, arg, body);
char_indices_as_byte_indices::check(cx, pat, arg, body);
const_sized_chunks::check(cx, pat, arg);
}

fn check_for_loop_arg(&self, cx: &LateContext<'_>, _: &Pat<'_>, arg: &Expr<'_>) {
Expand Down
5 changes: 5 additions & 0 deletions clippy_utils/src/sym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ generate! {
ambiguous_glob_reexports,
append,
arg,
array_chunks,
array_chunks_mut,
array_windows,
as_bytes,
as_deref,
as_deref_mut,
Expand Down Expand Up @@ -108,6 +111,8 @@ generate! {
checked_pow,
checked_rem_euclid,
checked_sub,
chunks_exact,
chunks_exact_mut,
clamp,
clippy_utils,
clone_into,
Expand Down
89 changes: 89 additions & 0 deletions tests/ui/const_sized_chunks_exact.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
#![warn(clippy::const_sized_chunks_exact)]
#![feature(array_chunks)]
#![allow(unused_variables)]
#![allow(dead_code)]

fn test_binding() {
let numbers = [0, 1, 2, 3, 4];

#[allow(clippy::needless_borrow)]
for [a, b] in numbers.array_chunks() {
//~^ const_sized_chunks_exact
}

#[allow(unused_mut)]
for [a, b] in numbers.array_chunks() {
//~^ const_sized_chunks_exact
}

for [a, b] in numbers.array_chunks() {
//~^ const_sized_chunks_exact
}
}

fn test_slice_like() {
let numbers = [2; 5];
for [a, b] in numbers.array_chunks() {
//~^ const_sized_chunks_exact
}

let numbers = [0, 1, 2, 3, 4];
for [a, b] in numbers.array_chunks() {
//~^ const_sized_chunks_exact
}

let numbers = &[0, 1, 2, 3, 4];
for [a, b] in numbers.array_chunks() {
//~^ const_sized_chunks_exact
}

let numbers = &&[0, 1, 2, 3, 4];
for [a, b] in numbers.array_chunks() {
//~^ const_sized_chunks_exact
}

let numbers = &&&[0, 1, 2, 3, 4];
for [a, b] in numbers.array_chunks() {
//~^ const_sized_chunks_exact
}

let numbers = Vec::from_iter(0..5);
for [a, b] in numbers.array_chunks() {
//~^ const_sized_chunks_exact
}

let numbers = &Vec::from_iter(0..5);
for [a, b] in numbers.array_chunks() {
//~^ const_sized_chunks_exact
}
}

fn test_const_eval() {
const N: usize = 2;

let numbers = [2; 5];
for [a, b] in numbers.array_chunks() {
//~^ const_sized_chunks_exact
}

for [a, b, c] in numbers.array_chunks() {
//~^ const_sized_chunks_exact
}
}

fn test_chunk_size() {
let numbers = Vec::from_iter(0..5);
for chunk in numbers.chunks_exact(0) {}

for [a] in numbers.array_chunks() {
//~^ const_sized_chunks_exact
}

for [a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w, x, y, z] in numbers.array_chunks() {
//~^ const_sized_chunks_exact
}

for chunk in numbers.chunks_exact(27) {}
}

fn main() {}
Loading