Lintcheck: Minor cleanup and function moves

Author: xFrednet

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,40 +83,21 @@ 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
         usize::MAX
     };
 
     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})")
     }
 }