Skip to content

Commit 9ebfb84

Browse files
Refactor and speed up cargo dev fmt (#14638)
Based on #14616 `cargo dev fmt` should now run almost instantly rather than taking a few seconds. changelog: None
2 parents c97b476 + 232be55 commit 9ebfb84

14 files changed

+278
-244
lines changed

Cargo.toml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
[package]
22
name = "clippy"
3-
# begin autogenerated version
43
version = "0.1.89"
5-
# end autogenerated version
64
description = "A bunch of helpful lints to avoid common pitfalls in Rust"
75
repository = "https://github.com/rust-lang/rust-clippy"
86
readme = "README.md"

clippy_config/Cargo.toml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
[package]
22
name = "clippy_config"
3-
# begin autogenerated version
43
version = "0.1.89"
5-
# end autogenerated version
64
edition = "2024"
75
publish = false
86

clippy_dev/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ clap = { version = "4.4", features = ["derive"] }
1010
indoc = "1.0"
1111
itertools = "0.12"
1212
opener = "0.7"
13-
shell-escape = "0.1"
1413
walkdir = "2.3"
1514

1615
[package.metadata.rust-analyzer]

clippy_dev/src/fmt.rs

Lines changed: 107 additions & 174 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,15 @@
1+
use crate::utils::{ClippyInfo, ErrAction, UpdateMode, panic_action, run_with_args_split, run_with_output};
12
use itertools::Itertools;
23
use rustc_lexer::{TokenKind, tokenize};
3-
use shell_escape::escape;
4-
use std::ffi::{OsStr, OsString};
4+
use std::fs;
5+
use std::io::{self, Read};
56
use std::ops::ControlFlow;
6-
use std::path::{Path, PathBuf};
7+
use std::path::PathBuf;
78
use std::process::{self, Command, Stdio};
8-
use std::{fs, io};
99
use walkdir::WalkDir;
1010

1111
pub enum Error {
12-
CommandFailed(String, String),
1312
Io(io::Error),
14-
RustfmtNotInstalled,
15-
WalkDir(walkdir::Error),
16-
IntellijSetupActive,
1713
Parse(PathBuf, usize, String),
1814
CheckFailed,
1915
}
@@ -24,50 +20,22 @@ impl From<io::Error> for Error {
2420
}
2521
}
2622

27-
impl From<walkdir::Error> for Error {
28-
fn from(error: walkdir::Error) -> Self {
29-
Self::WalkDir(error)
30-
}
31-
}
32-
3323
impl Error {
3424
fn display(&self) {
3525
match self {
3626
Self::CheckFailed => {
3727
eprintln!("Formatting check failed!\nRun `cargo dev fmt` to update.");
3828
},
39-
Self::CommandFailed(command, stderr) => {
40-
eprintln!("error: command `{command}` failed!\nstderr: {stderr}");
41-
},
4229
Self::Io(err) => {
4330
eprintln!("error: {err}");
4431
},
45-
Self::RustfmtNotInstalled => {
46-
eprintln!("error: rustfmt nightly is not installed.");
47-
},
48-
Self::WalkDir(err) => {
49-
eprintln!("error: {err}");
50-
},
51-
Self::IntellijSetupActive => {
52-
eprintln!(
53-
"error: a local rustc repo is enabled as path dependency via `cargo dev setup intellij`.\n\
54-
Not formatting because that would format the local repo as well!\n\
55-
Please revert the changes to `Cargo.toml`s with `cargo dev remove intellij`."
56-
);
57-
},
5832
Self::Parse(path, line, msg) => {
5933
eprintln!("error parsing `{}:{line}`: {msg}", path.display());
6034
},
6135
}
6236
}
6337
}
6438

65-
struct FmtContext {
66-
check: bool,
67-
verbose: bool,
68-
rustfmt_path: String,
69-
}
70-
7139
struct ClippyConf<'a> {
7240
name: &'a str,
7341
attrs: &'a str,
@@ -257,155 +225,120 @@ fn fmt_conf(check: bool) -> Result<(), Error> {
257225
Ok(())
258226
}
259227

