Clippy warning

[sgued]

• Have you checked to ensure there aren't other open Pull Requests for the same update/change?
• Are you doing the PR on the next branch?

This PR fixes clippy warnings and adds cargo clippy --workspace -- -Dwarnings to the CI to avoid merging future PRs with warnings.

It's based on top of #2009 which should probably be merged before it.

[Keats]

The reason why those were not enforced in the CI is that some people don't use those tools and I didn't really want to force them too when I can just run it myself eventually. I could be convinced of adding them to CI but leaning towards no for now.

[sgued]

IMHO enforcing the use of these tools is beneficial because:

• Most rust projects use them
• They play a huge role in making rust style consistent across the ecosystem
• They are overall of great quality, and the defaults are pretty good

If someone doesn't use them, it's more likely to be because they either don't know about them, or because they forgot to run them. In both cases teaching or reminding them about those tools would probably be for the best and IHMO reduces maintainer burden. Someone who forgot to run clippy would probably be fine with fixing their non-idiomatic code, and a new rust developer would probably be delighted to discover these tools. I would understand the reluctance if they were of bad quality or niche, but rustfmt and clippy are IMHO very good tools, and they are considered standard across the Rust ecosystem. We could also mention them in the PR template, it might help.

Not enforcing them poses a couple issues on the contrary. Many developers rely on their editor to automatically format code on save. Non formatted code means that theses developers must turn that setting off which makes it harder to format the new code of the PR.
Clippy warnings are also a great tool for development, and many people rely on them to improve their code. Having noise coming from code unrelated to the one from the ongoing PR is a pain.

Cargo fmt and clippy fixes PRs like this one can also be the source of merge conflicts, which are can be painful to resolve for devs inexperienced with git.

The decision is yours, if you don't want them in CI I'll remove them from this PR, but I think that would be a mistake.

[Keats]

Looks like there are some conflicts after merging the cargo fmt PR. Can you rebase?

[sgued]

Done. I have also fixed the new warnings that appeared after rebasing.

[Keats]

Thanks!