lintcheck: Add JSON output, diff subcommand

lintcheck/Cargo.toml

@@ -16,11 +16,13 @@ cargo_metadata = "0.15.3"
 clap = { version = "4.4", features = ["derive", "env"] }
 crates_io_api = "0.8.1"
 crossbeam-channel = "0.5.6"
+diff = "0.1.13"
 flate2 = "1.0"
 indicatif = "0.17.3"
 rayon = "1.5.1"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0.85"
+strip-ansi-escapes = "0.1.1"
 tar = "0.4"
 toml = "0.7.3"
 ureq = "2.2"

lintcheck/src/config.rs

@@ -1,8 +1,9 @@
-use clap::Parser;
+use clap::{Parser, Subcommand, ValueEnum};
 use std::num::NonZero;
 use std::path::PathBuf;
 
-#[derive(Clone, Debug, Parser)]
+#[derive(Parser, Clone, Debug)]
+#[command(args_conflicts_with_subcommands = true)]
 pub(crate) struct LintcheckConfig {
     /// Number of threads to use (default: all unless --fix or --recursive)
     #[clap(
@@ -35,12 +36,36 @@ pub(crate) struct LintcheckConfig {
     /// 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)]
     pub lint_filter: Vec<String>,
-    /// Change the reports table to use markdown links
+    /// Set the output format of the log file
-    #[clap(long)]
+    #[clap(long, short, default_value = "text")]
-    pub markdown: bool,
+    pub format: OutputFormat,
     /// Run clippy on the dependencies of crates specified in crates-toml
     #[clap(long, conflicts_with("max_jobs"))]
     pub recursive: bool,
+    #[command(subcommand)]
+    pub subcommand: Option<Commands>,
+}
+
+#[derive(Subcommand, Clone, Debug)]
+pub(crate) enum Commands {
+    Diff { old: PathBuf, new: PathBuf },
+}
+
+#[derive(ValueEnum, Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) enum OutputFormat {
+    Text,
+    Markdown,
+    Json,
+}
+
+impl OutputFormat {
+    fn file_extension(self) -> &'static str {
+        match self {
+            OutputFormat::Text => "txt",
+            OutputFormat::Markdown => "md",
+            OutputFormat::Json => "json",
+        }
+    }
 }
 
 impl LintcheckConfig {
@@ -53,7 +78,7 @@ impl LintcheckConfig {
         config.lintcheck_results_path = PathBuf::from(format!(
             "lintcheck-logs/{}_logs.{}",
             filename.display(),
-            if config.markdown { "md" } else { "txt" }
+            config.format.file_extension(),
         ));
 
         // look at the --threads arg, if 0 is passed, use the threads count

lintcheck/src/json.rs

@@ -0,0 +1,122 @@
+use std::collections::HashMap;
+use std::fmt::Write;
+use std::fs;
+use std::hash::Hash;
+use std::path::Path;
+
+use crate::ClippyWarning;
+
+/// Creates the log file output for [`crate::config::OutputFormat::Json`]
+pub(crate) fn output(clippy_warnings: &[ClippyWarning]) -> String {
+    serde_json::to_string(&clippy_warnings).unwrap()
+}
+
+fn load_warnings(path: &Path) -> Vec<ClippyWarning> {
+    let file = fs::read(path).unwrap_or_else(|e| panic!("failed to read {}: {e}", path.display()));
+
+    serde_json::from_slice(&file).unwrap_or_else(|e| panic!("failed to deserialize {}: {e}", path.display()))
+}
+
+/// Group warnings by their primary span location + lint name
+fn create_map(warnings: &[ClippyWarning]) -> HashMap<impl Eq + Hash + '_, Vec<&ClippyWarning>> {
+    let mut map = HashMap::<_, Vec<_>>::with_capacity(warnings.len());
+
+    for warning in warnings {
+        let span = warning.span();
+        let key = (&warning.lint_type, &span.file_name, span.byte_start, span.byte_end);
+
+        map.entry(key).or_default().push(warning);
+    }
+
+    map
+}
+
+fn print_warnings(title: &str, warnings: &[&ClippyWarning]) {
+    if warnings.is_empty() {
+        return;
+    }
+
+    println!("### {title}");
+    println!("```");
+    for warning in warnings {
+        print!("{}", warning.diag);
+    }
+    println!("```");
+}
+
+fn print_changed_diff(changed: &[(&[&ClippyWarning], &[&ClippyWarning])]) {
+    fn render(warnings: &[&ClippyWarning]) -> String {
+        let mut rendered = String::new();
+        for warning in warnings {
+            write!(&mut rendered, "{}", warning.diag).unwrap();
+        }
+        rendered
+    }
+
+    if changed.is_empty() {
+        return;
+    }
+
+    println!("### Changed");
+    println!("```diff");
+    for &(old, new) in changed {
+        let old_rendered = render(old);
+        let new_rendered = render(new);
+
+        for change in diff::lines(&old_rendered, &new_rendered) {
+            use diff::Result::{Both, Left, Right};
+
+            match change {
+                Both(unchanged, _) => {
+                    println!(" {unchanged}");
+                },
+                Left(removed) => {
+                    println!("-{removed}");
+                },
+                Right(added) => {
+                    println!("+{added}");
+                },
+            }
+        }
+    }
+    println!("```");
+}
+
+pub(crate) fn diff(old_path: &Path, new_path: &Path) {
+    let old_warnings = load_warnings(old_path);
+    let new_warnings = load_warnings(new_path);
+
+    let old_map = create_map(&old_warnings);
+    let new_map = create_map(&new_warnings);
+
+    let mut added = Vec::new();
+    let mut removed = Vec::new();
+    let mut changed = Vec::new();
+
+    for (key, new) in &new_map {
+        if let Some(old) = old_map.get(key) {
+            if old != new {
+                changed.push((old.as_slice(), new.as_slice()));
+            }
+        } else {
+            added.extend(new);
+        }
+    }
+
+    for (key, old) in &old_map {
+        if !new_map.contains_key(key) {
+            removed.extend(old);
+        }
+    }
+
+    print!(
+        "{} added, {} removed, {} changed\n\n",
+        added.len(),
+        removed.len(),
+        changed.len()
+    );
+
+    print_warnings("Added", &added);
+    print_warnings("Removed", &removed);
+    print_changed_diff(&changed);
+}

lintcheck/src/main.rs

@@ -12,18 +12,20 @@
     unused_lifetimes,
     unused_qualifications
 )]
