Lintcheck: Rework and limit diff output for GH's CI

[xFrednet]

Background

While working on #13136 I found an amazing limitation of GH's CI. The summary can at most have be 1MB of text. Here is the warning message:

$GITHUB_STEP_SUMMARY upload aborted, supports content up to a size of 1024k, got 46731k. For more information see: https://docs.github.com/actions/using-workflows/workflow-commands-for-github-actions#adding-a-markdown-summary

The PR produced a casual 61808 changes. Guess that's why those lints are not warn-by-default :P.

Changes:

This PR limits the lintcheck diff output in two ways.

1. The diff is limited to 200 messages per lint per section. Hidden messages are indicated by a message at the end of the section.
2. The output is first written to a file and only the first 1MB is written to >> $GITHUB_STEP_SUMMARY. The entire file is also written to the normal CI log. This helps for cases where several lints change and the total size exceeds the 1MB limit.

An example of these changes can be seen here: https://github.com/xFrednet/rust-clippy/actions/runs/10028799118?pr=4

---

changelog: none

r? @Alexendoo

Sorry for bombarding you with so many PR's lately 😅 Feel free to pass some of you reviews to me.

[flip1995]

2. The entire file is also written to the normal CI log. This helps for cases where several lints change and the total size exceeds the 1MB limit.

Could you upload the entire file as an artifact, like in the head and base steps? This should make it easier to just download the file from the action summary page, if someone wants to do triage.

[xFrednet]

Could you upload the entire file as an artifact.

I assume you mean the file markdown diff output? Yeah, I can do that, should be easy. It just means that the truncation has to be controlled by a CLI flag, but that's easy enough

[xFrednet]

My repo is currently running these changes with an additional dummy commit that generates a lot of warnings. Here is the link:

https://github.com/xFrednet/rust-clippy/actions/runs/10044508861

[xFrednet]

This is what the summary table looks like:

Do we want to also linkify the numbers or add a 🔗 emoji after them to jump directly to the relevant section? I considered this, but worry that this will become too noisy. The lints still have links to their sub sections

[xFrednet]

Example output with these changes: https://github.com/xFrednet/rust-clippy/actions/runs/10044508861

[Alexendoo]

No longer additional by the looks of it

Doesn't actually change the DOM since it gets auto closed by the </h2> but a isn't a void element so this should be <a></a>, same for below

[xFrednet]

GH seems to accept it for me, but it's probably better to use </a> instead

[Alexendoo]

Yeah it should result in the same DOM since HTML self closes elements - <h2><a /></h2> is the same as <h2><a></h2> because HTML treats /> and > identically

The open a tag is then auto closed when it hits the </h2> so it becomes <h2><a></a></h2> - but this seems like the kind of area where differing behaviour in markdown previewers would crop up

[xFrednet]

Makes sense, it's updated now :D

[Alexendoo]

If we put the lint name first in the key function we could create lint_warnings directly via something like

for (name, chunk) in &merge_join_by(...).chunk_by(...) {}
// or however it needs to work for `ChunkBy`'s lifetime

rather than resorting/grouping it

[xFrednet]

I like the idea. I currently can't figure out how to get chunk_by working on MergeBy. You suggestion produces the following error:

error[E0599]: no method named `chunk_by` found for struct `MergeBy` in the current scope
  --> src/json.rs:69:120
   |
69 |     for (_name, change) in &itertools::merge_join_by(old_warnings, new_warnings, |old, new| old.key().cmp(&new.key())).chunk_by(|change| change.into_left().lint.as_str()) {
   |                                                                                                                        ^^^^^^^^
   |
help: there is a method `chunks` with a similar name
   |
69 |     for (_name, change) in &itertools::merge_join_by(old_warnings, new_warnings, |old, new| old.key().cmp(&new.key())).chunks(|change| change.into_left().lint.as_str()) {
   |            

Do you know how to solve it? I could collect it first into a vec, but then it seems better to manually chunk them.

[Alexendoo]

Ah it was renamed in itertools 0.13 and we're on 0.12

[Alexendoo]

Nice, thanks!

@bors r+

[bors]

📌 Commit bdf3e58 has been approved by Alexendoo

It is now in the queue for this repository.

[bors]

⌛ Testing commit bdf3e58 with merge be82340...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Alexendoo
Pushing be82340 to master...