Check rust code with dprint / clippy

[michael-kerscher]

This issue collects infos from multiple places and some research I have done.

What

At the moment dprint is already used in the CI to check various file format for consistent formatting (config) https://github.com/google/comprehensive-rust/blob/a8460036650ecb8f980a3081a9f09cfcc6466580/.github/workflows/build.yml#L30-L31

Pull requests https://github.com/google/comprehensive-rust/pull/2025 and https://github.com/google/comprehensive-rust/pull/2410 argue already for a check with cargo clippy to have an in-depth check of rust as well.

https://github.com/google/comprehensive-rust/pull/2025#issuecomment-2427392137 mentions that clippy-driver could do the checking as in contrast cargo clippy as mentioned in https://github.com/google/comprehensive-rust/pull/2025#discussion_r1806091321

clippy-driver

An easy implementation would include clippy-driver as an dprint-plugin-exec command like

      {
        "command": "clippy-driver --edition 2021 -Cpanic=abort {{file_path}}",
        "exts": ["rs"]
      }

As already mentioned in https://github.com/google/comprehensive-rust/pull/2413#issuecomment-2421834318 this creates some problems as dprint only works on individual files while cargo clippy looks at the entire context (e.g. dependency crates).

https://github.com/google/comprehensive-rust/pull/2413#issuecomment-2427377949 argues that the code examples inside markdown files are pretty self-contained as they rely on playground and its crates.

Problems

And while that might be true, it is not as easy as it looks, because the rust playground provides the top 100 most downloaded crates from crates.io (https://play.rust-lang.org/help#features-crates) (explicit list). These dependencies are not considered by clippy-driver

dependency crates

An example that is using that can be found at concurrency/async-exercises/chat-async/src/bin/client.rs.

Output from `clippy-driver --edition=2021 src/concurrency/async-exercises/chat-async/src/bin/client.rs`

$ clippy-driver --edition=2021 src/concurrency/async-exercises/chat-async/src/bin/client.rs
error[E0433]: failed to resolve: use of undeclared crate or module `futures_util`
  --> src/concurrency/async-exercises/chat-async/src/bin/client.rs:17:5
   |
17 | use futures_util::stream::StreamExt;
   |     ^^^^^^^^^^^^ use of undeclared crate or module `futures_util`

error[E0432]: unresolved import `futures_util`
  --> src/concurrency/async-exercises/chat-async/src/bin/client.rs:18:5
   |
18 | use futures_util::SinkExt;
   |     ^^^^^^^^^^^^ use of undeclared crate or module `futures_util`

error[E0433]: failed to resolve: use of undeclared crate or module `tokio`
  --> src/concurrency/async-exercises/chat-async/src/bin/client.rs:20:5
   |
20 | use tokio::io::{AsyncBufReadExt, BufReader};
   |     ^^^^^ use of undeclared crate or module `tokio`

error[E0432]: unresolved import `http`
  --> src/concurrency/async-exercises/chat-async/src/bin/client.rs:19:5
   |
19 | use http::Uri;
   |     ^^^^ use of undeclared crate or module `http`

error[E0432]: unresolved import `tokio_websockets`
  --> src/concurrency/async-exercises/chat-async/src/bin/client.rs:21:5
   |
21 | use tokio_websockets::{ClientBuilder, Message};
   |     ^^^^^^^^^^^^^^^^ use of undeclared crate or module `tokio_websockets`

error[E0433]: failed to resolve: use of undeclared crate or module `tokio`
  --> src/concurrency/async-exercises/chat-async/src/bin/client.rs:23:3
   |
23 | #[tokio::main]
   |   ^^^^^ use of undeclared crate or module `tokio`

error[E0433]: failed to resolve: use of undeclared crate or module `tokio`
  --> src/concurrency/async-exercises/chat-async/src/bin/client.rs:36:9
   |
36 |         tokio::select! {
   |         ^^^^^ use of undeclared crate or module `tokio`

error[E0433]: failed to resolve: use of undeclared crate or module `tokio`
  --> src/concurrency/async-exercises/chat-async/src/bin/client.rs:30:17
   |
30 |     let stdin = tokio::io::stdin();
   |                 ^^^^^ use of undeclared crate or module `tokio`
   |
help: consider importing this module
   |
17 + use std::io;
   |
help: if you import `io`, refer to it directly
   |
30 -     let stdin = tokio::io::stdin();
30 +     let stdin = io::stdin();
   |

error[E0752]: `main` function is not allowed to be `async`
  --> src/concurrency/async-exercises/chat-async/src/bin/client.rs:24:1
   |
24 | async fn main() -> Result<(), tokio_websockets::Error> {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `main` function is not allowed to be `async`

error[E0433]: failed to resolve: use of undeclared crate or module `tokio_websockets`
  --> src/concurrency/async-exercises/chat-async/src/bin/client.rs:24:31
   |
24 | async fn main() -> Result<(), tokio_websockets::Error> {
   |                               ^^^^^^^^^^^^^^^^ use of undeclared crate or module `tokio_websockets`

error: aborting due to 10 previous errors

Some errors have detailed explanations: E0432, E0433, E0752.
For more information about an error, try `rustc --explain E0432`.

complex code

A solution should also take into account e.g. mdbook-course and other directories that are not as simple.

Other things

I also noticed that clippy-driver leaves some trash behind, etc. a binary dining-philosophers", whendprint` is checking that example it also gives an error that is not explaining a lot:

dprint check "src/concurrency/sync-exercises/dining-philosophers.rs" 
Error formatting /home/kerscher/workspace/comprehensive-rust/src/concurrency/sync-exercises/dining-philosophers.rs. Message: The original file text was greater than 100 characters, but the formatted text was empty. Perhaps dprint-plugin-exec has been misconfigured?
Had 1 error formatting.

Additionally it feels like abusing dprint a bit here. dprint is a formatting tool while clippy is a code linter. What is dprint fmt supposed to be doing?

cargo clippy

the workspace Cargo.toml already defines all members of the workspace that are relevant. We could use the workspace.lints-table to define what clippy rules we want and a cargo clippy run at the workspace level would check all the projects in that workspace

[michael-kerscher]

@mgeisler IIRC we talked about this at some point and there was some argument on why dprint + clippydriver would be a good choice but I don't find it anymore. Can you give the above summary a good look? From my observation I think cargo clippy on the workspace would be a good choice

[mgeisler]

there was some argument on why dprint + clippydriver would be a good choice but I don't find it anymore.

Yes, I think I remember: what dprint would bring is the ability to check Markdown code blocks. We already benefit from dprint running rustfmt on our code blocks, and I was hoping to get the same with Clippy.

So this might be the key: configure dprint somehow to not run clippy-driver on the .rs files themselves. As you note, cargo clippy will already cover those because the Rust files are part of the Cargo workspace.