Auto merge of #13250 - Alexendoo:lintcheck-diff-key, r=xFrednet

lintcheck: key lints on line start rather than byte start/end

Before: https://github.com/rust-lang/rust-clippy/actions/runs/10305798317
After: https://github.com/Alexendoo/rust-clippy/actions/runs/10333749860

If a single lint emits e.g. twice on the same line and the messages change for both there would still be two changed entries

Also slightly changes the output, the URL is no longer visible

r? `@xFrednet`

changelog: none
This commit is contained in:
bors 2024-08-11 09:00:46 +00:00
commit 94099f48c9
2 changed files with 28 additions and 25 deletions

View File

@ -12,21 +12,21 @@ const TRUNCATION_TOTAL_TARGET: usize = 1000;
#[derive(Debug, Deserialize, Serialize)]
struct LintJson {
lint: String,
krate: String,
file_name: String,
byte_pos: (u32, u32),
file_link: String,
/// The lint name e.g. `clippy::bytes_nth`
name: String,
/// The filename and line number e.g. `anyhow-1.0.86/src/error.rs:42`
file_line: String,
file_url: String,
rendered: String,
}
impl LintJson {
fn key(&self) -> impl Ord + '_ {
(self.lint.as_str(), self.file_name.as_str(), self.byte_pos)
(self.name.as_str(), self.file_line.as_str())
}
fn info_text(&self, action: &str) -> String {
format!("{action} `{}` in `{}` at {}", self.lint, self.krate, self.file_link)
format!("{action} `{}` at [`{}`]({})", self.name, self.file_line, self.file_url)
}
}
@ -36,13 +36,16 @@ pub(crate) fn output(clippy_warnings: Vec<ClippyWarning>) -> String {
.into_iter()
.map(|warning| {
let span = warning.span();
let file_name = span
.file_name
.strip_prefix("target/lintcheck/sources/")
.unwrap_or(&span.file_name);
let file_line = format!("{file_name}:{}", span.line_start);
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(),
name: warning.name,
file_line,
file_url: warning.url,
rendered: warning.diag.rendered.unwrap().trim().to_string(),
}
})
.collect();
@ -63,7 +66,7 @@ pub(crate) fn diff(old_path: &Path, new_path: &Path, truncate: bool) {
let mut lint_warnings = vec![];
for (name, changes) in &itertools::merge_join_by(old_warnings, new_warnings, |old, new| old.key().cmp(&new.key()))
.chunk_by(|change| change.as_ref().into_left().lint.to_string())
.chunk_by(|change| change.as_ref().into_left().name.clone())
{
let mut added = Vec::new();
let mut removed = Vec::new();
@ -162,7 +165,7 @@ fn print_warnings(title: &str, warnings: &[LintJson], truncate_after: usize) {
return;
}
print_h3(&warnings[0].lint, title);
print_h3(&warnings[0].name, title);
println!();
let warnings = truncate(warnings, truncate_after);
@ -171,7 +174,7 @@ fn print_warnings(title: &str, warnings: &[LintJson], truncate_after: usize) {
println!("{}", warning.info_text(title));
println!();
println!("```");
println!("{}", warning.rendered.trim_end());
println!("{}", warning.rendered);
println!("```");
println!();
}
@ -182,7 +185,7 @@ fn print_changed_diff(changed: &[(LintJson, LintJson)], truncate_after: usize) {
return;
}
print_h3(&changed[0].0.lint, "Changed");
print_h3(&changed[0].0.name, "Changed");
println!();
let changed = truncate(changed, truncate_after);
@ -191,7 +194,7 @@ fn print_changed_diff(changed: &[(LintJson, LintJson)], truncate_after: usize) {
println!("{}", new.info_text("Changed"));
println!();
println!("```diff");
for change in diff::lines(old.rendered.trim_end(), new.rendered.trim_end()) {
for change in diff::lines(&old.rendered, &new.rendered) {
use diff::Result::{Both, Left, Right};
match change {

View File

@ -51,7 +51,7 @@ impl RustcIce {
/// A single warning that clippy issued while checking a `Crate`
#[derive(Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
pub struct ClippyWarning {
pub lint: String,
pub name: String,
pub diag: Diagnostic,
pub krate: String,
/// The URL that points to the file and line of the lint emission
@ -60,8 +60,8 @@ pub struct ClippyWarning {
impl ClippyWarning {
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"))
let name = diag.code.clone()?.code;
if !(name.contains("clippy") || diag.message.contains("clippy"))
|| diag.message.contains("could not read cargo metadata")
{
return None;
@ -92,7 +92,7 @@ impl ClippyWarning {
};
Some(Self {
lint,
name,
diag,
url,
krate: krate.to_string(),
@ -108,7 +108,7 @@ impl ClippyWarning {
let mut file = span.file_name.clone();
let file_with_pos = format!("{file}:{}:{}", span.line_start, span.line_end);
match format {
OutputFormat::Text => format!("{file_with_pos} {} \"{}\"\n", self.lint, self.diag.message),
OutputFormat::Text => format!("{file_with_pos} {} \"{}\"\n", self.name, self.diag.message),
OutputFormat::Markdown => {
if file.starts_with("target") {
file.insert_str(0, "../");
@ -116,7 +116,7 @@ impl ClippyWarning {
let mut output = String::from("| ");
write!(output, "[`{file_with_pos}`]({file}#L{})", span.line_start).unwrap();
write!(output, r#" | `{:<50}` | "{}" |"#, self.lint, self.diag.message).unwrap();
write!(output, r#" | `{:<50}` | "{}" |"#, self.name, self.diag.message).unwrap();
output.push('\n');
output
},
@ -164,7 +164,7 @@ fn gather_stats(warnings: &[ClippyWarning]) -> (String, HashMap<&String, usize>)
let mut counter: HashMap<&String, usize> = HashMap::new();
warnings
.iter()
.for_each(|wrn| *counter.entry(&wrn.lint).or_insert(0) += 1);
.for_each(|wrn| *counter.entry(&wrn.name).or_insert(0) += 1);
// collect into a tupled list for sorting
let mut stats: Vec<(&&String, &usize)> = counter.iter().collect();