mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #10398 - Alexendoo:auto-lintcheck, r=xFrednet

Browse Source 
Run a diff of lintcheck against the merge base for pull requests

changelog: none
<!-- changelog_checked -->

This is an MVP of sorts, it consists of #9764 + a GitHub action that feeds the output to the [job summary](https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#adding-a-job-summary). It doesn't yet do anything fancy like `--recursive` or adding comments to the PR, so you'd have to click through to the action to see the results

Example output of a change (0be1ab82f8): https://github.com/Alexendoo/rust-clippy/actions/runs/4264858870#summary-11583333018

r? `@flip1995`
This commit is contained in:
bors 2024-06-16 10:03:12 +00:00
commit a2c9782128
5 changed files with 377 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

115
.github/workflows/lintcheck.yml vendored Normal file
View File

@ -0,0 +1,115 @@
name: Lintcheck
on: pull_request
env:
RUST_BACKTRACE: 1
CARGO_INCREMENTAL: 0
concurrency:
# For a given workflow, if we push to the same PR, cancel all previous builds on that PR.
group: "${{ github.workflow }}-${{ github.event.pull_request.number}}"
cancel-in-progress: true
jobs:
# Generates `lintcheck-logs/base.json` and stores it in a cache
base:
runs-on: ubuntu-latest
outputs:
key: ${{ steps.key.outputs.key }}
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 2
# HEAD is the generated merge commit `refs/pull/N/merge` between the PR and `master`, `HEAD^`
# being the commit from `master` that is the base of the merge
- name: Checkout base
run: git checkout HEAD^
# Use the lintcheck from the PR to generate the JSON in case the PR modifies lintcheck in some
# way
- name: Checkout current lintcheck
run: |
rm -rf lintcheck
git checkout ${{ github.sha }} -- lintcheck
- name: Create cache key
id: key
run: echo "key=lintcheck-base-${{ hashfiles('lintcheck/**') }}-$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"
- name: Cache results
id: cache
uses: actions/cache@v4
with:
path: lintcheck-logs/base.json
key: ${{ steps.key.outputs.key }}
- name: Run lintcheck
if: steps.cache.outputs.cache-hit != 'true'
run: cargo lintcheck --format json
- name: Rename JSON file
if: steps.cache.outputs.cache-hit != 'true'
run: mv lintcheck-logs/lintcheck_crates_logs.json lintcheck-logs/base.json
# Generates `lintcheck-logs/head.json` and stores it in a cache
head:
runs-on: ubuntu-latest
outputs:
key: ${{ steps.key.outputs.key }}
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Create cache key
id: key
run: echo "key=lintcheck-head-${{ github.sha }}" >> "$GITHUB_OUTPUT"
- name: Cache results
id: cache
uses: actions/cache@v4
with:
path: lintcheck-logs/head.json
key: ${{ steps.key.outputs.key }}
- name: Run lintcheck
if: steps.cache.outputs.cache-hit != 'true'
run: cargo lintcheck --format json
- name: Rename JSON file
if: steps.cache.outputs.cache-hit != 'true'
run: mv lintcheck-logs/lintcheck_crates_logs.json lintcheck-logs/head.json
# Retrieves `lintcheck-logs/base.json` and `lintcheck-logs/head.json` from the cache and prints
# the diff to the GH actions step summary
diff:
runs-on: ubuntu-latest
needs: [base, head]
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Restore base JSON
uses: actions/cache/restore@v4
with:
key: ${{ needs.base.outputs.key }}
path: lintcheck-logs/base.json
fail-on-cache-miss: true
- name: Restore head JSON
uses: actions/cache/restore@v4
with:
key: ${{ needs.head.outputs.key }}
path: lintcheck-logs/head.json
fail-on-cache-miss: true
- name: Diff results
run: cargo lintcheck diff lintcheck-logs/base.json lintcheck-logs/head.json >> $GITHUB_STEP_SUMMARY

