Apply manual fixes

This commit is contained in:
Samuel Moelius 2022-10-22 07:12:07 -04:00
parent e38bb1a963
commit bbee1c9d1f
4 changed files with 53 additions and 45 deletions

View File

@ -73,8 +73,7 @@ impl LintcheckConfig {
let sources_toml = env::var("LINTCHECK_TOML").unwrap_or_else(|_| { let sources_toml = env::var("LINTCHECK_TOML").unwrap_or_else(|_| {
clap_config clap_config
.get_one::<String>("crates-toml") .get_one::<String>("crates-toml")
.map(|s| &**s) .map_or("lintcheck/lintcheck_crates.toml", |s| &**s)
.unwrap_or("lintcheck/lintcheck_crates.toml")
.into() .into()
}); });

View File

@ -5,7 +5,7 @@ use std::net::TcpStream;
use std::process::{self, Command, Stdio}; use std::process::{self, Command, Stdio};
use std::{env, mem}; use std::{env, mem};
/// 1. Sends [DriverInfo] to the [crate::recursive::LintcheckServer] running on `addr` /// 1. Sends [`DriverInfo`] to the [`crate::recursive::LintcheckServer`] running on `addr`
/// 2. Receives [bool] from the server, if `false` returns `None` /// 2. Receives [bool] from the server, if `false` returns `None`
/// 3. Otherwise sends the stderr of running `clippy-driver` to the server /// 3. Otherwise sends the stderr of running `clippy-driver` to the server
fn run_clippy(addr: &str) -> Option<i32> { fn run_clippy(addr: &str) -> Option<i32> {

View File

@ -116,12 +116,13 @@ impl ClippyWarning {
let span = diag.spans.into_iter().find(|span| span.is_primary)?; let span = diag.spans.into_iter().find(|span| span.is_primary)?;
let file = match Path::new(&span.file_name).strip_prefix(env!("CARGO_HOME")) { let file = if let Ok(stripped) = Path::new(&span.file_name).strip_prefix(env!("CARGO_HOME")) {
Ok(stripped) => format!("$CARGO_HOME/{}", stripped.display()), format!("$CARGO_HOME/{}", stripped.display())
Err(_) => format!( } else {
format!(
"target/lintcheck/sources/{}-{}/{}", "target/lintcheck/sources/{}-{}/{}",
crate_name, crate_version, span.file_name crate_name, crate_version, span.file_name
), )
}; };
Some(Self { Some(Self {
@ -154,6 +155,7 @@ impl ClippyWarning {
} }
} }
#[allow(clippy::result_large_err)]
fn get(path: &str) -> Result<ureq::Response, ureq::Error> { fn get(path: &str) -> Result<ureq::Response, ureq::Error> {
const MAX_RETRIES: u8 = 4; const MAX_RETRIES: u8 = 4;
let mut retries = 0; let mut retries = 0;
@ -165,7 +167,7 @@ fn get(path: &str) -> Result<ureq::Response, ureq::Error> {
Err(e) => return Err(e), Err(e) => return Err(e),
} }
eprintln!("retrying in {retries} seconds..."); eprintln!("retrying in {retries} seconds...");
thread::sleep(Duration::from_secs(retries as u64)); thread::sleep(Duration::from_secs(u64::from(retries)));
retries += 1; retries += 1;
} }
} }
@ -232,7 +234,7 @@ impl CrateSource {
.expect("Failed to clone git repo!") .expect("Failed to clone git repo!")
.success() .success()
{ {
eprintln!("Failed to clone {url} into {}", repo_path.display()) eprintln!("Failed to clone {url} into {}", repo_path.display());
} }
} }
// check out the commit/branch/whatever // check out the commit/branch/whatever
@ -245,7 +247,7 @@ impl CrateSource {
.expect("Failed to check out commit") .expect("Failed to check out commit")
.success() .success()
{ {
eprintln!("Failed to checkout {commit} of repo at {}", repo_path.display()) eprintln!("Failed to checkout {commit} of repo at {}", repo_path.display());
} }
Crate { Crate {
@ -256,6 +258,12 @@ impl CrateSource {
} }
}, },
CrateSource::Path { name, path, options } => { CrateSource::Path { name, path, options } => {
fn is_cache_dir(entry: &DirEntry) -> bool {
std::fs::read(entry.path().join("CACHEDIR.TAG"))
.map(|x| x.starts_with(b"Signature: 8a477f597d28d172789f06886806bc55"))
.unwrap_or(false)
}
// copy path into the dest_crate_root but skip directories that contain a CACHEDIR.TAG file. // copy path into the dest_crate_root but skip directories that contain a CACHEDIR.TAG file.
// The target/ directory contains a CACHEDIR.TAG file so it is the most commonly skipped directory // The target/ directory contains a CACHEDIR.TAG file so it is the most commonly skipped directory
// as a result of this filter. // as a result of this filter.
@ -267,12 +275,6 @@ impl CrateSource {
println!("Copying {path:?} to {dest_crate_root:?}"); println!("Copying {path:?} to {dest_crate_root:?}");
fn is_cache_dir(entry: &DirEntry) -> bool {
std::fs::read(entry.path().join("CACHEDIR.TAG"))
.map(|x| x.starts_with(b"Signature: 8a477f597d28d172789f06886806bc55"))
.unwrap_or(false)
}
for entry in WalkDir::new(path).into_iter().filter_entry(|e| !is_cache_dir(e)) { for entry in WalkDir::new(path).into_iter().filter_entry(|e| !is_cache_dir(e)) {
let entry = entry.unwrap(); let entry = entry.unwrap();
let entry_path = entry.path(); let entry_path = entry.path();
@ -301,6 +303,7 @@ impl CrateSource {
impl Crate { impl Crate {
/// Run `cargo clippy` on the `Crate` and collect and return all the lint warnings that clippy /// Run `cargo clippy` on the `Crate` and collect and return all the lint warnings that clippy
/// issued /// issued
#[allow(clippy::too_many_arguments)]
fn run_clippy_lints( fn run_clippy_lints(
&self, &self,
cargo_clippy_path: &Path, cargo_clippy_path: &Path,
@ -345,14 +348,14 @@ impl Crate {
clippy_args.push(opt); clippy_args.push(opt);
} }
} else { } else {
clippy_args.extend(["-Wclippy::pedantic", "-Wclippy::cargo"]) clippy_args.extend(["-Wclippy::pedantic", "-Wclippy::cargo"]);
} }
if lint_filter.is_empty() { if lint_filter.is_empty() {
clippy_args.push("--cap-lints=warn"); clippy_args.push("--cap-lints=warn");
} else { } else {
clippy_args.push("--cap-lints=allow"); clippy_args.push("--cap-lints=allow");
clippy_args.extend(lint_filter.iter().map(|filter| filter.as_str())) clippy_args.extend(lint_filter.iter().map(std::string::String::as_str));
} }
if let Some(server) = server { if let Some(server) = server {
@ -463,7 +466,7 @@ fn read_crates(toml_path: &Path) -> (Vec<CrateSource>, RecursiveOptions) {
// flatten TomlCrates into CrateSources (one TomlCrates may represent several versions of a crate => // flatten TomlCrates into CrateSources (one TomlCrates may represent several versions of a crate =>
// multiple Cratesources) // multiple Cratesources)
let mut crate_sources = Vec::new(); let mut crate_sources = Vec::new();
tomlcrates.into_iter().for_each(|tk| { for tk in tomlcrates {
if let Some(ref path) = tk.path { if let Some(ref path) = tk.path {
crate_sources.push(CrateSource::Path { crate_sources.push(CrateSource::Path {
name: tk.name.clone(), name: tk.name.clone(),
@ -472,13 +475,13 @@ fn read_crates(toml_path: &Path) -> (Vec<CrateSource>, RecursiveOptions) {
}); });
} else if let Some(ref versions) = tk.versions { } else if let Some(ref versions) = tk.versions {
// if we have multiple versions, save each one // if we have multiple versions, save each one
versions.iter().for_each(|ver| { for ver in versions.iter() {
crate_sources.push(CrateSource::CratesIo { crate_sources.push(CrateSource::CratesIo {
name: tk.name.clone(), name: tk.name.clone(),
version: ver.to_string(), version: ver.to_string(),
options: tk.options.clone(), options: tk.options.clone(),
}); });
}) }
} else if tk.git_url.is_some() && tk.git_hash.is_some() { } else if tk.git_url.is_some() && tk.git_hash.is_some() {
// otherwise, we should have a git source // otherwise, we should have a git source
crate_sources.push(CrateSource::Git { crate_sources.push(CrateSource::Git {
@ -496,15 +499,18 @@ fn read_crates(toml_path: &Path) -> (Vec<CrateSource>, RecursiveOptions) {
|| tk.git_hash.is_some() != tk.git_url.is_some() || tk.git_hash.is_some() != tk.git_url.is_some()
{ {
eprintln!("tomlkrate: {tk:?}"); eprintln!("tomlkrate: {tk:?}");
if tk.git_hash.is_some() != tk.git_url.is_some() { assert_eq!(
panic!("Error: Encountered TomlCrate with only one of git_hash and git_url!"); tk.git_hash.is_some(),
} tk.git_url.is_some(),
if tk.path.is_some() && (tk.git_hash.is_some() || tk.versions.is_some()) { "Error: Encountered TomlCrate with only one of git_hash and git_url!"
panic!("Error: TomlCrate can only have one of 'git_.*', 'version' or 'path' fields"); );
} assert!(
tk.path.is_none() || (tk.git_hash.is_none() && tk.versions.is_none()),
"Error: TomlCrate can only have one of 'git_.*', 'version' or 'path' fields"
);
unreachable!("Failed to translate TomlCrate into CrateSource!"); unreachable!("Failed to translate TomlCrate into CrateSource!");
} }
}); }
// sort the crates // sort the crates
crate_sources.sort(); crate_sources.sort();
@ -566,6 +572,7 @@ fn lintcheck_needs_rerun(lintcheck_logs_path: &Path, paths: [&Path; 2]) -> bool
logs_modified < clippy_modified logs_modified < clippy_modified
} }
#[allow(clippy::too_many_lines)]
fn main() { fn main() {
// We're being executed as a `RUSTC_WRAPPER` as part of `--recursive` // We're being executed as a `RUSTC_WRAPPER` as part of `--recursive`
if let Ok(addr) = env::var("LINTCHECK_SERVER") { if let Ok(addr) = env::var("LINTCHECK_SERVER") {
@ -671,7 +678,7 @@ fn main() {
.unwrap(); .unwrap();
let server = config.recursive.then(|| { let server = config.recursive.then(|| {
let _ = fs::remove_dir_all("target/lintcheck/shared_target_dir/recursive"); fs::remove_dir_all("target/lintcheck/shared_target_dir/recursive").unwrap_or_default();
LintcheckServer::spawn(recursive_options) LintcheckServer::spawn(recursive_options)
}); });
@ -727,7 +734,7 @@ fn main() {
} }
write!(text, "{}", all_msgs.join("")).unwrap(); write!(text, "{}", all_msgs.join("")).unwrap();
text.push_str("\n\n### ICEs:\n"); text.push_str("\n\n### ICEs:\n");
for (cratename, msg) in ices.iter() { for (cratename, msg) in &ices {
let _ = write!(text, "{cratename}: '{msg}'"); let _ = write!(text, "{cratename}: '{msg}'");
} }
@ -780,10 +787,10 @@ fn print_stats(old_stats: HashMap<String, usize>, new_stats: HashMap<&String, us
let mut new_stats_deduped = new_stats; let mut new_stats_deduped = new_stats;
// remove duplicates from both hashmaps // remove duplicates from both hashmaps
same_in_both_hashmaps.iter().for_each(|(k, v)| { for (k, v) in &same_in_both_hashmaps {
assert!(old_stats_deduped.remove(k) == Some(*v)); assert!(old_stats_deduped.remove(k) == Some(*v));
assert!(new_stats_deduped.remove(k) == Some(*v)); assert!(new_stats_deduped.remove(k) == Some(*v));
}); }
println!("\nStats:"); println!("\nStats:");
@ -821,19 +828,21 @@ fn print_stats(old_stats: HashMap<String, usize>, new_stats: HashMap<&String, us
/// This function panics if creating one of the dirs fails. /// This function panics if creating one of the dirs fails.
fn create_dirs(krate_download_dir: &Path, extract_dir: &Path) { fn create_dirs(krate_download_dir: &Path, extract_dir: &Path) {
std::fs::create_dir("target/lintcheck/").unwrap_or_else(|err| { std::fs::create_dir("target/lintcheck/").unwrap_or_else(|err| {
if err.kind() != ErrorKind::AlreadyExists { assert_eq!(
panic!("cannot create lintcheck target dir"); err.kind(),
} ErrorKind::AlreadyExists,
"cannot create lintcheck target dir"
);
}); });
std::fs::create_dir(krate_download_dir).unwrap_or_else(|err| { std::fs::create_dir(krate_download_dir).unwrap_or_else(|err| {
if err.kind() != ErrorKind::AlreadyExists { assert_eq!(err.kind(), ErrorKind::AlreadyExists, "cannot create crate download dir");
panic!("cannot create crate download dir");
}
}); });
std::fs::create_dir(extract_dir).unwrap_or_else(|err| { std::fs::create_dir(extract_dir).unwrap_or_else(|err| {
if err.kind() != ErrorKind::AlreadyExists { assert_eq!(
panic!("cannot create crate extraction dir"); err.kind(),
} ErrorKind::AlreadyExists,
"cannot create crate extraction dir"
);
}); });
} }

View File

@ -1,7 +1,7 @@
//! In `--recursive` mode we set the `lintcheck` binary as the `RUSTC_WRAPPER` of `cargo check`, //! In `--recursive` mode we set the `lintcheck` binary as the `RUSTC_WRAPPER` of `cargo check`,
//! this allows [crate::driver] to be run for every dependency. The driver connects to //! this allows [`crate::driver`] to be run for every dependency. The driver connects to
//! [LintcheckServer] to ask if it should be skipped, and if not sends the stderr of running clippy //! [`LintcheckServer`] to ask if it should be skipped, and if not sends the stderr of running
//! on the crate to the server //! clippy on the crate to the server
use crate::ClippyWarning; use crate::ClippyWarning;
use crate::RecursiveOptions; use crate::RecursiveOptions;
@ -109,8 +109,8 @@ impl LintcheckServer {
Self { Self {
local_addr, local_addr,
sender,
receiver, receiver,
sender,
} }
} }