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

This commit is contained in:
Alex Macleod 2024-08-10 18:36:52 +00:00
parent 780c61f1ee
commit 6de27a0272
2 changed files with 28 additions and 25 deletions

View File

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

View File

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