Auto merge of #13064 - Alexendoo:lintcheck-shrink-json, r=xFrednet

Reduce the size of lintcheck JSON output

Saves about 80% of the size by picking out what we need rather than serialising the whole diagnostic

r? `@xFrednet`

changelog: none

lintcheck/Cargo.toml

@@ -16,6 +16,7 @@ clap = { version = "4.4", features = ["derive", "env"] }
 crossbeam-channel = "0.5.6"
 diff = "0.1.13"
 flate2 = "1.0"
+itertools = "0.12"
 rayon = "1.5.1"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0.85"

lintcheck/src/driver.rs

@@ -11,8 +11,6 @@
 fn run_clippy(addr: &str) -> Option<i32> {
     let driver_info = DriverInfo {
         package_name: env::var("CARGO_PKG_NAME").ok()?,
-        crate_name: env::var("CARGO_CRATE_NAME").ok()?,
-        version: env::var("CARGO_PKG_VERSION").ok()?,
     };
 
     let mut stream = BufReader::new(TcpStream::connect(addr).unwrap());

lintcheck/src/json.rs

@@ -1,37 +1,50 @@
-use std::collections::HashMap;
-use std::fmt::Write;
 use std::fs;
-use std::hash::Hash;
 use std::path::Path;
 
+use itertools::EitherOrBoth;
+use serde::{Deserialize, Serialize};
+
 use crate::ClippyWarning;
 
-/// Creates the log file output for [`crate::config::OutputFormat::Json`]
+#[derive(Deserialize, Serialize)]
-pub(crate) fn output(clippy_warnings: &[ClippyWarning]) -> String {
+struct LintJson {
-    serde_json::to_string(&clippy_warnings).unwrap()
+    lint: String,
+    file_name: String,
+    byte_pos: (u32, u32),
+    rendered: String,
 }
 
-fn load_warnings(path: &Path) -> Vec<ClippyWarning> {
+impl LintJson {
+    fn key(&self) -> impl Ord + '_ {
+        (self.file_name.as_str(), self.byte_pos, self.lint.as_str())
+    }
+}
+
+/// Creates the log file output for [`crate::config::OutputFormat::Json`]
+pub(crate) fn output(clippy_warnings: Vec<ClippyWarning>) -> String {
+    let mut lints: Vec<LintJson> = clippy_warnings
+        .into_iter()
+        .map(|warning| {
+            let span = warning.span();
+            LintJson {
+                file_name: span.file_name.clone(),
+                byte_pos: (span.byte_start, span.byte_end),
+                lint: warning.lint,
+                rendered: warning.diag.rendered.unwrap(),
+            }
+        })
+        .collect();
+    lints.sort_by(|a, b| a.key().cmp(&b.key()));
+    serde_json::to_string(&lints).unwrap()
+}
+
+fn load_warnings(path: &Path) -> Vec<LintJson> {
     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 print_warnings(title: &str, warnings: &[LintJson]) {
-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;
     }
@@ -39,31 +52,20 @@ fn print_warnings(title: &str, warnings: &[&ClippyWarning]) {
     println!("### {title}");
     println!("```");
     for warning in warnings {
-        print!("{}", warning.diag);
+        print!("{}", warning.rendered);
     }
     println!("```");
 }
 
-fn print_changed_diff(changed: &[(&[&ClippyWarning], &[&ClippyWarning])]) {
+fn print_changed_diff(changed: &[(LintJson, LintJson)]) {
-    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 {
+    for (old, new) in changed {
-        let old_rendered = render(old);
+        for change in diff::lines(&old.rendered, &new.rendered) {
-        let new_rendered = render(new);
-
-        for change in diff::lines(&old_rendered, &new_rendered) {
             use diff::Result::{Both, Left, Right};
 
             match change {
@@ -86,26 +88,19 @@ 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 {
+    for change in itertools::merge_join_by(old_warnings, new_warnings, |old, new| old.key().cmp(&new.key())) {
-        if let Some(old) = old_map.get(key) {
+        match change {
-            if old != new {
+            EitherOrBoth::Both(old, new) => {
-                changed.push((old.as_slice(), new.as_slice()));
+                if old.rendered != new.rendered {
+                    changed.push((old, new));
                 }
-        } else {
+            },
-            added.extend(new);
+            EitherOrBoth::Left(old) => removed.push(old),
-        }
+            EitherOrBoth::Right(new) => added.push(new),
-    }
-
-    for (key, old) in &old_map {
-        if !new_map.contains_key(key) {
-            removed.extend(old);
         }
     }
 

lintcheck/src/main.rs

@@ -38,7 +38,7 @@
 use cargo_metadata::diagnostic::{Diagnostic, DiagnosticSpan};
 use cargo_metadata::Message;
 use rayon::prelude::*;
-use serde::{Deserialize, Serialize};
+use serde::Deserialize;
 use walkdir::{DirEntry, WalkDir};
 
 const LINTCHECK_DOWNLOADS: &str = "target/lintcheck/downloads";
@@ -142,19 +142,17 @@ pub fn from_stderr_and_status(crate_name: &str, status: ExitStatus, stderr: &str
 }
 
 /// A single warning that clippy issued while checking a `Crate`
-#[derive(Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
+#[derive(Debug)]
 struct ClippyWarning {
-    crate_name: String,
+    lint: String,
-    crate_version: String,
-    lint_type: String,
     diag: Diagnostic,
 }
 
 #[allow(unused)]
 impl ClippyWarning {
-    fn new(mut diag: Diagnostic, crate_name: &str, crate_version: &str) -> Option<Self> {
+    fn new(mut diag: Diagnostic) -> Option<Self> {
-        let lint_type = diag.code.clone()?.code;
+        let lint = diag.code.clone()?.code;
-        if !(lint_type.contains("clippy") || diag.message.contains("clippy"))
+        if !(lint.contains("clippy") || diag.message.contains("clippy"))
             || diag.message.contains("could not read cargo metadata")
         {
             return None;
@@ -164,12 +162,7 @@ fn new(mut diag: Diagnostic, crate_name: &str, crate_version: &str) -> Option<Se
         let rendered = diag.rendered.as_mut().unwrap();
         *rendered = strip_ansi_escapes::strip_str(&rendered);
 
-        Some(Self {
+        Some(Self { lint, diag })
-            crate_name: crate_name.to_owned(),
-            crate_version: crate_version.to_owned(),
-            lint_type,
-            diag,
-        })
     }
 
     fn span(&self) -> &DiagnosticSpan {
@@ -181,7 +174,7 @@ fn to_output(&self, format: OutputFormat) -> String {
         let mut file = span.file_name.clone();
         let file_with_pos = format!("{file}:{}:{}", span.line_start, span.line_end);
         match format {
-            OutputFormat::Text => format!("{file_with_pos} {} \"{}\"\n", self.lint_type, self.diag.message),
+            OutputFormat::Text => format!("{file_with_pos} {} \"{}\"\n", self.lint, self.diag.message),
             OutputFormat::Markdown => {
                 if file.starts_with("target") {
                     file.insert_str(0, "../");
@@ -189,7 +182,7 @@ fn to_output(&self, format: OutputFormat) -> String {
 
                 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();
+                write!(output, r#" | `{:<50}` | "{}" |"#, self.lint, self.diag.message).unwrap();
                 output.push('\n');
                 output
             },
@@ -473,7 +466,7 @@ fn run_clippy_lints(
         // get all clippy warnings and ICEs
         let mut entries: Vec<ClippyCheckOutput> = Message::parse_stream(stdout.as_bytes())
             .filter_map(|msg| match msg {
-                Ok(Message::CompilerMessage(message)) => ClippyWarning::new(message.message, &self.name, &self.version),
+                Ok(Message::CompilerMessage(message)) => ClippyWarning::new(message.message),
                 _ => None,
             })
             .map(ClippyCheckOutput::ClippyWarning)
@@ -572,7 +565,7 @@ fn gather_stats(warnings: &[ClippyWarning]) -> (String, HashMap<&String, usize>)
     let mut counter: HashMap<&String, usize> = HashMap::new();
     warnings
         .iter()
-        .for_each(|wrn| *counter.entry(&wrn.lint_type).or_insert(0) += 1);
+        .for_each(|wrn| *counter.entry(&wrn.lint).or_insert(0) += 1);
 
     // collect into a tupled list for sorting
     let mut stats: Vec<(&&String, &usize)> = counter.iter().collect();
@@ -754,7 +747,7 @@ fn lintcheck(config: LintcheckConfig) {
                 panic!("Some crates ICEd");
             }
 
-            json::output(&warnings)
+            json::output(warnings)
         },
     };
 

lintcheck/src/recursive.rs

@@ -19,8 +19,6 @@
 #[derive(Debug, Eq, Hash, PartialEq, Clone, Serialize, Deserialize)]
 pub(crate) struct DriverInfo {
     pub package_name: String,
-    pub crate_name: String,
-    pub version: String,
 }
 
 pub(crate) fn serialize_line<T, W>(value: &T, writer: &mut W)
@@ -65,7 +63,7 @@ fn process_stream(
     let messages = stderr
         .lines()
         .filter_map(|json_msg| serde_json::from_str::<Diagnostic>(json_msg).ok())
-        .filter_map(|diag| ClippyWarning::new(diag, &driver_info.package_name, &driver_info.version));
+        .filter_map(ClippyWarning::new);
 
     for message in messages {
         sender.send(message).unwrap();