useless_let_if_seq should not warn when variable is set from multiple places. #3769
Labels
C-enhancement
Category: Enhancement of lints, like adding more cases or adding help messages
L-style
Lint: Belongs in the style lint group
Comments
|
These are different. The linked bug is about setting multiple variables where this bug is about setting one variable from multiple locations. |
|
Oh yeah, you're right! Reopening it, sorry. |
flip1995
added
C-enhancement
ghost
mentioned this issue
bors
added a commit
that referenced
this issue
May 15, 2020
Downgrade useless_let_if_seq to nursery
I feel that this lint has the wrong balance of incorrect suggestions for a default-enabled lint.
The immediate code I faced was something like:
```rust
fn main() {
let mut good = do1();
if !do2() {
good = false;
}
if good {
println!("good");
}
}
fn do1() -> bool { println!("1"); false }
fn do2() -> bool { println!("2"); false }
```
On this code Clippy calls it unidiomatic and suggests the following diff, which has different behavior in a way that I don't necessarily want.
```diff
- let mut good = do1();
- if !do2() {
- good = false;
- }
+ let good = if !do2() {
+ false
+ } else {
+ do1()
+ };
```
On exploring issues filed about this lint, I have found that other users have also struggled with inappropriate suggestions (#4124, #3043, #2918, #2176) and suggestions that make the code worse (#3769, #2749). Overall I believe that this lint is still at nursery quality for now and should not be enabled.
---
changelog: Remove useless_let_if_seq from default set of enabled lints
bors
added a commit
that referenced
this issue
May 15, 2020
Downgrade useless_let_if_seq to nursery
I feel that this lint has the wrong balance of incorrect suggestions for a default-enabled lint.
The immediate code I faced was something like:
```rust
fn main() {
let mut good = do1();
if !do2() {
good = false;
}
if good {
println!("good");
}
}
fn do1() -> bool { println!("1"); false }
fn do2() -> bool { println!("2"); false }
```
On this code Clippy calls it unidiomatic and suggests the following diff, which has different behavior in a way that I don't necessarily want.
```diff
- let mut good = do1();
- if !do2() {
- good = false;
- }
+ let good = if !do2() {
+ false
+ } else {
+ do1()
+ };
```
On exploring issues filed about this lint, I have found that other users have also struggled with inappropriate suggestions (#4124, #3043, #2918, #2176) and suggestions that make the code worse (#3769, #2749). Overall I believe that this lint is still at nursery quality for now and should not be enabled.
---
changelog: Remove useless_let_if_seq from default set of enabled lints
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I have some code that looks like this.
However clippy gives me a
useless_let_if_seqsaying that I should merge the first if into thelet. I don't think this is a helpful suggestion because 1) It makes the first check look special and 2) it distracts a bit from thetrueuntil markedfalse.After thinking about this a bit I think that if the variable defined from the
letis set from multiple locations in the code then this warning should probably be suppressed.The text was updated successfully, but these errors were encountered: