Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 11, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 21 additions & 18 deletions lintcheck/src/json.rs
View file
Open in desktop
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,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();
Expand All @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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!();
}
Expand All @@ -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);
Expand All @@ -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 {
Expand Down
14 changes: 7 additions & 7 deletions lintcheck/src/output.rs
View file
Open in desktop
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