-
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 all 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,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) { | ||
|
Comment on lines
+27
to
+28
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You want |
||
| // 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); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be |
||
|
|
||
| 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) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You actually need to be more conservative than this. The only time where the other suggestion is correct is when any iterator would work.
|
||
| // 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::<CHUNK_SIZE>(); | ||
| // 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()"), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally this would only suggest |
||
| applicability, | ||
| ); | ||
| } | ||
| }, | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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::<Vec<_>>() | ||
| .chunks_exact(2); | ||
| //~^ chunks_exact_with_const_size | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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::<CHUNK_SIZE>()` 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 | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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(); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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::<CHUNK_SIZE>()` 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::<CHUNK_SIZE>()` 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 | ||
|
|
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.
This doesn't actually constrain the method correctly. You want to for a single reference to a slice.