Lintcheck

.github/workflows/lintcheck.yml

@@ -132,3 +132,46 @@ jobs:
         path: |
           full_diff.md
           truncated_diff.md
+
+  # Run lintcheck in fix mode to check that all suggestions are good. This only
+  # runs on `clippy::all` + `clippy::pedantic` as lints and suggestions from these
+  # categories generally not conflict and overlap.
+  fix:
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v4
+      with:
+        fetch-depth: 2
+
+    # HEAD is the generated merge commit `refs/pull/N/merge` between the PR and `master`, `HEAD^`
+    # being the commit from `master` that is the base of the merge
+    - name: Checkout base
+      run: git checkout HEAD^
+
+    # Use the lintcheck from the PR to generate the JSON in case the PR modifies lintcheck in some
+    # way
+    - name: Checkout current lintcheck
+      run: |
+        rm -rf lintcheck
+        git checkout ${{ github.sha }} -- lintcheck
+
+    - name: Cache lintcheck bin
+      id: cache-lintcheck-bin
+      uses: actions/cache@v4
+      with:
+        path: target/debug/lintcheck
+        key: lintcheck-bin-${{ hashfiles('lintcheck/**') }}
+
+    - name: Build lintcheck
+      if: steps.cache-lintcheck-bin.outputs.cache-hit != 'true'
+      run: cargo build --manifest-path=lintcheck/Cargo.toml
+
+    - name: Create cache key
+      id: key
+      run: echo "key=lintcheck-base-${{ hashfiles('lintcheck/**') }}-$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"
+
+    - name: Run lintcheck
+      if: steps.cache-json.outputs.cache-hit != 'true'
+      run: ./target/debug/lintcheck --crates-toml ./lintcheck/ci_crates.toml --fix

lintcheck/lintcheck_crates.toml

@@ -12,33 +12,33 @@
 [crates]
 
 # Some binaries
-cargo = {name = "cargo", version = '0.80.0', online_link = 'https://docs.rs/cargo/{version}/src/{file}.html#{line}'}
-ripgrep = {name = "ripgrep", version = '14.1.0'}
-mdbook = {name = "mdbook", version = '0.4.40'}
+cargo = {name = "cargo", version = '0.80.0', mode = "bin", online_link = 'https://docs.rs/cargo/{version}/src/{file}.html#{line}' }
+ripgrep = {name = "ripgrep", version = '14.1.0', mode = "bin" }
+mdbook = {name = "mdbook", version = '0.4.40', mode = "lib" }
 
 # Common libraries
-rayon = {name = "rayon", version = '1.10.0'}
-serde = {name = "serde", version = '1.0.204'}
-bitflags = {name = "bitflags", version = '2.6.0'}
-log = {name = "log", version = '0.4.22'}
-quote = {name = "quote", version = '1.0.36'}
-proc-macro2 = {name = "proc-macro2", version = '1.0.86'}
-rand = {name = "rand", version = '0.8.5'}
-rand_core = {name = "rand_core", version = '0.6.4'}
-regex = {name = "regex", version = '1.10.5'}
-syn = {name = "syn", version = '2.0.71'}
-anyhow = {name = "anyhow", version = '1.0.86'}
-async-trait = { name = 'async-trait', version = '0.1.81' }
-cxx = {name = "cxx", version = '1.0.124'}
-ryu = {name = "ryu", version = '1.0.18'}
-thiserror = {name = "thiserror", version = '1.0.63'}
-serde_yaml = {name = "serde_yaml", version = '0.9.33'}
-puffin = {name = "puffin", version = '0.19.0'}
-bumpalo = {name = "bumpalo", version = '3.16.0'}
-wasmi = {name = "wasmi", version = '0.35.0'}
-base64 = { name = 'base64', version = '0.22.1' }
-once_cell = { name = 'once_cell', version = '1.19.0' }
-tokio = { name = 'tokio', version = '1.38.1' }
+rayon = {name = "rayon", version = '1.10.0', mode = "lib"}
+serde = {name = "serde", version = '1.0.204', mode = "lib"}
+bitflags = {name = "bitflags", version = '2.6.0', mode = "lib"}
+log = {name = "log", version = '0.4.22', mode = "lib"}
+quote = {name = "quote", version = '1.0.36', mode = "lib"}
+proc-macro2 = {name =  "proc-macro2", version = '1.0.86', mode = "lib"}
+rand = {name = "rand", version = '0.8.5', mode = "lib"}
+rand_core = {name = "rand_core", version = '0.6.4', mode = "lib"}
+regex = {name = "regex", version = '1.10.5', mode = "lib"}
+syn = {name = "syn", version = '2.0.71', mode = "lib"}
+anyhow = {name = "anyhow", version = '1.0.86', mode = "lib"}
+async-trait = {name =  'async-trait', version = '0.1.81' , mode = "lib"}
+cxx = {name = "cxx", version = '1.0.124', mode = "lib"}
+ryu = {name = "ryu", version = '1.0.18', mode = "lib"}
+thiserror = {name = "thiserror", version = '1.0.63', mode = "lib"}
+serde_yaml = {name = "serde_yaml", version = '0.9.33', mode = "lib"}
+puffin = {name = "puffin", version = '0.19.0', mode = "lib"}
+bumpalo = {name = "bumpalo", version = '3.16.0', mode = "lib"}
+wasmi = {name = "wasmi", version = '0.35.0', mode = "lib"}
+base64 = {name = 'base64', version = '0.22.1', mode = "lib"}
+once_cell = {name = 'once_cell', version = '1.19.0', mode = "lib"}
+tokio = {name = 'tokio', version = '1.38.1', mode = "lib"}
 
 [recursive]
 ignore = [

lintcheck/src/config.rs

@@ -30,8 +30,13 @@ pub(crate) struct LintcheckConfig {
     /// Only process a single crate on the list
     #[clap(long, value_name = "CRATE")]
     pub only: Option<String>,
-    /// Runs cargo clippy --fix and checks if all suggestions apply
-    #[clap(long, conflicts_with("max_jobs"))]
+    /// Runs cargo clippy --fix and checks if all suggestions apply.
+    #[clap(
+        long,
+        conflicts_with("max_jobs"),
+        conflicts_with("warn_all"),
+        conflicts_with("recursive")
+    )]
     pub fix: bool,
     /// Apply a filter to only collect specified lints, this also overrides `allow` attributes
     #[clap(long = "filter", value_name = "clippy_lint_name", use_value_delimiter = true)]

