rust: allow `clippy::collapsible_match` globally
Subsystems (4)
Miguel Ojeda
maintainerPATCH 1/2 rust: allow `clippy::collapsible_match` globally
The clippy::collapsible_match lint [1] can make code harder to read in certain cases [2], e.g.
CLIPPY P rust/libmacros.so - due to command line change
warning: this `if` can be collapsed into the outer `match`
--> rust/pin-init/internal/src/helpers.rs:91:17
|
91 | / if nesting == 1 {
92 | | impl_generics.push(tt.clone());
93 | | impl_generics.push(tt);
94 | | skip_until_comma = false;
95 | | }
| |_________________^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_match
= note: `-W clippy::collapsible-match` implied by `-W clippy::all`
= help: to override `-W clippy::all` add `#[allow(clippy::collapsible_match)]`
help: collapse nested if block
|
90 ~ TokenTree::Punct(p) if skip_until_comma && p.as_char() == ','
91 ~ && nesting == 1 => {
92 | impl_generics.push(tt.clone());
93 | impl_generics.push(tt);
94 | skip_until_comma = false;
95 ~ }
|The lint does not have much upside -- when the suggestion may be a good one, it would still read fine when nested anyway. And it is the kind of lint that may easily bias people to just apply the suggestion instead of allowing it.
Thus just let developers decide on their own.
--- Makefile | 1 + 1 file changed, 1 insertion(+)
Makefile +1 -0
@@ -486,6 +486,7 @@ export rust_common_flags := --edition=2021 \ -Wclippy::as_ptr_cast_mut \ -Wclippy::as_underscore \ -Wclippy::cast_lossless \ + -Aclippy::collapsible_match \ -Wclippy::ignored_unit_patterns \ -Aclippy::incompatible_msrv \ -Wclippy::mut_mut \
base-commit: 897d54018cc9aa97fd1529ca08a53b429d05a566
Signature · 1 line
2.53.0
Miguel Ojeda
maintainerPATCH 2/2 rust: allow `clippy::collapsible_if` globally
Similar to clippy::collapsible_match (globally allowed in the previous commit), the clippy::collapsible_if lint [1] can make code harder to read in certain cases.
Thus just let developers decide on their own.
In addition, remove the existing expect we had.
--- Makefile | 1 + drivers/android/binder/range_alloc/array.rs | 1 - 2 files changed, 1 insertion(+), 1 deletion(-)
Makefile +1 -0
@@ -486,6 +486,7 @@ export rust_common_flags := --edition=2021 \ -Wclippy::as_ptr_cast_mut \ -Wclippy::as_underscore \ -Wclippy::cast_lossless \ + -Aclippy::collapsible_if \ -Aclippy::collapsible_match \ -Wclippy::ignored_unit_patterns \ -Aclippy::incompatible_msrv \
drivers/android/binder/range_alloc/array.rs +0 -1
@@ -204,7 +204,6 @@ pub(crate) fn reservation_abort(&mut self, offset: usize) -> Result<FreedRange>// caller will mark them as unused, which means that they can be freed if the system comes // under memory pressure. let mut freed_range =FreedRange ::interior_pages (offset, size); -#[expect(clippy::collapsible_if)] // reads better like this if offset %PAGE_SIZE !=0 {if i ==0 || self.ranges[i -1 ].endpoint () <= (offset &PAGE_MASK ) { freed_range.start_page_idx -=1 ;
Signature · 1 line
2.53.0
Re: [PATCH 2/2] rust: allow `clippy::collapsible_if` globally
Quoted 6 lines · Miguel Ojeda
Similar to
clippy::collapsible_match(globally allowed in the previous commit), theclippy::collapsible_iflint [1] can make code harder to read in certain cases.Thus just let developers decide on their own.
Quoted 7 lines
In addition, remove the existing
expectwe had.Cc [email protected] # Needed in 6.12.y and later (Rust is pinned in older LTSs).Suggested-by Gary Guo <[email protected]>Signed-off-by Miguel Ojeda <[email protected]>
Quoted 4 lines
--- Makefile | 1 + drivers/android/binder/range_alloc/array.rs | 1 - 2 files changed, 1 insertion(+), 1 deletion(-)
Quoted 27 lines · Miguel Ojeda
The
clippy::collapsible_matchlint [1] can make code harder to read in certain cases [2], e.g.CLIPPY P rust/libmacros.so - due to command line change warning: this `if` can be collapsed into the outer `match` --> rust/pin-init/internal/src/helpers.rs:91:17 | 91 | / if nesting == 1 { 92 | | impl_generics.push(tt.clone()); 93 | | impl_generics.push(tt); 94 | | skip_until_comma = false; 95 | | } | |_________________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_match = note: `-W clippy::collapsible-match` implied by `-W clippy::all` = help: to override `-W clippy::all` add `#[allow(clippy::collapsible_match)]` help: collapse nested if block | 90 ~ TokenTree::Punct(p) if skip_until_comma && p.as_char() == ',' 91 ~ && nesting == 1 => { 92 | impl_generics.push(tt.clone()); 93 | impl_generics.push(tt); 94 | skip_until_comma = false; 95 ~ } |
Quoted 4 lines
The lint does not have much upside -- when the suggestion may be a good one, it would still read fine when nested anyway. And it is the kind of lint that may easily bias people to just apply the suggestion instead of allowing it.
It's also just wrong.
pattern => {
if x {
...
}
}is not equal to
pattern if x => { ... }because the former prevents other arms later (e.g. a _) from being to act while the latter doesn't.
See https://github.com/rust-lang/rust-clippy/pull/16878
Quoted 7 lines
Thus just let developers decide on their own.
Cc [email protected] # Needed in 6.12.y and later (Rust is pinned in older LTSs).Link https://lore.kernel.org/rust-for-linux/CANiq72nWYJna_hdFxjQCQZK6yJBrr1Mb86iKavivV0U0BgufeA@mail.gmail.com/ [2] open hereSigned-off-by Miguel Ojeda <[email protected]>
Best,
Gary
Quoted 18 lines
--- Makefile | 1 + 1 file changed, 1 insertion(+)Makefile +1
-0@@ -486,6 +486,7 @@ export rust_common_flags := --edition=2021 \ -Wclippy::as_ptr_cast_mut \ -Wclippy::as_underscore \ -Wclippy::cast_lossless \ + -Aclippy::collapsible_match \ -Wclippy::ignored_unit_patterns \ -Aclippy::incompatible_msrv \ -Wclippy::mut_mut \base-commit:
897d54018cc9aa97fd1529ca08a53b429d05a566
Quoted 33 lines · Miguel Ojeda
The
clippy::collapsible_matchlint [1] can make code harder to read in certain cases [2], e.g.CLIPPY P rust/libmacros.so - due to command line change warning: this `if` can be collapsed into the outer `match` --> rust/pin-init/internal/src/helpers.rs:91:17 | 91 | / if nesting == 1 { 92 | | impl_generics.push(tt.clone()); 93 | | impl_generics.push(tt); 94 | | skip_until_comma = false; 95 | | } | |_________________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_match = note: `-W clippy::collapsible-match` implied by `-W clippy::all` = help: to override `-W clippy::all` add `#[allow(clippy::collapsible_match)]` help: collapse nested if block | 90 ~ TokenTree::Punct(p) if skip_until_comma && p.as_char() == ',' 91 ~ && nesting == 1 => { 92 | impl_generics.push(tt.clone()); 93 | impl_generics.push(tt); 94 | skip_until_comma = false; 95 ~ } |The lint does not have much upside -- when the suggestion may be a good one, it would still read fine when nested anyway. And it is the kind of lint that may easily bias people to just apply the suggestion instead of allowing it.
Quoted 6 lines
Thus just let developers decide on their own.
Cc [email protected] # Needed in 6.12.y and later (Rust is pinned in older LTSs).Link https://lore.kernel.org/rust-for-linux/CANiq72nWYJna_hdFxjQCQZK6yJBrr1Mb86iKavivV0U0BgufeA@mail.gmail.com/ [2] open hereSigned-off-by Miguel Ojeda <[email protected]>
Applied (both patches) to rust-fixes -- thanks!
[ In addition, as Gary points out [3], the suggestion is also wrong [4] and
in the process of being fixed [5], possibly for Rust 1.97.0:
Link: https://github.com/rust-lang/rust-clippy/issues/16875 [4] Link: https://github.com/rust-lang/rust-clippy/pull/16878 [5] - Miguel ]
Ideally, we would have an Acked-by on the second one from Binder maintainers, but it is trivial.
Cheers,
Miguel