Skip to content

Improve speed of cargo dev fmt #14862

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

Merged
merged 2 commits into from
May 21, 2025
Merged
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
133 changes: 44 additions & 89 deletions clippy_dev/src/fmt.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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),
Expand Down Expand Up @@ -260,50 +260,27 @@ 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"]),
))
.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() {
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion clippy_dev/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 6 additions & 14 deletions clippy_dev/src/rename_lint.rs
Original file line number Diff line number Diff line change
@@ -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.
///
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 2 additions & 5 deletions clippy_dev/src/update_lints.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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()..)
Expand Down
Loading