Type safe CLI implementation for clippy-dev

[flip1995]

Use the derive feature of clap to generate CLI of clippy-dev. Adding new commands will be easier in the future and we get better compile time checking through exhaustive matching.

---

I think I tested everything locally. But I would appreciate if the reviewer could go over it again, so that everything keeps working.

changelog: none

[rustbot]

r? @Alexendoo

rustbot has assigned @Alexendoo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Alexendoo]

IIRC we were using the manual API so that we didn't have to compile syn & friends to use cargo dev, are we fine with that trade-off?

[flip1995]

This branch:

$ cargo clean
$ time cargo dev
   Compiling [...]
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 3.14s
     Running `target/debug/clippy_dev`
cargo dev  11.40s user 1.96s system 420% cpu 3.173 total

master:

$ cargo clean
$ time cargo dev
   Compiling [...]
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 2.20s
     Running `target/debug/clippy_dev`
cargo dev  7.72s user 1.48s system 412% cpu 2.231 total

So it takes a bit longer to compile, but not significantly longer. I think this trade-off is fine.

[flip1995]

but not significantly longer

Well it takes 50% longer. But compiling from a clean build in 11.4s instead of in 7.7s isn't too bad IMO

[flip1995]

Maybe some context, why I'm doing this helps here: I'm working on automating the sync and release process using josh instead of subtree. To do this, I'll have to add a few more deps to clippy_dev anyway, i.e. chrono, directories, and xshell.

So the number of deps will grow anyway and so will the commands. I find the current command definition already bloated, so I wanted to clean up the command generation first, before adding to it.

Here's the PoC commit: #12759

[Alexendoo]

I don't love it but it's palatable. Is it possible for us to have changes that exist in the clippy repo but not upstream? If we could add a [workspace] to our root Cargo.toml that only exists locally that would allow us to be sure the syn compilation can be shared by clippy itself mitigating the extra time spent here

[flip1995]

Sadly this is not possible, without a lot of overhead when doing the sync. One idea might be to add Cargo.toml.local files and then move our whole dev workflow (including building and testing) to cargo dev, which would use the .local files, rather than the Cargo.toml files that are used in rustc.

[Alexendoo]

That makes sense, it could also be pretty finicky on syncs if we can't easily modify the rust repo/clippy repo versions directly in the same PR

An idea to keep normal Cargo.tomls around:

• Move our current root package (cargo-clippy/clippy-driver) into a directory
• Upstream, create a virtual workspace src/tools/clippy/workspace/Cargo.toml (unused in the rust repo)
• Use a composition filter to map it to be the root Cargo.toml in the Clippy repo with the rest of the files mapped as normal

[flip1995]

Yeah, with Josh we could do something like this. That's actually a good idea. But first we'd need to move to Josh.

If you want, I can also rebase my Josh work on master using the non-derive Clap interface and we wait with merging this until we have all the virtual workspace infra in place.

[Alexendoo]

Spent too long trying to find {n} in the clap docs 😅. looks like they're not documented as it's a temporary solution for clap-rs/clap#2389

Merging this now seems fine to me, ideally we can have a local workspace in the future but if we don't it's not a huge deal to duplicate syn sometimes

@bors r+

[bors]

📌 Commit 537ab6c has been approved by Alexendoo

It is now in the queue for this repository.

[bors]

⌛ Testing commit 537ab6c with merge 3ef3a13...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Alexendoo
Pushing 3ef3a13 to master...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Alexendoo
Pushing 3ef3a13 to master...

[bors]

👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request.