Skip to content

lintcheck: Add JSON output, diff subcommand #9764

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.

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions lintcheck/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ publish = false
cargo_metadata = "0.14"
clap = "3.2"
crossbeam-channel = "0.5.6"
diff = "0.1.13"
flate2 = "1.0"
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.5"
ureq = "2.2"
Expand Down
51 changes: 46 additions & 5 deletions lintcheck/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,41 @@ fn get_clap_config() -> ArgMatches {
Arg::new("markdown")
.long("markdown")
.help("Change the reports table to use markdown links"),
Arg::new("json")
.long("json")
.help("Output the diagnostics as JSON")
.conflicts_with("markdown"),
Arg::new("recursive")
.long("--recursive")
.help("Run clippy on the dependencies of crates specified in crates-toml")
.conflicts_with("threads")
.conflicts_with("fix"),
])
.subcommand(
Command::new("diff")
.about("Prints the difference between two `lintcheck --json` results")
.args([Arg::new("old").required(true), Arg::new("new").required(true)]),
)
.get_matches()
}

#[derive(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",
}
}
}

#[derive(Debug, Clone)]
pub(crate) struct LintcheckConfig {
/// max number of jobs to spawn (default 1)
Expand All @@ -57,10 +83,12 @@ pub(crate) struct LintcheckConfig {
pub fix: bool,
/// A list of lints that this lintcheck run should focus on
pub lint_filter: Vec<String>,
/// Indicate if the output should support markdown syntax
pub markdown: bool,
/// The output format of the log file
pub format: OutputFormat,
/// Run clippy on the dependencies of crates
pub recursive: bool,
/// Diff the two `lintcheck --json` results
pub diff: Option<(PathBuf, PathBuf)>,
}

impl LintcheckConfig {
Expand All @@ -77,7 +105,13 @@ impl LintcheckConfig {
.into()
});

let markdown = clap_config.contains_id("markdown");
let format = if clap_config.contains_id("markdown") {
OutputFormat::Markdown
} else if clap_config.contains_id("json") {
OutputFormat::Json
} else {
OutputFormat::Text
};
let sources_toml_path = PathBuf::from(sources_toml);

// for the path where we save the lint results, get the filename without extension (so for
Expand All @@ -86,7 +120,7 @@ impl LintcheckConfig {
let lintcheck_results_path = PathBuf::from(format!(
"lintcheck-logs/{}_logs.{}",
filename.display(),
if markdown { "md" } else { "txt" }
format.file_extension(),
));

// look at the --threads arg, if 0 is passed, ask rayon rayon how many threads it would spawn and
Expand Down Expand Up @@ -117,15 +151,22 @@ impl LintcheckConfig {
})
.unwrap_or_default();

let diff = clap_config.subcommand_matches("diff").map(|args| {
let path = |arg| PathBuf::from(args.get_one::<String>(arg).unwrap());

(path("old"), path("new"))
});

LintcheckConfig {
max_jobs,
sources_toml_path,
lintcheck_results_path,
only: clap_config.get_one::<String>("only").map(String::from),
fix: clap_config.contains_id("fix"),
lint_filter,
markdown,
format,
recursive: clap_config.contains_id("recursive"),
diff,
}
}
}
122 changes: 122 additions & 0 deletions lintcheck/src/json.rs
Original file line number Diff line number Diff line change
@@ -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);
}
Loading