Propose stable clippy lint group instead of unstable.

[AndrewKhoma]

From the clippy lint docs (https://doc.rust-lang.org/clippy/lints.html#nursery):

The clippy::nursery group contains lints which are buggy or need more work. It is not recommended to enable the whole group, but rather cherry-pick lints that are useful for your code base and your use case.

I propose to include suspicious lint group instead (https://doc.rust-lang.org/clippy/lints.html#suspicious):

The clippy::suspicious group is similar to the correctness lints in that it contains lints that trigger on code that is really sus and should be fixed. As opposed to correctness lints, it might be possible that the linted code is intentionally written like it is.

[geeknoid]

@ralfbiedert I think this is a reasonable change. I'm surprised that the suspicious group wasn't in there in the first place, and not including unstable checks is probably a good idea for general guidelines (even if we chose to leave those on in our repos)

[ralfbiedert]

Thanks for the PR. The suspicious group was an oversight, and we should enable it. The nursery ones have in my observation been mostly helpful (e.g., we sometimes have to disable missing_const_for_fn IIRC).

The issue I'd have with

rather cherry-pick lints that are useful for your code base and your use case.

is that I'd still want to enable all of them, and only disable the ones that in a particular code base really get in the way. How about we suggest something like this (wording TBD):

cargo = { level = "warn", priority = -1 }
complexity = { level = "warn", priority = -1 }
correctness = { level = "warn", priority = -1 }
pedantic = { level = "warn", priority = -1 }
perf = { level = "warn", priority = -1 }
style = { level = "warn", priority = -1 }
suspicious = { level = "warn", priority = -1 }
nursery = { level = "warn", priority = -1 }  # optional, might cause more false positives
                                             # but generally recommended

[geeknoid]

@ralfbiedert I was thinking that using the nursery group is similar to using or depending on nightly compiler builds. It's stuff that's not fully tested, not fully agreed it's the right lint to enforce, still have some false positives, etc.

[AndrewKhoma]

seems like the overall Rust nursery is about Rust experimentation:
https://users.rust-lang.org/t/what-does-being-in-rust-nursery-mean/55463/2

[ralfbiedert]

I was thinking that using the nursery group is similar to using or depending on nightly compiler builds. It's stuff that's not fully tested, not fully agreed it's the right lint to enforce, still have some false positives, etc.

Yes, but I think the resulting "mechanism of action" is different:

I'd argue we don't use the nightly Rust compiler because 1) it would force all our customers to nightly, 2) we don't want to rely on features that will never come, and 3) it's tested less.

Most of the premises (e.g., not fully tested or agreed) are true for clippy lints, but: 1) 'nursery' use is not observable by customers, 2) if a lint is removed it will not affect the resulting code, and 3) if a lint has has issues, it can be disabled on various levels.

Nursery might change in scope, but the same is true for all categories. What passes today on suspicious might fail tomorrow.

Going through the list of nursery lints, there are quite a few I remember having fired once we enabled them (e.g., future_not_send, needless_pass_by_ref_mut, redundant_clone and use_self). We could list them manually, but then we'd have to regularly keep coming back to the conversation what to include which I'd like to avoid.

All that said, I'm ok with us not enabling the / a category because it has been marked as 'not recommended yet' upstream (and admittedly some lints, like missing_const_for_fn, had a tendency to misfire around traits which could be confusing for newcomers).

But I'd still want it to be mentioned (commented out or so), because lint groups are easy to forget (we forgot suspicious after all), and overall we have seen it leads to better code. How about:

cargo = { level = "warn", priority = -1 }
complexity = { level = "warn", priority = -1 }
correctness = { level = "warn", priority = -1 }
pedantic = { level = "warn", priority = -1 }
perf = { level = "warn", priority = -1 }
style = { level = "warn", priority = -1 }
suspicious = { level = "warn", priority = -1 }
# nursery = { level = "warn", priority = -1 }  # optional, might cause more false positives