Skip to content

Commit b67908a

Browse files
committed
Lintcheck: Minor cleanup and function moves
1 parent 4fea519 commit b67908a

File tree

1 file changed

+106
-107
lines changed

1 file changed

+106
-107
lines changed

lintcheck/src/json.rs

+106-107
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use serde::{Deserialize, Serialize};
1010

1111
use crate::ClippyWarning;
1212

13+
/// This is the total number. 300 warnings results in 100 messages per section.
1314
const DEFAULT_LIMIT_PER_LINT: usize = 300;
1415
const TRUNCATION_TOTAL_TARGET: usize = 1000;
1516

@@ -59,77 +60,6 @@ fn load_warnings(path: &Path) -> Vec<LintJson> {
5960
serde_json::from_slice(&file).unwrap_or_else(|e| panic!("failed to deserialize {}: {e}", path.display()))
6061
}
6162

62-
fn print_warnings(title: &str, warnings: &[LintJson], truncate_after: usize) {
63-
if warnings.is_empty() {
64-
return;
65-
}
66-
67-
print_h3(&warnings[0].lint, title);
68-
println!();
69-
70-
let warnings = truncate(warnings, truncate_after);
71-
72-
for warning in warnings {
73-
println!("{}", warning.info_text(title));
74-
println!();
75-
println!("```");
76-
println!("{}", warning.rendered.trim_end());
77-
println!("```");
78-
println!();
79-
}
80-
}
81-
82-
fn print_changed_diff(changed: &[(LintJson, LintJson)], truncate_after:usize) {
83-
if changed.is_empty() {
84-
return;
85-
}
86-
87-
print_h3(&changed[0].0.lint, "Changed");
88-
println!();
89-
90-
let changes = truncate(changed, truncate_after);
91-
92-
for (old, new) in changed {
93-
println!("{}", new.info_text("Changed"));
94-
println!();
95-
println!("```diff");
96-
for change in diff::lines(old.rendered.trim_end(), new.rendered.trim_end()) {
97-
use diff::Result::{Both, Left, Right};
98-
99-
match change {
100-
Both(unchanged, _) => {
101-
println!(" {unchanged}");
102-
},
103-
Left(removed) => {
104-
println!("-{removed}");
105-
},
106-
Right(added) => {
107-
println!("+{added}");
108-
},
109-
}
110-
}
111-
println!("```");
112-
}
113-
}
114-
115-
fn print_h3(lint: &str, title: &str) {
116-
let html_id = to_html_id(lint);
117-
// We have to use HTML here to be able to manually add an id.
118-
println!(r#"<h3 id="user-content-{html_id}-{title}">{title}</h3>"#);
119-
}
120-
121-
fn truncate<T>(list: &[T], truncate_after: usize) -> &[T] {
122-
if list.len() > truncate_after {
123-
println!("{} warnings have been truncated for this summary.", list.len() - truncate_after);
124-
println!();
125-
126-
list.split_at(truncate_after).0
127-
} else {
128-
list
129-
}
130-
131-
}
132-
13363
pub(crate) fn diff(old_path: &Path, new_path: &Path, truncate: bool) {
13464
let old_warnings = load_warnings(old_path);
13565
let new_warnings = load_warnings(new_path);
@@ -156,40 +86,21 @@ pub(crate) fn diff(old_path: &Path, new_path: &Path, truncate: bool) {
15686
println!();
15787

15888
let truncate_after = if truncate {
159-
// Max 9 ensures that we at least have three messages per lint
160-
DEFAULT_LIMIT_PER_LINT.min(TRUNCATION_TOTAL_TARGET / lint_warnings.len()).max(9)
89+
// Max 15 ensures that we at least have five messages per lint
90+
DEFAULT_LIMIT_PER_LINT
91+
.min(TRUNCATION_TOTAL_TARGET / lint_warnings.len())
92+
.max(15)
16193
} else {
16294
// No lint should ever each this number of lint emissions, so this is equivialent to
16395
// No truncation
16496
usize::MAX
16597
};
16698

16799
for lint in lint_warnings {
168-
print_lint_warnings(&lint, truncate_after)
100+
print_lint_warnings(&lint, truncate_after);
169101
}
170102
}
171103

172-
fn print_lint_warnings(lint: &LintWarnings, truncate_after: usize) {
173-
let name = &lint.name;
174-
let html_id = to_html_id(name);
175-
176-
// The additional anchor is added for non GH viewers that don't prefix ID's
177-
println!(r#"<h2 id="user-content-{html_id}">{name}</h2>"#);
178-
println!();
179-
180-
print!(
181-
r##"{}, {}, {}"##,
182-
count_string(name, "added", lint.added.len()),
183-
count_string(name, "removed", lint.removed.len()),
184-
count_string(name, "changed", lint.changed.len()),
185-
);
186-
println!();
187-
188-
print_warnings("Added", &lint.added, truncate_after / 3);
189-
print_warnings("Removed", &lint.removed, truncate_after / 3);
190-
print_changed_diff(&lint.changed, truncate_after / 3);
191-
}
192-
193104
#[derive(Debug)]
194105
struct LintWarnings {
195106
name: String,
@@ -209,7 +120,7 @@ fn group_by_lint(
209120
F: FnMut(&T) -> bool,
210121
{
211122
let mut items = vec![];
212-
while iter.peek().map_or(false, |item| condition(item)) {
123+
while iter.peek().map_or(false, &mut condition) {
213124
items.push(iter.next().unwrap());
214125
}
215126
items
@@ -234,7 +145,7 @@ fn group_by_lint(
234145
let mut changed_iter = changed.into_iter().peekable();
235146

236147
let mut lints = vec![];
237-
for name in lint_names.into_iter() {
148+
for name in lint_names {
238149
lints.push(LintWarnings {
239150
added: collect_while(&mut added_iter, |warning| warning.lint == name),
240151
removed: collect_while(&mut removed_iter, |warning| warning.lint == name),
@@ -246,13 +157,26 @@ fn group_by_lint(
246157
lints
247158
}
248159

249-
// This function limits the number of lints in the collection if needed.
250-
//
251-
// It's honestly amazing that a function like this is needed. But some restriction
252-
// lints (Looking at you `implicit_return` and `if_then_some_else_none`) trigger a lot
253-
// like 60'000+ times in our CI.
254-
//
255-
// Each lint in the list will be limited to 200 messages
160+
fn print_lint_warnings(lint: &LintWarnings, truncate_after: usize) {
161+
let name = &lint.name;
162+
let html_id = to_html_id(name);
163+
164+
// The additional anchor is added for non GH viewers that don't prefix ID's
165+
println!(r#"## `{name}` <a id="user-content-{html_id}"/>"#);
166+
println!();
167+
168+
print!(
169+
r##"{}, {}, {}"##,
170+
count_string(name, "added", lint.added.len()),
171+
count_string(name, "removed", lint.removed.len()),
172+
count_string(name, "changed", lint.changed.len()),
173+
);
174+
println!();
175+
176+
print_warnings("Added", &lint.added, truncate_after / 3);
177+
print_warnings("Removed", &lint.removed, truncate_after / 3);
178+
print_changed_diff(&lint.changed, truncate_after / 3);
179+
}
256180

257181
fn print_summary_table(lints: &[LintWarnings]) {
258182
println!("| Lint | Added | Removed | Changed |");
@@ -269,8 +193,83 @@ fn print_summary_table(lints: &[LintWarnings]) {
269193
}
270194
}
271195

196+
fn print_warnings(title: &str, warnings: &[LintJson], truncate_after: usize) {
197+
if warnings.is_empty() {
198+
return;
199+
}
200+
201+
print_h3(&warnings[0].lint, title);
202+
println!();
203+
204+
let warnings = truncate(warnings, truncate_after);
205+
206+
for warning in warnings {
207+
println!("{}", warning.info_text(title));
208+
println!();
209+
println!("```");
210+
println!("{}", warning.rendered.trim_end());
211+
println!("```");
212+
println!();
213+
}
214+
}
215+
216+
fn print_changed_diff(changed: &[(LintJson, LintJson)], truncate_after: usize) {
217+
if changed.is_empty() {
218+
return;
219+
}
220+
221+
print_h3(&changed[0].0.lint, "Changed");
222+
println!();
223+
224+
let changed = truncate(changed, truncate_after);
225+
226+
for (old, new) in changed {
227+
println!("{}", new.info_text("Changed"));
228+
println!();
229+
println!("```diff");
230+
for change in diff::lines(old.rendered.trim_end(), new.rendered.trim_end()) {
231+
use diff::Result::{Both, Left, Right};
232+
233+
match change {
234+
Both(unchanged, _) => {
235+
println!(" {unchanged}");
236+
},
237+
Left(removed) => {
238+
println!("-{removed}");
239+
},
240+
Right(added) => {
241+
println!("+{added}");
242+
},
243+
}
244+
}
245+
println!("```");
246+
}
247+
}
248+
249+
fn truncate<T>(list: &[T], truncate_after: usize) -> &[T] {
250+
if list.len() > truncate_after {
251+
println!(
252+
"{} warnings have been truncated for this summary.",
253+
list.len() - truncate_after
254+
);
255+
println!();
256+
257+
list.split_at(truncate_after).0
258+
} else {
259+
list
260+
}
261+
}
262+
263+
fn print_h3(lint: &str, title: &str) {
264+
let html_id = to_html_id(lint);
265+
// We have to use HTML here to be able to manually add an id.
266+
println!(r#"### {title} <a id="user-content-{html_id}-{title}"/>"#);
267+
}
268+
269+
/// GitHub's markdown parsers doesn't like IDs with `::` and `_`. This simplifies
270+
/// the lint name for the HTML ID.
272271
fn to_html_id(lint_name: &str) -> String {
273-
lint_name.replace("clippy::", "").replace("_", "-")
272+
lint_name.replace("clippy::", "").replace('_', "-")
274273
}
275274

276275
/// This generates the `x added` string for the start of the job summery.
@@ -281,10 +280,10 @@ fn count_string(lint: &str, label: &str, count: usize) -> String {
281280
if count == 0 {
282281
format!("0 {label}")
283282
} else {
284-
let lint_id = to_html_id(lint);
283+
let html_id = to_html_id(lint);
285284
// GitHub's job summaries don't add HTML ids to headings. That's why we
286285
// manually have to add them. GitHub prefixes these manual ids with
287286
// `user-content-` and that's how we end up with these awesome links :D
288-
format!("[{count} {label}](#user-content-{lint_id}-{label})")
287+
format!("[{count} {label}](#user-content-{html_id}-{label})")
289288
}
290289
}

0 commit comments

Comments
 (0)