Workspace Clippy lint management

[nyurik]

Make all lints managed centrally in one place.

Note that this approach forces pedantic lints on the entire codebase - which means that when the new versions of rust are released, some new lint may fail the build. At the same time, this allows new lints to always be noticed and manually excluded.

The big list of allowed lints can be slowly worked through and forbidden one by one.

Other changes

This PR also fixes one missing semicolon, formats a few Cargo.toml files, and adjusts the justfile clippy recipe to cover more cases.

Note that this PR effectively closes #874 because now the list of lints is tracked in code.

TODO

• Why is there a separate MSRV in clippy.toml that does not match the one in Cargo.toml?
• Unrelated, but really puzzling - why do you have a copy of gix-packetline as gix-packetline-blocking?

[Byron]

That's great, thank you!

I also removed a configuration file that I thought was only for clippy.

• Why is there a separate MSRV in clippy.toml that does not match the one in Cargo.toml?

Clippy should be close to the latest stable release if it's affordable, i.e. if it doesn't want to change everything. Now that can be controlled much better, so maybe one can increase the clippy level.
rust-version is a difficult field as one can't really increase it without disturbing someone downstream. So as long as it compiles on this level, it should stay on this level. Interestingly, I am not quite sure if this is truly tested, and if rustc really sets itself to this level internally - I doubt it. So chances are it doesn't really compile with this level anyway, but I also don't really know.

• Unrelated, but really puzzling - why do you have a copy of gix-packetline as gix-packetline-blocking?

gix-filter uses it and needs it in blocking mode. But if it would be using the original gix-packetline it would have to configure it, and thus prevent anybody downstream who uses gix-filter from using an async version of gix-packetline, which is also used for fetching and pushing.
To resolve this, there is a managed copy of gix-packetline, a crate which fortunately is very stable already.

[Byron]

And while trying to use Clippy v1.80, I realized that the actual reason for leaving it at the MSRV of 1.74 is to prevent it from suggesting methods that aren't available on the MSRV compiler yet.

Maybe it's an option to use the latest clippy (or to not configure the language level at all) and instead allow all new lints that it would find.

[Byron]

Actually, I think I will leave it as is and just increase the clippy version once the MSRV is increased - it seems like the easiest and least surprising choice.

[nyurik]

• Why is there a separate MSRV in clippy.toml that does not match the one in Cargo.toml?

Clippy should be close to the latest stable release if it's affordable, i.e. if it doesn't want to change everything.

@Byron Sorry, not sure I understand. msrv in clippy.toml does not mean "use Clippy v1.74.0", it means "only suggest language features that were made on or after Rust v1.74.0". Which is exactly the same thing as the meaning of rust-version field. So if the one in Cargo.toml is older than the one in clippy.toml, it makes no sense: in essence, you allow Clippy to suggest something that might not compile in the older version of Rust, while you officially declare that the crate does support that older Rust version.

• Unrelated, but really puzzling - why do you have a copy of gix-packetline as gix-packetline-blocking?

gix-filter uses it and needs it in blocking mode. But if it would be using the original gix-packetline it would have to configure it, and thus prevent anybody downstream who uses gix-filter from using an async version of gix-packetline, which is also used for fetching and pushing. To resolve this, there is a managed copy of gix-packetline, a crate which fortunately is very stable already.

Can you solve this by simply aliasing it in Cargo.toml? E.g. gix-packetline-blocking = { package = "gix-packetline", ... } in Cargo.toml's dependencies?

[Byron]

So if the one in Cargo.toml is older than the one in clippy.toml, it makes no sense: in essence, you allow Clippy to suggest something that might not compile in the older version of Rust, while you officially declare that the crate does support that older Rust version.

You are right. Effectively though, I think there is no breaking 'suggestion' between 1.65 and 1.74 (but there are breaking suggestions in 1.80).

What I really would like to do is have a tool that determines the lowest possible Rustc compiler, and sets the value accordingly. The current value is just something low that worked, but definitely not the lowest possible.

Can you solve this by simply aliasing it in Cargo.toml? E.g. gix-packetline-blocking = { package = "gix-packetline", ... } in Cargo.toml's dependencies?

Cargo will still do dependency resolution based on the actual name, so the 'new name' only affects the name that can be used in code. If that would be possible, it would definitely allow to get rid of the copy.

[NobodyXu]

I think you could use cargo-hack to determine minimum msrv?

Just needs to specify a lower bound (and a possible upper bound) to avoid having too many iterations

[Byron]

I think I would run it if I had a 'for the lazy' script that runs it over all crates in a workspace. If it auto-edits the manifests, it should be good to go after do-all-the-work-and-edit-manifests && say "all done here - commit now"

[nyurik]

For msrv, you can use cargo msrv tool. But I still don't understand why you would want to specify version twice - once in Cargo.toml and once in clippy.toml

[Byron]

Thanks!
It's the difference between the minimum version that compiles the crate successfully, and the latest available lints that don't actually affect the compiler version.

[nyurik]

@Byron but the latest available lints are determined by the version of clippy that you run, not the value of the msrv field in the clippy.toml

[Byron]

I don't think that's entirely true - I am running the latest version of clippy and only if I increase the version of the msrv field in the clippy.toml will it start to report lints that are gated behind higher version numbers. It does respect the msrv field.
It's definitely true that clippy doesn't respect the rust-version field of the manifest though.

[nyurik]

that's so weird - are you certain? When I was implementing some of the Clippy lints, I used the msrv value internally to decide if I should suggest something or not. See this example -- so if the lint runs in the context of a Rust version that supports inlined format values, the check is performed, and otherwise it is skipped.

The internal msrv value should be controlled by the rust-version (from Cargo.toml) and by msrv value (from clippy.toml). If both are set, it picks the one from clippy.toml and gives a warning. So in gitoxide case, there is a conflict that it incorrectly resolves to the newer rust version - i.e. it makes suggestions as if the Rust version is newer, even though it might be compiled with the older one.

[Byron]

I am certain that the msrv field takes precedence, as setting it to 1.80 for example will surface lints that offer API changes that are available in Rust versions more recent than 1.74.

Probably you are completely right - the msrv only gates lints that would suggest using APIs that are available up to that version, while other lints that are independent of that would still be available depending on the clippy version used.

If this is true, then there is no benefit of setting the msrv in Clipp.toml at all, and I'd do better (and have less warnings) by removing it.

If you conquer, I'd welcome a PR that removes clippy.toml entirely - maybe I just saw what I wanted to see and kept it around for the wrong reasons.

[nyurik]

Thx, that was my point exactly - I removed it in #1558