diff options
| author | xFrednet <xFrednet@gmail.com> | 2024-07-22 17:38:03 +0200 |
|---|---|---|
| committer | xFrednet <xFrednet@gmail.com> | 2024-07-22 18:02:39 +0200 |
| commit | 1f879fc0cf423c1ee1b8f30dcdbf0a708061c8be (patch) | |
| tree | 479c5cce6709e61e053cfb6d46ba220129b638d6 | |
| parent | 4fea5199e4936692b2682a61a9205077604b9bbb (diff) | |
| download | rust-1f879fc0cf423c1ee1b8f30dcdbf0a708061c8be.tar.gz rust-1f879fc0cf423c1ee1b8f30dcdbf0a708061c8be.zip | |
Lintcheck: Minor cleanup and function moves
| -rw-r--r-- | lintcheck/src/json.rs | 216 |
1 files changed, 106 insertions, 110 deletions
diff --git a/lintcheck/src/json.rs b/lintcheck/src/json.rs index 8cee26b4ccc..2fa527c5f87 100644 --- a/lintcheck/src/json.rs +++ b/lintcheck/src/json.rs @@ -1,7 +1,4 @@ -#![expect(unused)] - use std::collections::BTreeSet; -use std::fmt::Write; use std::fs; use std::path::Path; @@ -10,6 +7,7 @@ use serde::{Deserialize, Serialize}; use crate::ClippyWarning; +/// This is the total number. 300 warnings results in 100 messages per section. const DEFAULT_LIMIT_PER_LINT: usize = 300; const TRUNCATION_TOTAL_TARGET: usize = 1000; @@ -59,77 +57,6 @@ fn load_warnings(path: &Path) -> Vec<LintJson> { serde_json::from_slice(&file).unwrap_or_else(|e| panic!("failed to deserialize {}: {e}", path.display())) } -fn print_warnings(title: &str, warnings: &[LintJson], truncate_after: usize) { - if warnings.is_empty() { - return; - } - - print_h3(&warnings[0].lint, title); - println!(); - - let warnings = truncate(warnings, truncate_after); - - for warning in warnings { - println!("{}", warning.info_text(title)); - println!(); - println!("```"); - println!("{}", warning.rendered.trim_end()); - println!("```"); - println!(); - } -} - -fn print_changed_diff(changed: &[(LintJson, LintJson)], truncate_after:usize) { - if changed.is_empty() { - return; - } - - print_h3(&changed[0].0.lint, "Changed"); - println!(); - - let changes = truncate(changed, truncate_after); - - for (old, new) in changed { - println!("{}", new.info_text("Changed")); - println!(); - println!("```diff"); - for change in diff::lines(old.rendered.trim_end(), new.rendered.trim_end()) { - use diff::Result::{Both, Left, Right}; - - match change { - Both(unchanged, _) => { - println!(" {unchanged}"); - }, - Left(removed) => { - println!("-{removed}"); - }, - Right(added) => { - println!("+{added}"); - }, - } - } - println!("```"); - } -} - -fn print_h3(lint: &str, title: &str) { - let html_id = to_html_id(lint); - // We have to use HTML here to be able to manually add an id. - println!(r#"<h3 id="user-content-{html_id}-{title}">{title}</h3>"#); -} - -fn truncate<T>(list: &[T], truncate_after: usize) -> &[T] { - if list.len() > truncate_after { - println!("{} warnings have been truncated for this summary.", list.len() - truncate_after); - println!(); - - list.split_at(truncate_after).0 - } else { - list - } - -} - pub(crate) fn diff(old_path: &Path, new_path: &Path, truncate: bool) { let old_warnings = load_warnings(old_path); let new_warnings = load_warnings(new_path); @@ -156,8 +83,10 @@ pub(crate) fn diff(old_path: &Path, new_path: &Path, truncate: bool) { println!(); let truncate_after = if truncate { - // Max 9 ensures that we at least have three messages per lint - DEFAULT_LIMIT_PER_LINT.min(TRUNCATION_TOTAL_TARGET / lint_warnings.len()).max(9) + // Max 15 ensures that we at least have five messages per lint + DEFAULT_LIMIT_PER_LINT + .min(TRUNCATION_TOTAL_TARGET / lint_warnings.len()) + .max(15) } else { // No lint should ever each this number of lint emissions, so this is equivialent to // No truncation @@ -165,31 +94,10 @@ pub(crate) fn diff(old_path: &Path, new_path: &Path, truncate: bool) { }; for lint in lint_warnings { - print_lint_warnings(&lint, truncate_after) + print_lint_warnings(&lint, truncate_after); } } -fn print_lint_warnings(lint: &LintWarnings, truncate_after: usize) { - let name = &lint.name; - let html_id = to_html_id(name); - - // The additional anchor is added for non GH viewers that don't prefix ID's - println!(r#"<h2 id="user-content-{html_id}">{name}</h2>"#); - println!(); - - print!( - r##"{}, {}, {}"##, - count_string(name, "added", lint.added.len()), - count_string(name, "removed", lint.removed.len()), - count_string(name, "changed", lint.changed.len()), - ); - println!(); - - print_warnings("Added", &lint.added, truncate_after / 3); - print_warnings("Removed", &lint.removed, truncate_after / 3); - print_changed_diff(&lint.changed, truncate_after / 3); -} - #[derive(Debug)] struct LintWarnings { name: String, @@ -209,7 +117,7 @@ fn group_by_lint( F: FnMut(&T) -> bool, { let mut items = vec![]; - while iter.peek().map_or(false, |item| condition(item)) { + while iter.peek().map_or(false, &mut condition) { items.push(iter.next().unwrap()); } items @@ -234,7 +142,7 @@ fn group_by_lint( let mut changed_iter = changed.into_iter().peekable(); let mut lints = vec![]; - for name in lint_names.into_iter() { + for name in lint_names { lints.push(LintWarnings { added: collect_while(&mut added_iter, |warning| warning.lint == name), removed: collect_while(&mut removed_iter, |warning| warning.lint == name), @@ -246,13 +154,26 @@ fn group_by_lint( lints } -// This function limits the number of lints in the collection if needed. -// -// It's honestly amazing that a function like this is needed. But some restriction -// lints (Looking at you `implicit_return` and `if_then_some_else_none`) trigger a lot -// like 60'000+ times in our CI. -// -// Each lint in the list will be limited to 200 messages +fn print_lint_warnings(lint: &LintWarnings, truncate_after: usize) { + let name = &lint.name; + let html_id = to_html_id(name); + + // The additional anchor is added for non GH viewers that don't prefix ID's + println!(r#"## `{name}` <a id="user-content-{html_id}"/>"#); + println!(); + + print!( + r##"{}, {}, {}"##, + count_string(name, "added", lint.added.len()), + count_string(name, "removed", lint.removed.len()), + count_string(name, "changed", lint.changed.len()), + ); + println!(); + + print_warnings("Added", &lint.added, truncate_after / 3); + print_warnings("Removed", &lint.removed, truncate_after / 3); + print_changed_diff(&lint.changed, truncate_after / 3); +} fn print_summary_table(lints: &[LintWarnings]) { println!("| Lint | Added | Removed | Changed |"); @@ -269,8 +190,83 @@ fn print_summary_table(lints: &[LintWarnings]) { } } +fn print_warnings(title: &str, warnings: &[LintJson], truncate_after: usize) { + if warnings.is_empty() { + return; + } + + print_h3(&warnings[0].lint, title); + println!(); + + let warnings = truncate(warnings, truncate_after); + + for warning in warnings { + println!("{}", warning.info_text(title)); + println!(); + println!("```"); + println!("{}", warning.rendered.trim_end()); + println!("```"); + println!(); + } +} + +fn print_changed_diff(changed: &[(LintJson, LintJson)], truncate_after: usize) { + if changed.is_empty() { + return; + } + + print_h3(&changed[0].0.lint, "Changed"); + println!(); + + let changed = truncate(changed, truncate_after); + + for (old, new) in changed { + println!("{}", new.info_text("Changed")); + println!(); + println!("```diff"); + for change in diff::lines(old.rendered.trim_end(), new.rendered.trim_end()) { + use diff::Result::{Both, Left, Right}; + + match change { + Both(unchanged, _) => { + println!(" {unchanged}"); + }, + Left(removed) => { + println!("-{removed}"); + }, + Right(added) => { + println!("+{added}"); + }, + } + } + println!("```"); + } +} + +fn truncate<T>(list: &[T], truncate_after: usize) -> &[T] { + if list.len() > truncate_after { + println!( + "{} warnings have been truncated for this summary.", + list.len() - truncate_after + ); + println!(); + + list.split_at(truncate_after).0 + } else { + list + } +} + +fn print_h3(lint: &str, title: &str) { + let html_id = to_html_id(lint); + // We have to use HTML here to be able to manually add an id. + println!(r#"### {title} <a id="user-content-{html_id}-{title}"/>"#); +} + +/// GitHub's markdown parsers doesn't like IDs with `::` and `_`. This simplifies +/// the lint name for the HTML ID. fn to_html_id(lint_name: &str) -> String { - lint_name.replace("clippy::", "").replace("_", "-") + lint_name.replace("clippy::", "").replace('_', "-") } /// This generates the `x added` string for the start of the job summery. @@ -281,10 +277,10 @@ fn count_string(lint: &str, label: &str, count: usize) -> String { if count == 0 { format!("0 {label}") } else { - let lint_id = to_html_id(lint); + let html_id = to_html_id(lint); // GitHub's job summaries don't add HTML ids to headings. That's why we // manually have to add them. GitHub prefixes these manual ids with // `user-content-` and that's how we end up with these awesome links :D - format!("[{count} {label}](#user-content-{lint_id}-{label})") + format!("[{count} {label}](#user-content-{html_id}-{label})") } } |
