New lint: detect if expressions with simple boolean assignments to the same target

[samueltardieu]

Closes #10430

changelog: [needless_bool_assign] new lint to detect simple boolean assignment to the same target in if branches

[rustbot]

r? @Manishearth

(rustbot has picked a reviewer for you, use r? to override)

[matthiaskrgr]

If we have comments inside the if expr that explain what is happening, will clippy remove these? This might be annoying :/

    if needless_bool_condition() {
    // the crazy brown fox
        a.field = true;
    } else {
    // jumps over the lazy cow
        a.field = true;
    }

[Manishearth]

Yeah I'm concerned about sticking this within an existing lint, and worried about impact.

[samueltardieu]

I agree with both remarks. I'll make this into a new lint and check for comments. I'll put it in draft status in the meantime.

[samueltardieu]

@Manishearth I made it into a new lint.

@matthiaskrgr The lint does not trigger when there are comments in the if expression being linted.

Note that expanded code is excluded already.

[Manishearth]

cc @rust-lang/clippy Do we think this lint is going to be too noisy? Should we nursery it first or something?

[samueltardieu]

I don't expect this lint to trigger much except on beginner programmers' code (as does needless_bool) but I might be wrong.

[xFrednet]

cc @rust-lang/clippy Do we think this lint is going to be too noisy? Should we nursery it first or something?

If we want to have a trial period, I'd first put this into pedantic. Though I'd guess that it won't be noisy if it really just checks for assignments and nothing else. Having it warn-by-default will probably be fine. Maybe let's see what lintcheck or another tool with more crates says?

[samueltardieu]

Maybe let's see what lintcheck or another tool with more crates says?

lintcheck does not trigger this lint. Running it with --filter needless_bool_assign yields:

clippy 0.1.69

### Reports



### Stats:

| lint                                               | count |
| -------------------------------------------------- | ----- |


### ICEs:

[samueltardieu]

Checked with the 500 most recently downloaded crates of crates.io: this lint was not triggered (the complete 470klines report is available at https://rfc1149.net/tmp/top500_logs.txt.gz).

[samueltardieu]

@Manishearth Is there anything more you'd like me to do on this one?

[Manishearth]

I don't think we have consensus as a team on whether we should release this lint enabled by default.

(Also I'm a bit busy and will be slow to review)

[samueltardieu]

This has been lingering for more than one month now. What is the next step?

(I'll rebase it in the meantime)

[Manishearth]

@bors r+

given lintcheck results I guess it's fine

[bors]

📌 Commit 69da902 has been approved by Manishearth

It is now in the queue for this repository.

[bors]

⌛ Testing commit 69da902 with merge 7a870ae...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Manishearth
Pushing 7a870ae to master...