-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add lint to suggest as_chunks over chunks_exact with constant #16002
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
base: master
Are you sure you want to change the base?
Changes from 10 commits
3d3d2be
4a4c955
d332ec5
610acda
632e37b
bc1d010
1d99b91
2731771
7082eef
dc9fa9d
afaf92b
dfd8065
3cfa99a
94181ab
370d0ae
24798af
5e05b1d
ae2cb12
43a7a72
64aae82
83e0039
1e276d7
c50ccf8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| 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; | ||
| 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_span: Span, | ||
| 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; | ||
| } | ||
|
|
||
| // Check if argument is a constant | ||
| let constant_eval = ConstEvalCtxt::new(cx); | ||
| if let Some(Constant::Int(_)) = constant_eval.eval(arg) { | ||
| // Determine the suggested method name | ||
| let suggestion_method = if method_name == sym::chunks_exact_mut { | ||
| "as_chunks_mut" | ||
| } else { | ||
| "as_chunks" | ||
| }; | ||
|
|
||
| // Build the suggestion with proper applicability tracking | ||
| let mut applicability = Applicability::MachineApplicable; | ||
| let recv_str = snippet_with_applicability(cx, recv.span, "_", &mut applicability); | ||
| let arg_str = snippet_with_applicability(cx, arg.span, "_", &mut applicability); | ||
|
|
||
| let suggestion = format!("{recv_str}.{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| { | ||
| diag.span_suggestion( | ||
| expr_span, | ||
| "consider using `as_chunks` instead", | ||
| suggestion, | ||
| applicability, | ||
| ); | ||
| }, | ||
| ); | ||
| } | ||
|
|
||
| // Check for Rust version | ||
| if !msrv.meets(cx, msrvs::AS_CHUNKS) {} | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
|
|
@@ -5649,7 +5677,7 @@ impl Methods { | |
| } | ||
| } | ||
| // Handle method calls whose receiver and arguments may come from expansion | ||
| if let ExprKind::MethodCall(path, recv, args, _call_span) = expr.kind { | ||
| if let ExprKind::MethodCall(path, recv, args, call_span) = expr.kind { | ||
| let method_span = path.ident.span; | ||
|
|
||
| // Those methods do their own method name checking as they deal with multiple methods. | ||
|
|
@@ -5715,6 +5743,9 @@ impl Methods { | |
| unwrap_expect_used::Variant::Unwrap, | ||
| ); | ||
| }, | ||
| (name @ (sym::chunks_exact | sym::chunks_exact_mut), [arg]) => { | ||
| chunks_exact_with_const_size::check(cx, recv, arg, expr.span, call_span, name, self.msrv); | ||
| }, | ||
|
||
| _ => {}, | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| #![warn(clippy::chunks_exact_with_const_size)] | ||
| #![allow(unused)] | ||
|
|
||
| fn main() { | ||
| let slice = [1, 2, 3, 4, 5, 6, 7, 8]; | ||
|
|
||
| // Should trigger lint - literal constant | ||
| let result = slice.as_chunks::<4>(); | ||
| //~^ chunks_exact_with_const_size | ||
|
|
||
| // Should trigger lint - const value | ||
| const CHUNK_SIZE: usize = 4; | ||
| let result = slice.as_chunks::<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.as_chunks::<3>(); | ||
| //~^ chunks_exact_with_const_size | ||
|
|
||
| // Should trigger - mutable variant | ||
| let mut arr = [1, 2, 3, 4, 5, 6, 7, 8]; | ||
| let result = arr.as_chunks_mut::<4>(); | ||
| //~^ chunks_exact_with_const_size | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| #![warn(clippy::chunks_exact_with_const_size)] | ||
| #![allow(unused)] | ||
|
|
||
| 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 | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| error: using `chunks_exact` with a constant chunk size | ||
| --> tests/ui/chunks_exact_with_const_size.rs:8:24 | ||
| | | ||
| LL | let result = slice.chunks_exact(4); | ||
| | ------^^^^^^^^^^^^^^^ | ||
| | | | ||
| | help: consider using `as_chunks` instead: `slice.as_chunks::<4>()` | ||
| | | ||
| = 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:13:24 | ||
| | | ||
| LL | let result = slice.chunks_exact(CHUNK_SIZE); | ||
| | ------^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | | | ||
| | help: consider using `as_chunks` instead: `slice.as_chunks::<CHUNK_SIZE>()` | ||
|
|
||
| error: using `chunks_exact` with a constant chunk size | ||
| --> tests/ui/chunks_exact_with_const_size.rs:22:24 | ||
| | | ||
| LL | let result = slice.chunks_exact(3); | ||
| | ------^^^^^^^^^^^^^^^ | ||
| | | | ||
| | help: consider using `as_chunks` instead: `slice.as_chunks::<3>()` | ||
|
|
||
| error: using `chunks_exact_mut` with a constant chunk size | ||
| --> tests/ui/chunks_exact_with_const_size.rs:27:22 | ||
| | | ||
| LL | let result = arr.chunks_exact_mut(4); | ||
| | ----^^^^^^^^^^^^^^^^^^^ | ||
| | | | ||
| | help: consider using `as_chunks` instead: `arr.as_chunks_mut::<4>()` | ||
|
|
||
| error: aborting due to 4 previous errors | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The suggestion doesn't need to point at the whole expression either -- you can suggest replacing just the span of
chunks_exact(N)(socall_span) withas_chunks::<4>().0.iter()(notice the lack of the starting., ascall_spandoesn't include it). This has an additional advantage of not requiring you to get a snippet for the receiver, as you won't need that in the suggestion anymoreThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 370d0ae