From d931d1b5e6d1ccb9290313dde986844966f3ee2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Sat, 27 Feb 2021 00:29:42 +0100 Subject: [PATCH] lintcheck: refactor: introduce a basic LintcheckConfig struct which holds the job limit and paths to the sources and log files --- clippy_dev/src/lintcheck.rs | 111 +++++++++++++++++++++--------------- 1 file changed, 66 insertions(+), 45 deletions(-) diff --git a/clippy_dev/src/lintcheck.rs b/clippy_dev/src/lintcheck.rs index 65e438bc0..423daa0d1 100644 --- a/clippy_dev/src/lintcheck.rs +++ b/clippy_dev/src/lintcheck.rs @@ -287,6 +287,61 @@ impl Crate { } } +#[derive(Debug)] +struct LintcheckConfig { + // max number of jobs to spawn (default 1) + max_jobs: usize, + // we read the sources to check from here + sources_toml_path: PathBuf, + // we save the clippy lint results here + lintcheck_results_path: PathBuf, +} + +impl LintcheckConfig { + fn from_clap(clap_config: &ArgMatches) -> Self { + // first, check if we got anything passed via the LINTCHECK_TOML env var, + // if not, ask clap if we got any value for --crates-toml + // if not, use the default "clippy_dev/lintcheck_crates.toml" + let sources_toml = env::var("LINTCHECK_TOML").unwrap_or( + clap_config + .value_of("crates-toml") + .clone() + .unwrap_or("clippy_dev/lintcheck_crates.toml") + .to_string(), + ); + + let sources_toml_path = PathBuf::from(sources_toml); + + // for the path where we save the lint results, get the filename without extenstion ( so for + // wasd.toml, use "wasd"....) + let filename: PathBuf = sources_toml_path.file_stem().unwrap().into(); + let lintcheck_results_path = PathBuf::from(format!("lintcheck-logs/{}_logs.txt", filename.display())); + + let max_jobs = match clap_config.value_of("threads") { + Some(threads) => { + let threads: usize = threads + .parse() + .expect(&format!("Failed to parse '{}' to a digit", threads)); + if threads == 0 { + // automatic choice + // Rayon seems to return thread count so half that for core count + (rayon::current_num_threads() / 2) as usize + } else { + threads + } + }, + // no -j passed, use a single thread + None => 1, + }; + + LintcheckConfig { + max_jobs, + sources_toml_path, + lintcheck_results_path, + } + } +} + /// takes a single json-formatted clippy warnings and returns true (we are interested in that line) /// or false (we aren't) fn filter_clippy_warnings(line: &str) -> bool { @@ -310,19 +365,6 @@ fn filter_clippy_warnings(line: &str) -> bool { false } -/// get the path to lintchecks crate sources .toml file, check LINTCHECK_TOML first but if it's -/// empty use the default path -fn lintcheck_config_toml(toml_path: Option<&str>) -> PathBuf { - PathBuf::from( - env::var("LINTCHECK_TOML").unwrap_or( - toml_path - .clone() - .unwrap_or("clippy_dev/lintcheck_crates.toml") - .to_string(), - ), - ) -} - /// Builds clippy inside the repo to make sure we have a clippy executable we can use. fn build_clippy() { let status = Command::new("cargo") @@ -336,9 +378,7 @@ fn build_clippy() { } /// Read a `toml` file and return a list of `CrateSources` that we want to check with clippy -fn read_crates(toml_path: &PathBuf) -> (String, Vec) { - // save it so that we can use the name of the sources.toml as name for the logfile later. - let toml_filename = toml_path.file_stem().unwrap().to_str().unwrap().to_string(); +fn read_crates(toml_path: &PathBuf) -> Vec { let toml_content: String = std::fs::read_to_string(&toml_path).unwrap_or_else(|_| panic!("Failed to read {}", toml_path.display())); let crate_list: SourceList = @@ -398,7 +438,7 @@ fn read_crates(toml_path: &PathBuf) -> (String, Vec) { // sort the crates crate_sources.sort(); - (toml_filename, crate_sources) + crate_sources } /// Parse the json output of clippy and return a `ClippyWarning` @@ -476,16 +516,15 @@ fn lintcheck_needs_rerun(toml_path: &PathBuf) -> bool { /// lintchecks `main()` function pub fn run(clap_config: &ArgMatches) { + let config = LintcheckConfig::from_clap(clap_config); + println!("Compiling clippy..."); build_clippy(); println!("Done compiling"); - let clap_toml_path: Option<&str> = clap_config.value_of("crates-toml"); - let toml_path: PathBuf = lintcheck_config_toml(clap_toml_path); - // if the clippy bin is newer than our logs, throw away target dirs to force clippy to // refresh the logs - if lintcheck_needs_rerun(&toml_path) { + if lintcheck_needs_rerun(&config.sources_toml_path) { let shared_target_dir = "target/lintcheck/shared_target_dir"; match std::fs::metadata(&shared_target_dir) { Ok(metadata) => { @@ -520,9 +559,8 @@ pub fn run(clap_config: &ArgMatches) { // download and extract the crates, then run clippy on them and collect clippys warnings // flatten into one big list of warnings - let (filename, crates) = read_crates(&toml_path); - let file = format!("lintcheck-logs/{}_logs.txt", filename); - let old_stats = read_stats_from_file(&file); + let crates = read_crates(&config.sources_toml_path); + let old_stats = read_stats_from_file(&config.lintcheck_results_path); let clippy_warnings: Vec = if let Some(only_one_crate) = clap_config.value_of("only") { // if we don't have the specified crate in the .toml, throw an error @@ -562,23 +600,7 @@ pub fn run(clap_config: &ArgMatches) { // order to achive some kind of parallelism // by default, use a single thread - let num_cpus = match clap_config.value_of("threads") { - Some(threads) => { - let threads: usize = threads - .parse() - .expect(&format!("Failed to parse '{}' to a digit", threads)); - if threads == 0 { - // automatic choice - // Rayon seems to return thread count so half that for core count - (rayon::current_num_threads() / 2) as usize - } else { - threads - } - }, - // no -j passed, use a single thread - None => 1, - }; - + let num_cpus = config.max_jobs; let num_crates = crates.len(); // check all crates (default) @@ -612,15 +634,14 @@ pub fn run(clap_config: &ArgMatches) { ices.iter() .for_each(|(cratename, msg)| text.push_str(&format!("{}: '{}'", cratename, msg))); - println!("Writing logs to {}", file); - write(&file, text).unwrap(); + println!("Writing logs to {}", config.lintcheck_results_path.display()); + write(&config.lintcheck_results_path, text).unwrap(); print_stats(old_stats, new_stats); } /// read the previous stats from the lintcheck-log file -fn read_stats_from_file(file_path: &String) -> HashMap { - let file_path = PathBuf::from(file_path); +fn read_stats_from_file(file_path: &PathBuf) -> HashMap { let file_content: String = match std::fs::read_to_string(file_path).ok() { Some(content) => content, None => {