diff options
| -rw-r--r-- | clippy_dev/src/fmt.rs | 133 | ||||
| -rw-r--r-- | clippy_dev/src/main.rs | 2 | ||||
| -rw-r--r-- | clippy_dev/src/rename_lint.rs | 20 | ||||
| -rw-r--r-- | clippy_dev/src/update_lints.rs | 7 | ||||
| -rw-r--r-- | clippy_dev/src/utils.rs | 140 | ||||
| -rw-r--r-- | rustfmt.toml | 6 | ||||
| -rw-r--r-- | tests/ui/non_expressive_names_error_recovery.fixed (renamed from tests/ui/skip_rustfmt/non_expressive_names_error_recovery.fixed) | 0 | ||||
| -rw-r--r-- | tests/ui/non_expressive_names_error_recovery.rs (renamed from tests/ui/skip_rustfmt/non_expressive_names_error_recovery.rs) | 0 | ||||
| -rw-r--r-- | tests/ui/non_expressive_names_error_recovery.stderr (renamed from tests/ui/skip_rustfmt/non_expressive_names_error_recovery.stderr) | 2 |
9 files changed, 158 insertions, 152 deletions
diff --git a/clippy_dev/src/fmt.rs b/clippy_dev/src/fmt.rs index 13d6b1285dc..c1b6b370706 100644 --- a/clippy_dev/src/fmt.rs +++ b/clippy_dev/src/fmt.rs @@ -1,5 +1,6 @@ use crate::utils::{ - ClippyInfo, ErrAction, FileUpdater, UpdateMode, UpdateStatus, panic_action, run_with_args_split, run_with_output, + ErrAction, FileUpdater, UpdateMode, UpdateStatus, expect_action, run_with_output, split_args_for_threads, + walk_dir_no_dot_or_target, }; use itertools::Itertools; use rustc_lexer::{TokenKind, tokenize}; @@ -9,7 +10,6 @@ use std::io::{self, Read}; use std::ops::ControlFlow; use std::path::PathBuf; use std::process::{self, Command, Stdio}; -use walkdir::WalkDir; pub enum Error { Io(io::Error), @@ -260,7 +260,7 @@ fn fmt_syms(update_mode: UpdateMode) { ); } -fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) { +fn run_rustfmt(update_mode: UpdateMode) { let mut rustfmt_path = String::from_utf8(run_with_output( "rustup which rustfmt", Command::new("rustup").args(["which", "rustfmt"]), @@ -268,42 +268,19 @@ fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) { .expect("invalid rustfmt path"); rustfmt_path.truncate(rustfmt_path.trim_end().len()); - let mut cargo_path = String::from_utf8(run_with_output( - "rustup which cargo", - Command::new("rustup").args(["which", "cargo"]), - )) - .expect("invalid cargo path"); - cargo_path.truncate(cargo_path.trim_end().len()); - - // Start all format jobs first before waiting on the results. - let mut children = Vec::with_capacity(16); - for &path in &[ - ".", - "clippy_config", - "clippy_dev", - "clippy_lints", - "clippy_lints_internal", - "clippy_utils", - "rustc_tools_util", - "lintcheck", - ] { - let mut cmd = Command::new(&cargo_path); - cmd.current_dir(clippy.path.join(path)) - .args(["fmt"]) - .env("RUSTFMT", &rustfmt_path) - .stdout(Stdio::null()) - .stdin(Stdio::null()) - .stderr(Stdio::piped()); - if update_mode.is_check() { - cmd.arg("--check"); - } - match cmd.spawn() { - Ok(x) => children.push(("cargo fmt", x)), - Err(ref e) => panic_action(&e, ErrAction::Run, "cargo fmt".as_ref()), - } - } + let args: Vec<_> = walk_dir_no_dot_or_target() + .filter_map(|e| { + let e = expect_action(e, ErrAction::Read, "."); + e.path() + .as_os_str() + .as_encoded_bytes() + .ends_with(b".rs") + .then(|| e.into_path().into_os_string()) + }) + .collect(); - run_with_args_split( + let mut children: Vec<_> = split_args_for_threads( + 32, || { let mut cmd = Command::new(&rustfmt_path); if update_mode.is_check() { @@ -312,66 +289,44 @@ fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) { cmd.stdout(Stdio::null()) .stdin(Stdio::null()) .stderr(Stdio::piped()) - .args(["--config", "show_parse_errors=false"]); + .args(["--unstable-features", "--skip-children"]); cmd }, - |cmd| match cmd.spawn() { - Ok(x) => children.push(("rustfmt", x)), - Err(ref e) => panic_action(&e, ErrAction::Run, "rustfmt".as_ref()), - }, - WalkDir::new("tests") - .into_iter() - .filter_entry(|p| p.path().file_name().is_none_or(|x| x != "skip_rustfmt")) - .filter_map(|e| { - let e = e.expect("error reading `tests`"); - e.path() - .as_os_str() - .as_encoded_bytes() - .ends_with(b".rs") - .then(|| e.into_path().into_os_string()) - }), - ); + args.iter(), + ) + .map(|mut cmd| expect_action(cmd.spawn(), ErrAction::Run, "rustfmt")) + .collect(); - for (name, child) in &mut children { - match child.wait() { - Ok(status) => match (update_mode, status.exit_ok()) { - (UpdateMode::Check | UpdateMode::Change, Ok(())) => {}, - (UpdateMode::Check, Err(_)) => { - let mut s = String::new(); - if let Some(mut stderr) = child.stderr.take() - && stderr.read_to_string(&mut s).is_ok() - { - eprintln!("{s}"); - } - eprintln!("Formatting check failed!\nRun `cargo dev fmt` to update."); - process::exit(1); - }, - (UpdateMode::Change, Err(e)) => { - let mut s = String::new(); - if let Some(mut stderr) = child.stderr.take() - && stderr.read_to_string(&mut s).is_ok() - { - eprintln!("{s}"); - } - panic_action(&e, ErrAction::Run, name.as_ref()); - }, + for child in &mut children { + let status = expect_action(child.wait(), ErrAction::Run, "rustfmt"); + match (update_mode, status.exit_ok()) { + (UpdateMode::Check | UpdateMode::Change, Ok(())) => {}, + (UpdateMode::Check, Err(_)) => { + let mut s = String::new(); + if let Some(mut stderr) = child.stderr.take() + && stderr.read_to_string(&mut s).is_ok() + { + eprintln!("{s}"); + } + eprintln!("Formatting check failed!\nRun `cargo dev fmt` to update."); + process::exit(1); + }, + (UpdateMode::Change, e) => { + let mut s = String::new(); + if let Some(mut stderr) = child.stderr.take() + && stderr.read_to_string(&mut s).is_ok() + { + eprintln!("{s}"); + } + expect_action(e, ErrAction::Run, "rustfmt"); }, - Err(ref e) => panic_action(e, ErrAction::Run, name.as_ref()), } } } // the "main" function of cargo dev fmt -pub fn run(clippy: &ClippyInfo, update_mode: UpdateMode) { - if clippy.has_intellij_hook { - eprintln!( - "error: a local rustc repo is enabled as path dependency via `cargo dev setup intellij`.\n\ - Not formatting because that would format the local repo as well!\n\ - Please revert the changes to `Cargo.toml`s with `cargo dev remove intellij`." - ); - return; - } - run_rustfmt(clippy, update_mode); +pub fn run(update_mode: UpdateMode) { + run_rustfmt(update_mode); fmt_syms(update_mode); if let Err(e) = fmt_conf(update_mode.is_check()) { e.display(); diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index ebcd8611d78..26aa269fb63 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -26,7 +26,7 @@ fn main() { allow_staged, allow_no_vcs, } => dogfood::dogfood(fix, allow_dirty, allow_staged, allow_no_vcs), - DevCommand::Fmt { check } => fmt::run(&clippy, utils::UpdateMode::from_check(check)), + DevCommand::Fmt { check } => fmt::run(utils::UpdateMode::from_check(check)), DevCommand::UpdateLints { check } => update_lints::update(utils::UpdateMode::from_check(check)), DevCommand::NewLint { pass, diff --git a/clippy_dev/src/rename_lint.rs b/clippy_dev/src/rename_lint.rs index be8b27c7a9e..d62597428e2 100644 --- a/clippy_dev/src/rename_lint.rs +++ b/clippy_dev/src/rename_lint.rs @@ -1,13 +1,12 @@ use crate::update_lints::{RenamedLint, find_lint_decls, generate_lint_files, read_deprecated_lints}; use crate::utils::{ - FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, Version, delete_dir_if_exists, delete_file_if_exists, - try_rename_dir, try_rename_file, + ErrAction, FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, Version, delete_dir_if_exists, + delete_file_if_exists, expect_action, try_rename_dir, try_rename_file, walk_dir_no_dot_or_target, }; use rustc_lexer::TokenKind; use std::ffi::OsString; use std::fs; use std::path::Path; -use walkdir::WalkDir; /// Runs the `rename_lint` command. /// @@ -133,17 +132,10 @@ pub fn rename(clippy_version: Version, old_name: &str, new_name: &str, uplift: b } let mut update_fn = file_update_fn(old_name, new_name, mod_edit); - for file in WalkDir::new(".").into_iter().filter_entry(|e| { - // Skip traversing some of the larger directories. - e.path() - .as_os_str() - .as_encoded_bytes() - .get(2..) - .is_none_or(|x| x != "target".as_bytes() && x != ".git".as_bytes()) - }) { - let file = file.expect("error reading clippy directory"); - if file.path().as_os_str().as_encoded_bytes().ends_with(b".rs") { - updater.update_file(file.path(), &mut update_fn); + for e in walk_dir_no_dot_or_target() { + let e = expect_action(e, ErrAction::Read, "."); + if e.path().as_os_str().as_encoded_bytes().ends_with(b".rs") { + updater.update_file(e.path(), &mut update_fn); } } generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); diff --git a/clippy_dev/src/update_lints.rs b/clippy_dev/src/update_lints.rs index 25ba2c72049..320462a2c96 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -1,5 +1,5 @@ use crate::utils::{ - ErrAction, File, FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, panic_action, update_text_region_fn, + ErrAction, File, FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, expect_action, update_text_region_fn, }; use itertools::Itertools; use std::collections::HashSet; @@ -201,10 +201,7 @@ pub fn find_lint_decls() -> Vec<Lint> { /// Reads the source files from the given root directory fn read_src_with_module(src_root: &Path) -> impl use<'_> + Iterator<Item = (DirEntry, String)> { WalkDir::new(src_root).into_iter().filter_map(move |e| { - let e = match e { - Ok(e) => e, - Err(ref e) => panic_action(e, ErrAction::Read, src_root), - }; + let e = expect_action(e, ErrAction::Read, src_root); let path = e.path().as_os_str().as_encoded_bytes(); if let Some(path) = path.strip_suffix(b".rs") && let Some(path) = path.get("clippy_lints/src/".len()..) diff --git a/clippy_dev/src/utils.rs b/clippy_dev/src/utils.rs index 255e36afe69..c4808b7048b 100644 --- a/clippy_dev/src/utils.rs +++ b/clippy_dev/src/utils.rs @@ -1,14 +1,16 @@ use core::fmt::{self, Display}; +use core::num::NonZero; use core::ops::Range; use core::slice; use core::str::FromStr; use rustc_lexer::{self as lexer, FrontmatterAllowed}; -use std::env; use std::ffi::OsStr; use std::fs::{self, OpenOptions}; use std::io::{self, Read as _, Seek as _, SeekFrom, Write}; use std::path::{Path, PathBuf}; use std::process::{self, Command, ExitStatus, Stdio}; +use std::{env, thread}; +use walkdir::WalkDir; #[cfg(not(windows))] static CARGO_CLIPPY_EXE: &str = "cargo-clippy"; @@ -45,6 +47,14 @@ pub fn panic_action(err: &impl Display, action: ErrAction, path: &Path) -> ! { panic!("error {} `{}`: {}", action.as_str(), path.display(), *err) } +#[track_caller] +pub fn expect_action<T>(res: Result<T, impl Display>, action: ErrAction, path: impl AsRef<Path>) -> T { + match res { + Ok(x) => x, + Err(ref e) => panic_action(e, action, path.as_ref()), + } +} + /// Wrapper around `std::fs::File` which panics with a path on failure. pub struct File<'a> { pub inner: fs::File, @@ -55,9 +65,9 @@ impl<'a> File<'a> { #[track_caller] pub fn open(path: &'a (impl AsRef<Path> + ?Sized), options: &mut OpenOptions) -> Self { let path = path.as_ref(); - match options.open(path) { - Ok(inner) => Self { inner, path }, - Err(e) => panic_action(&e, ErrAction::Open, path), + Self { + inner: expect_action(options.open(path), ErrAction::Open, path), + path, } } @@ -84,10 +94,7 @@ impl<'a> File<'a> { /// Read the entire contents of a file to the given buffer. #[track_caller] pub fn read_append_to_string<'dst>(&mut self, dst: &'dst mut String) -> &'dst mut String { - match self.inner.read_to_string(dst) { - Ok(_) => {}, - Err(e) => panic_action(&e, ErrAction::Read, self.path), - } + expect_action(self.inner.read_to_string(dst), ErrAction::Read, self.path); dst } @@ -107,9 +114,7 @@ impl<'a> File<'a> { }, Err(e) => Err(e), }; - if let Err(e) = res { - panic_action(&e, ErrAction::Write, self.path); - } + expect_action(res, ErrAction::Write, self.path); } } @@ -660,47 +665,91 @@ pub fn try_rename_dir(old_name: &Path, new_name: &Path) -> bool { } pub fn write_file(path: &Path, contents: &str) { - fs::write(path, contents).unwrap_or_else(|e| panic_action(&e, ErrAction::Write, path)); + expect_action(fs::write(path, contents), ErrAction::Write, path); } #[must_use] pub fn run_with_output(path: &(impl AsRef<Path> + ?Sized), cmd: &mut Command) -> Vec<u8> { fn f(path: &Path, cmd: &mut Command) -> Vec<u8> { - match cmd - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::inherit()) - .output() - { - Ok(x) => match x.status.exit_ok() { - Ok(()) => x.stdout, - Err(ref e) => panic_action(e, ErrAction::Run, path), - }, - Err(ref e) => panic_action(e, ErrAction::Run, path), - } + let output = expect_action( + cmd.stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .output(), + ErrAction::Run, + path, + ); + expect_action(output.status.exit_ok(), ErrAction::Run, path); + output.stdout } f(path.as_ref(), cmd) } -pub fn run_with_args_split( - mut make_cmd: impl FnMut() -> Command, - mut run_cmd: impl FnMut(&mut Command), - args: impl Iterator<Item: AsRef<OsStr>>, -) { - let mut cmd = make_cmd(); - let mut len = 0; - for arg in args { - len += arg.as_ref().len(); - cmd.arg(arg); - // Very conservative limit - if len > 10000 { - run_cmd(&mut cmd); - cmd = make_cmd(); - len = 0; +/// Splits an argument list across multiple `Command` invocations. +/// +/// The argument list will be split into a number of batches based on +/// `thread::available_parallelism`, with `min_batch_size` setting a lower bound on the size of each +/// batch. +/// +/// If the size of the arguments would exceed the system limit additional batches will be created. +pub fn split_args_for_threads( + min_batch_size: usize, + make_cmd: impl FnMut() -> Command, + args: impl ExactSizeIterator<Item: AsRef<OsStr>>, +) -> impl Iterator<Item = Command> { + struct Iter<F, I> { + make_cmd: F, + args: I, + min_batch_size: usize, + batch_size: usize, + thread_count: usize, + } + impl<F, I> Iterator for Iter<F, I> + where + F: FnMut() -> Command, + I: ExactSizeIterator<Item: AsRef<OsStr>>, + { + type Item = Command; + fn next(&mut self) -> Option<Self::Item> { + if self.thread_count > 1 { + self.thread_count -= 1; + } + let mut cmd = (self.make_cmd)(); + let mut cmd_len = 0usize; + for arg in self.args.by_ref().take(self.batch_size) { + cmd.arg(arg.as_ref()); + // `+ 8` to account for the `argv` pointer on unix. + // Windows is complicated since the arguments are first converted to UTF-16ish, + // but this needs to account for the space between arguments and whatever additional + // is needed to escape within an argument. + cmd_len += arg.as_ref().len() + 8; + cmd_len += 8; + + // Windows has a command length limit of 32767. For unix systems this is more + // complicated since the limit includes environment variables and room needs to be + // left to edit them once the program starts, but the total size comes from + // `getconf ARG_MAX`. + // + // For simplicity we use 30000 here under a few assumptions. + // * Individual arguments aren't super long (the final argument is still added) + // * `ARG_MAX` is set to a reasonable amount. Basically every system will be configured way above + // what windows supports, but POSIX only requires `4096`. + if cmd_len > 30000 { + self.batch_size = self.args.len().div_ceil(self.thread_count).max(self.min_batch_size); + break; + } + } + (cmd_len != 0).then_some(cmd) } } - if len != 0 { - run_cmd(&mut cmd); + let thread_count = thread::available_parallelism().map_or(1, NonZero::get); + let batch_size = args.len().div_ceil(thread_count).max(min_batch_size); + Iter { + make_cmd, + args, + min_batch_size, + batch_size, + thread_count, } } @@ -720,3 +769,12 @@ pub fn delete_dir_if_exists(path: &Path) { Err(ref e) => panic_action(e, ErrAction::Delete, path), } } + +/// Walks all items excluding top-level dot files/directories and any target directories. +pub fn walk_dir_no_dot_or_target() -> impl Iterator<Item = ::walkdir::Result<::walkdir::DirEntry>> { + WalkDir::new(".").into_iter().filter_entry(|e| { + e.path() + .file_name() + .is_none_or(|x| x != "target" && x.as_encoded_bytes().first().copied() != Some(b'.')) + }) +} diff --git a/rustfmt.toml b/rustfmt.toml index 0dc6adce7bf..0ed58a2dfc1 100644 --- a/rustfmt.toml +++ b/rustfmt.toml @@ -6,4 +6,8 @@ edition = "2024" error_on_line_overflow = true imports_granularity = "Module" style_edition = "2024" -ignore = ["tests/ui/crashes/ice-10912.rs"] +ignore = [ + "tests/ui/crashes/ice-9405.rs", + "tests/ui/crashes/ice-10912.rs", + "tests/ui/non_expressive_names_error_recovery.rs", +] diff --git a/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.fixed b/tests/ui/non_expressive_names_error_recovery.fixed index c96a53ba2cd..c96a53ba2cd 100644 --- a/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.fixed +++ b/tests/ui/non_expressive_names_error_recovery.fixed diff --git a/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.rs b/tests/ui/non_expressive_names_error_recovery.rs index a3a35eb26d1..a3a35eb26d1 100644 --- a/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.rs +++ b/tests/ui/non_expressive_names_error_recovery.rs diff --git a/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.stderr b/tests/ui/non_expressive_names_error_recovery.stderr index 4998b9bd2cc..28d9a42a9a1 100644 --- a/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.stderr +++ b/tests/ui/non_expressive_names_error_recovery.stderr @@ -1,5 +1,5 @@ error: expected one of `!`, `(`, `+`, `,`, `::`, `<`, or `>`, found `)` - --> tests/ui/skip_rustfmt/non_expressive_names_error_recovery.rs:6:19 + --> tests/ui/non_expressive_names_error_recovery.rs:6:19 | LL | fn aa(a: Aa<String) { | ^ expected one of 7 possible tokens |