Skip to content

Commit

Permalink
Auto merge of #5599 - dtolnay:letif, r=flip1995
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed May 15, 2020
2 parents e22ccf5 + 95399f8 commit cac9ad0
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 4 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/let_if_seq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ declare_clippy_lint! {
/// };
/// ```
pub USELESS_LET_IF_SEQ,
style,
nursery,
"unidiomatic `let mut` declaration followed by initialization in `if`"
}

Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1266,7 +1266,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT),
LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
LintId::of(&len_zero::LEN_ZERO),
LintId::of(&let_if_seq::USELESS_LET_IF_SEQ),
LintId::of(&let_underscore::LET_UNDERSCORE_LOCK),
LintId::of(&lifetimes::EXTRA_UNUSED_LIFETIMES),
LintId::of(&lifetimes::NEEDLESS_LIFETIMES),
Expand Down Expand Up @@ -1476,7 +1475,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&inherent_to_string::INHERENT_TO_STRING),
LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
LintId::of(&len_zero::LEN_ZERO),
LintId::of(&let_if_seq::USELESS_LET_IF_SEQ),
LintId::of(&literal_representation::INCONSISTENT_DIGIT_GROUPING),
LintId::of(&loops::EMPTY_LOOP),
LintId::of(&loops::FOR_KV_MAP),
Expand Down Expand Up @@ -1728,6 +1726,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&floating_point_arithmetic::IMPRECISE_FLOPS),
LintId::of(&floating_point_arithmetic::SUBOPTIMAL_FLOPS),
LintId::of(&future_not_send::FUTURE_NOT_SEND),
LintId::of(&let_if_seq::USELESS_LET_IF_SEQ),
LintId::of(&missing_const_for_fn::MISSING_CONST_FOR_FN),
LintId::of(&mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL),
LintId::of(&mutex_atomic::MUTEX_INTEGER),
Expand Down
2 changes: 1 addition & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2469,7 +2469,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
},
Lint {
name: "useless_let_if_seq",
group: "style",
group: "nursery",
desc: "unidiomatic `let mut` declaration followed by initialization in `if`",
deprecation: None,
module: "let_if_seq",
Expand Down

0 comments on commit cac9ad0

Please sign in to comment.