Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clippy wants #79

Open
24 of 44 tasks
repi opened this issue Jun 7, 2022 · 5 comments
Open
24 of 44 tasks

Clippy wants #79

repi opened this issue Jun 7, 2022 · 5 comments
Labels

Comments

@repi
Copy link
Contributor

repi commented Jun 7, 2022

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.

  • cargo clippy --fix often doesn't work and it is unclear to the user which lints it works for and which it skips
  • A lint to require types in crate to be used fully qualified, like log::info!("hej") instead of use log::info; info!("hej") by specifying log crate in clippy.toml section for the lint.
  • A lint to disallow returning Option<()> as we've seen that use in code just to (too) easily early out with ? on empty options.

Related tracking issues for other Rust components:

@repi repi added the tools label Jun 7, 2022
@MarijnS95
Copy link

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

Seems like you're also tracking this in #16 though :)

This was referenced Aug 6, 2022
@repi repi mentioned this issue Oct 2, 2022
21 tasks
@linyihai
Copy link

linyihai commented Sep 5, 2023

this is a awsome list, thanks

@MarijnS95
Copy link

@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
Copy link
Contributor Author

repi commented Nov 16, 2023

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
Copy link

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants