diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cb2755be0ee..c8cd55feb1e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6252,6 +6252,7 @@ Released 2018-09-13 [`chars_last_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#chars_last_cmp [`chars_next_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#chars_next_cmp [`checked_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#checked_conversions +[`chunks_exact_with_const_size`]: https://rust-lang.github.io/rust-clippy/master/index.html#chunks_exact_with_const_size [`clear_with_drain`]: https://rust-lang.github.io/rust-clippy/master/index.html#clear_with_drain [`clone_double_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_double_ref [`clone_on_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index a754eea31165..733ed29f2760 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -352,6 +352,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::methods::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS_INFO, crate::methods::CHARS_LAST_CMP_INFO, crate::methods::CHARS_NEXT_CMP_INFO, + crate::methods::CHUNKS_EXACT_WITH_CONST_SIZE_INFO, crate::methods::CLEAR_WITH_DRAIN_INFO, crate::methods::CLONED_INSTEAD_OF_COPIED_INFO, crate::methods::CLONE_ON_COPY_INFO, diff --git a/clippy_lints/src/methods/chunks_exact_with_const_size.rs b/clippy_lints/src/methods/chunks_exact_with_const_size.rs new file mode 100644 index 000000000000..fdd3bef7b08a --- /dev/null +++ b/clippy_lints/src/methods/chunks_exact_with_const_size.rs @@ -0,0 +1,91 @@ +use clippy_utils::consts::{ConstEvalCtxt, Constant}; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::msrvs::{self, Msrv}; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::sym; +use rustc_errors::Applicability; +use rustc_hir::{Expr, Node, PatKind}; +use rustc_lint::LateContext; +use rustc_span::{Span, Symbol}; + +use super::CHUNKS_EXACT_WITH_CONST_SIZE; + +pub(super) fn check( + cx: &LateContext<'_>, + recv: &Expr<'_>, + arg: &Expr<'_>, + expr: &Expr<'_>, + call_span: Span, + method_name: Symbol, + msrv: Msrv, +) { + // Check if receiver is slice-like + if !cx.typeck_results().expr_ty_adjusted(recv).peel_refs().is_slice() { + return; + } + + let constant_eval = ConstEvalCtxt::new(cx); + if let Some(Constant::Int(_)) = constant_eval.eval(arg) { + // Check for Rust version + if !msrv.meets(cx, msrvs::AS_CHUNKS) { + return; + } + + let suggestion_method = if method_name == sym::chunks_exact_mut { + "as_chunks_mut" + } else { + "as_chunks" + }; + + let mut applicability = Applicability::MachineApplicable; + let arg_str = snippet_with_applicability(cx, arg.span, "_", &mut applicability); + + let as_chunks = format_args!("{suggestion_method}::<{arg_str}>()"); + + span_lint_and_then( + cx, + CHUNKS_EXACT_WITH_CONST_SIZE, + call_span, + format!("using `{method_name}` with a constant chunk size"), + |diag| { + if let Node::LetStmt(let_stmt) = cx.tcx.parent_hir_node(expr.hir_id) { + // The `ChunksExact(Mut)` struct is stored for later -- this likely means that the user intends to + // not only use it as an iterator, but also access the remainder using + // `(into_)remainder`. For now, just give a help message in this case. + // TODO: give a suggestion that replaces this: + // ``` + // let chunk_iter = bytes.chunks_exact(CHUNK_SIZE); + // let remainder_chunk = chunk_iter.remainder(); + // for chunk in chunk_iter { + // /* ... */ + // } + // ``` + // with this: + // ``` + // let chunk_iter = bytes.as_chunks::(); + // let remainder_chunk = chunk_iter.1; + // for chunk in chunk_iter.0.iter() { + // /* ... */ + // } + // ``` + + diag.span_help(call_span, format!("consider using `{as_chunks}` instead")); + + // Try to extract the variable name to provide a more helpful note + if let PatKind::Binding(_, _, ident, _) = let_stmt.pat.kind { + diag.note(format!( + "you can access the chunks using `{ident}.0.iter()`, and the remainder using `{ident}.1`" + )); + } + } else { + diag.span_suggestion( + call_span, + "consider using `as_chunks` instead", + format!("{as_chunks}.0.iter()"), + applicability, + ); + } + }, + ); + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 5b917e2bfefa..5da1b53acf47 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -9,6 +9,7 @@ mod chars_last_cmp; mod chars_last_cmp_with_unwrap; mod chars_next_cmp; mod chars_next_cmp_with_unwrap; +mod chunks_exact_with_const_size; mod clear_with_drain; mod clone_on_copy; mod clone_on_ref_ptr; @@ -2087,6 +2088,32 @@ declare_clippy_lint! { "replace `.bytes().nth()` with `.as_bytes().get()`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for usage of `chunks_exact` or `chunks_exact_mut` with a constant chunk size. + /// + /// ### Why is this bad? + /// `as_chunks` provides better ergonomics and type safety by returning arrays instead of slices. + /// It was stabilized in Rust 1.88. + /// + /// ### Example + /// ```no_run + /// let slice = [1, 2, 3, 4, 5, 6]; + /// let mut it = slice.chunks_exact(2); + /// for chunk in it {} + /// ``` + /// Use instead: + /// ```no_run + /// let slice = [1, 2, 3, 4, 5, 6]; + /// let (chunks, remainder) = slice.as_chunks::<2>(); + /// for chunk in chunks {} + /// ``` + #[clippy::version = "1.93.0"] + pub CHUNKS_EXACT_WITH_CONST_SIZE, + style, + "using `chunks_exact` with constant when `as_chunks` is more ergonomic" +} + declare_clippy_lint! { /// ### What it does /// Checks for the usage of `_.to_owned()`, `vec.to_vec()`, or similar when calling `_.clone()` would be clearer. @@ -4787,6 +4814,7 @@ impl_lint_pass!(Methods => [ ITER_NTH, ITER_NTH_ZERO, BYTES_NTH, + CHUNKS_EXACT_WITH_CONST_SIZE, ITER_SKIP_NEXT, GET_UNWRAP, GET_LAST_WITH_LEN, @@ -5043,6 +5071,9 @@ impl Methods { _ => {}, } }, + (name @ (sym::chunks_exact | sym::chunks_exact_mut), [arg]) => { + chunks_exact_with_const_size::check(cx, recv, arg, expr, call_span, name, self.msrv); + }, (sym::and_then, [arg]) => { let biom_option_linted = bind_instead_of_map::check_and_then_some(cx, expr, recv, arg); let biom_result_linted = bind_instead_of_map::check_and_then_ok(cx, expr, recv, arg); diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 86d17a8231d5..d82ee2487ee9 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -23,7 +23,7 @@ macro_rules! msrv_aliases { // names may refer to stabilized feature flags or library items msrv_aliases! { - 1,88,0 { LET_CHAINS } + 1,88,0 { LET_CHAINS, AS_CHUNKS } 1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT, CONST_CHAR_IS_DIGIT, UNSIGNED_IS_MULTIPLE_OF, INTEGER_SIGN_CAST } 1,85,0 { UINT_FLOAT_MIDPOINT, CONST_SIZE_OF_VAL } 1,84,0 { CONST_OPTION_AS_SLICE, MANUAL_DANGLING_PTR } diff --git a/clippy_utils/src/sym.rs b/clippy_utils/src/sym.rs index 8e8a80a6a9c9..969bb2e47eec 100644 --- a/clippy_utils/src/sym.rs +++ b/clippy_utils/src/sym.rs @@ -111,6 +111,8 @@ generate! { checked_pow, checked_rem_euclid, checked_sub, + chunks_exact, + chunks_exact_mut, clamp, clippy_utils, clone_into, diff --git a/tests/ui/chunks_exact_with_const_size.rs b/tests/ui/chunks_exact_with_const_size.rs new file mode 100644 index 000000000000..9cd5d3c18698 --- /dev/null +++ b/tests/ui/chunks_exact_with_const_size.rs @@ -0,0 +1,39 @@ +#![warn(clippy::chunks_exact_with_const_size)] +#![allow(unused)] +#![allow(clippy::iter_cloned_collect)] + +fn main() { + let slice = [1, 2, 3, 4, 5, 6, 7, 8]; + + // Should trigger lint - literal constant + let result = slice.chunks_exact(4); + //~^ chunks_exact_with_const_size + + // Should trigger lint - const value + const CHUNK_SIZE: usize = 4; + let result = slice.chunks_exact(CHUNK_SIZE); + //~^ chunks_exact_with_const_size + + // Should NOT trigger - runtime value + let size = 4; + let mut it = slice.chunks_exact(size); + for chunk in it {} + + // Should trigger lint - simple iteration + let result = slice.chunks_exact(3); + //~^ chunks_exact_with_const_size + + // Should trigger - mutable variant + let mut arr = [1, 2, 3, 4, 5, 6, 7, 8]; + let result = arr.chunks_exact_mut(4); + //~^ chunks_exact_with_const_size + + // Should trigger - multiline expression + #[rustfmt::skip] + let result = slice + .iter() + .copied() + .collect::>() + .chunks_exact(2); + //~^ chunks_exact_with_const_size +} diff --git a/tests/ui/chunks_exact_with_const_size.stderr b/tests/ui/chunks_exact_with_const_size.stderr new file mode 100644 index 000000000000..7139fffad746 --- /dev/null +++ b/tests/ui/chunks_exact_with_const_size.stderr @@ -0,0 +1,69 @@ +error: using `chunks_exact` with a constant chunk size + --> tests/ui/chunks_exact_with_const_size.rs:9:24 + | +LL | let result = slice.chunks_exact(4); + | ^^^^^^^^^^^^^^^ + | +help: consider using `as_chunks::<4>()` instead + --> tests/ui/chunks_exact_with_const_size.rs:9:24 + | +LL | let result = slice.chunks_exact(4); + | ^^^^^^^^^^^^^^^ + = note: you can access the chunks using `result.0.iter()`, and the remainder using `result.1` + = note: `-D clippy::chunks-exact-with-const-size` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::chunks_exact_with_const_size)]` + +error: using `chunks_exact` with a constant chunk size + --> tests/ui/chunks_exact_with_const_size.rs:14:24 + | +LL | let result = slice.chunks_exact(CHUNK_SIZE); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `as_chunks::()` instead + --> tests/ui/chunks_exact_with_const_size.rs:14:24 + | +LL | let result = slice.chunks_exact(CHUNK_SIZE); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + = note: you can access the chunks using `result.0.iter()`, and the remainder using `result.1` + +error: using `chunks_exact` with a constant chunk size + --> tests/ui/chunks_exact_with_const_size.rs:23:24 + | +LL | let result = slice.chunks_exact(3); + | ^^^^^^^^^^^^^^^ + | +help: consider using `as_chunks::<3>()` instead + --> tests/ui/chunks_exact_with_const_size.rs:23:24 + | +LL | let result = slice.chunks_exact(3); + | ^^^^^^^^^^^^^^^ + = note: you can access the chunks using `result.0.iter()`, and the remainder using `result.1` + +error: using `chunks_exact_mut` with a constant chunk size + --> tests/ui/chunks_exact_with_const_size.rs:28:22 + | +LL | let result = arr.chunks_exact_mut(4); + | ^^^^^^^^^^^^^^^^^^^ + | +help: consider using `as_chunks_mut::<4>()` instead + --> tests/ui/chunks_exact_with_const_size.rs:28:22 + | +LL | let result = arr.chunks_exact_mut(4); + | ^^^^^^^^^^^^^^^^^^^ + = note: you can access the chunks using `result.0.iter()`, and the remainder using `result.1` + +error: using `chunks_exact` with a constant chunk size + --> tests/ui/chunks_exact_with_const_size.rs:37:10 + | +LL | .chunks_exact(2); + | ^^^^^^^^^^^^^^^ + | +help: consider using `as_chunks::<2>()` instead + --> tests/ui/chunks_exact_with_const_size.rs:37:10 + | +LL | .chunks_exact(2); + | ^^^^^^^^^^^^^^^ + = note: you can access the chunks using `result.0.iter()`, and the remainder using `result.1` + +error: aborting due to 5 previous errors + diff --git a/tests/ui/chunks_exact_with_const_size_unfixable.rs b/tests/ui/chunks_exact_with_const_size_unfixable.rs new file mode 100644 index 000000000000..0e6438215cd1 --- /dev/null +++ b/tests/ui/chunks_exact_with_const_size_unfixable.rs @@ -0,0 +1,20 @@ +#![warn(clippy::chunks_exact_with_const_size)] +#![allow(unused)] + +fn main() { + let slice = [1, 2, 3, 4, 5, 6, 7, 8]; + const CHUNK_SIZE: usize = 4; + + // Should trigger lint with help message only (not suggestion) - stored in variable + let mut chunk_iter = slice.chunks_exact(CHUNK_SIZE); + //~^ chunks_exact_with_const_size + for chunk in chunk_iter.by_ref() {} + let _remainder = chunk_iter.remainder(); + + // Similar for mutable version - help message only + let mut arr2 = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; + let mut chunk_iter = arr2.chunks_exact_mut(CHUNK_SIZE); + //~^ chunks_exact_with_const_size + for chunk in chunk_iter.by_ref() {} + let _remainder = chunk_iter.into_remainder(); +} diff --git a/tests/ui/chunks_exact_with_const_size_unfixable.stderr b/tests/ui/chunks_exact_with_const_size_unfixable.stderr new file mode 100644 index 000000000000..9146bca894a2 --- /dev/null +++ b/tests/ui/chunks_exact_with_const_size_unfixable.stderr @@ -0,0 +1,30 @@ +error: using `chunks_exact` with a constant chunk size + --> tests/ui/chunks_exact_with_const_size_unfixable.rs:9:32 + | +LL | let mut chunk_iter = slice.chunks_exact(CHUNK_SIZE); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `as_chunks::()` instead + --> tests/ui/chunks_exact_with_const_size_unfixable.rs:9:32 + | +LL | let mut chunk_iter = slice.chunks_exact(CHUNK_SIZE); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + = note: you can access the chunks using `chunk_iter.0.iter()`, and the remainder using `chunk_iter.1` + = note: `-D clippy::chunks-exact-with-const-size` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::chunks_exact_with_const_size)]` + +error: using `chunks_exact_mut` with a constant chunk size + --> tests/ui/chunks_exact_with_const_size_unfixable.rs:16:31 + | +LL | let mut chunk_iter = arr2.chunks_exact_mut(CHUNK_SIZE); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `as_chunks_mut::()` instead + --> tests/ui/chunks_exact_with_const_size_unfixable.rs:16:31 + | +LL | let mut chunk_iter = arr2.chunks_exact_mut(CHUNK_SIZE); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: you can access the chunks using `chunk_iter.0.iter()`, and the remainder using `chunk_iter.1` + +error: aborting due to 2 previous errors + diff --git a/tests/ui/explicit_write_in_test.stderr b/tests/ui/explicit_write_in_test.stderr deleted file mode 100644 index e69de29bb2d1..000000000000