Redesign `rust-analyzer::flycheck`

[alibektas]

The current implementation of flycheck is flawed in that it instantiates at most a single "flycheck instance" for each workspace. With the introduction of rust-analyzer.toml files, more fine-grained configuration has become possible. Therefore, we need more flexibility in flycheck instances. At the same time, we want to avoid having many `flycheck' threads, as this is clearly inefficient.

Another issue that also needs to be addressed is how hard it is to test any kind of flycheck activity. The new model should make testing less of a problem for us.

Some key points about the new approach

• A thread pool will be used to avoid unnecessary initialization/destruction of threads. The number of threads will be configurable.
• Threads are used to execute whatever flycheck op is waiting to be executed. To map the results of a flycheck' instance to the corresponding workspace, it was previously sufficient to keep a workspace id'. The new model could use a hash of workspace_id * target * other things I can't think of to compute an id.
• More points to come in the next few days.

[Veykril]

Dumping some quick notes/thoughts:

Current architecture:

• "spawn" one flycheck worker per loaded workspace, we eagerly built the command at this stage
  • the command building should likely be deferred instead, as we want it to be dynamic according to our needs (see Feature Request: Detect no_std per-crate and ignore check.allTargets #14205)
• on initial project load finishing, instruct all workers to check their full workspace
• Whenever a file is saved
  • figure out which crates the file belongs to
    • then, instruct all flycheck workers of workspaces that contain any of the crates to flycheck either:
      • the relevant package belonging to the crate (if the workspace c onfig is not set)
      • or the full workspace otherwise
  • if no flycheck was triggered, isntruct all workers to check their full workspace

Notably, this has the downside that we can only check a single package (or target even) per workspace. We also can bring down the system if there are a lot of workspaces and we check all of them at once.

Also, we do want to keep the property of having at most one flycheck worker run for a given workspace (multiple is non-sensical as they will block on another for the cargo lock anyways).

So maybe a job queue isn't actually needed and we can keep the per workspace workers, from the looks of it we mainly need to make them more flexible wrt. the command they invoke?

• some relevant issues to consider:
  • Don't pass --all-targets when a target is specified #18110
  • check.workspace option no longer implies cargo check will always use --workspace flag #18104
  • Feature Request: Detect no_std per-crate and ignore check.allTargets #14205
  • Allow flycheck to be configured to only check transitive dependents of the current crate #17404

[joshka]

Add

• Multiple flychecks with different commands for single workspace #17414

Taking a big picture look at this, I think I'd look at this as solving the big hairy goal of how to optimize the developer inner loop. To me, that means where possible, low latency results (on every keypress, not on save) for all the activities that a developer needs to focus on, while simultaneously prioritizing the most important ones (e.g. run impacted unit tests first, run recently failing tests first, run fast tests first, run clippy concurrently), in combination with showing the most important information up front (i.e. I rarely ever want to see clippy warnings when there's errors, and I probably never want to see these if there's unit test failures / build errors).

Put more plainly, I'd like to stay in flow, while having my machine do all the things which are required as part of delivering quality software (build, test, lint, etc.)

Maybe this could be expanded out to be a worthy GSoC project for someone?

So maybe a job queue isn't actually needed and we can keep the per workspace workers, from the looks of it we mainly need to make them more flexible wrt. the command they invoke?

I think we probably do need a queue, but it's more likely a priority queue than a linear one.