lintcheck/src/input.rs

@@ -8,7 +8,7 @@ use std::time::Duration;
 use serde::Deserialize;
 use walkdir::{DirEntry, WalkDir};
 
-use crate::{Crate, LINTCHECK_DOWNLOADS, LINTCHECK_SOURCES};
+use crate::{CheckMode, Crate, LINTCHECK_DOWNLOADS, LINTCHECK_SOURCES};
 
 const DEFAULT_DOCS_LINK: &str = "https://docs.rs/{krate}/{version}/src/{krate_}/{file}.html#{line}";
 const DEFAULT_GITHUB_LINK: &str = "{url}/blob/{hash}/src/{file}#L{line}";
@@ -37,6 +37,7 @@ struct TomlCrate {
     git_hash: Option<String>,
     path: Option<String>,
     options: Option<Vec<String>>,
+    mode: Option<CheckMode>,
     /// Magic values:
     /// * `{krate}` will be replaced by `self.name`
     /// * `{krate_}` will be replaced by `self.name` with all `-` replaced by `_`
@@ -79,9 +80,10 @@ impl TomlCrate {
 #[derive(Debug, Deserialize, Eq, Hash, PartialEq, Ord, PartialOrd)]
 pub struct CrateWithSource {
     pub name: String,
-    pub source: CrateSource,
-    pub file_link: String,
-    pub options: Option<Vec<String>>,
+    source: CrateSource,
+    file_link: String,
+    options: Option<Vec<String>>,
+    mode: Option<CheckMode>,
 }
 
 #[derive(Debug, Deserialize, Eq, Hash, PartialEq, Ord, PartialOrd)]
@@ -112,6 +114,7 @@ pub fn read_crates(toml_path: &Path) -> (Vec<CrateWithSource>, RecursiveOptions)
                 },
                 file_link: tk.file_link(DEFAULT_PATH_LINK),
                 options: tk.options.clone(),
+                mode: tk.mode,
             });
         } else if let Some(ref version) = tk.version {
             crate_sources.push(CrateWithSource {
@@ -121,6 +124,7 @@ pub fn read_crates(toml_path: &Path) -> (Vec<CrateWithSource>, RecursiveOptions)
                 },
                 file_link: tk.file_link(DEFAULT_DOCS_LINK),
                 options: tk.options.clone(),
+                mode: tk.mode,
             });
         } else if tk.git_url.is_some() && tk.git_hash.is_some() {
             // otherwise, we should have a git source
@@ -132,6 +136,7 @@ pub fn read_crates(toml_path: &Path) -> (Vec<CrateWithSource>, RecursiveOptions)
                 },
                 file_link: tk.file_link(DEFAULT_GITHUB_LINK),
                 options: tk.options.clone(),
