-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[tuple_array_conversions
]: move from complexity
to nursery
#11146
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @dswij (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
@@ -31,7 +31,7 @@ declare_clippy_lint! { | |||
/// ``` | |||
#[clippy::version = "1.72.0"] | |||
pub TUPLE_ARRAY_CONVERSIONS, | |||
complexity, | |||
pedantic, |
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 should be nursery
instead.
I'll try to fix most of these issues before the next sync (in 2 weeks).
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.
Ok, that makes sense too!
Due to outstanding issues: * rust-lang#11082 * rust-lang#11085 * rust-lang#11100 (rust-lang#11105) * rust-lang#11124 * rust-lang#11144
tuple_array_conversions
]: move from complexity
to pedantic
tuple_array_conversions
]: move from complexity
to nursery
35e7588
to
4102a30
Compare
Bit sad about this but it makes sense. Once it's been rewritten hopefully we can bump it up to |
Will say though that IMO it's the third, it conveys the intent best (if it's not asymmetric) and even avoids an unnecessary closure which is pretty nice |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
1 similar comment
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
…Frednet Move tuple_array_conversions to nursery changelog: Move [`tuple_array_conversions`] to `nursery` (Now allow-by-default) <!-- FIY: Ignore this change, if the commit gets backported and also #11146 --> [#11172](#11172) The nursery change got lost in #11146 and it ended up in pedantic, this puts it in nursery and gives something to backport r? `@xFrednet`
…sery, r=xFrednet Move tuple_array_conversions to nursery changelog: Move [`tuple_array_conversions`] to `nursery` (Now allow-by-default) <!-- FIY: Ignore this change, if the commit gets backported and also rust-lang/rust-clippy#11146 --> [rust-lang#11172](rust-lang/rust-clippy#11172) The nursery change got lost in rust-lang#11146 and it ended up in pedantic, this puts it in nursery and gives something to backport r? `@xFrednet`
The lint suggestion is arguably often less readable and more complex than the original code.
For example, which of the following is the most readable:
The lint can be useful, but really only applies if the tuple is either long enough that naming the fields is silly (maybe at least 4 entries long), or if the author intends the fields to be homogenous, which is author intent and can't be determined by the lint. Therefore I think the lint should be marked as pedantic.
Currently, there are also a lot of false positives with the lint:
tuple_array_conversions
lint makes code less readable when slice is involved #11082tuple_array_conversions
suggested code erases binding names and hides asymmetry #11085tuple_array_conversions
]: Check if all types are same #11105)tuple_array_conversions
triggers even when binding is used for more than tuple-array conversion #11124tuple_array_conversion
false positive on destructured tuple values #11144Should fix those issues before enabling it for everyone.
changelog: Move [
tuple_array_conversions
] tonursery
(Now allow-by-default)#11146