-#![allow(clippy::collapsible_else_if)]
+#![allow(clippy::collapsible_else_if, clippy::needless_borrows_for_generic_args)]
 
 mod config;
 mod driver;
+mod json;
 mod recursive;
 
-use crate::config::LintcheckConfig;
+use crate::config::{Commands, LintcheckConfig, OutputFormat};
 use crate::recursive::LintcheckServer;
 
 use std::collections::{HashMap, HashSet};
 use std::env::consts::EXE_SUFFIX;
-use std::fmt::{self, Write as _};
+use std::fmt::{self, Display, Write as _};
+use std::hash::Hash;
 use std::io::{self, ErrorKind};
 use std::path::{Path, PathBuf};
 use std::process::{Command, ExitStatus};
@@ -31,7 +33,7 @@ use std::sync::atomic::{AtomicUsize, Ordering};
 use std::time::Duration;
 use std::{env, fs, thread};
 
-use cargo_metadata::diagnostic::Diagnostic;
+use cargo_metadata::diagnostic::{Diagnostic, DiagnosticSpan};
 use cargo_metadata::Message;
 use rayon::prelude::*;
 use serde::{Deserialize, Serialize};
@@ -111,6 +113,17 @@ struct RustcIce {
     pub crate_name: String,
     pub ice_content: String,
 }