+                mode: tk.mode,
             });
         } else {
             panic!("Invalid crate source: {tk:?}");
@@ -180,7 +185,7 @@ impl CrateWithSource {
     /// copies a local folder
     #[expect(clippy::too_many_lines)]
     fn download_and_extract(&self) -> Crate {
-        #[allow(clippy::result_large_err)]
+        #[expect(clippy::result_large_err)]
         fn get(path: &str) -> Result<ureq::Response, ureq::Error> {
             const MAX_RETRIES: u8 = 4;
             let mut retries = 0;
@@ -199,6 +204,7 @@ impl CrateWithSource {
         let name = &self.name;
         let options = &self.options;
         let file_link = &self.file_link;
+        let mode = self.mode;
         match &self.source {
             CrateSource::CratesIo { version } => {
                 let extract_dir = PathBuf::from(LINTCHECK_SOURCES);
@@ -232,6 +238,7 @@ impl CrateWithSource {
                     path: extract_dir.join(format!("{name}-{version}/")),
                     options: options.clone(),
                     base_url: file_link.clone(),
+                    mode,
                 }
             },
             CrateSource::Git { url, commit } => {
@@ -274,6 +281,7 @@ impl CrateWithSource {
                     path: repo_path,
                     options: options.clone(),
                     base_url: file_link.clone(),
+                    mode,
                 }
             },
             CrateSource::Path { path } => {
@@ -314,6 +322,7 @@ impl CrateWithSource {
                     path: dest_crate_root,
                     options: options.clone(),
                     base_url: file_link.clone(),
+                    mode,
                 }
             },
         }

lintcheck/src/json.rs

@@ -90,6 +90,8 @@ pub(crate) fn diff(old_path: &Path, new_path: &Path, truncate: bool) {
         }
     }
 
+    println!();
+    println!();
     print_summary_table(&lint_warnings);
     println!();
 

lintcheck/src/main.rs

@@ -55,6 +55,16 @@ struct Crate {
     path: PathBuf,
     options: Option<Vec<String>>,
     base_url: String,
+    mode: Option<CheckMode>,
+}
+
+#[derive(Debug, Copy, Clone, Eq, Hash, PartialEq, Ord, PartialOrd, serde::Deserialize)]
+#[serde(rename_all = "lowercase")]
+enum CheckMode {
+    Bin,
+    Lib,
+    Tests,
+    All,
 }
 
 impl Crate {
@@ -116,11 +126,46 @@ impl Crate {
         clippy_args.extend(lint_levels_args.iter().map(String::as_str));
 
         let mut cmd = Command::new("cargo");
-        cmd.arg(if config.fix { "fix" } else { "check" })
-            .arg("--quiet")
+
+        if config.fix {
+            cmd.arg("fix");
+            cmd.arg("--allow-no-vcs");
+            // Fix should never be run in combination with `--recursive`. This
+            // should never happen as the CLI flags conflict, but you know life.
+            assert!(
+                server.is_none(),
+                "--fix was combined with `--recursive`. Server: {server:#?}"
+            );
+        } else {
+            cmd.arg("check");
+            cmd.arg("--message-format=json");
+        }
+
+        // It turns out that `cargo fix` by default uses `--all-targets`. This is
+        // on contrast to `cargo check`. (See rust-lang/cargo#13527). Sadly, there
+        // is currently no simple way to opt out of this.
+        //
+        // We can also not just default to `--tests` or something uniform, as those
+        // don't always compile and almost doubles the run times.
+        //
+        // The "simple" solution is to require a mode when running rustfix. This is
+        // turned into a flag like `--bins`, `--lib` etc.
+        if let Some(mode) = self.mode {
+            cmd.arg(mode.as_arg());
+        } else if config.fix {
+            eprintln!(
+                "\n\
+                WARNING: `--fix` requires the mode to be specified.\n\
+                |        Try adding a mode to the specified crate\n"
+            );
+            return Vec::new();
+        }
+
+        cmd.arg("--quiet")
             .current_dir(&self.path)
             .env("CLIPPY_ARGS", clippy_args.join("__CLIPPY_HACKERY__"));
 
+        // This is only used for `--recursive`
         if let Some(server) = server {
             // `cargo clippy` is a wrapper around `cargo check` that mainly sets `RUSTC_WORKSPACE_WRAPPER` to
             // `clippy-driver`. We do the same thing here with a couple changes:
@@ -145,10 +190,6 @@ impl Crate {
             return Vec::new();
         };
 
-        if !config.fix {
-            cmd.arg("--message-format=json");
-        }
-
         let shared_target_dir = shared_target_dir(&format!("_{thread_index:?}"));
         let all_output = cmd
             // use the looping index to create individual target dirs
@@ -206,6 +247,17 @@ impl Crate {
     }
 }
 
+impl CheckMode {
+    fn as_arg(self) -> &'static str {
+        match self {
+            Self::Bin => "--bins",
+            Self::Lib => "--lib",
+            Self::Tests => "--tests",
+            Self::All => "--all-targets",
+        }
+    }
+}
+
 /// The target directory can sometimes be stored in the file name of spans.
 /// This is problematic since the directory in constructed from the thread
 /// ID and also used in our CI to determine if two lint emissions are the

lintcheck/src/recursive.rs

@@ -79,6 +79,7 @@ fn process_stream(
     }
 }
 
+#[derive(Debug)]
 pub(crate) struct LintcheckServer {
     pub local_addr: SocketAddr,
     receiver: Receiver<ClippyWarning>,