lintcheck: Add JSON output, diff subcommand

Author: Alexendoo

lintcheck/Cargo.toml

@@ -13,10 +13,12 @@ publish = false
 cargo_metadata = "0.14"
 clap = "3.2"
 crossbeam-channel = "0.5.6"
+diff = "0.1.13"
 flate2 = "1.0"
 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.5"
 ureq = "2.2"

lintcheck/src/config.rs

@@ -34,15 +34,41 @@ fn get_clap_config() -> ArgMatches {
             Arg::new("markdown")
                 .long("markdown")
                 .help("Change the reports table to use markdown links"),
+            Arg::new("json")
+                .long("json")
+                .help("Output the diagnostics as JSON")
+                .conflicts_with("markdown"),
             Arg::new("recursive")
                 .long("--recursive")
                 .help("Run clippy on the dependencies of crates specified in crates-toml")
                 .conflicts_with("threads")
                 .conflicts_with("fix"),
         ])
+        .subcommand(
+            Command::new("diff")
+                .about("Prints the difference between two `lintcheck --json` results")
+                .args([Arg::new("old").required(true), Arg::new("new").required(true)]),
+        )
         .get_matches()
 }
 
+#[derive(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",
+        }
+    }
+}
+
 #[derive(Debug, Clone)]
 pub(crate) struct LintcheckConfig {
     /// max number of jobs to spawn (default 1)
@@ -57,10 +83,12 @@ pub(crate) struct LintcheckConfig {
     pub fix: bool,
     /// A list of lints that this lintcheck run should focus on
     pub lint_filter: Vec<String>,
-    /// Indicate if the output should support markdown syntax
-    pub markdown: bool,
+    /// The output format of the log file
+    pub format: OutputFormat,
     /// Run clippy on the dependencies of crates
     pub recursive: bool,
+    /// Diff the two `lintcheck --json` results
+    pub diff: Option<(PathBuf, PathBuf)>,
 }
 
 impl LintcheckConfig {
@@ -77,7 +105,13 @@ impl LintcheckConfig {
                 .into()
         });
 
-        let markdown = clap_config.contains_id("markdown");
+        let format = if clap_config.contains_id("markdown") {
+            OutputFormat::Markdown
+        } else if clap_config.contains_id("json") {
+            OutputFormat::Json
+        } else {
+            OutputFormat::Text
+        };
         let sources_toml_path = PathBuf::from(sources_toml);
 
         // for the path where we save the lint results, get the filename without extension (so for
@@ -86,7 +120,7 @@ impl LintcheckConfig {
         let lintcheck_results_path = PathBuf::from(format!(
             "lintcheck-logs/{}_logs.{}",
             filename.display(),
-            if markdown { "md" } else { "txt" }
+            format.file_extension(),
         ));
 
         // look at the --threads arg, if 0 is passed, ask rayon rayon how many threads it would spawn and
@@ -117,15 +151,22 @@ impl LintcheckConfig {
             })
             .unwrap_or_default();
 
+        let diff = clap_config.subcommand_matches("diff").map(|args| {
+            let path = |arg| PathBuf::from(args.get_one::<String>(arg).unwrap());
+
+            (path("old"), path("new"))
+        });
+
         LintcheckConfig {
             max_jobs,
             sources_toml_path,
             lintcheck_results_path,
             only: clap_config.get_one::<String>("only").map(String::from),
             fix: clap_config.contains_id("fix"),
             lint_filter,
-            markdown,
+            format,
             recursive: clap_config.contains_id("recursive"),
+            diff,
         }
     }
 }

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);
+}