260-
fn run_rustfmt(context: &FmtContext) -> Result<(), Error> {
261-
// if we added a local rustc repo as path dependency to clippy for rust analyzer, we do NOT want to
262-
// format because rustfmt would also format the entire rustc repo as it is a local
263-
// dependency
264-
if fs::read_to_string("Cargo.toml")
265-
.expect("Failed to read clippy Cargo.toml")
266-
.contains("[target.'cfg(NOT_A_PLATFORM)'.dependencies]")
267-
{
268-
return Err(Error::IntellijSetupActive);
269-
}
270-
271-
check_for_rustfmt(context)?;
228+
fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) {
229+
let mut rustfmt_path = String::from_utf8(run_with_output(
230+
"rustup which rustfmt",
231+
Command::new("rustup").args(["which", "rustfmt"]),
232+
))
233+
.expect("invalid rustfmt path");
234+
rustfmt_path.truncate(rustfmt_path.trim_end().len());
272235

273-
cargo_fmt(context, ".".as_ref())?;
274-
cargo_fmt(context, "clippy_dev".as_ref())?;
275-
cargo_fmt(context, "rustc_tools_util".as_ref())?;
276-
cargo_fmt(context, "lintcheck".as_ref())?;
236+
let mut cargo_path = String::from_utf8(run_with_output(
237+
"rustup which cargo",
238+
Command::new("rustup").args(["which", "cargo"]),
239+
))
240+
.expect("invalid cargo path");
241+
cargo_path.truncate(cargo_path.trim_end().len());
242+
243+
// Start all format jobs first before waiting on the results.
244+
let mut children = Vec::with_capacity(16);
245+
for &path in &[
246+
".",
247+
"clippy_config",
248+
"clippy_dev",
249+
"clippy_lints",
250+
"clippy_utils",
251+
"rustc_tools_util",
252+
"lintcheck",
253+
] {
254+
let mut cmd = Command::new(&cargo_path);
255+
cmd.current_dir(clippy.path.join(path))
256+
.args(["fmt"])
257+
.env("RUSTFMT", &rustfmt_path)
258+
.stdout(Stdio::null())
259+
.stdin(Stdio::null())
260+
.stderr(Stdio::piped());
261+
if update_mode.is_check() {
262+
cmd.arg("--check");
263+
}
264+
match cmd.spawn() {
265+
Ok(x) => children.push(("cargo fmt", x)),
266+
Err(ref e) => panic_action(&e, ErrAction::Run, "cargo fmt".as_ref()),
267+
}
268+
}
277269

278-
let chunks = WalkDir::new("tests")
279-
.into_iter()
280-
.filter_map(|entry| {
281-
let entry = entry.expect("failed to find tests");
282-
let path = entry.path();
283-
if path.extension() != Some("rs".as_ref())
284-
|| path
285-
.components()
286-
.nth_back(1)
287-
.is_some_and(|c| c.as_os_str() == "syntax-error-recovery")
288-
|| entry.file_name() == "ice-3891.rs"
289-
{
290-
None
291-
} else {
292-
Some(entry.into_path().into_os_string())
270+
run_with_args_split(
271+
|| {
272+
let mut cmd = Command::new(&rustfmt_path);
273+
if update_mode.is_check() {
274+
cmd.arg("--check");
293275
}
294-
})
295-
.chunks(250);
276+
cmd.stdout(Stdio::null())
277+
.stdin(Stdio::null())
278+
.stderr(Stdio::piped())
279+
.args(["--config", "show_parse_errors=false"]);
280+
cmd
281+
},
282+
|cmd| match cmd.spawn() {
283+
Ok(x) => children.push(("rustfmt", x)),
284+
Err(ref e) => panic_action(&e, ErrAction::Run, "rustfmt".as_ref()),
285+
},
286+
WalkDir::new("tests")
287+
.into_iter()
288+
.filter_entry(|p| p.path().file_name().is_none_or(|x| x != "skip_rustfmt"))
289+
.filter_map(|e| {
290+
let e = e.expect("error reading `tests`");
291+
e.path()
292+
.as_os_str()
293+
.as_encoded_bytes()
294+
.ends_with(b".rs")
295+
.then(|| e.into_path().into_os_string())
296+
}),
297+
);
296298

297-
for chunk in &chunks {
298-
rustfmt(context, chunk)?;
299+
for (name, child) in &mut children {
300+
match child.wait() {
301+
Ok(status) => match (update_mode, status.exit_ok()) {
302+
(UpdateMode::Check | UpdateMode::Change, Ok(())) => {},
303+
(UpdateMode::Check, Err(_)) => {
304+
let mut s = String::new();
305+
if let Some(mut stderr) = child.stderr.take()
306+
&& stderr.read_to_string(&mut s).is_ok()
307+
{
308+
eprintln!("{s}");
309+
}
310+
eprintln!("Formatting check failed!\nRun `cargo dev fmt` to update.");
311+
process::exit(1);
312+
},
313+
(UpdateMode::Change, Err(e)) => {
314+
let mut s = String::new();
315+
if let Some(mut stderr) = child.stderr.take()
316+
&& stderr.read_to_string(&mut s).is_ok()
317+
{
318+
eprintln!("{s}");
319+
}
320+
panic_action(&e, ErrAction::Run, name.as_ref());
321+
},
322+
},
323+
Err(ref e) => panic_action(e, ErrAction::Run, name.as_ref()),
324+
}
299325
}
300-
Ok(())
301326
}
302327

303328
// the "main" function of cargo dev fmt
304-
pub fn run(check: bool, verbose: bool) {
305-
let output = Command::new("rustup")
306-
.args(["which", "rustfmt"])
307-
.stderr(Stdio::inherit())
308-
.output()
309-
.expect("error running `rustup which rustfmt`");
310-
if !output.status.success() {
311-
eprintln!("`rustup which rustfmt` did not execute successfully");
312-
process::exit(1);
329+
pub fn run(clippy: &ClippyInfo, update_mode: UpdateMode) {
330+
if clippy.has_intellij_hook {
331+
eprintln!(
332+
"error: a local rustc repo is enabled as path dependency via `cargo dev setup intellij`.\n\
333+
Not formatting because that would format the local repo as well!\n\
334+
Please revert the changes to `Cargo.toml`s with `cargo dev remove intellij`."
335+
);
336+
return;
313337
}
314-
let mut rustfmt_path = String::from_utf8(output.stdout).expect("invalid rustfmt path");
315-
rustfmt_path.truncate(rustfmt_path.trim_end().len());
338+
run_rustfmt(clippy, update_mode);
316339

317-
let context = FmtContext {
318-
check,
319-
verbose,
320-
rustfmt_path,
321-
};
322-
if let Err(e) = run_rustfmt(&context).and_then(|()| fmt_conf(check)) {
340+
if let Err(e) = fmt_conf(update_mode.is_check()) {
323341
e.display();
324342
process::exit(1);
325343
}
326344
}
327-
328-
fn format_command(program: impl AsRef<OsStr>, dir: impl AsRef<Path>, args: &[impl AsRef<OsStr>]) -> String {
329-
let arg_display: Vec<_> = args.iter().map(|a| escape(a.as_ref().to_string_lossy())).collect();
330-
331-
format!(
332-
"cd {} && {} {}",
333-
escape(dir.as_ref().to_string_lossy()),
334-
escape(program.as_ref().to_string_lossy()),
335-
arg_display.join(" ")
336-
)
337-
}
338-
339-
fn exec_fmt_command(
340-
context: &FmtContext,
341-
program: impl AsRef<OsStr>,
342-
dir: impl AsRef<Path>,
343-
args: &[impl AsRef<OsStr>],
344-
) -> Result<(), Error> {
345-
if context.verbose {
346-
println!("{}", format_command(&program, &dir, args));
347-
}
348-
349-
let output = Command::new(&program)
350-
.env("RUSTFMT", &context.rustfmt_path)
351-
.current_dir(&dir)
352-
.args(args.iter())
353-
.output()
354-
.unwrap();
355-
let success = output.status.success();
356-
357-
match (context.check, success) {
358-
(_, true) => Ok(()),
359-
(true, false) => Err(Error::CheckFailed),
360-
(false, false) => {
361-
let stderr = std::str::from_utf8(&output.stderr).unwrap_or("");
362-
Err(Error::CommandFailed(
363-
format_command(&program, &dir, args),
364-
String::from(stderr),
365-
))
366-
},
367-
}
368-
}
369-
370-
fn cargo_fmt(context: &FmtContext, path: &Path) -> Result<(), Error> {
371-
let mut args = vec!["fmt", "--all"];
372-
if context.check {
373-
args.push("--check");
374-
}
375-
exec_fmt_command(context, "cargo", path, &args)
376-
}
377-
378-
fn check_for_rustfmt(context: &FmtContext) -> Result<(), Error> {
379-
let program = "rustfmt";
380-
let dir = std::env::current_dir()?;
381-
let args = &["--version"];
382-
383-
if context.verbose {
384-
println!("{}", format_command(program, &dir, args));
385-
}
386-
387-
let output = Command::new(program).current_dir(&dir).args(args.iter()).output()?;
388-
389-
if output.status.success() {
390-
Ok(())
391-
} else if std::str::from_utf8(&output.stderr)
392-
.unwrap_or("")
393-
.starts_with("error: 'rustfmt' is not installed")
394-
{
395-
Err(Error::RustfmtNotInstalled)
396-
} else {
397-
Err(Error::CommandFailed(
398-
format_command(program, &dir, args),
399-
std::str::from_utf8(&output.stderr).unwrap_or("").to_string(),
400-
))
401-
}
402-
}
403-
404-
fn rustfmt(context: &FmtContext, paths: impl Iterator<Item = OsString>) -> Result<(), Error> {
405-
let mut args = Vec::new();
406-
if context.check {
407-
args.push(OsString::from("--check"));
408-
}
409-
args.extend(paths);
410-
exec_fmt_command(context, &context.rustfmt_path, std::env::current_dir()?, &args)
411-
}

clippy_dev/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#![feature(
22
rustc_private,
3+
exit_status_error,
34
if_let_guard,
45
let_chains,
56
os_str_slice,

clippy_dev/src/main.rs

Lines changed: 1 addition & 4 deletions
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, verbose } => fmt::run(check, verbose),
29+
DevCommand::Fmt { check } => fmt::run(&clippy, utils::UpdateMode::from_check(check)),
3030
DevCommand::UpdateLints { check } => update_lints::update(utils::UpdateMode::from_check(check)),
3131
DevCommand::NewLint {
3232
pass,
@@ -125,9 +125,6 @@ enum DevCommand {
125125
#[arg(long)]
126126
/// Use the rustfmt --check option
127127
check: bool,
128-
#[arg(short, long)]
129-
/// Echo commands run
130-
verbose: bool,
131128
},
132129
#[command(name = "update_lints")]
133130
/// Updates lint registration and information from the source code

0 commit comments

Comments
 (0)