2
lintcheck/Cargo.toml
View File

@ -16,11 +16,13 @@ cargo_metadata = "0.15.3"
clap = { version = "4.4", features = ["derive", "env"] }
crates_io_api = "0.8.1"
crossbeam-channel = "0.5.6"
diff = "0.1.13"
flate2 = "1.0"
indicatif = "0.17.3"
rayon = "1.5.1"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0.85"
strip-ansi-escapes = "0.1.1"
tar = "0.4"
toml = "0.7.3"
ureq = "2.2"

37
lintcheck/src/config.rs
View File

@ -1,8 +1,9 @@
use clap::Parser;
use clap::{Parser, Subcommand, ValueEnum};
use std::num::NonZero;
use std::path::PathBuf;
#[derive(Clone, Debug, Parser)]
#[derive(Parser, Clone, Debug)]
#[command(args_conflicts_with_subcommands = true)]
pub(crate) struct LintcheckConfig {
/// Number of threads to use (default: all unless --fix or --recursive)
#[clap(
@ -35,12 +36,36 @@ pub(crate) struct LintcheckConfig {
/// Apply a filter to only collect specified lints, this also overrides `allow` attributes
#[clap(long = "filter", value_name = "clippy_lint_name", use_value_delimiter = true)]
pub lint_filter: Vec<String>,
/// Change the reports table to use markdown links
#[clap(long)]
pub markdown: bool,
/// Set the output format of the log file
#[clap(long, short, default_value = "text")]
pub format: OutputFormat,
/// Run clippy on the dependencies of crates specified in crates-toml
#[clap(long, conflicts_with("max_jobs"))]
pub recursive: bool,
#[command(subcommand)]
pub subcommand: Option<Commands>,
}
#[derive(Subcommand, Clone, Debug)]
pub(crate) enum Commands {
Diff { old: PathBuf, new: PathBuf },
}
#[derive(ValueEnum, Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum OutputFormat {
Text,
Markdown,
Json,
}
impl OutputFormat {
fn file_extension(self) -> &'static str {
match self {
OutputFormat::Text => "txt",
OutputFormat::Markdown => "md",
OutputFormat::Json => "json",
}
}
}
impl LintcheckConfig {
@ -53,7 +78,7 @@ pub fn new() -> Self {
config.lintcheck_results_path = PathBuf::from(format!(
"lintcheck-logs/{}_logs.{}",
filename.display(),
if config.markdown { "md" } else { "txt" }
config.format.file_extension(),
));
// look at the --threads arg, if 0 is passed, use the threads count

122
lintcheck/src/json.rs Normal file
View File

@ -0,0 +1,122 @@
use std::collections::HashMap;
use std::fmt::Write;
use std::fs;
use std::hash::Hash;
use std::path::Path;
use crate::ClippyWarning;
/// Creates the log file output for [`crate::config::OutputFormat::Json`]
pub(crate) fn output(clippy_warnings: &[ClippyWarning]) -> String {
serde_json::to_string(&clippy_warnings).unwrap()
}
fn load_warnings(path: &Path) -> Vec<ClippyWarning> {
let file = fs::read(path).unwrap_or_else(|e| panic!("failed to read {}: {e}", path.display()));
serde_json::from_slice(&file).unwrap_or_else(|e| panic!("failed to deserialize {}: {e}", path.display()))
}
/// Group warnings by their primary span location + lint name
fn create_map(warnings: &[ClippyWarning]) -> HashMap<impl Eq + Hash + '_, Vec<&ClippyWarning>> {
let mut map = HashMap::<_, Vec<_>>::with_capacity(warnings.len());
for warning in warnings {
let span = warning.span();
let key = (&warning.lint_type, &span.file_name, span.byte_start, span.byte_end);
map.entry(key).or_default().push(warning);
}
map
}
fn print_warnings(title: &str, warnings: &[&ClippyWarning]) {
if warnings.is_empty() {
return;
}
println!("### {title}");
println!("```");
for warning in warnings {
print!("{}", warning.diag);
}
println!("```");
}
fn print_changed_diff(changed: &[(&[&ClippyWarning], &[&ClippyWarning])]) {
fn render(warnings: &[&ClippyWarning]) -> String {
let mut rendered = String::new();
for warning in warnings {
write!(&mut rendered, "{}", warning.diag).unwrap();
}
rendered
}
if changed.is_empty() {
return;
}
println!("### Changed");
println!("```diff");
for &(old, new) in changed {
let old_rendered = render(old);
let new_rendered = render(new);
for change in diff::lines(&old_rendered, &new_rendered) {
use diff::Result::{Both, Left, Right};
match change {
Both(unchanged, _) => {
println!(" {unchanged}");
},
Left(removed) => {
println!("-{removed}");
},
Right(added) => {
println!("+{added}");
},
}
}
}
println!("```");
}
pub(crate) fn diff(old_path: &Path, new_path: &Path) {
let old_warnings = load_warnings(old_path);
let new_warnings = load_warnings(new_path);
let old_map = create_map(&old_warnings);
let new_map = create_map(&new_warnings);
let mut added = Vec::new();
let mut removed = Vec::new();
let mut changed = Vec::new();
for (key, new) in &new_map {
if let Some(old) = old_map.get(key) {
if old != new {
changed.push((old.as_slice(), new.as_slice()));
}
} else {
added.extend(new);
}
}
for (key, old) in &old_map {
if !new_map.contains_key(key) {
removed.extend(old);
}
}
print!(
"{} added, {} removed, {} changed\n\n",
added.len(),
removed.len(),
changed.len()
);
print_warnings("Added", &added);
print_warnings("Removed", &removed);
print_changed_diff(&changed);
}

169
lintcheck/src/main.rs
View File

@ -12,18 +12,20 @@
unused_lifetimes,
unused_qualifications
)]
#![allow(clippy::collapsible_else_if)]
#![allow(clippy::collapsible_else_if, clippy::needless_borrows_for_generic_args)]
mod config;
mod driver;
mod json;
mod recursive;
use crate::config::LintcheckConfig;
use crate::config::{Commands, LintcheckConfig, OutputFormat};
use crate::recursive::LintcheckServer;
use std::collections::{HashMap, HashSet};
use std::env::consts::EXE_SUFFIX;
use std::fmt::{self, Write as _};
use std::fmt::{self, Display, Write as _};
use std::hash::Hash;
use std::io::{self, ErrorKind};
use std::path::{Path, PathBuf};
use std::process::{Command, ExitStatus};
@ -31,7 +33,7 @@
use std::time::Duration;
use std::{env, fs, thread};
use cargo_metadata::diagnostic::Diagnostic;
use cargo_metadata::diagnostic::{Diagnostic, DiagnosticSpan};
use cargo_metadata::Message;
use rayon::prelude::*;
use serde::{Deserialize, Serialize};
@ -111,6 +113,17 @@ struct RustcIce {
pub crate_name: String,
pub ice_content: String,
}
impl Display for RustcIce {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
f,
"{}:\n{}\n========================================\n",
self.crate_name, self.ice_content
)
}
}
impl RustcIce {
pub fn from_stderr_and_status(crate_name: &str, status: ExitStatus, stderr: &str) -> Option<Self> {
if status.code().unwrap_or(0) == 101
@ -127,60 +140,58 @@ pub fn from_stderr_and_status(crate_name: &str, status: ExitStatus, stderr: &str
}
/// A single warning that clippy issued while checking a `Crate`
#[derive(Debug)]
#[derive(Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
struct ClippyWarning {
file: String,
line: usize,
column: usize,
crate_name: String,
crate_version: String,
lint_type: String,
message: String,
diag: Diagnostic,
}
#[allow(unused)]
impl ClippyWarning {
fn new(diag: Diagnostic, crate_name: &str, crate_version: &str) -> Option<Self> {
let lint_type = diag.code?.code;
fn new(mut diag: Diagnostic, crate_name: &str, crate_version: &str) -> Option<Self> {
let lint_type = diag.code.clone()?.code;
if !(lint_type.contains("clippy") || diag.message.contains("clippy"))
|| diag.message.contains("could not read cargo metadata")
{
return None;
}
let span = diag.spans.into_iter().find(|span| span.is_primary)?;
let file = if let Ok(stripped) = Path::new(&span.file_name).strip_prefix(env!("CARGO_HOME")) {
format!("$CARGO_HOME/{}", stripped.display())
} else {
format!(
"target/lintcheck/sources/{crate_name}-{crate_version}/{}",
span.file_name
)
};
// --recursive bypasses cargo so we have to strip the rendered output ourselves
let rendered = diag.rendered.as_mut().unwrap();
*rendered = String::from_utf8(strip_ansi_escapes::strip(&rendered).unwrap()).unwrap();
Some(Self {
file,
line: span.line_start,
column: span.column_start,
crate_name: crate_name.to_owned(),
crate_version: crate_version.to_owned(),
lint_type,
message: diag.message,
diag,
})
}
fn to_output(&self, markdown: bool) -> String {
let file_with_pos = format!("{}:{}:{}", &self.file, &self.line, &self.column);
if markdown {
let mut file = self.file.clone();
if !file.starts_with('$') {
file.insert_str(0, "../");
}
fn span(&self) -> &DiagnosticSpan {
self.diag.spans.iter().find(|span| span.is_primary).unwrap()
}
let mut output = String::from("| ");
let _: fmt::Result = write!(output, "[`{file_with_pos}`]({file}#L{})", self.line);
let _: fmt::Result = write!(output, r#" | `{:<50}` | "{}" |"#, self.lint_type, self.message);
output.push('\n');
output
} else {
format!("{file_with_pos} {} \"{}\"\n", self.lint_type, self.message)
fn to_output(&self, format: OutputFormat) -> String {
let span = self.span();
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_type, 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_type, self.diag.message).unwrap();
output.push('\n');
output
},
OutputFormat::Json => unreachable!("JSON output is handled via serde"),
}
}
}
@ -333,7 +344,7 @@ fn is_cache_dir(entry: &DirEntry) -> bool {
impl Crate {
/// Run `cargo clippy` on the `Crate` and collect and return all the lint warnings that clippy
/// issued
#[allow(clippy::too_many_arguments)]
#[allow(clippy::too_many_arguments, clippy::too_many_lines)]
fn run_clippy_lints(
&self,
cargo_clippy_path: &Path,
@ -372,7 +383,25 @@ fn run_clippy_lints(
vec!["--quiet", "--message-format=json", "--"]
};
let mut clippy_args = Vec::<&str>::new();
let cargo_home = env!("CARGO_HOME");
// `src/lib.rs` -> `target/lintcheck/sources/crate-1.2.3/src/lib.rs`
let remap_relative = format!("={}", self.path.display());
// Fallback for other sources, `~/.cargo/...` -> `$CARGO_HOME/...`
let remap_cargo_home = format!("{cargo_home}=$CARGO_HOME");
// `~/.cargo/registry/src/github.com-1ecc6299db9ec823/crate-2.3.4/src/lib.rs`
// -> `crate-2.3.4/src/lib.rs`
let remap_crates_io = format!("{cargo_home}/registry/src/github.com-1ecc6299db9ec823/=");
let mut clippy_args = vec![
"--remap-path-prefix",
&remap_relative,
"--remap-path-prefix",
&remap_cargo_home,
"--remap-path-prefix",
&remap_crates_io,
];
if let Some(options) = &self.options {
for opt in options {
clippy_args.push(opt);
@ -554,10 +583,10 @@ fn read_crates(toml_path: &Path) -> (Vec<CrateSource>, RecursiveOptions) {
}
/// Generate a short list of occurring lints-types and their count
fn gather_stats(clippy_warnings: &[ClippyWarning]) -> (String, HashMap<&String, usize>) {
fn gather_stats(warnings: &[ClippyWarning]) -> (String, HashMap<&String, usize>) {
// count lint type occurrences
let mut counter: HashMap<&String, usize> = HashMap::new();
clippy_warnings
warnings
.iter()
.for_each(|wrn| *counter.entry(&wrn.lint_type).or_insert(0) += 1);
@ -595,6 +624,11 @@ fn main() {
let config = LintcheckConfig::new();
if let Some(Commands::Diff { old, new }) = config.subcommand {
json::diff(&old, &new);
return;
}
println!("Compiling clippy...");
build_clippy();
println!("Done compiling");
@ -619,7 +653,6 @@ fn main() {
// flatten into one big list of warnings
let (crates, recursive_options) = read_crates(&config.sources_toml_path);
let old_stats = read_stats_from_file(&config.lintcheck_results_path);
let counter = AtomicUsize::new(1);
let lint_filter: Vec<String> = config
@ -711,39 +744,51 @@ fn main() {
}
}
// generate some stats
let (stats_formatted, new_stats) = gather_stats(&warnings);
let text = match config.format {
OutputFormat::Text | OutputFormat::Markdown => output(&warnings, &raw_ices, clippy_ver, &config),
OutputFormat::Json => {
if !raw_ices.is_empty() {
for ice in raw_ices {
println!("{ice}");
}
panic!("Some crates ICEd");
}
let mut all_msgs: Vec<String> = warnings.iter().map(|warn| warn.to_output(config.markdown)).collect();
json::output(&warnings)
},
};
println!("Writing logs to {}", config.lintcheck_results_path.display());
fs::create_dir_all(config.lintcheck_results_path.parent().unwrap()).unwrap();
fs::write(&config.lintcheck_results_path, text).unwrap();
}
/// Creates the log file output for [`OutputFormat::Text`] and [`OutputFormat::Markdown`]
fn output(warnings: &[ClippyWarning], ices: &[RustcIce], clippy_ver: String, config: &LintcheckConfig) -> String {
// generate some stats
let (stats_formatted, new_stats) = gather_stats(warnings);
let old_stats = read_stats_from_file(&config.lintcheck_results_path);
let mut all_msgs: Vec<String> = warnings.iter().map(|warn| warn.to_output(config.format)).collect();
all_msgs.sort();
all_msgs.push("\n\n### Stats:\n\n".into());
all_msgs.push(stats_formatted);
// save the text into lintcheck-logs/logs.txt
let mut text = clippy_ver; // clippy version number on top
text.push_str("\n### Reports\n\n");
if config.markdown {
if config.format == OutputFormat::Markdown {
text.push_str("| file | lint | message |\n");
text.push_str("| --- | --- | --- |\n");
}
write!(text, "{}", all_msgs.join("")).unwrap();
text.push_str("\n\n### ICEs:\n");
for ice in &raw_ices {
let _: fmt::Result = write!(
text,
"{}:\n{}\n========================================\n\n",
ice.crate_name, ice.ice_content
);
for ice in ices {
writeln!(text, "{ice}").unwrap();
}
println!("Writing logs to {}", config.lintcheck_results_path.display());
if !raw_ices.is_empty() {
println!("WARNING: at least one ICE reported, check log file");
}
fs::create_dir_all(config.lintcheck_results_path.parent().unwrap()).unwrap();
fs::write(&config.lintcheck_results_path, text).unwrap();
print_stats(old_stats, new_stats, &config.lint_filter);
text
}
/// read the previous stats from the lintcheck-log file