Skip to content

Commit 3da4c10

Browse files
Improve speed of cargo dev fmt (#14862)
This stops using `cargo fmt` and instead calls rustfmt directly with the list of all files. All `cargo fmt` does is find the crate roots and passes the edition from `cargo.toml`. Since the edition is set in `rustfmt.toml` for the test files and we're already iterating through all the files this is not needed. `--skip-children` is used since we already pass all the files, so the automatic detection isn't buying us anything other than running slower. ~Second commit~ (part of the first commit now) is a change to only use the `ignore` option in `rustfmt.toml` rather than having a way in `cargo dev fmt` to ignore files. r? @samueltardieu changelog: none
2 parents 9fa448a + 106ac79 commit 3da4c10

9 files changed

+158
-152
lines changed

clippy_dev/src/fmt.rs

Lines changed: 44 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::utils::{
2-
ClippyInfo, ErrAction, FileUpdater, UpdateMode, UpdateStatus, panic_action, run_with_args_split, run_with_output,
2+
ErrAction, FileUpdater, UpdateMode, UpdateStatus, expect_action, run_with_output, split_args_for_threads,
3+
walk_dir_no_dot_or_target,
34
};
45
use itertools::Itertools;
56
use rustc_lexer::{TokenKind, tokenize};
@@ -9,7 +10,6 @@ use std::io::{self, Read};
910
use std::ops::ControlFlow;
1011
use std::path::PathBuf;
1112
use std::process::{self, Command, Stdio};
12-
use walkdir::WalkDir;
1313

1414
pub enum Error {
1515
Io(io::Error),
@@ -260,50 +260,27 @@ fn fmt_syms(update_mode: UpdateMode) {
260260
);
261261
}
262262

263-
fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) {
263+
fn run_rustfmt(update_mode: UpdateMode) {
264264
let mut rustfmt_path = String::from_utf8(run_with_output(
265265
"rustup which rustfmt",
266266
Command::new("rustup").args(["which", "rustfmt"]),
267267
))
268268
.expect("invalid rustfmt path");
269269
rustfmt_path.truncate(rustfmt_path.trim_end().len());
270270

271-
let mut cargo_path = String::from_utf8(run_with_output(
272-
"rustup which cargo",
273-
Command::new("rustup").args(["which", "cargo"]),
274-
))
275-
.expect("invalid cargo path");
276-
cargo_path.truncate(cargo_path.trim_end().len());
277-
278-
// Start all format jobs first before waiting on the results.
279-
let mut children = Vec::with_capacity(16);
280-
for &path in &[
281-
".",
282-
"clippy_config",
283-
"clippy_dev",
284-
"clippy_lints",
285-
"clippy_lints_internal",
286-
"clippy_utils",
287-
"rustc_tools_util",
288-
"lintcheck",
289-
] {
290-
let mut cmd = Command::new(&cargo_path);
291-
cmd.current_dir(clippy.path.join(path))
292-
.args(["fmt"])
293-
.env("RUSTFMT", &rustfmt_path)
294-
.stdout(Stdio::null())
295-
.stdin(Stdio::null())
296-
.stderr(Stdio::piped());
297-
if update_mode.is_check() {
298-
cmd.arg("--check");
299-
}
300-
match cmd.spawn() {
301-
Ok(x) => children.push(("cargo fmt", x)),
302-
Err(ref e) => panic_action(&e, ErrAction::Run, "cargo fmt".as_ref()),
303-
}
304-
}
271+
let args: Vec<_> = walk_dir_no_dot_or_target()
272+
.filter_map(|e| {
273+
let e = expect_action(e, ErrAction::Read, ".");
274+
e.path()
275+
.as_os_str()
276+
.as_encoded_bytes()
277+
.ends_with(b".rs")
278+
.then(|| e.into_path().into_os_string())
279+
})
280+
.collect();
305281

306-
run_with_args_split(
282+
let mut children: Vec<_> = split_args_for_threads(
283+
32,
307284
|| {
308285
let mut cmd = Command::new(&rustfmt_path);
309286
if update_mode.is_check() {
@@ -312,66 +289,44 @@ fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) {
312289
cmd.stdout(Stdio::null())
313290
.stdin(Stdio::null())
314291
.stderr(Stdio::piped())
315-
.args(["--config", "show_parse_errors=false"]);
292+
.args(["--unstable-features", "--skip-children"]);
316293
cmd
317294
},
318-
|cmd| match cmd.spawn() {
319-
Ok(x) => children.push(("rustfmt", x)),
320-
Err(ref e) => panic_action(&e, ErrAction::Run, "rustfmt".as_ref()),
321-
},
322-
WalkDir::new("tests")
323-
.into_iter()
324-
.filter_entry(|p| p.path().file_name().is_none_or(|x| x != "skip_rustfmt"))
325-
.filter_map(|e| {
326-
let e = e.expect("error reading `tests`");
327-
e.path()
328-
.as_os_str()
329-
.as_encoded_bytes()
330-
.ends_with(b".rs")
331-
.then(|| e.into_path().into_os_string())
332-
}),
333-
);
295+
args.iter(),
296+
)
297+
.map(|mut cmd| expect_action(cmd.spawn(), ErrAction::Run, "rustfmt"))
298+
.collect();
334299

335-
for (name, child) in &mut children {
336-
match child.wait() {
337-
Ok(status) => match (update_mode, status.exit_ok()) {
338-
(UpdateMode::Check | UpdateMode::Change, Ok(())) => {},
339-
(UpdateMode::Check, Err(_)) => {
340-
let mut s = String::new();
341-
if let Some(mut stderr) = child.stderr.take()
342-
&& stderr.read_to_string(&mut s).is_ok()
343-
{
344-
eprintln!("{s}");
345-
}
346-
eprintln!("Formatting check failed!\nRun `cargo dev fmt` to update.");
347-
process::exit(1);
348-
},
349-
(UpdateMode::Change, Err(e)) => {
350-
let mut s = String::new();
351-
if let Some(mut stderr) = child.stderr.take()
352-
&& stderr.read_to_string(&mut s).is_ok()
353-
{
354-
eprintln!("{s}");
355-
}
356-
panic_action(&e, ErrAction::Run, name.as_ref());
357-
},
300+
for child in &mut children {
301+
let status = expect_action(child.wait(), ErrAction::Run, "rustfmt");
302+
match (update_mode, status.exit_ok()) {
303+
(UpdateMode::Check | UpdateMode::Change, Ok(())) => {},
304+
(UpdateMode::Check, Err(_)) => {
305+
let mut s = String::new();
306+
if let Some(mut stderr) = child.stderr.take()
307+
&& stderr.read_to_string(&mut s).is_ok()
308+
{
309+
eprintln!("{s}");
310+
}
311+
eprintln!("Formatting check failed!\nRun `cargo dev fmt` to update.");
312+
process::exit(1);
313+
},
314+
(UpdateMode::Change, e) => {
315+
let mut s = String::new();
316+
if let Some(mut stderr) = child.stderr.take()
317+
&& stderr.read_to_string(&mut s).is_ok()
318+
{
319+
eprintln!("{s}");
320+
}
321+
expect_action(e, ErrAction::Run, "rustfmt");
358322
},
359-
Err(ref e) => panic_action(e, ErrAction::Run, name.as_ref()),
360323
}
361324
}
362325
}
363326

364327
// the "main" function of cargo dev fmt
365-
pub fn run(clippy: &ClippyInfo, update_mode: UpdateMode) {
366-
if clippy.has_intellij_hook {
367-
eprintln!(
368-
"error: a local rustc repo is enabled as path dependency via `cargo dev setup intellij`.\n\
369-
Not formatting because that would format the local repo as well!\n\
370-
Please revert the changes to `Cargo.toml`s with `cargo dev remove intellij`."
371-
);
372-
return;
373-
}
374-
run_rustfmt(clippy, update_mode);
328+
pub fn run(update_mode: UpdateMode) {
329+
run_rustfmt(update_mode);
375330
fmt_syms(update_mode);
376331
if let Err(e) = fmt_conf(update_mode.is_check()) {
377332
e.display();

clippy_dev/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ fn main() {
2626
allow_staged,
2727
allow_no_vcs,
2828
} => dogfood::dogfood(fix, allow_dirty, allow_staged, allow_no_vcs),
29-
DevCommand::Fmt { check } => fmt::run(&clippy, utils::UpdateMode::from_check(check)),
29+
DevCommand::Fmt { check } => fmt::run(utils::UpdateMode::from_check(check)),
3030
DevCommand::UpdateLints { check } => update_lints::update(utils::UpdateMode::from_check(check)),
3131
DevCommand::NewLint {
3232
pass,

clippy_dev/src/rename_lint.rs

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
use crate::update_lints::{RenamedLint, find_lint_decls, generate_lint_files, read_deprecated_lints};
22
use crate::utils::{
3-
FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, Version, delete_dir_if_exists, delete_file_if_exists,
4-
try_rename_dir, try_rename_file,
3+
ErrAction, FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, Version, delete_dir_if_exists,
4+
delete_file_if_exists, expect_action, try_rename_dir, try_rename_file, walk_dir_no_dot_or_target,
55
};
66
use rustc_lexer::TokenKind;
77
use std::ffi::OsString;
88
use std::fs;
99
use std::path::Path;
10-
use walkdir::WalkDir;
1110

1211
/// Runs the `rename_lint` command.
1312
///
@@ -133,17 +132,10 @@ pub fn rename(clippy_version: Version, old_name: &str, new_name: &str, uplift: b
133132
}
134133

135134
let mut update_fn = file_update_fn(old_name, new_name, mod_edit);
136-
for file in WalkDir::new(".").into_iter().filter_entry(|e| {
137-
// Skip traversing some of the larger directories.
138-
e.path()
139-
.as_os_str()
140-
.as_encoded_bytes()
141-
.get(2..)
142-
.is_none_or(|x| x != "target".as_bytes() && x != ".git".as_bytes())
143-
}) {
144-
let file = file.expect("error reading clippy directory");
145-
if file.path().as_os_str().as_encoded_bytes().ends_with(b".rs") {
146-
updater.update_file(file.path(), &mut update_fn);
135+
for e in walk_dir_no_dot_or_target() {
136+
let e = expect_action(e, ErrAction::Read, ".");
137+
if e.path().as_os_str().as_encoded_bytes().ends_with(b".rs") {
138+
updater.update_file(e.path(), &mut update_fn);
147139
}
148140
}
149141
generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints);

clippy_dev/src/update_lints.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::utils::{
2-
ErrAction, File, FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, panic_action, update_text_region_fn,
2+
ErrAction, File, FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, expect_action, update_text_region_fn,
33
};
44
use itertools::Itertools;
55
use std::collections::HashSet;
@@ -201,10 +201,7 @@ pub fn find_lint_decls() -> Vec<Lint> {
201201
/// Reads the source files from the given root directory
202202
fn read_src_with_module(src_root: &Path) -> impl use<'_> + Iterator<Item = (DirEntry, String)> {
203203
WalkDir::new(src_root).into_iter().filter_map(move |e| {
204-
let e = match e {
205-
Ok(e) => e,
206-
Err(ref e) => panic_action(e, ErrAction::Read, src_root),
207-
};
204+
let e = expect_action(e, ErrAction::Read, src_root);
208205
let path = e.path().as_os_str().as_encoded_bytes();
209206
if let Some(path) = path.strip_suffix(b".rs")
210207
&& let Some(path) = path.get("clippy_lints/src/".len()..)

0 commit comments

Comments
 (0)