diff --git a/lintcheck/Cargo.toml b/lintcheck/Cargo.toml index ae9e77b8eed..3c86dfe324f 100644 --- a/lintcheck/Cargo.toml +++ b/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" diff --git a/lintcheck/src/driver.rs b/lintcheck/src/driver.rs index 47724a2fedb..041be5081f2 100644 --- a/lintcheck/src/driver.rs +++ b/lintcheck/src/driver.rs @@ -11,8 +11,6 @@ fn run_clippy(addr: &str) -> Option { 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()); diff --git a/lintcheck/src/json.rs b/lintcheck/src/json.rs index 43d0413c7ce..1a652927988 100644 --- a/lintcheck/src/json.rs +++ b/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`] -pub(crate) fn output(clippy_warnings: &[ClippyWarning]) -> String { - serde_json::to_string(&clippy_warnings).unwrap() +#[derive(Deserialize, Serialize)] +struct LintJson { + lint: String, + file_name: String, + byte_pos: (u32, u32), + rendered: String, } -fn load_warnings(path: &Path) -> Vec { +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) -> String { + let mut lints: Vec = 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 { 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> { - 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]) { +fn print_warnings(title: &str, warnings: &[LintJson]) { 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 render(warnings: &[&ClippyWarning]) -> String { - let mut rendered = String::new(); - for warning in warnings { - write!(&mut rendered, "{}", warning.diag).unwrap(); - } - rendered - } - +fn print_changed_diff(changed: &[(LintJson, LintJson)]) { 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) { + for (old, new) in changed { + 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 { - 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); + for change in itertools::merge_join_by(old_warnings, new_warnings, |old, new| old.key().cmp(&new.key())) { + match change { + EitherOrBoth::Both(old, new) => { + if old.rendered != new.rendered { + changed.push((old, new)); + } + }, + EitherOrBoth::Left(old) => removed.push(old), + EitherOrBoth::Right(new) => added.push(new), } } diff --git a/lintcheck/src/main.rs b/lintcheck/src/main.rs index 70b8af0fdb9..26a67beb442 100644 --- a/lintcheck/src/main.rs +++ b/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, - crate_version: String, - lint_type: String, + lint: String, diag: Diagnostic, } #[allow(unused)] impl ClippyWarning { - fn new(mut diag: Diagnostic, crate_name: &str, crate_version: &str) -> Option { - let lint_type = diag.code.clone()?.code; - if !(lint_type.contains("clippy") || diag.message.contains("clippy")) + fn new(mut diag: Diagnostic) -> Option { + let lint = diag.code.clone()?.code; + 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 &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 = 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) }, }; diff --git a/lintcheck/src/recursive.rs b/lintcheck/src/recursive.rs index 994fa3c3b23..24dddfe6563 100644 --- a/lintcheck/src/recursive.rs +++ b/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(value: &T, writer: &mut W) @@ -65,7 +63,7 @@ fn process_stream( let messages = stderr .lines() .filter_map(|json_msg| serde_json::from_str::(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();