Skip to content

Commit

Permalink
Auto merge of rust-lang#13133 - xFrednet:lintcheck-linkify-summary, r…
Browse files Browse the repository at this point in the history
…=Alexendoo

Lintcheck: Cleanup of Lintcheck's CI summery

This PR makes three changes to lintcheck's job summary:
* Adds links to the *Added*, *Removed*, *Changed* sections
* Added the crate name to the warning info
* Removes empty lines from the rendered output

This is what the new output roughly looks like:

![image](https://github.com/user-attachments/assets/3faae0a6-e5ee-4e70-9d4d-d19b18dc6a3a)
![image](https://github.com/user-attachments/assets/028c3a92-98dc-4e00-b7bb-fecf9450f5b1)

[:framed_picture: Old Output :framed_picture:](https://github.com/xFrednet/rust-clippy/actions/runs/10019681444)

[:framed_picture: New Output :framed_picture:](https://github.com/xFrednet/rust-clippy/actions/runs/10019598141)

The links for the sections are a bit weird as you have to click on them twice. I believe this is a bug from GH's side. But it works reasonably well :D

---

changelog: none

r? `@Alexendoo`
  • Loading branch information
bors committed Jul 21, 2024
2 parents 8fe5c75 + 69c3289 commit 7f0ed11
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 13 deletions.
50 changes: 40 additions & 10 deletions lintcheck/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::ClippyWarning;
#[derive(Deserialize, Serialize)]
struct LintJson {
lint: String,
krate: String,
file_name: String,
byte_pos: (u32, u32),
file_link: String,
Expand All @@ -19,6 +20,10 @@ impl LintJson {
fn key(&self) -> impl Ord + '_ {
(self.file_name.as_str(), self.byte_pos, self.lint.as_str())
}

fn info_text(&self, action: &str) -> String {
format!("{action} `{}` in `{}` at {}", self.lint, self.krate, self.file_link)
}
}

/// Creates the log file output for [`crate::config::OutputFormat::Json`]
Expand All @@ -30,6 +35,7 @@ pub(crate) fn output(clippy_warnings: Vec<ClippyWarning>) -> String {
LintJson {
file_name: span.file_name.clone(),
byte_pos: (span.byte_start, span.byte_end),
krate: warning.krate,
file_link: warning.url,
lint: warning.lint,
rendered: warning.diag.rendered.unwrap(),
Expand All @@ -51,12 +57,16 @@ fn print_warnings(title: &str, warnings: &[LintJson]) {
return;
}

println!("### {title}");
//We have to use HTML here to be able to manually add an id.
println!(r#"<h3 id="{title}">{title}</h3>"#);
println!();
for warning in warnings {
println!("{title} `{}` at {}", warning.lint, warning.file_link);
println!("{}", warning.info_text(title));
println!();
println!("```");
print!("{}", warning.rendered);
println!("{}", warning.rendered.trim_end());
println!("```");
println!();
}
}

Expand All @@ -65,11 +75,14 @@ fn print_changed_diff(changed: &[(LintJson, LintJson)]) {
return;
}

println!("### Changed");
//We have to use HTML here to be able to manually add an id.
println!(r#"<h3 id="changed">Changed</h3>"#);
println!();
for (old, new) in changed {
println!("Changed `{}` at {}", new.lint, new.file_link);
println!("{}", new.info_text("Changed"));
println!();
println!("```diff");
for change in diff::lines(&old.rendered, &new.rendered) {
for change in diff::lines(old.rendered.trim_end(), new.rendered.trim_end()) {
use diff::Result::{Both, Left, Right};

match change {
Expand Down Expand Up @@ -109,13 +122,30 @@ pub(crate) fn diff(old_path: &Path, new_path: &Path) {
}

print!(
"{} added, {} removed, {} changed\n\n",
added.len(),
removed.len(),
changed.len()
r##"{}, {}, {}"##,
count_string("added", added.len()),
count_string("removed", removed.len()),
count_string("changed", changed.len()),
);
println!();
println!();

print_warnings("Added", &added);
print_warnings("Removed", &removed);
print_changed_diff(&changed);
}

/// This generates the `x added` string for the start of the job summery.
/// It linkifies them if possible to jump to the respective heading.
fn count_string(label: &str, count: usize) -> String {
// Headlines are only added, if anything will be displayed under the headline.
// We therefore only want to add links to them if they exist
if count == 0 {
format!("0 {label}")
} else {
// 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-{label})")
}
}
1 change: 1 addition & 0 deletions lintcheck/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ impl Crate {
Ok(Message::CompilerMessage(message)) => ClippyWarning::new(
normalize_diag(message.message, shared_target_dir.to_str().unwrap()),
&self.base_url,
&self.name,
),
_ => None,
})
Expand Down
10 changes: 8 additions & 2 deletions lintcheck/src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,13 @@ impl RustcIce {
pub struct ClippyWarning {
pub lint: String,
pub diag: Diagnostic,
pub krate: String,
/// The URL that points to the file and line of the lint emission
pub url: String,
}

impl ClippyWarning {
pub fn new(mut diag: Diagnostic, base_url: &str) -> Option<Self> {
pub fn new(mut diag: Diagnostic, base_url: &str, krate: &str) -> Option<Self> {
let lint = diag.code.clone()?.code;
if !(lint.contains("clippy") || diag.message.contains("clippy"))
|| diag.message.contains("could not read cargo metadata")
Expand Down Expand Up @@ -90,7 +91,12 @@ impl ClippyWarning {
file.clone()
};

Some(Self { lint, diag, url })
Some(Self {
lint,
diag,
url,
krate: krate.to_string(),
})
}

pub fn span(&self) -> &DiagnosticSpan {
Expand Down
2 changes: 1 addition & 1 deletion lintcheck/src/recursive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ fn process_stream(
let messages = stderr
.lines()
.filter_map(|json_msg| serde_json::from_str::<Diagnostic>(json_msg).ok())
.filter_map(|diag| ClippyWarning::new(diag, &base_url));
.filter_map(|diag| ClippyWarning::new(diag, &base_url, &driver_info.package_name));

for message in messages {
sender.send(message).unwrap();
Expand Down

0 comments on commit 7f0ed11

Please sign in to comment.