Move collection_is_never_read from nursery to suspicious

[schubart]

This seems like a useful lint. I have not heard about any false positives. Let's make it more widely available.

changelog: Moved [collection_is_never_read] to suspicious

[rustbot]

r? @dswij

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

[schubart]

Needs more work. Closing this for now, apologies.

[schubart]

Many of the tests for other lints are failing because those tests create a collection, add some stuff to it, and then don't do anything with it. So these are true positives, but obviously irrelevant because these tests are not complete and meaningful programs.

Is there a recommended way to deal with this in bulk or should I update all these tests and allow collection_is_never_read?

[Jarcho]

Just allow the lint in any test where needed.

[schubart]

OK. That'll take a while.

[rustbot]

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git rebase -i master
$ # delete any merge commits in the editor that appears
$ git push --force-with-lease

The following commits are merge commits:

• 8f547a0

[schubart]

I've put #![allow(clippy::collection_is_never_read)] in all the unrelated tests that were failing.

The PR is ready now.

[bors]

☔ The latest upstream changes (presumably #11596) made this pull request unmergeable. Please resolve the merge conflicts.

[schubart]

Merge conflicts resolved, tests passed.

[dswij]

Thanks for the PR! (and allow-ing all the tests) @schubart

I think moving to suspicious makes sense, but I'll open up a discussion in Zulip with the team to see if there's any objection here.

[flip1995]

Before moving this to suspicious and make it warn-by-default, can we do a lintcheck run and check the results, please?

[flip1995]

clippy 0.1.75 (5ec94d2043a 2023-10-09)

Reports

target/lintcheck/sources/futures-channel-0.3.28/src/mpsc/mod.rs:761:9 clippy::collection_is_never_read "collection is never read"
target/lintcheck/sources/futures-channel-0.3.28/src/mpsc/mod.rs:842:9 clippy::collection_is_never_read "collection is never read"
target/lintcheck/sources/strum_macros-0.25.2/src/macros/enum_properties.rs:22:9 clippy::collection_is_never_read "collection is never read"
target/lintcheck/sources/strum_macros-0.25.2/src/macros/enum_properties.rs:23:9 clippy::collection_is_never_read "collection is never read"
target/lintcheck/sources/toml_edit-0.20.2/src/encode.rs:467:5 clippy::collection_is_never_read "collection is never read"
target/lintcheck/sources/wasm-bindgen-0.2.87/src/externref.rs:64:21 clippy::collection_is_never_read "collection is never read"

Stats:

lint	count
clippy::collection_is_never_read	6

Results on the top 400 most popular crates. I haven't looked at the code where the lint triggered at, yet.

EDIT: the futures-channel triggers look like FPs to me, the wasm-bindgen is definitely a FP.

[schubart]

• futures_channel is a false positive involving Hasher, similar to what Alex Macleod mentioned in Zulip. I'll work on it.
• strum_macros is a true positive: Remove unused vectors Peternator7/strum#304
• toml_edit is a true positive: chore: Stop building unnecessary string toml-rs/toml#620
• wasm-bindgen: I'd argue that's a very specific corner case in some unsafe low-level code. Summary:

// poor man's `try_reserve_exact` until that's stable
unsafe {
    // ...
    let mut old: Vec<usize> = mem::replace(&mut self.data, new_vec);
    old.set_len(0);
}

I think in a case like this, suppressing the lint would be appropriate.

[bors]

☔ The latest upstream changes (presumably #12030) made this pull request unmergeable. Please resolve the merge conflicts.

[xFrednet]

Hey @schubart, this is a ping from triage, since there hasn't been any activity in some time. Are you still planning to continue this implementation?

If you have any questions, you're always welcome to ask them in this PR or on Zulip.

@rustbot author

[schubart]

Hey @schubart, this is a ping from triage, since there hasn't been any activity in some time. Are you still planning to continue this implementation?

Yes, I think the concerns raised here and on Zulip can be addressed, but I don't have time to work on this at the moment.

I've already reverted this PR to "draft" but I can close it entirely for the time being, if that helps.

[xFrednet]

I've already reverted this PR to "draft" but I can close it entirely for the time being, if that helps.

Okay, I'm trying to organize our old PRs a bit and mark things as inactive in case someone else is interested in completing a PR.

So, I'll close this for now. When you have the time again, we would love to have you back. You can just create a new PR :D. Thank you for the time, you already put into this!

@rustbot label +S-inactive-closed -S-waiting-on-author -S-waiting-on-review