+
+impl Display for RustcIce {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "{}:\n{}\n========================================\n",
+            self.crate_name, self.ice_content
+        )
+    }
+}
+
 impl RustcIce {
     pub fn from_stderr_and_status(crate_name: &str, status: ExitStatus, stderr: &str) -> Option<Self> {
         if status.code().unwrap_or(0) == 101
@@ -127,60 +140,58 @@ impl RustcIce {
 }
 
 /// A single warning that clippy issued while checking a `Crate`
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
 struct ClippyWarning {
-    file: String,
+    crate_name: String,
-    line: usize,
+    crate_version: String,
-    column: usize,
     lint_type: String,
-    message: String,
+    diag: Diagnostic,
 }
 
 #[allow(unused)]
 impl ClippyWarning {
-    fn new(diag: Diagnostic, crate_name: &str, crate_version: &str) -> Option<Self> {
+    fn new(mut diag: Diagnostic, crate_name: &str, crate_version: &str) -> Option<Self> {
-        let lint_type = diag.code?.code;
+        let lint_type = diag.code.clone()?.code;
         if !(lint_type.contains("clippy") || diag.message.contains("clippy"))
             || diag.message.contains("could not read cargo metadata")
         {
             return None;
         }
 
-        let span = diag.spans.into_iter().find(|span| span.is_primary)?;
+        // --recursive bypasses cargo so we have to strip the rendered output ourselves
-
+        let rendered = diag.rendered.as_mut().unwrap();
-        let file = if let Ok(stripped) = Path::new(&span.file_name).strip_prefix(env!("CARGO_HOME")) {
+        *rendered = String::from_utf8(strip_ansi_escapes::strip(&rendered).unwrap()).unwrap();
-            format!("$CARGO_HOME/{}", stripped.display())
-        } else {
-            format!(
-                "target/lintcheck/sources/{crate_name}-{crate_version}/{}",
-                span.file_name
-            )
-        };
 
         Some(Self {
-            file,
+            crate_name: crate_name.to_owned(),
-            line: span.line_start,
+            crate_version: crate_version.to_owned(),
-            column: span.column_start,
             lint_type,
-            message: diag.message,
+            diag,
         })
     }
 
-    fn to_output(&self, markdown: bool) -> String {
+    fn span(&self) -> &DiagnosticSpan {
-        let file_with_pos = format!("{}:{}:{}", &self.file, &self.line, &self.column);
+        self.diag.spans.iter().find(|span| span.is_primary).unwrap()
-        if markdown {
+    }
-            let mut file = self.file.clone();
-            if !file.starts_with('$') {
-                file.insert_str(0, "../");
-            }
 
-            let mut output = String::from("| ");
+    fn to_output(&self, format: OutputFormat) -> String {
-            let _: fmt::Result = write!(output, "[`{file_with_pos}`]({file}#L{})", self.line);
+        let span = self.span();
-            let _: fmt::Result = write!(output, r#" | `{:<50}` | "{}" |"#, self.lint_type, self.message);
+        let mut file = span.file_name.clone();
-            output.push('\n');
+        let file_with_pos = format!("{file}:{}:{}", span.line_start, span.line_end);
-            output
+        match format {
-        } else {
+            OutputFormat::Text => format!("{file_with_pos} {} \"{}\"\n", self.lint_type, self.diag.message),
-            format!("{file_with_pos} {} \"{}\"\n", self.lint_type, self.message)
+            OutputFormat::Markdown => {
+                if file.starts_with("target") {
+                    file.insert_str(0, "../");
+                }
+
+                let mut output = String::from("| ");
+                write!(output, "[`{file_with_pos}`]({file}#L{})", span.line_start).unwrap();
+                write!(output, r#" | `{:<50}` | "{}" |"#, self.lint_type, self.diag.message).unwrap();
+                output.push('\n');
+                output
+            },
+            OutputFormat::Json => unreachable!("JSON output is handled via serde"),
         }
     }
 }
@@ -333,7 +344,7 @@ impl CrateSource {
 impl Crate {
     /// Run `cargo clippy` on the `Crate` and collect and return all the lint warnings that clippy
     /// issued
-    #[allow(clippy::too_many_arguments)]
+    #[allow(clippy::too_many_arguments, clippy::too_many_lines)]
     fn run_clippy_lints(
         &self,
         cargo_clippy_path: &Path,
@@ -372,7 +383,25 @@ impl Crate {
             vec!["--quiet", "--message-format=json", "--"]
         };
 
-        let mut clippy_args = Vec::<&str>::new();
+        let cargo_home = env!("CARGO_HOME");
+
+        // `src/lib.rs` -> `target/lintcheck/sources/crate-1.2.3/src/lib.rs`
+        let remap_relative = format!("={}", self.path.display());
+        // Fallback for other sources, `~/.cargo/...` -> `$CARGO_HOME/...`
+        let remap_cargo_home = format!("{cargo_home}=$CARGO_HOME");
+        // `~/.cargo/registry/src/github.com-1ecc6299db9ec823/crate-2.3.4/src/lib.rs`
+        //     -> `crate-2.3.4/src/lib.rs`
+        let remap_crates_io = format!("{cargo_home}/registry/src/github.com-1ecc6299db9ec823/=");
+
+        let mut clippy_args = vec![
+            "--remap-path-prefix",
+            &remap_relative,
+            "--remap-path-prefix",
+            &remap_cargo_home,
+            "--remap-path-prefix",
+            &remap_crates_io,
+        ];
+
         if let Some(options) = &self.options {
             for opt in options {
                 clippy_args.push(opt);
@@ -554,10 +583,10 @@ fn read_crates(toml_path: &Path) -> (Vec<CrateSource>, RecursiveOptions) {
 }
 
 /// Generate a short list of occurring lints-types and their count
-fn gather_stats(clippy_warnings: &[ClippyWarning]) -> (String, HashMap<&String, usize>) {
+fn gather_stats(warnings: &[ClippyWarning]) -> (String, HashMap<&String, usize>) {
     // count lint type occurrences
     let mut counter: HashMap<&String, usize> = HashMap::new();
-    clippy_warnings
+    warnings
         .iter()
         .for_each(|wrn| *counter.entry(&wrn.lint_type).or_insert(0) += 1);
 
@@ -595,6 +624,11 @@ fn main() {
 
     let config = LintcheckConfig::new();
 
+    if let Some(Commands::Diff { old, new }) = config.subcommand {
+        json::diff(&old, &new);
+        return;
+    }
+
     println!("Compiling clippy...");
     build_clippy();
     println!("Done compiling");
@@ -619,7 +653,6 @@ fn main() {
     // flatten into one big list of warnings
 
     let (crates, recursive_options) = read_crates(&config.sources_toml_path);
-    let old_stats = read_stats_from_file(&config.lintcheck_results_path);
 
     let counter = AtomicUsize::new(1);
     let lint_filter: Vec<String> = config
@@ -711,39 +744,51 @@ fn main() {
         }
     }
 
-    // generate some stats
+    let text = match config.format {
-    let (stats_formatted, new_stats) = gather_stats(&warnings);
+        OutputFormat::Text | OutputFormat::Markdown => output(&warnings, &raw_ices, clippy_ver, &config),
+        OutputFormat::Json => {
+            if !raw_ices.is_empty() {
+                for ice in raw_ices {
+                    println!("{ice}");
+                }
+                panic!("Some crates ICEd");
+            }
 
-    let mut all_msgs: Vec<String> = warnings.iter().map(|warn| warn.to_output(config.markdown)).collect();
+            json::output(&warnings)
+        },
+    };
+
+    println!("Writing logs to {}", config.lintcheck_results_path.display());
+    fs::create_dir_all(config.lintcheck_results_path.parent().unwrap()).unwrap();
+    fs::write(&config.lintcheck_results_path, text).unwrap();
+}
+
+/// Creates the log file output for [`OutputFormat::Text`] and [`OutputFormat::Markdown`]
+fn output(warnings: &[ClippyWarning], ices: &[RustcIce], clippy_ver: String, config: &LintcheckConfig) -> String {
+    // generate some stats
+    let (stats_formatted, new_stats) = gather_stats(warnings);
+    let old_stats = read_stats_from_file(&config.lintcheck_results_path);
+
+    let mut all_msgs: Vec<String> = warnings.iter().map(|warn| warn.to_output(config.format)).collect();
     all_msgs.sort();
     all_msgs.push("\n\n### Stats:\n\n".into());
     all_msgs.push(stats_formatted);
 
-    // save the text into lintcheck-logs/logs.txt
     let mut text = clippy_ver; // clippy version number on top
     text.push_str("\n### Reports\n\n");
-    if config.markdown {
+    if config.format == OutputFormat::Markdown {
         text.push_str("| file | lint | message |\n");
         text.push_str("| --- | --- | --- |\n");
     }
     write!(text, "{}", all_msgs.join("")).unwrap();
     text.push_str("\n\n### ICEs:\n");
-    for ice in &raw_ices {
+    for ice in ices {
-        let _: fmt::Result = write!(
+        writeln!(text, "{ice}").unwrap();
-            text,
-            "{}:\n{}\n========================================\n\n",
-            ice.crate_name, ice.ice_content
-        );
     }
 
-    println!("Writing logs to {}", config.lintcheck_results_path.display());
-    if !raw_ices.is_empty() {
-        println!("WARNING: at least one ICE reported, check log file");
-    }
-    fs::create_dir_all(config.lintcheck_results_path.parent().unwrap()).unwrap();
-    fs::write(&config.lintcheck_results_path, text).unwrap();
-
     print_stats(old_stats, new_stats, &config.lint_filter);
+
+    text
 }
 
 /// read the previous stats from the lintcheck-log file