diff options
| author | bors <bors@rust-lang.org> | 2022-09-30 22:29:40 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-09-30 22:29:40 +0000 |
| commit | 31b17411a6cd5c4b36cde6ff008de1d3ec128ac4 (patch) | |
| tree | 535340407dc55a2b7119b8fd8b0c1738dbd4f9ec | |
| parent | a78551bb14733216f734660d0598d392a6c5db20 (diff) | |
| parent | fc77d91469244bbba292d6746d7f206dbba812b5 (diff) | |
| download | rust-31b17411a6cd5c4b36cde6ff008de1d3ec128ac4.tar.gz rust-31b17411a6cd5c4b36cde6ff008de1d3ec128ac4.zip | |
Auto merge of #9510 - Alexendoo:lintcheck-server2, r=matthiaskrgr
Add `cargo lintcheck --recursive` to check dependencies of crates r? `@matthiaskrgr` This just adds the mode without changing how a regular run works Takes a fair bit longer to run than a `-j4` or regular run ``` cargo lintcheck -j4 468.33s user 43.78s system 290% cpu 2:56.42 total cargo lintcheck 428.81s user 41.85s system 199% cpu 3:55.37 total cargo lintcheck --recursive 679.83s user 45.01s system 210% cpu 5:43.72 total ``` But finds more results: <details> <summary>Stats</summary> <pre><code>clippy::explicit_counter_loop 0 => 1 clippy::needless_question_mark 0 => 3 clippy::unnecessary_cast 0 => 2 clippy::to_string_in_format_args 0 => 4 clippy::deprecated_cfg_attr 0 => 23 clippy::redundant_closure 0 => 9 clippy::drop_copy 0 => 4 clippy::double_must_use 0 => 1 clippy::transmute_num_to_bytes 0 => 9 clippy::bind_instead_of_map 0 => 14 clippy::float_cmp 0 => 16 clippy::is_digit_ascii_radix 0 => 16 clippy::manual_swap 0 => 1 clippy::needless_match 0 => 2 clippy::vec_init_then_push 0 => 1 clippy::never_loop 0 => 1 clippy::option_map_or_none 0 => 4 clippy::tabs_in_doc_comments 0 => 1 clippy::naive_bytecount 0 => 1 clippy::collapsible_if 0 => 24 clippy::copy_iterator 0 => 5 clippy::unused_io_amount 0 => 2 clippy::result_large_err 0 => 141 clippy::useless_conversion 0 => 24 clippy::flat_map_option 0 => 8 clippy::useless_format 0 => 2 clippy::module_inception 0 => 1 clippy::drop_ref 0 => 2 clippy::unnecessary_fold 0 => 2 clippy::neg_multiply 0 => 1 clippy::while_let_loop 0 => 6 clippy::missing_inline_in_public_items 0 => 37 clippy::unnecessary_mut_passed 0 => 1 unknown_lints 0 => 15 clippy::wildcard_dependencies 0 => 3 clippy::same_item_push 0 => 2 clippy::useless_asref 0 => 1 clippy::unnecessary_unwrap 0 => 4 clippy::iter_not_returning_iterator 0 => 5 clippy::comparison_to_empty 0 => 10 clippy::ref_option_ref 0 => 4 clippy::unused_peekable 0 => 1 clippy::needless_range_loop 0 => 8 clippy::absurd_extreme_comparisons 0 => 2 clippy::unnecessary_operation 0 => 2 clippy::for_kv_map 0 => 5 clippy::unnecessary_owned_empty_strings 0 => 3 clippy::transmutes_expressible_as_ptr_casts 0 => 1 clippy::toplevel_ref_arg 0 => 2 clippy::uninit_vec 0 => 3 clippy::filter_next 0 => 1 clippy::wildcard_in_or_patterns 0 => 6 clippy::cast_ptr_alignment 0 => 48 clippy::manual_memcpy 0 => 1 clippy::assign_op_pattern 0 => 313 clippy::unnecessary_lazy_evaluations 0 => 14 clippy::println_empty_string 0 => 2 clippy::redundant_pattern 0 => 2 clippy::declare_interior_mutable_const 0 => 8 clippy::large_stack_arrays 0 => 4 clippy::match_bool 0 => 4 clippy::unicode_not_nfc 0 => 2075 clippy::inconsistent_digit_grouping 0 => 4 clippy::no_effect_underscore_binding 0 => 2 clippy::let_and_return 0 => 5 clippy::transmute_ptr_to_ref 0 => 12 clippy::op_ref 0 => 13 clippy::unnecessary_join 0 => 4 clippy::into_iter_on_ref 0 => 13 clippy::from_str_radix_10 0 => 7 clippy::ptr_offset_with_cast 0 => 48 clippy::erasing_op 0 => 1 clippy::swap_ptr_to_ref 0 => 3 clippy::needless_bitwise_bool 0 => 2 clippy::extend_with_drain 0 => 19 clippy::only_used_in_recursion 0 => 4 clippy::needless_late_init 0 => 8 clippy::excessive_precision 0 => 1959 clippy::match_ref_pats 0 => 10 clippy::unit_arg 0 => 20 clippy::bool_comparison 0 => 4 clippy::bool_assert_comparison 0 => 1 clippy::eq_op 0 => 6 clippy::cast_abs_to_unsigned 0 => 6 clippy::format_in_format_args 0 => 1 clippy::iter_cloned_collect 0 => 4 clippy::ptr_eq 0 => 3 clippy::needless_bool 0 => 5 clippy::transmute_ptr_to_ptr 0 => 16 clippy::needless_option_take 0 => 2 clippy::flat_map_identity 0 => 1 clippy::needless_splitn 0 => 2 clippy::blocks_in_if_conditions 0 => 1 clippy::write_literal 0 => 1 clippy::manual_split_once 0 => 1 clippy::result_unit_err 0 => 36 clippy::unused_unit 0 => 11 clippy::single_match 0 => 22 clippy::manual_find 0 => 3 clippy::derive_ord_xor_partial_ord 0 => 6 clippy::char_lit_as_u8 0 => 2 clippy::let_unit_value 0 => 2 clippy::needless_continue 0 => 19 clippy::zero_sized_map_values 0 => 4 clippy::needless_arbitrary_self_type 0 => 6 clippy::partialeq_to_none 0 => 11 clippy::partialeq_ne_impl 0 => 1 clippy::invalid_upcast_comparisons 0 => 1 clippy::mut_range_bound 0 => 4 clippy::match_result_ok 0 => 2 clippy::ptr_arg 0 => 8 clippy::iter_nth_zero 0 => 18 clippy::needless_for_each 0 => 1 clippy::manual_unwrap_or 0 => 1 clippy::transmute_int_to_float 0 => 6 clippy::cast_slice_from_raw_parts 0 => 1 clippy::match_wild_err_arm 0 => 2 clippy::match_like_matches_macro 4 => 116 clippy::enum_glob_use 50 => 380 clippy::get_first 3 => 33 clippy::needless_doctest_main 10 => 26 clippy::struct_excessive_bools 19 => 51 clippy::cast_possible_wrap 46 => 538 clippy::manual_string_new 10 => 27 clippy::match_same_arms 53 => 1039 clippy::manual_non_exhaustive 1 => 33 clippy::redundant_pattern_matching 2 => 13 clippy::new_without_default 5 => 73 clippy::option_as_ref_deref 2 => 9 clippy::unwrap_or_else_default 2 => 4 clippy::case_sensitive_file_extension_comparisons 6 => 9 clippy::cast_precision_loss 45 => 110 clippy::needless_pass_by_value 26 => 187 clippy::redundant_closure_for_method_calls 170 => 539 clippy::let_underscore_drop 33 => 133 clippy::single_match_else 51 => 138 clippy::needless_borrow 24 => 382 clippy::redundant_else 37 => 151 clippy::type_complexity 2 => 22 clippy::ptr_as_ptr 93 => 1135 clippy::needless_lifetimes 7 => 100 clippy::single_char_add_str 2 => 22 clippy::similar_names 99 => 352 clippy::cargo_common_metadata 25 => 276 clippy::int_plus_one 1 => 2 clippy::missing_safety_doc 9 => 152 clippy::redundant_slicing 2 => 13 clippy::mut_mut 2 => 17 clippy::derive_partial_eq_without_eq 8 => 141 clippy::derive_hash_xor_eq 2 => 20 clippy::from_iter_instead_of_collect 2 => 17 clippy::verbose_bit_mask 1 => 8 clippy::too_many_lines 58 => 162 clippy::module_name_repetitions 178 => 1104 clippy::explicit_into_iter_loop 12 => 32 clippy::cast_lossless 45 => 478 clippy::many_single_char_names 9 => 23 clippy::unnested_or_patterns 27 => 127 clippy::upper_case_acronyms 5 => 29 clippy::needless_return 5 => 97 clippy::precedence 1 => 11 clippy::len_zero 2 => 70 clippy::manual_strip 2 => 30 clippy::derivable_impls 2 => 12 clippy::unused_self 20 => 187 clippy::enum_variant_names 1 => 6 clippy::self_named_constructors 1 => 3 clippy::explicit_auto_deref 19 => 314 clippy::semicolon_if_nothing_returned 137 => 1861 clippy::should_implement_trait 1 => 7 clippy::expl_impl_clone_on_copy 159 => 1318 clippy::stable_sort_primitive 4 => 12 clippy::mem_replace_with_default 7 => 46 clippy::borrow_deref_ref 5 => 140 clippy::large_enum_variant 1 => 4 clippy::map_unwrap_or 30 => 203 clippy::zero_ptr 3 => 25 clippy::filter_map_next 2 => 6 clippy::identity_op 5 => 76 clippy::checked_conversions 1 => 8 clippy::len_without_is_empty 9 => 47 clippy::missing_errors_doc 372 => 2333 clippy::fn_params_excessive_bools 3 => 7 clippy::single_component_path_imports 6 => 28 clippy::unreadable_literal 366 => 9814 clippy::field_reassign_with_default 1 => 5 clippy::redundant_clone 1 => 8 clippy::cloned_instead_of_copied 36 => 78 clippy::too_many_arguments 4 => 22 clippy::option_map_unit_fn 7 => 9 clippy::extra_unused_lifetimes 1 => 24 clippy::unnecessary_wraps 26 => 128 clippy::used_underscore_binding 1 => 50 clippy::inconsistent_struct_constructor 2 => 7 clippy::manual_range_contains 9 => 120 clippy::map_clone 7 => 46 clippy::cast_slice_different_sizes 1 => 4 clippy::missing_panics_doc 114 => 603 renamed_and_removed_lints 3 => 9 clippy::items_after_statements 155 => 309 clippy::inefficient_to_string 5 => 6 clippy::comparison_chain 1 => 19 clippy::crate_in_macro_def 3 => 6 clippy::write_with_newline 2 => 36 clippy::manual_saturating_arithmetic 1 => 2 clippy::clone_on_copy 1 => 86 clippy::negative_feature_names 3 => 16 clippy::redundant_field_names 112 => 1013 clippy::from_over_into 2 => 28 clippy::wildcard_imports 178 => 376 clippy::unusual_byte_groupings 19 => 65 clippy::option_option 2 => 10 clippy::nonminimal_bool 1 => 17 clippy::borrow_as_ptr 2 => 172 clippy::redundant_static_lifetimes 24 => 1701 clippy::or_fun_call 1 => 63 clippy::single_char_pattern 3 => 79 clippy::explicit_iter_loop 72 => 148 clippy::collapsible_else_if 2 => 27 clippy::manual_str_repeat 1 => 6 clippy::if_same_then_else 3 => 31 clippy::while_let_on_iterator 4 => 28 clippy::multiple_crate_versions 5 => 19 clippy::cast_possible_truncation 115 => 1172 clippy::explicit_deref_methods 1 => 38 clippy::default_trait_access 48 => 130 clippy::question_mark 2 => 28 clippy::must_use_candidate 612 => 5369 clippy::manual_map 1 => 12 clippy::bool_to_int_with_if 2 => 15 clippy::doc_markdown 202 => 1709 clippy::cast_sign_loss 60 => 477 clippy::wrong_self_convention 11 => 45 clippy::transmute_float_to_int 6 => 18 clippy::return_self_not_must_use 66 => 736 clippy::range_plus_one 1 => 36 clippy::manual_assert 11 => 62 clippy::trivially_copy_pass_by_ref 40 => 189 clippy::match_on_vec_items 2 => 7 clippy::inline_always 59 => 1079 clippy::if_not_else 31 => 205 clippy::implicit_clone 10 => 32 clippy::match_wildcard_for_single_variants 16 => 101 clippy::doc_link_with_quotes 7 => 35 clippy::redundant_feature_names 4 => 41 </code></pre></details> changelog: none
| -rw-r--r-- | lintcheck/Cargo.toml | 2 | ||||
| -rw-r--r-- | lintcheck/README.md | 20 | ||||
| -rw-r--r-- | lintcheck/lintcheck_crates.toml | 8 | ||||
| -rw-r--r-- | lintcheck/src/config.rs | 10 | ||||
| -rw-r--r-- | lintcheck/src/driver.rs | 67 | ||||
| -rw-r--r-- | lintcheck/src/main.rs | 164 | ||||
| -rw-r--r-- | lintcheck/src/recursive.rs | 123 |
7 files changed, 339 insertions, 55 deletions
diff --git a/lintcheck/Cargo.toml b/lintcheck/Cargo.toml index 737c845c045..de31c16b819 100644 --- a/lintcheck/Cargo.toml +++ b/lintcheck/Cargo.toml @@ -12,9 +12,11 @@ publish = false [dependencies] cargo_metadata = "0.14" clap = "3.2" +crossbeam-channel = "0.5.6" flate2 = "1.0" rayon = "1.5.1" serde = { version = "1.0", features = ["derive"] } +serde_json = "1.0.85" tar = "0.4" toml = "0.5" ureq = "2.2" diff --git a/lintcheck/README.md b/lintcheck/README.md index 6f3d23382ce..6142de5e313 100644 --- a/lintcheck/README.md +++ b/lintcheck/README.md @@ -69,9 +69,27 @@ is checked. is explicitly specified in the options. ### Fix mode -You can run `./lintcheck/target/debug/lintcheck --fix` which will run Clippy with `--fix` and +You can run `cargo lintcheck --fix` which will run Clippy with `--fix` and print a warning if Clippy's suggestions fail to apply (if the resulting code does not build). This lets us spot bad suggestions or false positives automatically in some cases. Please note that the target dir should be cleaned afterwards since clippy will modify the downloaded sources which can lead to unexpected results when running lintcheck again afterwards. + +### Recursive mode +You can run `cargo lintcheck --recursive` to also run Clippy on the dependencies +of the crates listed in the crates source `.toml`. e.g. adding `rand 0.8.5` +would also lint `rand_core`, `rand_chacha`, etc. + +Particularly slow crates in the dependency graph can be ignored using +`recursive.ignore`: + +```toml +[crates] +cargo = {name = "cargo", versions = ['0.64.0']} + +[recursive] +ignore = [ + "unicode-normalization", +] +``` diff --git a/lintcheck/lintcheck_crates.toml b/lintcheck/lintcheck_crates.toml index ebbe9c9ae67..52f7fee47b6 100644 --- a/lintcheck/lintcheck_crates.toml +++ b/lintcheck/lintcheck_crates.toml @@ -33,3 +33,11 @@ cfg-expr = {name = "cfg-expr", versions = ['0.7.1']} puffin = {name = "puffin", git_url = "https://github.com/EmbarkStudios/puffin", git_hash = "02dd4a3"} rpmalloc = {name = "rpmalloc", versions = ['0.2.0']} tame-oidc = {name = "tame-oidc", versions = ['0.1.0']} + +[recursive] +ignore = [ + # Takes ~30s to lint + "combine", + # Has 1.2 million `clippy::match_same_arms`s + "unicode-normalization", +] diff --git a/lintcheck/src/config.rs b/lintcheck/src/config.rs index 1742cf677c0..b344db634f6 100644 --- a/lintcheck/src/config.rs +++ b/lintcheck/src/config.rs @@ -34,11 +34,16 @@ fn get_clap_config() -> ArgMatches { Arg::new("markdown") .long("markdown") .help("Change the reports table to use markdown links"), + Arg::new("recursive") + .long("--recursive") + .help("Run clippy on the dependencies of crates specified in crates-toml") + .conflicts_with("threads") + .conflicts_with("fix"), ]) .get_matches() } -#[derive(Debug)] +#[derive(Debug, Clone)] pub(crate) struct LintcheckConfig { /// max number of jobs to spawn (default 1) pub max_jobs: usize, @@ -54,6 +59,8 @@ pub(crate) struct LintcheckConfig { pub lint_filter: Vec<String>, /// Indicate if the output should support markdown syntax pub markdown: bool, + /// Run clippy on the dependencies of crates + pub recursive: bool, } impl LintcheckConfig { @@ -119,6 +126,7 @@ impl LintcheckConfig { fix: clap_config.contains_id("fix"), lint_filter, markdown, + recursive: clap_config.contains_id("recursive"), } } } diff --git a/lintcheck/src/driver.rs b/lintcheck/src/driver.rs new file mode 100644 index 00000000000..63221bab32d --- /dev/null +++ b/lintcheck/src/driver.rs @@ -0,0 +1,67 @@ +use crate::recursive::{deserialize_line, serialize_line, DriverInfo}; + +use std::io::{self, BufReader, Write}; +use std::net::TcpStream; +use std::process::{self, Command, Stdio}; +use std::{env, mem}; + +/// 1. Sends [DriverInfo] to the [crate::recursive::LintcheckServer] running on `addr` +/// 2. Receives [bool] from the server, if `false` returns `None` +/// 3. Otherwise sends the stderr of running `clippy-driver` to the server +fn run_clippy(addr: &str) -> Option<i32> { + let driver_info = DriverInfo { + package_name: env::var("CARGO_PKG_NAME").ok()?, + crate_name: env::var("CARGO_CRATE_NAME").ok()?, + version: env::var("CARGO_PKG_VERSION").ok()?, + }; + + let mut stream = BufReader::new(TcpStream::connect(addr).unwrap()); + + serialize_line(&driver_info, stream.get_mut()); + + let should_run = deserialize_line::<bool, _>(&mut stream); + if !should_run { + return None; + } + + // Remove --cap-lints allow so that clippy runs and lints are emitted + let mut include_next = true; + let args = env::args().skip(1).filter(|arg| match arg.as_str() { + "--cap-lints=allow" => false, + "--cap-lints" => { + include_next = false; + false + }, + _ => mem::replace(&mut include_next, true), + }); + + let output = Command::new(env::var("CLIPPY_DRIVER").expect("missing env CLIPPY_DRIVER")) + .args(args) + .stdout(Stdio::inherit()) + .output() + .expect("failed to run clippy-driver"); + + stream + .get_mut() + .write_all(&output.stderr) + .unwrap_or_else(|e| panic!("{e:?} in {driver_info:?}")); + + match output.status.code() { + Some(0) => Some(0), + code => { + io::stderr().write_all(&output.stderr).unwrap(); + Some(code.expect("killed by signal")) + }, + } +} + +pub fn drive(addr: &str) { + process::exit(run_clippy(addr).unwrap_or_else(|| { + Command::new("rustc") + .args(env::args_os().skip(2)) + .status() + .unwrap() + .code() + .unwrap() + })) +} diff --git a/lintcheck/src/main.rs b/lintcheck/src/main.rs index 9ee25280f04..cc2b3e1acec 100644 --- a/lintcheck/src/main.rs +++ b/lintcheck/src/main.rs @@ -8,13 +8,17 @@ #![allow(clippy::collapsible_else_if)] mod config; +mod driver; +mod recursive; -use config::LintcheckConfig; +use crate::config::LintcheckConfig; +use crate::recursive::LintcheckServer; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::env; +use std::env::consts::EXE_SUFFIX; use std::fmt::Write as _; -use std::fs::write; +use std::fs; use std::io::ErrorKind; use std::path::{Path, PathBuf}; use std::process::Command; @@ -22,22 +26,12 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use std::thread; use std::time::Duration; -use cargo_metadata::diagnostic::DiagnosticLevel; +use cargo_metadata::diagnostic::{Diagnostic, DiagnosticLevel}; use cargo_metadata::Message; use rayon::prelude::*; use serde::{Deserialize, Serialize}; use walkdir::{DirEntry, WalkDir}; -#[cfg(not(windows))] -const CLIPPY_DRIVER_PATH: &str = "target/debug/clippy-driver"; -#[cfg(not(windows))] -const CARGO_CLIPPY_PATH: &str = "target/debug/cargo-clippy"; - -#[cfg(windows)] -const CLIPPY_DRIVER_PATH: &str = "target/debug/clippy-driver.exe"; -#[cfg(windows)] -const CARGO_CLIPPY_PATH: &str = "target/debug/cargo-clippy.exe"; - const LINTCHECK_DOWNLOADS: &str = "target/lintcheck/downloads"; const LINTCHECK_SOURCES: &str = "target/lintcheck/sources"; @@ -45,6 +39,13 @@ const LINTCHECK_SOURCES: &str = "target/lintcheck/sources"; #[derive(Debug, Serialize, Deserialize)] struct SourceList { crates: HashMap<String, TomlCrate>, + #[serde(default)] + recursive: RecursiveOptions, +} + +#[derive(Debug, Serialize, Deserialize, Default)] +struct RecursiveOptions { + ignore: HashSet<String>, } /// A crate source stored inside the .toml @@ -105,12 +106,7 @@ struct ClippyWarning { #[allow(unused)] impl ClippyWarning { - fn new(cargo_message: Message, krate: &Crate) -> Option<Self> { - let diag = match cargo_message { - Message::CompilerMessage(message) => message.message, - _ => return None, - }; - + fn new(diag: Diagnostic, crate_name: &str, crate_version: &str) -> Option<Self> { let lint_type = diag.code?.code; if !(lint_type.contains("clippy") || diag.message.contains("clippy")) || diag.message.contains("could not read cargo metadata") @@ -124,12 +120,12 @@ impl ClippyWarning { Ok(stripped) => format!("$CARGO_HOME/{}", stripped.display()), Err(_) => format!( "target/lintcheck/sources/{}-{}/{}", - krate.name, krate.version, span.file_name + crate_name, crate_version, span.file_name ), }; Some(Self { - crate_name: krate.name.clone(), + crate_name: crate_name.to_owned(), file, line: span.line_start, column: span.column_start, @@ -142,8 +138,6 @@ impl ClippyWarning { fn to_output(&self, markdown: bool) -> String { let file_with_pos = format!("{}:{}:{}", &self.file, &self.line, &self.column); if markdown { - let lint = format!("`{}`", self.lint_type); - let mut file = self.file.clone(); if !file.starts_with('$') { file.insert_str(0, "../"); @@ -151,7 +145,7 @@ impl ClippyWarning { let mut output = String::from("| "); let _ = write!(output, "[`{}`]({}#L{})", file_with_pos, file, self.line); - let _ = write!(output, r#" | {:<50} | "{}" |"#, lint, self.message); + let _ = write!(output, r#" | `{:<50}` | "{}" |"#, self.lint_type, self.message); output.push('\n'); output } else { @@ -243,6 +237,7 @@ impl CrateSource { } // check out the commit/branch/whatever if !Command::new("git") + .args(["-c", "advice.detachedHead=false"]) .arg("checkout") .arg(commit) .current_dir(&repo_path) @@ -309,10 +304,12 @@ impl Crate { fn run_clippy_lints( &self, cargo_clippy_path: &Path, + clippy_driver_path: &Path, target_dir_index: &AtomicUsize, total_crates_to_lint: usize, config: &LintcheckConfig, lint_filter: &Vec<String>, + server: &Option<LintcheckServer>, ) -> Vec<ClippyWarning> { // advance the atomic index by one let index = target_dir_index.fetch_add(1, Ordering::SeqCst); @@ -336,36 +333,67 @@ impl Crate { let shared_target_dir = clippy_project_root().join("target/lintcheck/shared_target_dir"); - let mut args = if config.fix { + let mut cargo_clippy_args = if config.fix { vec!["--fix", "--"] } else { vec!["--", "--message-format=json", "--"] }; + let mut clippy_args = Vec::<&str>::new(); if let Some(options) = &self.options { for opt in options { - args.push(opt); + clippy_args.push(opt); } } else { - args.extend(&["-Wclippy::pedantic", "-Wclippy::cargo"]) + clippy_args.extend(&["-Wclippy::pedantic", "-Wclippy::cargo"]) } if lint_filter.is_empty() { - args.push("--cap-lints=warn"); + clippy_args.push("--cap-lints=warn"); } else { - args.push("--cap-lints=allow"); - args.extend(lint_filter.iter().map(|filter| filter.as_str())) + clippy_args.push("--cap-lints=allow"); + clippy_args.extend(lint_filter.iter().map(|filter| filter.as_str())) + } + + if let Some(server) = server { + let target = shared_target_dir.join("recursive"); + + // `cargo clippy` is a wrapper around `cargo check` that mainly sets `RUSTC_WORKSPACE_WRAPPER` to + // `clippy-driver`. We do the same thing here with a couple changes: + // + // `RUSTC_WRAPPER` is used instead of `RUSTC_WORKSPACE_WRAPPER` so that we can lint all crate + // dependencies rather than only workspace members + // + // The wrapper is set to the `lintcheck` so we can force enable linting and ignore certain crates + // (see `crate::driver`) + let status = Command::new("cargo") + .arg("check") + .arg("--quiet") + .current_dir(&self.path) + .env("CLIPPY_ARGS", clippy_args.join("__CLIPPY_HACKERY__")) + .env("CARGO_TARGET_DIR", target) + .env("RUSTC_WRAPPER", env::current_exe().unwrap()) + // Pass the absolute path so `crate::driver` can find `clippy-driver`, as it's executed in various + // different working directories + .env("CLIPPY_DRIVER", clippy_driver_path) + .env("LINTCHECK_SERVER", server.local_addr.to_string()) + .status() + .expect("failed to run cargo"); + + assert_eq!(status.code(), Some(0)); + + return Vec::new(); } - let all_output = std::process::Command::new(&cargo_clippy_path) + cargo_clippy_args.extend(clippy_args); + + let all_output = Command::new(&cargo_clippy_path) // use the looping index to create individual target dirs .env( "CARGO_TARGET_DIR", shared_target_dir.join(format!("_{:?}", thread_index)), ) - // lint warnings will look like this: - // src/cargo/ops/cargo_compile.rs:127:35: warning: usage of `FromIterator::from_iter` - .args(&args) + .args(&cargo_clippy_args) .current_dir(&self.path) .output() .unwrap_or_else(|error| { @@ -404,7 +432,10 @@ impl Crate { // get all clippy warnings and ICEs let warnings: Vec<ClippyWarning> = Message::parse_stream(stdout.as_bytes()) - .filter_map(|msg| ClippyWarning::new(msg.unwrap(), &self)) + .filter_map(|msg| match msg { + Ok(Message::CompilerMessage(message)) => ClippyWarning::new(message.message, &self.name, &self.version), + _ => None, + }) .collect(); warnings @@ -423,8 +454,8 @@ 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: &Path) -> Vec<CrateSource> { +/// Read a `lintcheck_crates.toml` file +fn read_crates(toml_path: &Path) -> (Vec<CrateSource>, RecursiveOptions) { 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 = @@ -484,7 +515,7 @@ fn read_crates(toml_path: &Path) -> Vec<CrateSource> { // sort the crates crate_sources.sort(); - crate_sources + (crate_sources, crate_list.recursive) } /// Generate a short list of occurring lints-types and their count @@ -516,20 +547,20 @@ fn gather_stats(clippy_warnings: &[ClippyWarning]) -> (String, HashMap<&String, /// check if the latest modification of the logfile is older than the modification date of the /// clippy binary, if this is true, we should clean the lintchec shared target directory and recheck -fn lintcheck_needs_rerun(lintcheck_logs_path: &Path) -> bool { +fn lintcheck_needs_rerun(lintcheck_logs_path: &Path, paths: [&Path; 2]) -> bool { if !lintcheck_logs_path.exists() { return true; } let clippy_modified: std::time::SystemTime = { - let mut times = [CLIPPY_DRIVER_PATH, CARGO_CLIPPY_PATH].iter().map(|p| { + let [cargo, driver] = paths.map(|p| { std::fs::metadata(p) .expect("failed to get metadata of file") .modified() .expect("failed to get modification date") }); // the oldest modification of either of the binaries - std::cmp::max(times.next().unwrap(), times.next().unwrap()) + std::cmp::max(cargo, driver) }; let logs_modified: std::time::SystemTime = std::fs::metadata(lintcheck_logs_path) @@ -543,6 +574,11 @@ fn lintcheck_needs_rerun(lintcheck_logs_path: &Path) -> bool { } fn main() { + // We're being executed as a `RUSTC_WRAPPER` as part of `--recursive` + if let Ok(addr) = env::var("LINTCHECK_SERVER") { + driver::drive(&addr); + } + // assert that we launch lintcheck from the repo root (via cargo lintcheck) if std::fs::metadata("lintcheck/Cargo.toml").is_err() { eprintln!("lintcheck needs to be run from clippy's repo root!\nUse `cargo lintcheck` alternatively."); @@ -555,9 +591,15 @@ fn main() { build_clippy(); println!("Done compiling"); + let cargo_clippy_path = fs::canonicalize(format!("target/debug/cargo-clippy{EXE_SUFFIX}")).unwrap(); + let clippy_driver_path = fs::canonicalize(format!("target/debug/clippy-driver{EXE_SUFFIX}")).unwrap(); + // if the clippy bin is newer than our logs, throw away target dirs to force clippy to // refresh the logs - if lintcheck_needs_rerun(&config.lintcheck_results_path) { + if lintcheck_needs_rerun( + &config.lintcheck_results_path, + [&cargo_clippy_path, &clippy_driver_path], + ) { let shared_target_dir = "target/lintcheck/shared_target_dir"; // if we get an Err here, the shared target dir probably does simply not exist if let Ok(metadata) = std::fs::metadata(&shared_target_dir) { @@ -569,10 +611,6 @@ fn main() { } } - let cargo_clippy_path: PathBuf = PathBuf::from(CARGO_CLIPPY_PATH) - .canonicalize() - .expect("failed to canonicalize path to clippy binary"); - // assert that clippy is found assert!( cargo_clippy_path.is_file(), @@ -580,7 +618,7 @@ fn main() { cargo_clippy_path.display() ); - let clippy_ver = std::process::Command::new(CARGO_CLIPPY_PATH) + let clippy_ver = std::process::Command::new(&cargo_clippy_path) .arg("--version") .output() .map(|o| String::from_utf8_lossy(&o.stdout).into_owned()) @@ -589,7 +627,7 @@ fn main() { // download and extract the crates, then run clippy on them and collect clippy's warnings // flatten into one big list of warnings - let crates = read_crates(&config.sources_toml_path); + 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); @@ -639,11 +677,31 @@ fn main() { .build_global() .unwrap(); - let clippy_warnings: Vec<ClippyWarning> = crates + let server = config.recursive.then(|| { + let _ = fs::remove_dir_all("target/lintcheck/shared_target_dir/recursive"); + + LintcheckServer::spawn(recursive_options) + }); + + let mut clippy_warnings: Vec<ClippyWarning> = crates .par_iter() - .flat_map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &counter, crates.len(), &config, &lint_filter)) + .flat_map(|krate| { + krate.run_clippy_lints( + &cargo_clippy_path, + &clippy_driver_path, + &counter, + crates.len(), + &config, + &lint_filter, + &server, + ) + }) .collect(); + if let Some(server) = server { + clippy_warnings.extend(server.warnings()); + } + // if we are in --fix mode, don't change the log files, terminate here if config.fix { return; @@ -681,8 +739,8 @@ fn main() { } println!("Writing logs to {}", config.lintcheck_results_path.display()); - std::fs::create_dir_all(config.lintcheck_results_path.parent().unwrap()).unwrap(); - write(&config.lintcheck_results_path, text).unwrap(); + 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); } diff --git a/lintcheck/src/recursive.rs b/lintcheck/src/recursive.rs new file mode 100644 index 00000000000..67dcfc2b199 --- /dev/null +++ b/lintcheck/src/recursive.rs @@ -0,0 +1,123 @@ +//! 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 +//! [LintcheckServer] to ask if it should be skipped, and if not sends the stderr of running clippy +//! on the crate to the server + +use crate::ClippyWarning; +use crate::RecursiveOptions; + +use std::collections::HashSet; +use std::io::{BufRead, BufReader, Read, Write}; +use std::net::{SocketAddr, TcpListener, TcpStream}; +use std::sync::{Arc, Mutex}; +use std::thread; + +use cargo_metadata::diagnostic::Diagnostic; +use crossbeam_channel::{Receiver, Sender}; +use serde::de::DeserializeOwned; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Eq, Hash, PartialEq, Clone, Serialize, Deserialize)] +pub(crate) struct DriverInfo { + pub package_name: String, + pub crate_name: String, + pub version: String, +} + +pub(crate) fn serialize_line<T, W>(value: &T, writer: &mut W) +where + T: Serialize, + W: Write, +{ + let mut buf = serde_json::to_vec(&value).expect("failed to serialize"); + buf.push(b'\n'); + writer.write_all(&buf).expect("write_all failed"); +} + +pub(crate) fn deserialize_line<T, R>(reader: &mut R) -> T +where + T: DeserializeOwned, + R: BufRead, +{ + let mut string = String::new(); + reader.read_line(&mut string).expect("read_line failed"); + serde_json::from_str(&string).expect("failed to deserialize") +} + +fn process_stream( + stream: TcpStream, + sender: &Sender<ClippyWarning>, + options: &RecursiveOptions, + seen: &Mutex<HashSet<DriverInfo>>, +) { + let mut stream = BufReader::new(stream); + + let driver_info: DriverInfo = deserialize_line(&mut stream); + + let unseen = seen.lock().unwrap().insert(driver_info.clone()); + let ignored = options.ignore.contains(&driver_info.package_name); + let should_run = unseen && !ignored; + + serialize_line(&should_run, stream.get_mut()); + + let mut stderr = String::new(); + stream.read_to_string(&mut stderr).unwrap(); + + let messages = stderr + .lines() + .filter_map(|json_msg| serde_json::from_str::<Diagnostic>(json_msg).ok()) + .filter_map(|diag| ClippyWarning::new(diag, &driver_info.package_name, &driver_info.version)); + + for message in messages { + sender.send(message).unwrap(); + } +} + +pub(crate) struct LintcheckServer { + pub local_addr: SocketAddr, + receiver: Receiver<ClippyWarning>, + sender: Arc<Sender<ClippyWarning>>, +} + +impl LintcheckServer { + pub fn spawn(options: RecursiveOptions) -> Self { + let listener = TcpListener::bind("localhost:0").unwrap(); + let local_addr = listener.local_addr().unwrap(); + + let (sender, receiver) = crossbeam_channel::unbounded::<ClippyWarning>(); + let sender = Arc::new(sender); + // The spawned threads hold a `Weak<Sender>` so that they don't keep the channel connected + // indefinitely + let sender_weak = Arc::downgrade(&sender); + + // Ignore dependencies multiple times, e.g. for when it's both checked and compiled for a + // build dependency + let seen = Mutex::default(); + + thread::spawn(move || { + thread::scope(|s| { + s.spawn(|| { + while let Ok((stream, _)) = listener.accept() { + let sender = sender_weak.upgrade().expect("received connection after server closed"); + let options = &options; + let seen = &seen; + s.spawn(move || process_stream(stream, &sender, options, seen)); + } + }); + }); + }); + + Self { + local_addr, + sender, + receiver, + } + } + + pub fn warnings(self) -> impl Iterator<Item = ClippyWarning> { + // causes the channel to become disconnected so that the receiver iterator ends + drop(self.sender); + + self.receiver.into_iter() + } +} |
