Skip to content

Commit

Permalink
lintcheck: key lints on line start rather than byte start/end
Browse files Browse the repository at this point in the history
  • Loading branch information
Alexendoo committed Aug 10, 2024
1 parent b2ec6f8 commit 9f5cef2
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 25 deletions.
44 changes: 26 additions & 18 deletions lintcheck/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -36,13 +36,21 @@ pub(crate) fn output(clippy_warnings: Vec<ClippyWarning>) -> String {
.into_iter()
.map(|warning| {
let span = warning.span();
let file_name = if let Some(stripped) = span.file_name.strip_prefix("target/lintcheck/sources/") {
stripped
} else if let Some(stripped) = span.file_name.strip_prefix("/rustc/") {
// In CI paths pointing into stdlib appear as `/rustc/HASH/..` where the `HASH` can change between
// rustups, remove the hash component
&format!("/rustc{}", stripped.trim_start_matches(|ch| ch != '/'))
} else {
&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();
Expand All @@ -63,7 +71,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();
Expand Down Expand Up @@ -162,7 +170,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);
Expand All @@ -171,7 +179,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!();
}
Expand All @@ -182,7 +190,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);
Expand All @@ -191,7 +199,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 {
Expand Down
14 changes: 7 additions & 7 deletions lintcheck/src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -92,7 +92,7 @@ impl ClippyWarning {
};

Some(Self {
lint,
name,
diag,
url,
krate: krate.to_string(),
Expand All @@ -108,15 +108,15 @@ 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, "../");
}

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
},
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 9f5cef2

Please sign in to comment.