EmbarkStudios / rust-ecosystem

Rust wants & tracking for Embark 🦀
http://embark.rs
Apache License 2.0
880 stars 18 forks source link

Clippy wants #79

Open repi opened 2 years ago

repi commented 2 years ago

Wish list of Clippy lints and features & bug fixes we would like to see added/improved/fixed based on the Rust projects we've been developing at Embark.

This is not a complete list, but an attempt to keep a bit of structure for our own sake, and may be of interest for Clippy lint developers as well.

Should have

Lints & Clippy features or fixes that would directly improve or help our workflows

Nice to have

Lints & Clippy features or fixes that would be nice to have, but are lower priority to us than the above list.

Not filed / found

Fixes or enhancements that believe there is no issue for yet, if you find one please do comment and we'll update it here.


Related tracking issues for other Rust components:

MarijnS95 commented 2 years ago

You are perhaps also interested in unused-crate-dependencies, were the "false-positives" around multi-target crates figured out / resolved: https://github.com/rust-lang/rust/issues/95513

Seems like you're also tracking this in https://github.com/EmbarkStudios/rust-ecosystem/issues/16 though :)

linyihai commented 11 months ago

this is a awsome list, thanks

MarijnS95 commented 9 months ago

@repi what are your thoughts on the new [lints] table? https://blog.rust-lang.org/2023/11/16/Rust-1.74.0.html

I considered converting to it and submitting a PR here, but a massive downside is that the table isn't forced down on all crates from the workspace; instead every individual workspace member crate has to opt-in by setting [lints] workspace = true.

repi commented 9 months ago

it is unfortunate and a miss as it makes it very error prone to opt into the workspace lints (rather than opting out), we should have tested the new clippy workspace lint setting more in nightly and given feedback about that.

for our main projects we'll likely still use the new [lints] and update every crate to use it (cc @Jake-Shadle ), and likely write some small automation script for CI to verify that it is used in all of them (or maybe fit for cargo-deny, though a bit odd to have in there).

for all of our smaller open source projects we will likely not change to use the new functionality just because of this reason, it is easier to use .cargo/config.toml RUSTFLAGS override and have it automatically be enabled for all crates in the project, or cargo cranky.

hope these new lint support can be improved to be implicitly inherited from the workspace without changes in the future, this seems to be the main discussion issue for that:

MarijnS95 commented 9 months ago

we should have tested the new clippy workspace lint setting more in nightly and given feedback about that.

Same for us, I've been subscribed to the relevant issues but was never able to follow up on testing this.

That said it likely won't have made a difference given the same reasons in the linked issue: this is an MVP, and implicit/forced inheritance is an addition that can be supplied in later updates.

Keep us posted if you figure out some trivial CI tooling, we might adopt that :)